linux-kernel - Re: [PATCH 2/3] regulator: add set_pulldown in regulator operations

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] [day] [month] [year] [list]

Message-ID: <20120217072542.GC11464@opensource.wolfsonmicro.com>
Date:	Thu, 16 Feb 2012 23:25:43 -0800
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	"Kim, Milo" <Milo.Kim@...com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] regulator: add set_pulldown in regulator operations

On Thu, Feb 16, 2012 at 10:41:45PM -0800, Kim, Milo wrote:
> Some PMUs provide programmable active shutdown switches
> that help discharge the load if the supply is off.
> For providing this feature, set_pulldown() is added in the ops.

> The set_pulldown() is not exported function.
> This function is called internally when the regulator is disabled.
> Then pull-down bits can be programmed in each regulator driver.

There's several problems here.  One is that "pulldown" is a really
confusing name as a pulldown is usually a feature of open drain logic.
"Discharge" would be a better term.

> +			if (rdev->desc->ops->set_pulldown) {
> +				ret = rdev->desc->ops->set_pulldown(rdev);
> +				if (ret < 0) {
> +					rdev_err(rdev,
> +						"failed to set pulldown\n");
> +					return ret;
> +				}

Another is that since the operation is defined not to take any arguments
there's no need to have an operation - the driver may as well just
enable the discharge unconditionally at system startup as it can never
be disabled.  For a lot of hardware this is all that's needed anyway as
the device is smart enough to remove the discharge when the regulator is
enabled.

It's also not altogether clear that actively discharging regulators is
a good default - often it'll increase leakage current a tiny amount and
nothing really cares that much about how quickly the voltage collapses.

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