Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1311090 > unrolled thread

Re: [PATCH v2 1/2] regulator: Add coupled regulator

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 1/2] regulator: Add coupled regulator Rob Herring <robh@kernel.org> - 2016-01-17 01:10 +0100
    Re: [PATCH v2 1/2] regulator: Add coupled regulator Mark Brown <broonie@kernel.org> - 2016-01-18 17:30 +0100
      Re: [PATCH v2 1/2] regulator: Add coupled regulator Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-01-21 16:50 +0100
        Re: [PATCH v2 1/2] regulator: Add coupled regulator Mark Brown <broonie@kernel.org> - 2016-01-21 17:30 +0100

#1311090 — Re: [PATCH v2 1/2] regulator: Add coupled regulator

On Fri, Jan 15, 2016 at 09:57:34AM +0100, Maxime Ripard wrote:
> Hi Rob,
> 
> On Tue, Jan 12, 2016 at 08:31:00AM -0600, Rob Herring wrote:
> > On Tue, Jan 12, 2016 at 02:37:21PM +0100, Maxime Ripard wrote:
> > > Some boards, in order to power devices that have a quite high power
> > > consumption, wire multiple regulators in parallel.
> > > 
> > > In such a case, the regulators need to be kept in sync, all of them being
> > > enabled or disabled in parallel.
> > > 
> > > This also requires to expose only the voltages that are common to all the
> > > regulators.

[...]

> > > +Coupled voltage regulators
> > > +
> > > +Required properties:
> > > +- compatible		: Must be "coupled-voltage-regulator".
> > > +
> > > +Optional properties:
> > > +- vinX-supply		: Phandle to the regulators it aggregates
> > > +
> > > +Any property defined as part of the core regulator binding defined in
> > > +regulator.txt can also be used.
> > > +
> > > +Example:
> > > +	vcc_wifi: wifi_reg {
> > > +		compatible = "coupled-voltage-regulator";
> > > +		regulator-name = "vcc-wifi";
> > > +		vin0-supply = <&reg_ldo3>;
> > > +		vin1-supply = <&reg_ldo4>;
> > > +	};
> > 
> > Why not just make ?-supply a list of phandles? That would be simpler 
> > than a virtual regulator.
> 
> I'm not sure I get what you're saying. Do you want to remove that
> driver entirely, or just allow the -supply properties in the device
> tree to take a list?

Both actually, I think. If a power supply input for a device has 2 
supplies connected then list both of them for that device rather than 
create a virtual device in DT. Of course, you could keep a virtual 
regulator independent of DT. That is more of an implementation choice.

> In the former case, the rationale behind this driver is that the
> regulators powering a device also have to be kept in sync, both by
> enabling and disabling all of them at once, but also by all having
> them at the same voltages.

None of this has anything to do with the binding other than you can 
write a regulator driver and already have the mechanism to instantiate 
it with DT.

> We could push that code in the consumer drivers, but that has some
> significant drawbacks:
>   - That would duplicate that code in all the drivers, leading to the
>     usual drawbacks of code duplication, especially when it's not
>     really trivial to handle (or at least, when there's a few
>     gotchas).

Either you could keep the driver and the consumer driver is responsible 
for instantiating the regulator. It could also be implemented as a helper 
library in the regulator core. 

>   - When you come to consider it from an hardware point of view, the
>     device usually have a single pin that powers it. It's the board
>     designer that chose to route that pin to multiple regulators, so
>     it's really the board that is wired that way, and putting that
>     code in the consumer drivers would be an abstraction leak imho.

That's a good point. Perhaps the regulator core needs to be able to 
parse the list and return the single ptr to the virtual regulator.

>   - We might not even have a driver for these regulators, or at least
>     one that play by the rules. In our case, that's an out-of-tree
>     WiFi driver.

Support of out of tree things has never been a winning argument for 
upstream.

With an out of tree binding as well?

> In the latter case, I remember Mark saying several times that he was
> not in favour of such a change, even recently:
> https://lkml.org/lkml/2015/10/14/238

I think this case is somewhat different. What I'm suggesting is closer 
to the hardware which is what Mark also argued for, but we should let 
him speak. If the input supply name on a device is VCC and you have REG1 
and REG2 attached, then having "vcc-supply = <&REG1>, <&REG2>;" makes 
perfect sense in terms of matching the h/w.

Also, simplefb is not a good thing to compare with. 

Rob

[toc] | [next] | [standalone]

#1311656

[Multipart message — attachments visible in raw view] — view raw

