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:39
> Subject: Re: [PATCH 3/4] drm/panfrost: Add bus_ace optional clock support for RZ/G2L
> 
> On 20/03/2026 12:30, Biju Das wrote:
> > 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",
> 
> You can simply change the format string to "%d".
> 
> Explanation:
> 
> PTR_ERR returns a long (which matches the kernel's idea that a long is the same size as a pointer).
> But the standard return code size is int.
> So technically the assignment to err is truncating the type. However, the IS_ERR() check uses
> MAX_ERRNO which is 4095 so all error values will fit in an int. So we know the assignment into 'int'
> isn't going to truncate. [ Also it's just an error message... ;) ]

OK will fix this in next version.

Cheers,
Biju