linux-kernel - Re: [Patch v2 2/7] Regulator: DA9055 Regulator driver

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 10 Oct 2012 12:56:23 +0900
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Ashish Jangam <ashish.jangam@...tcummins.com>
Cc:	Liam Girdwood <lrg@...com>, Samuel Ortiz <sameo@...ux.intel.com>,
	David Dajun Chen <dchen@...semi.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [Patch v2 2/7] Regulator: DA9055 Regulator driver

On Tue, Oct 09, 2012 at 04:30:16PM +0530, Ashish Jangam wrote:
> On Tue, 2012-10-09 at 15:37 +0530, Mark Brown wrote:
> > On Mon, Oct 08, 2012 at 07:00:39PM +0530, Ashish Jangam wrote:

> > > +		/* Set the GPIO I/P pin for controlling the regulator state. */
> > > +		ret = devm_gpio_request_one(config->dev, gpio, GPIOF_DIR_IN,
> > > +					    name);
> > > +		if (ret < 0)
> > > +			goto err;

> > We never actually appear to use this GPIO anywhere...  why are we
> > requesting it?

> DA9055 regulator changes its state by detecting the rising/failing edge at
> GPI DA9055. Therefore we just need to set the DA9055 GPIO direction to input.

Right, so there's several problems here.  One is that this code is very
obscure - you're really doing pinmux here rather than actually using it
as a GPIO, a better comment would clarify this.  The other is that
you're requiring a defined gpio_base in platform data, it would be
better to allow this to be dynamically assigned as the driver can find
it's own GPIOs easily enough.

> >   Also, why is the ability to read the regulator state via
> > a GPIO associated with controlling it via a GPIO, it's unusual for these
> > things to be tied together.

> There is no connection between state just to differentiate between two strings/labels.
> If required I can change the string.

It's nothing to do with the name, it's that it looks due to the above
like the input GPIO is used by the CPU to read the state of the device.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/