On Sat, Jan 16, 2016 at 06:04:34PM -0600, Rob Herring wrote:
> On Fri, Jan 15, 2016 at 09:57:34AM +0100, Maxime Ripard wrote:

> > We could push that code in the consumer drivers, but that has some
> > significant drawbacks:
> >   - That would duplicate that code in all the drivers, leading to the
> >     usual drawbacks of code duplication, especially when it's not
> >     really trivial to handle (or at least, when there's a few
> >     gotchas).

> Either you could keep the driver and the consumer driver is responsible 
> for instantiating the regulator. It could also be implemented as a helper 
> library in the regulator core. 

No, anything that is visible to consumer drivers is completely silly.
The whole point with both the DT bindings and the API is to hide details
of how the supply is implemented from the consumers, any implementation
must be transparent to consumers otherwise it's unusable.

> >   - When you come to consider it from an hardware point of view, the
> >     device usually have a single pin that powers it. It's the board
> >     designer that chose to route that pin to multiple regulators, so
> >     it's really the board that is wired that way, and putting that
> >     code in the consumer drivers would be an abstraction leak imho.

> That's a good point. Perhaps the regulator core needs to be able to 
> parse the list and return the single ptr to the virtual regulator.

Exactly, if we don't want to represent the combination directly.  For
most uses it's probably OK but I can see us in a situation where we
might want to do things like only use one of the regulators in low load
situations where we might want to attach properties to the merge of the
two regulators rather than just referencing them both.  I'm not sure
that's realistic though or that we wouldn't just be working that use
case out dynamically at runtime.

I'm ambivalent on which way is better, it does complicate the
implementation to support doing this as lists and while it makes the DT
more elegant I'm not clear that it's worth the effort especially when it
comes to constraint combining.  But perhaps the implementation turns out
to be simpler than I would anticiapte.

> >   - We might not even have a driver for these regulators, or at least
> >     one that play by the rules. In our case, that's an out-of-tree
> >     WiFi driver.

> Support of out of tree things has never been a winning argument for 
> upstream.

> With an out of tree binding as well?

Since the consumer driver shouldn't be worrying about implementation
details of the supply it doesn't matter if the device on this particular
board is out of tree.

> > In the latter case, I remember Mark saying several times that he was
> > not in favour of such a change, even recently:
> > https://lkml.org/lkml/2015/10/14/238

> I think this case is somewhat different. What I'm suggesting is closer 

Yes, that's a different case - that's the case where all the supplies
in the device are combined into a single list rather than the case where
we have one supply with multiple regulators providing it.

[toc] | [prev] | [next] | [standalone]

#1314289

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Mon, Jan 18, 2016 at 04:25:38PM +0000, Mark Brown wrote:
> > >   - When you come to consider it from an hardware point of view, the
> > >     device usually have a single pin that powers it. It's the board
> > >     designer that chose to route that pin to multiple regulators, so
> > >     it's really the board that is wired that way, and putting that
> > >     code in the consumer drivers would be an abstraction leak imho.
> 
> > That's a good point. Perhaps the regulator core needs to be able to 
> > parse the list and return the single ptr to the virtual regulator.
> 
> Exactly, if we don't want to represent the combination directly.  For
> most uses it's probably OK but I can see us in a situation where we
> might want to do things like only use one of the regulators in low load
> situations where we might want to attach properties to the merge of the
> two regulators rather than just referencing them both.  I'm not sure
> that's realistic though or that we wouldn't just be working that use
> case out dynamically at runtime.
> 
> I'm ambivalent on which way is better, it does complicate the
> implementation to support doing this as lists and while it makes the DT
> more elegant I'm not clear that it's worth the effort especially when it
> comes to constraint combining.  But perhaps the implementation turns out
> to be simpler than I would anticiapte.

I guess a separate driver would make it easier to deal with cases like
the one you suggested (shutting down when the load is going to be
lower). I don't see how we could have a good DT representation of that
if we're going to use lists.

Anyway, I'm fine with both approaches, just let me know what you
prefer.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1314333

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jan 21, 2016 at 04:46:49PM +0100, Maxime Ripard wrote:

> I guess a separate driver would make it easier to deal with cases like
> the one you suggested (shutting down when the load is going to be
> lower). I don't see how we could have a good DT representation of that
> if we're going to use lists.

We can have a driver regardless of what the DT looks like, it's a
question of how things get instantiated.

> Anyway, I'm fine with both approaches, just let me know what you
> prefer.

Without seeing an implementation of the lists it's hard to say.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web