[PATCH v2 2/5] clk: bcm: rpi: Turn firmware clock on/off when preparing/unpreparing

[Stefan Wahren]

Hi,

Am 25.09.25 um 09:57 schrieb Marek Szyprowski:
> On 31.07.2025 23:06, Maíra Canal wrote:
>> Currently, when we prepare or unprepare RPi's clocks, we don't actually
>> enable/disable the firmware clock. This means that
>> `clk_disable_unprepare()` doesn't actually change the clock state at
>> all, nor does it lowers the clock rate.
>>
>> >From the Mailbox Property Interface documentation [1], we can see that
>> we should use `RPI_FIRMWARE_SET_CLOCK_STATE` to set the clock state
>> off/on. Therefore, use `RPI_FIRMWARE_SET_CLOCK_STATE` to create a
>> prepare and an unprepare hook for RPi's firmware clock.
>>
>> As now the clocks are actually turned off, some of them are now marked
>> CLK_IS_CRITICAL, as those are required to be on during the whole system
>> operation.
>>
>> Link:https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface [1]
>> Signed-off-by: Maíra Canal<mcanal at igalia.com>
>>
>> ---
>>
>> About the pixel clock: currently, if we actually disable the pixel
>> clock during a hotplug, the system will crash. This happens in the
>> RPi 4.
>>
>> The crash happens after we disabled the CRTC (thus, the pixel clock),
>> but before the end of atomic commit tail. As vc4's pixel valve doesn't
>> directly hold a reference to its clock – we use the HDMI encoder to
>> manage the pixel clock – I believe we might be disabling the clock
>> before we should.
>>
>> After this investigation, I decided to keep things as they current are:
>> the pixel clock is never disabled, as fixing it would go out of
>> the scope of this series.
>> ---
>>    drivers/clk/bcm/clk-raspberrypi.c | 56 ++++++++++++++++++++++++++++++++++++++-
>>    1 file changed, 55 insertions(+), 1 deletion(-)
> This patch landed recently in linux-next as commit 919d6924ae9b ("clk:
> bcm: rpi: Turn firmware clock on/off when preparing/unpreparing"). In my
> tests I found that it breaks booting of RaspberryPi3B+ board in ARM
> 32bit mode. Surprisingly the same board in ARM 64bit mode correctly
> boots a kernel compiled from the same source. The RPi3B+ board freezes
> after loading the DRM modules (kernel compiled from arm/multi_v7_defconfig):
thanks for spotting and bisecting this. Sorry, I only reviewed the 
changes and didn't had the time to test any affected board.

I was able to reproduce this issue and the following workaround avoid 
the hang in my case:

diff --git a/drivers/clk/bcm/clk-raspberrypi.c 
b/drivers/clk/bcm/clk-raspberrypi.c
index 1a9162f0ae31..94fd4f6e2837 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -137,6 +137,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
         [RPI_FIRMWARE_V3D_CLK_ID] = {
                 .export = true,
                 .maximize = true,
+               .flags = CLK_IS_CRITICAL,
         },
         [RPI_FIRMWARE_PIXEL_CLK_ID] = {
                 .export = true,

The proper fix should be in the clock consumer drivers. I found that 
vc4_v3d doesn't ensure that the clock is enabled before accessing the 
registers. Unfortunately the following change doesn't fix the issue for 
me :-(

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index bb09df5000bd..5e43523732b4 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -441,7 +441,7 @@ static int vc4_v3d_bind(struct device *dev, struct 
device *master, void *data)
         vc4->v3d = v3d;
         v3d->vc4 = vc4;

-       v3d->clk = devm_clk_get_optional(dev, NULL);
+       v3d->clk = devm_clk_get_optional_enabled(dev, NULL);
         if (IS_ERR(v3d->clk))
                 return dev_err_probe(dev, PTR_ERR(v3d->clk), "Failed to 
get V3D clock\n");

Best regards