Messages in this thread | | | Date | Thu, 16 Feb 2012 23:25:43 -0800 | From | Mark Brown <> | 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. [unhandled content-type:application/pgp-signature] | |