Hi Steven Price, Thanks for the feedback.
> -----Original Message----- > From: Steven Price <[email protected]> > Sent: 20 March 2026 12:16 > Subject: Re: [PATCH 3/4] drm/panfrost: Add bus_ace optional clock support for > RZ/G2L > > On 04/03/2026 13:48, Biju wrote: > > From: Biju Das <[email protected]> > > > > On RZ/G2L SoCs, the GPU MMU requires a bus_ace clock to operate correctly. > > Without it, unbind/bind cycles leave the GPU non-operational, > > manifesting as an AS_ACTIVE bit stuck and a soft reset timeout falling > > back to hard reset. Add bus_ace_clock as an optional clock, wiring it > > into init/fini, and the runtime suspend/resume paths alongside the > > existing optional bus_clock. > > > > Signed-off-by: Biju Das <[email protected]> > > --- > > drivers/gpu/drm/panfrost/panfrost_device.c | 24 > > ++++++++++++++++++++++ drivers/gpu/drm/panfrost/panfrost_device.h | > > 1 + > > 2 files changed, 25 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > > b/drivers/gpu/drm/panfrost/panfrost_device.c > > index 01e702a0b2f0..87dae0ed748a 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > > @@ -70,8 +70,23 @@ static int panfrost_clk_init(struct panfrost_device > > *pfdev) > > goto disable_clock; > > } > > > > + pfdev->bus_ace_clock = devm_clk_get_optional(pfdev->base.dev, > > "bus_ace"); > > + if (IS_ERR(pfdev->bus_ace_clock)) { > > + err = PTR_ERR(pfdev->bus_ace_clock); > > + dev_err(pfdev->base.dev, "get bus_ace_clock failed %ld\n", > > + PTR_ERR(pfdev->bus_ace_clock)); > > + err = PTR_ERR(pfdev->bus_ace_clock); > > You've assigned err twice (with the same value), and you can simplify the > dev_err() line by using err Oops, forgot to take out the bottom assignment. > rather than the same PTR_ERR() expression again. I get a warning, if I use "err" in dev_err() panfrost_device.c:76:42: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘int’ [-Wformat=] 76 | dev_err(pfdev->base.dev, "get bus_ace_clock failed %ld\n", Cheers, Biju > > With that fixed: > > Reviewed-by: Steven Price <[email protected]> > > Thanks, > Steve > > > + goto disable_bus_clock; > > + } > > + > > + err = clk_prepare_enable(pfdev->bus_ace_clock); > > + if (err) > > + goto disable_bus_clock; > > + > > return 0; > > > > +disable_bus_clock: > > + clk_disable_unprepare(pfdev->bus_clock); > > disable_clock: > > clk_disable_unprepare(pfdev->clock); > > > > @@ -80,6 +95,7 @@ static int panfrost_clk_init(struct panfrost_device > > *pfdev) > > > > static void panfrost_clk_fini(struct panfrost_device *pfdev) { > > + clk_disable_unprepare(pfdev->bus_ace_clock); > > clk_disable_unprepare(pfdev->bus_clock); > > clk_disable_unprepare(pfdev->clock); > > } > > @@ -432,6 +448,10 @@ static int panfrost_device_runtime_resume(struct > > device *dev) > > ret = clk_enable(pfdev->bus_clock); > > if (ret) > > goto err_bus_clk; > > + > > + ret = clk_enable(pfdev->bus_ace_clock); > > + if (ret) > > + goto err_bus_ace_clk; > > } > > > > panfrost_device_reset(pfdev, true); > > @@ -439,6 +459,9 @@ static int panfrost_device_runtime_resume(struct > > device *dev) > > > > return 0; > > > > +err_bus_ace_clk: > > + if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) > > + clk_disable(pfdev->bus_clock); > > err_bus_clk: > > if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) > > clk_disable(pfdev->clock); > > @@ -462,6 +485,7 @@ static int panfrost_device_runtime_suspend(struct > > device *dev) > > panfrost_gpu_power_off(pfdev); > > > > if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) { > > + clk_disable(pfdev->bus_ace_clock); > > clk_disable(pfdev->bus_clock); > > clk_disable(pfdev->clock); > > reset_control_assert(pfdev->rstc); > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h > > b/drivers/gpu/drm/panfrost/panfrost_device.h > > index 0f3992412205..ec55c136b1b6 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > > @@ -136,6 +136,7 @@ struct panfrost_device { > > void __iomem *iomem; > > struct clk *clock; > > struct clk *bus_clock; > > + struct clk *bus_ace_clock; > > struct regulator_bulk_data *regulators; > > struct reset_control *rstc; > > /* pm_domains for devices with more than one. */
