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/.

>  {
>       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/.

Reply via email to