RE: [PATCH 3/4] drm/panfrost: Add bus_ace optional clock support for RZ/G2L

[Biju Das]

Hi Steven Price,

Thanks for the feedback.

> -----Original Message-----
> From: Steven Price <steven.price@xxxxxxx>
> 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 <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > 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 <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> >  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 <steven.price@xxxxxxx>
> 
> 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. */