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<[email protected]> >>> >>> --- >>> >>> 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 <[email protected]> Tested-by: Marek Szyprowski <[email protected]> > 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"); Well, this can be sorted out in the drivers as a next step. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
