| Message ID | 20180709125402.32427-1-broonie@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | regulator: core: Don't link consumers on the same device | expand |
Hi Mark, On 2018-07-09 14:54, Mark Brown wrote: > In some cases a device may end up supplying itself, for example when a > DCDC is used to improve the efficiency of LDOs or when a LDO is provided > to clean up sensitive supplies. In these cases the driver core will > warn loudly about attempts to add links so suppress them here. > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > > Compile tested only. This patch doesn't help, because wm8994 creates separate struct device for each registered regulator, so you need to check the regulator parents too, see my fixes below in the patch. I don't like such fixes for this specific case. It's not that uncommon that a complex device provides various functionalities used by its components, so it is not a bad design to use regulator provided by its parent/grandparent/other ancestor device. Frankly, either the device_is_dependent() function should be exported and used in regulator core or device_link_add() should have another flag: DL_FLAG_SKIP_DEPENDANT. > drivers/regulator/core.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index b5db0257bc40..1302e66e80ac 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -1741,7 +1741,8 @@ struct regulator *_regulator_get(struct device *dev, const char *id, > rdev->use_count = 0; > } > > - device_link_add(dev, &rdev->dev, DL_FLAG_STATELESS); > + if (dev != &rdev->dev) if (dev != &rdev->dev && dev != rdev->dev.parent) > + device_link_add(dev, &rdev->dev, DL_FLAG_STATELESS); > > return regulator; > } > @@ -1840,7 +1841,7 @@ static void _regulator_put(struct regulator *regulator) > if (r->dev == regulator->dev) > count++; > > - if (count == 1) > + if (count == 1 && regulator->dev != &rdev->dev) if (count == 1 && regulator->dev != &rdev->dev && regulator->dev != rdev->dev.parent) > device_link_remove(regulator->dev, &rdev->dev); > > /* remove any sysfs entries */ Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
On Mon, Jul 09, 2018 at 03:32:14PM +0200, Marek Szyprowski wrote: > This patch doesn't help, because wm8994 creates separate struct device for > each registered regulator, so you need to check the regulator parents too, > see my fixes below in the patch. Ugh, right - I'd not looked at enough of the context to see what was being checked. > I don't like such fixes for this specific case. It's not that uncommon > that a complex device provides various functionalities used by its > components, so it is not a bad design to use regulator provided by > its parent/grandparent/other ancestor device. > Frankly, either the device_is_dependent() function should be exported and > used in regulator core or device_link_add() should have another flag: > DL_FLAG_SKIP_DEPENDANT. Or it should just make the warning optional at the core level.
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index b5db0257bc40..1302e66e80ac 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1741,7 +1741,8 @@ struct regulator *_regulator_get(struct device *dev, const char *id, rdev->use_count = 0; } - device_link_add(dev, &rdev->dev, DL_FLAG_STATELESS); + if (dev != &rdev->dev) + device_link_add(dev, &rdev->dev, DL_FLAG_STATELESS); return regulator; } @@ -1840,7 +1841,7 @@ static void _regulator_put(struct regulator *regulator) if (r->dev == regulator->dev) count++; - if (count == 1) + if (count == 1 && regulator->dev != &rdev->dev) device_link_remove(regulator->dev, &rdev->dev); /* remove any sysfs entries */
In some cases a device may end up supplying itself, for example when a DCDC is used to improve the efficiency of LDOs or when a LDO is provided to clean up sensitive supplies. In these cases the driver core will warn loudly about attempts to add links so suppress them here. Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- Compile tested only. drivers/regulator/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.18.0.rc2