The ISP clock should also be marked critical to prevent the clock
framework from disabling it on boot, which can lead to bus lockups when
the kernel tries to submit buffers to the VC4 ISP firmware.

Link: https://github.com/raspberrypi/linux/issues/7100
Fixes: 919d6924ae9b ("clk: bcm: rpi: Turn firmware clock on/off when preparing/unpreparing")
Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 1a9162f0ae31e330c46f6eafdd00350599b0eede..dcc3ae5fb91650b042d52f02499922786b43afde 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -151,6 +151,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
 	[RPI_FIRMWARE_ISP_CLK_ID] = {
 		.export = true,
 		.minimize = true,
+		.flags = CLK_IS_CRITICAL,
 	},
 	[RPI_FIRMWARE_PIXEL_BVB_CLK_ID] = {
 		.export = true,

---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251028-rpi-isp-clk-eb5c48346e51

Best regards,
-- 
Jai Luthra <jai.luthra@ideasonboard.com>

On 10/28/25 04:46, Jai Luthra wrote:
> The ISP clock should also be marked critical to prevent the clock
> framework from disabling it on boot, which can lead to bus lockups when
> the kernel tries to submit buffers to the VC4 ISP firmware.

The commit message suggests that the driver that interfaces with the VC4 
ISP firmware is not ensuring that the clock remains enabled for the 
duration of the transactions. That would be a driver bug, would not it?

> 
> Link: https://github.com/raspberrypi/linux/issues/7100
> Fixes: 919d6924ae9b ("clk: bcm: rpi: Turn firmware clock on/off when preparing/unpreparing")
> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
> ---
>   drivers/clk/bcm/clk-raspberrypi.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
> index 1a9162f0ae31e330c46f6eafdd00350599b0eede..dcc3ae5fb91650b042d52f02499922786b43afde 100644
> --- a/drivers/clk/bcm/clk-raspberrypi.c
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -151,6 +151,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
>   	[RPI_FIRMWARE_ISP_CLK_ID] = {
>   		.export = true,
>   		.minimize = true,
> +		.flags = CLK_IS_CRITICAL,
>   	},
>   	[RPI_FIRMWARE_PIXEL_BVB_CLK_ID] = {
>   		.export = true,
> 
> ---
> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> change-id: 20251028-rpi-isp-clk-eb5c48346e51
> 
> Best regards,


-- 
Florian

Hi Florian,

Quoting Florian Fainelli (2025-11-05 03:50:23)
> On 10/28/25 04:46, Jai Luthra wrote:
> > The ISP clock should also be marked critical to prevent the clock
> > framework from disabling it on boot, which can lead to bus lockups when
> > the kernel tries to submit buffers to the VC4 ISP firmware.
> 
> The commit message suggests that the driver that interfaces with the VC4 
> ISP firmware is not ensuring that the clock remains enabled for the 
> duration of the transactions. That would be a driver bug, would not it?
> 

Yes, from https://github.com/raspberrypi/linux/issues/7100#issuecomment-3467699055
the VC4 firmware should be responsible for stopping and starting that clock
when necessary.

The downstream bcm2835-isp linux driver currently does not consume this
clock at all, so I'm not sure if the cleaner solution is to drop this
altogether from linux, or mark it as a consumer, along with fixing the
firmware driver.

> > 
> > Link: https://github.com/raspberrypi/linux/issues/7100
> > Fixes: 919d6924ae9b ("clk: bcm: rpi: Turn firmware clock on/off when preparing/unpreparing")
> > Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
> > ---
> >   drivers/clk/bcm/clk-raspberrypi.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
> > index 1a9162f0ae31e330c46f6eafdd00350599b0eede..dcc3ae5fb91650b042d52f02499922786b43afde 100644
> > --- a/drivers/clk/bcm/clk-raspberrypi.c
> > +++ b/drivers/clk/bcm/clk-raspberrypi.c
> > @@ -151,6 +151,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
> >       [RPI_FIRMWARE_ISP_CLK_ID] = {
> >               .export = true,
> >               .minimize = true,
> > +             .flags = CLK_IS_CRITICAL,
> >       },
> >       [RPI_FIRMWARE_PIXEL_BVB_CLK_ID] = {
> >               .export = true,
> > 
> > ---
> > base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> > change-id: 20251028-rpi-isp-clk-eb5c48346e51
> > 
> > Best regards,
> 
> 
> -- 
> Florian

Thanks,
Jai

Am 28.10.25 um 12:46 schrieb Jai Luthra:
> The ISP clock should also be marked critical to prevent the clock
> framework from disabling it on boot, which can lead to bus lockups when
> the kernel tries to submit buffers to the VC4 ISP firmware.
>
> Link: https://github.com/raspberrypi/linux/issues/7100
> Fixes: 919d6924ae9b ("clk: bcm: rpi: Turn firmware clock on/off when preparing/unpreparing")
> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
Reviewed-by: Stefan Wahren <wahrenst@gmx.net>

Thanks
> ---
>   drivers/clk/bcm/clk-raspberrypi.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
> index 1a9162f0ae31e330c46f6eafdd00350599b0eede..dcc3ae5fb91650b042d52f02499922786b43afde 100644
> --- a/drivers/clk/bcm/clk-raspberrypi.c
> +++ b/drivers/clk/bcm/clk-raspberrypi.c
> @@ -151,6 +151,7 @@ raspberrypi_clk_variants[RPI_FIRMWARE_NUM_CLK_ID] = {
>   	[RPI_FIRMWARE_ISP_CLK_ID] = {
>   		.export = true,
>   		.minimize = true,
> +		.flags = CLK_IS_CRITICAL,
>   	},
>   	[RPI_FIRMWARE_PIXEL_BVB_CLK_ID] = {
>   		.export = true,
>
> ---
> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> change-id: 20251028-rpi-isp-clk-eb5c48346e51
>
> Best regards,