On Wed, Oct 18, 2023 at 12:35 PM Robert Foss <rfoss@xxxxxxxxxx> wrote:
>
> On Wed, Oct 11, 2023 at 4:38 PM Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
> >
> > From: Thierry Reding <treding@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> > 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.
