-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Sync the firmware clocks driver with upstream #3664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Maxime Ripard <[email protected]>
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]>
Signed-off-by: Maxime Ripard <[email protected]>
Signed-off-by: Maxime Ripard <[email protected]>
|
Thanks! |
|
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. |
|
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? |
|
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. |
|
I can't see any evidence of a BCM2837-based device that uses it - where should I be looking? |
|
Aren't they all using it at least for cpufreq? |
|
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? |
|
And bcm2709-rpi.dtsi. |
|
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/ |
|
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. |
|
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. |
|
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! |
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
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
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.