linux-kernel - Re: [PATCH] regulator: core: Fix resolve supply

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241113173642.4d58645b@kmaincent-XPS-13-7390>
Date: Wed, 13 Nov 2024 17:36:42 +0100
From: Kory Maincent <kory.maincent@...tlin.com>
To: Mark Brown <broonie@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>, linux-kernel@...r.kernel.org,
 thomas.petazzoni@...tlin.com, Liam Girdwood <lgirdwood@...il.com>
Subject: Re: [PATCH] regulator: core: Fix resolve supply

On Wed, 13 Nov 2024 16:05:46 +0000
Mark Brown <broonie@...nel.org> wrote:

> On Wed, Nov 13, 2024 at 04:36:14PM +0100, Kory Maincent wrote:
> 
> > The regulator should not use the device parent to resolve the regulator
> > supply. It fails to resolve the correct supply when the of_node
> > variable in the regulator_config structure is not within the parent
> > node.  
> 
> I can't understand what you are saying here at all.  What is "it", why
> and in what way does it fail and why would we expect it to succeed?  How
> does your proposed change fix whatever the issue is?  The DT binding is
> against the actual device, not the virtual device.  Please describe both
> the problem and the fix more specifically.

Sorry, I was not explicit enough.
 
> > Fixes: 6261b06de565 ("regulator: Defer lookup of supply to regulator_get")  
> 
> >  static int regulator_resolve_supply(struct regulator_dev *rdev)
> >  {
> >  	struct regulator_dev *r;
> > -	struct device *dev = rdev->dev.parent;
> > +	struct device *dev = &rdev->dev;  
> 
> The rdev is a virtual device, it's not going to have any OF
> configuration

It does:
https://elixir.bootlin.com/linux/v6.11.7/source/drivers/regulator/core.c#L5687

My issue is that it does not look for the regulator supply in the OF node
described in the regulator_config structure:
https://elixir.bootlin.com/linux/v6.11.7/source/include/linux/regulator/driver.h#L445
It looks at the parent device OF node instead.

My use case is that a PSE controller have several PIs (Power Interface) which
can have different regulator supply. A regulator device is registered for each
PIs. The OF node used by the regulator core to lookup for the regulator supply
is the PSE controller node and its children instead of the node of the PI which
is described by the regulator_config->of_node.

> , and given that prior to the refactoring in the commit
> you're referencing in the Fixes: we were using the struct device passed
> into regulator_register() which should be the same device as we're using
> here so if there is an issue it doesn't look like it was introduced in
> the refactoring.  What makes you believe that there is an issue in htat
> commit?

Yes you are right I looked at it too quickly sorry.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com