[Aarch64-laptops] [EOAN][UNSTABLE][PATCH] UBUNTU: SAUCE: arm64: dts: qcom: Add support for AArch64 laptops

Bjorn Andersson bjorn.andersson at linaro.org
Thu Aug 8 16:52:29 UTC 2019


On Thu 08 Aug 08:10 PDT 2019, Seth Forshee wrote:

> On Tue, Jul 30, 2019 at 03:28:05PM -0400, Dimitri John Ledkov wrote:
> > From: AArch64 Laptops <aarch64laptops at gmail.com>
> > 
> > This is squashed merged of multiple patches from
> > https://github.com/aarch64-laptops/linux/tree/laptops-ubuntu
> > 
> > Note that some of these, with different names are getting upstreamed
> > targetted for v5.4. Once those are all merged and available, this
> > patch will become obsolete and will be dropped.
> 
> Are they in linux-next or a maintainer tree already? If so I think it
> would be preferable to cherry pick the individual patches.
> 
> Otherwise, adding some device trees seems pretty harmless. But one
> comment ...
> 
> <snip>
> 
> > diff --git a/drivers/regulator/qcom-rpmh-regulator.c b/drivers/regulator/qcom-rpmh-regulator.c
> > index b2c2d01d1637..aa0272497be4 100644
> > --- a/drivers/regulator/qcom-rpmh-regulator.c
> > +++ b/drivers/regulator/qcom-rpmh-regulator.c
> > @@ -207,7 +207,7 @@ static int rpmh_regulator_is_enabled(struct regulator_dev *rdev)
> >  {
> >  	struct rpmh_vreg *vreg = rdev_get_drvdata(rdev);
> >  
> > -	return vreg->enabled;
> > +	return vreg->enabled > 0;
> >  }
> 
> This is clearly a bug in the driver. I think it should be a separate
> patch from the dts files, and it should be able to go into 5.3.
> 

The rpmh regulator driver does not have a way to query the state of the
hardware during initialization, so it has to cache the current state;
with is initialized to -EINVAL.

So the upstream driver returns an error here, which according to the
regulator driver api seems to be the right thing to do. Then, iirc, the
regulator core interprets this as the regulator being on (!0) and
requests it to be turned off - which causes the display to be powered
down.

So this interaction needs to be investigated further to come up with a
proper solution.


PS. Note that the display drivers might very well be loaded way after
regulator_init_complete(), where the regulator framework will turn off
any unused regulators. Any lying in the driver (always returning false,
like I do here) will cause unused regulators to stay on forever.

Regards,
Bjorn



More information about the kernel-team mailing list