Skip to content

Conversation

@mripard
Copy link
Contributor

@mripard mripard commented Jun 10, 2020

Hi,

Here's a PR sync'ing the current state of the firmware clocks driver in the latest revision of the RPi4 HDMI patchset, and that should be fairly close to the merged version.

mripard added 7 commits June 10, 2020 18:56
While the firmware allows us to discover the available clocks, we need to
discriminate those clocks to only register the ones meaningful to Linux.
The firmware also doesn't provide a clock name, so having a list of the ID
will help us to give clocks a proper name later on.

Acked-by: Nicolas Saenz Julienne <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
The raspberrypi firmware clock driver has a min_rate / max_rate clamping by
storing the info it needs in a private structure.

However, the CCF already provides such a facility, so we can switch to it
to remove the boilerplate.

Reviewed-by: Nicolas Saenz Julienne <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
We've registered the firmware clocks using their ID as name, but it's much
more convenient to register them using their proper name. Since the
firmware doesn't provide it, we have to duplicate it.

Acked-by: Nicolas Saenz Julienne <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
The CPU clock has had so far a bunch of quirks to expose the clock tree
properly, but since we reverted to exposing them through the MMIO driver,
we can remove that code from the firmware driver.

Signed-off-by: Maxime Ripard <[email protected]>
@mripard mripard mentioned this pull request Jun 10, 2020
@pelwell pelwell merged commit c9c0a30 into raspberrypi:rpi-5.4.y Jun 11, 2020
@pelwell
Copy link
Contributor

pelwell commented Jun 11, 2020

Thanks!

@pelwell
Copy link
Contributor

pelwell commented Jun 11, 2020

No, that's not going to work - you've left many of our platforms with no firmware clocks support. I've got a fix in the pipeline.

@pelwell
Copy link
Contributor

pelwell commented Jun 11, 2020

OK, so I've decided I don't understand your policy of which devices get a firmware_clocks node. Currently only the vc4 KMS driver on bcm2711 uses it, which would possibly make it reasonable to include only in bcm2711.dtsi or bcm2711-rpi.dtsi (or even bcm2711-rpi-4-b.dts, at a push), but you also explicitly declare it in all the bcm2710 and bcm2837 platforms directly, rather than in bcm2837-rpi.dts.

Can you explain your logic?

@mripard
Copy link
Contributor Author

mripard commented Jun 11, 2020

I should have explained it better then. The new firmware clocks DT binding makes it a subnode of the main firmware node. Therefore, I added it for all the boards that are using that binding (which is as far as I can see all the ones based on BCM2837 and BCM2711). The other SoCs don't have that binding and will be instantiated by registering the platform_device by hand in the main firmware driver.

@pelwell
Copy link
Contributor

pelwell commented Jun 11, 2020

I can't see any evidence of a BCM2837-based device that uses it - where should I be looking?

@mripard
Copy link
Contributor Author

mripard commented Jun 11, 2020

Aren't they all using it at least for cpufreq?

@pelwell
Copy link
Contributor

pelwell commented Jun 11, 2020

Yes, all Pis except those with BCM2835s are using it, which means that as it stands Pi 2 (which has a BCM2836) currently has no frequency scaling.

Wouldn't it be easier to add the firmware_clocks node in bcm2836-rpi.dtsi and (at least in downstream) bcm2711-rpi.dtsi, rather than changing all the .dts files?

@pelwell
Copy link
Contributor

pelwell commented Jun 11, 2020

And bcm2709-rpi.dtsi.

@mripard
Copy link
Contributor Author

mripard commented Jun 12, 2020

I overlooked the patch you did to remove the platform_device registration in the main firmware driver. We can do it either way, just let me know if you prefer to do the change you suggested or just reverting 025822b and apply https://lore.kernel.org/linux-arm-kernel/c853e53cdf1f98a3e63741f5bdb0631dbba3031e.1591860665.git-series.maxime@cerno.tech/

@pelwell
Copy link
Contributor

pelwell commented Jun 12, 2020

If the patch is accepted upstream and removes the warning (which it appears it will) then we'd prefer to revert and take the upstreamed patch, but that doesn't mean I like the non-DT platform_device approach.

@pelwell
Copy link
Contributor

pelwell commented Jun 12, 2020

You might also have seen that I pushed a patch to rpi-5.4.y that simplifies the firmware_clocks DT support and fixes the Pi 2B case in the process. I haven't yet ported your patches to rpi-5.6.y and rpi-5.7.y, so let me know how you feel about taking that approach from the outset i.e. squashing my DT patch with yours.

@mripard
Copy link
Contributor Author

mripard commented Jun 12, 2020

I'm fine with your solution, and since you pushed it already, I think we can leave it at that.

We would still have to apply the firmware patch I linked earlier upstream to deal with the old DTs, but I guess we should send a variant of yours too. I'll take care of it.

Thanks for fixing this!

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Jun 17, 2020
kernel: Sync the firmware clocks driver with upstream
See: raspberrypi/linux#3664

firmware: board_info: Add and use BT_FLOWCONTROL trait

firmware: logging: Add missing checks for uart_output_enabled

firmware: host_applications: Install debug_sym.h

firmware: logging: Inherit uart_2ndstage config from the bootloader
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Jun 17, 2020
kernel: Sync the firmware clocks driver with upstream
See: raspberrypi/linux#3664

firmware: board_info: Add and use BT_FLOWCONTROL trait

firmware: logging: Add missing checks for uart_output_enabled

firmware: host_applications: Install debug_sym.h

firmware: logging: Inherit uart_2ndstage config from the bootloader
@mripard mripard deleted the mr/rpi-fw-clk branch April 21, 2021 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants