On 14.09.2026 10:45, Boris Brezillon wrote:
> On Sat, 12 Sep 2026 00:28:06 +0100
> Adrián Larumbe <[email protected]> wrote:
>
> > Gather all clock enables and disables into a single function to avoid
> > repetitions between driver init/fini and device resume/suspend, since
> > these clocks are always handled in bulk.
> >
> > Also do clk (un)prepares and dis/enables at the same time, since the
> > clk_prepare_* family of functions can simply increase the refcnt of
> > an already prepared clock.
> >
> > Signed-off-by: Adrián Larumbe <[email protected]>
>
> Reviewed-by: Boris Brezillon <[email protected]>
>
> Two nits below.
>
> > ---
> > drivers/gpu/drm/panfrost/panfrost_device.c | 118
> > +++++++++++++----------------
> > 1 file changed, 52 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
> > b/drivers/gpu/drm/panfrost/panfrost_device.c
> > index b3a53504bd01..2fcbd223204c 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > @@ -34,10 +34,46 @@ static void panfrost_reset_fini(struct panfrost_device
> > *pfdev)
> > reset_control_assert(pfdev->rstc);
> > }
> >
> > -static int panfrost_clk_init(struct panfrost_device *pfdev)
> > +static int panfrost_clk_enable(struct panfrost_device *pfdev, bool
> > enable_ace_clock)
>
> I don't want to have a flag added for each new clock we decide to keep
> enabled on RPM suspend, so how about s/enable_ace_clock/on_resume/?
> Also, since it's enabling more than one clk, I'd go
> s/panfrost_clk_enable/panfrost_clks_enable/.
I agree with you. I was thinking if new clocks also had different needs in the
future,
maybe we could pass an array of option clocks but at this point it's pure
make-believe.
> > {
> > int err;
> > +
> > + err = clk_prepare_enable(pfdev->clock);
> > + if (err)
> > + return err;
> > +
> > + err = clk_prepare_enable(pfdev->bus_clock);
> > + if (err)
> > + goto disable_clock;
> > +
> > + if (enable_ace_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);
> > +
> > + return err;
> > +}
> > +
> > +static void panfrost_clk_disable(struct panfrost_device *pfdev, bool
> > disable_ace_clock)
>
> Similarly, I'd go s/disable_ace_clock/on_suspend/ and
> s/panfrost_clk_disable/panfrost_clks_disable/.
Noted.
Adrian Larumbe
