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

[Mark Brown]

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.

Attachment: signature.asc
Description: Digital signature