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

[Marek Szyprowski]

On 01.10.2025 22:50, Melissa Wen wrote:
> On 26/09/2025 07:36, Stefan Wahren wrote:
>> 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.
>
> Hi,
>
> I see in the downstream kernel the CLOCK_V3D was removed in favor of 
> firmware clock:
> https://github.com/raspberrypi/linux/blob/rpi-6.12.y/drivers/clk/bcm/clk-bcm2835.c#L2076 
>
>
> Also, v3d in RPi4 is set to use the firmware clock:
> https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi#L97 
>
>
> I think v3d clock is missed on boot, but I also think the issue should 
> be solved by setting the v3d firmware clock for Pi3.
> WDYT? Can you check it on your side? Something like:
>
> diff --git a/arch/arm/boot/dts/broadcom/bcm2835-rpi-common.dtsi 
> b/arch/arm/boot/dts/broadcom/bcm2835-rpi-common.dtsi
> index 8b3c21d9f333..3289cb5dfa8e 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2835-rpi-common.dtsi
> +++ b/arch/arm/boot/dts/broadcom/bcm2835-rpi-common.dtsi
> @@ -14,6 +14,7 @@ &hdmi {
>  };
>
>  &v3d {
> +       clocks = <&firmware_clocks 5>;
>         power-domains = <&power RPI_POWER_DOMAIN_V3D>;
>  };
>
This works for me and fixes the mentioned issue. Feel free to add:

Reported-by: Marek Szyprowski <m.szyprowski at samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski at samsung.com>


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland