linux-kernel - Re: [PATCH 3/5] regulator: axp20x: Update the bindings to use a local parent regulator

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: <20140605142729.GG5765@lukather>
Date:	Thu, 5 Jun 2014 16:27:29 +0200
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	Mark Brown <broonie@...nel.org>
Cc:	carlo@...one.org, Boris Brezillon <boris@...e-electrons.com>,
	lgirdwood@...il.com, lee.jones@...aro.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	kevin.z.m.zh@...il.com, sunny@...winnertech.com,
	shuge@...winnertech.com, zhuzhenhua@...winnertech.com
Subject: Re: [PATCH 3/5] regulator: axp20x: Update the bindings to use a
 local parent regulator

On Tue, Jun 03, 2014 at 03:43:19PM +0100, Mark Brown wrote:
> On Tue, Jun 03, 2014 at 03:12:04PM +0200, Maxime Ripard wrote:
> > On Wed, May 28, 2014 at 07:50:51PM +0100, Mark Brown wrote:
> > > On Wed, May 28, 2014 at 07:11:07PM +0200, Maxime Ripard wrote:
> 
> > > >  		axp_mic_reg: ldo5 {
> > > > +			vin-supply = <&axp_ipsout_reg>;
> 
> > > This is saying that every single regulator on the device is powered by a
> > > (presumably) single input called "vin".  This would certainly be an
> > > unusual package design...  As ever the name of the supply should follow
> > > the name of the pin on the device.
> 
> > Not exactly. Each regulator has its own input pin, named
> > <regulator_name>in in the schematics, with the exception of LDO2 and
> > LDO4 that share the same pin.
> 
> OK, so the supplies should be called <regulator_name>in then not vin.

Which they already do. So I guess we don't have to change the bindings
after all. One last thing I fail to understand though, is why do we
need to have that duplication of the informations between the mfd
driver and the regulator one.

You already list the regulators available and their supply in the
regulator driver, why do you need to set the regulator parents in the
mfd driver as well?

My guess is that it's to work around the fact that
regulator_dev_lookup only looks for the regulator's device of_node (so
not the PMIC one, but one of its child), which doesn't have the supply
properties, and then just falls back on the regulator alias
list. Would it make some sense to add a lookup in the parent device
of_node (which would be the "main" PMIC node in our case)?

Also, there's also the fact that all the supply properties seems to
also be mandatory in the DT, even though the regulator itself might
not be used at all on the board, and the input voltage not wired to
anything.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)