| Message ID | 20220504065041.6718-1-zev@bewilderbeest.net |
|---|---|
| Headers | show |
| Series | regulator: core: Add support for external outputs | expand |
On Tue, May 03, 2022 at 11:50:35PM PDT, Zev Weiss wrote: >Hello, > >This patch series is a new approach at implementing some functionality >I previously attempted as a separate driver in drivers/misc [0], but I >think (as suggested by Greg in that thread) ultimately makes more >sense being built into the regulator subsystem. > <snip> Sorry for the broken threading here (missed a comma in the CC list on patch 1 that 'git send-email --dry-run' didn't catch, then forgot to add the necessary flags on the second attempt to tack it onto this cover letter from the first). In case it's not obvious, the patches corresponding to this are here: https://lore.kernel.org/openbmc/20220504065252.6955-1-zev@bewilderbeest.net/T/ Zev
On Tue, May 03, 2022 at 11:50:35PM -0700, Zev Weiss wrote: > The DT bindings changes (patches 1 and 2) consist of a boolean > regulator property to mark it as supplying an external output, and a > reg-external-output binding to act as a downstream device representing > that output. The redundancy between the two maybe isn't entirely > ideal, but it was the cleanest approach I've been able to come up with > so far in terms of working with the regulator subsystem; I'm certainly > open to suggestions for better ways of going about this. Nothing in the series articulates what the purpose of the redundancy is - your description of this is a consumer, why would the regulator itself care what's connected to it?
On Wed, May 04, 2022 at 05:56:13AM PDT, Mark Brown wrote: >On Tue, May 03, 2022 at 11:50:35PM -0700, Zev Weiss wrote: > >> The DT bindings changes (patches 1 and 2) consist of a boolean >> regulator property to mark it as supplying an external output, and a >> reg-external-output binding to act as a downstream device representing >> that output. The redundancy between the two maybe isn't entirely >> ideal, but it was the cleanest approach I've been able to come up with >> so far in terms of working with the regulator subsystem; I'm certainly >> open to suggestions for better ways of going about this. > >Nothing in the series articulates what the purpose of the redundancy is >- your description of this is a consumer, why would the regulator itself >care what's connected to it? Hi Mark, thanks for the review. To some extent that was an additional (maybe excessive) protective measure against regulators ending up userspace-controllable when they shouldn't be, since I had previously gotten the impression that there was some concern about that possibility. More functionally though, it was also basically a hack to allow the 'state' sysfs attribute's mode to get set properly in regulator_register(), before the consumer is known. Though if things are rearranged based on what you said in another message about putting the enable/disable control in a consumer driver instead of the regulator itself, it should be easy to get rid of. Thanks, Zev
On Wed, May 04, 2022 at 01:28:11PM -0700, Zev Weiss wrote: > To some extent that was an additional (maybe excessive) protective measure > against regulators ending up userspace-controllable when they shouldn't be, > since I had previously gotten the impression that there was some concern > about that possibility. The problem there is that userspace shouldn't be controlling the regulator, it should be controlling the things using the regulator so that whatever happens is coordinated with the rest of the management of the device.