[PATCH 2/2] fbdev/simplefb: Add support for generic power-domains

[Robert Foss]

On Wed, Oct 18, 2023 at 12:35 PM Robert Foss <rfoss at kernel.org> wrote:
>
> On Wed, Oct 11, 2023 at 4:38 PM Thierry Reding <thierry.reding at gmail.com> wrote:
> >
> > From: Thierry Reding <treding at nvidia.com>
> >
> > The simple-framebuffer device tree bindings document the power-domains
> > property, so make sure that simplefb supports it. This ensures that the
> > power domains remain enabled as long as simplefb is active.
> >
> > Signed-off-by: Thierry Reding <treding at nvidia.com>
> > ---
> >  drivers/video/fbdev/simplefb.c | 93 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 91 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> > index 18025f34fde7..e69fb0ad2d54 100644
> > --- a/drivers/video/fbdev/simplefb.c
> > +++ b/drivers/video/fbdev/simplefb.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/of_clk.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/parser.h>
> > +#include <linux/pm_domain.h>
> >  #include <linux/regulator/consumer.h>
> >
> >  static const struct fb_fix_screeninfo simplefb_fix = {
> > @@ -78,6 +79,11 @@ struct simplefb_par {
> >         unsigned int clk_count;
> >         struct clk **clks;
> >  #endif
> > +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> > +       unsigned int num_genpds;
> > +       struct device **genpds;
> > +       struct device_link **genpd_links;
> > +#endif
> >  #if defined CONFIG_OF && defined CONFIG_REGULATOR
> >         bool regulators_enabled;
> >         u32 regulator_count;
> > @@ -432,6 +438,83 @@ static void simplefb_regulators_enable(struct simplefb_par *par,
> >  static void simplefb_regulators_destroy(struct simplefb_par *par) { }
> >  #endif
> >
> > +#if defined CONFIG_OF && defined CONFIG_PM_GENERIC_DOMAINS
> > +static void simplefb_detach_genpds(void *res)
> > +{
> > +       struct simplefb_par *par = res;
> > +       unsigned int i = par->num_genpds;
> > +
> > +       if (par->num_genpds <= 1)
> > +               return;
> > +
> > +       while (i--) {
> > +               if (par->genpd_links[i])
> > +                       device_link_del(par->genpd_links[i]);
> > +
> > +               if (!IS_ERR_OR_NULL(par->genpds[i]))
> > +                       dev_pm_domain_detach(par->genpds[i], true);
> > +       }
> > +}
> > +
> > +static int simplefb_attach_genpd(struct simplefb_par *par,
> > +                                struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       unsigned int i;
> > +       int err;
> > +
> > +       par->num_genpds = of_count_phandle_with_args(dev->of_node,
> > +                                                    "power-domains",
> > +                                                    "#power-domain-cells");
> > +       /*
> > +        * Single power-domain devices are handled by the driver core, so
> > +        * nothing to do here.
> > +        */
> > +       if (par->num_genpds <= 1)
> > +               return 0;
> > +
> > +       par->genpds = devm_kcalloc(dev, par->num_genpds, sizeof(*par->genpds),
> > +                                  GFP_KERNEL);
> > +       if (!par->genpds)
> > +               return -ENOMEM;
> > +
> > +       par->genpd_links = devm_kcalloc(dev, par->num_genpds,
> > +                                       sizeof(*par->genpd_links),
> > +                                       GFP_KERNEL);
> > +       if (!par->genpd_links)
> > +               return -ENOMEM;
> > +
> > +       for (i = 0; i < par->num_genpds; i++) {
> > +               par->genpds[i] = dev_pm_domain_attach_by_id(dev, i);
> > +               if (IS_ERR(par->genpds[i])) {
> > +                       err = PTR_ERR(par->genpds[i]);
> > +                       if (err == -EPROBE_DEFER) {
> > +                               simplefb_detach_genpds(par);
> > +                               return err;
> > +                       }
> > +
> > +                       dev_warn(dev, "failed to attach domain %u: %d\n", i, err);
> > +                       continue;
> > +               }
> > +
> > +               par->genpd_links[i] = device_link_add(dev, par->genpds[i],
> > +                                                     DL_FLAG_STATELESS |
> > +                                                     DL_FLAG_PM_RUNTIME |
> > +                                                     DL_FLAG_RPM_ACTIVE);
> > +               if (!par->genpd_links[i])
> > +                       dev_warn(dev, "failed to link power-domain %u\n", i);
> > +       }
> > +
> > +       return devm_add_action_or_reset(dev, simplefb_detach_genpds, par);
> > +}
> > +#else
> > +static int simplefb_attach_genpd(struct simplefb_par *par,
> > +                                struct platform_device *pdev)
> > +{
> > +       return 0;
> > +}
> > +#endif
> > +
> >  static int simplefb_probe(struct platform_device *pdev)
> >  {
> >         int ret;
> > @@ -518,6 +601,10 @@ static int simplefb_probe(struct platform_device *pdev)
> >         if (ret < 0)
> >                 goto error_clocks;
> >
> > +       ret = simplefb_attach_genpd(par, pdev);
> > +       if (ret < 0)
> > +               goto error_regulators;
> > +
> >         simplefb_clocks_enable(par, pdev);
> >         simplefb_regulators_enable(par, pdev);
> >
> > @@ -534,18 +621,20 @@ static int simplefb_probe(struct platform_device *pdev)
> >         ret = devm_aperture_acquire_for_platform_device(pdev, par->base, par->size);
> >         if (ret) {
> >                 dev_err(&pdev->dev, "Unable to acquire aperture: %d\n", ret);
> > -               goto error_regulators;
> > +               goto error_genpds;
> >         }
> >         ret = register_framebuffer(info);
> >         if (ret < 0) {
> >                 dev_err(&pdev->dev, "Unable to register simplefb: %d\n", ret);
> > -               goto error_regulators;
> > +               goto error_genpds;
> >         }
> >
> >         dev_info(&pdev->dev, "fb%d: simplefb registered!\n", info->node);
> >
> >         return 0;
> >
> > +error_genpds:
> > +       simplefb_detach_genpds(par);
> >  error_regulators:
> >         simplefb_regulators_destroy(par);
>
> I saw an error on a rhel9.3 kernel build, it may or may not be hit on
> an upstream build.
>
> drivers/video/fbdev/simplefb.c: In function 'simplefb_probe':
> drivers/video/fbdev/simplefb.c:650:1: warning: label
> 'error_regulators' defined but not used [-Wunused-label]
>   650 | error_regulators:
>       | ^~~~~~~~~~~~~~~~
>
>

Scratch that. After applying on an upstream build, it builds cleanly.