Re: [PATCH 0/6] regulator: core: Add support for external outputs

From: Zev Weiss
Date: Wed May 04 2022 - 16:28:21 EST


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