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

[Stefan Wahren]

Hi Marek,

Am 26.09.25 um 09:27 schrieb Marek Szyprowski:
> On 25.09.2025 18:48, Stefan Wahren wrote:
>> 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,
>>
> Right, this fixes (frankly speaking 'hides') the issue. Feel free to add:
>
> Reported-by: Marek Szyprowski <m.szyprowski at samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski at samsung.com>
>
AFAIK the offending clock change isn't in the downstream kernel, so I 
like to see the opinion of María and the Raspberry Pi people first.

Currently I know that in the error case the following clocks are 
disabled during boot of Raspberry Pi 3B+:
fw-clk-vec
fw-clk-isp
fw-clk-v3d

So it's very likely that the vc4 drivers tries to access the register 
after the these clocks has been disabled and then the system freeze. The 
workaround above was just a wild guess, so currently I don't know why 
this change avoid the freeze.

Best regards