Add the documentation which describe the voltage selection gpio support.
This property can be applied to each subnode within the 'regulators'
node so each regulator can be configured differently.
Signed-off-by: Marco Felsch <[email protected]>
---
Changelog:
v3:
- adapt binding description
Documentation/devicetree/bindings/mfd/da9062.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
index edca653a5777..b9cccd4c3f76 100644
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ b/Documentation/devicetree/bindings/mfd/da9062.txt
@@ -66,6 +66,14 @@ Sub-nodes:
details of individual regulator device can be found in:
Documentation/devicetree/bindings/regulator/regulator.txt
+ Optional regulator device-specific properties:
+ - dlg,vsel-sense-gpios : A GPIO reference to a local general purpose input,
+ the datasheet calls it GPI. The regulator sense the input signal and select
+ the active or suspend voltage settings. If the signal is active the
+ active-settings are applied else the suspend-settings are applied.
+ Attention: Sharing the same GPI for other purposes or across multiple
+ regulators is possible but the polarity setting must equal.
+
- rtc : This node defines settings required for the Real-Time Clock associated
with the DA9062. There are currently no entries in this binding, however
compatible = "dlg,da9062-rtc" should be added if a node is created.
--
2.20.1
On Fri, Nov 29, 2019 at 06:25:34PM +0100, Marco Felsch wrote:
> + Optional regulator device-specific properties:
> + - dlg,vsel-sense-gpios : A GPIO reference to a local general purpose input,
> + the datasheet calls it GPI. The regulator sense the input signal and select
> + the active or suspend voltage settings. If the signal is active the
> + active-settings are applied else the suspend-settings are applied.
> + Attention: Sharing the same GPI for other purposes or across multiple
> + regulators is possible but the polarity setting must equal.
I'm really confused by this. As far as I understand it it seems
to be doing pinmuxing on the chip using the GPIO bindings which
is itself a bit odd and I don't see anything here that configures
whatever sets the state of the pins. Don't we need another GPIO
to set the vsel-sense inputs on the PMIC?
Hi Mark,
On 19-12-04 13:46, Mark Brown wrote:
> On Fri, Nov 29, 2019 at 06:25:34PM +0100, Marco Felsch wrote:
>
> > + Optional regulator device-specific properties:
> > + - dlg,vsel-sense-gpios : A GPIO reference to a local general purpose input,
> > + the datasheet calls it GPI. The regulator sense the input signal and select
> > + the active or suspend voltage settings. If the signal is active the
> > + active-settings are applied else the suspend-settings are applied.
> > + Attention: Sharing the same GPI for other purposes or across multiple
> > + regulators is possible but the polarity setting must equal.
>
> I'm really confused by this. As far as I understand it it seems
> to be doing pinmuxing on the chip using the GPIO bindings which
> is itself a bit odd and I don't see anything here that configures
> whatever sets the state of the pins. Don't we need another GPIO
> to set the vsel-sense inputs on the PMIC?
Yes the PMIC is very configurable and it took a while till I understand
it.. @Adam please correct me if I'm wrong.
The PMIC regulators regardless of the type: ldo or buck can be
simplified drawn as:
da9062-gpio da9062-regulator
+-------------------------------------------------------
| PMIC
|
> GPIO0 +--------------------------+
| | REGULATOR-0 |
> GPIO1 -------+ | |
| +-- > vsel-in voltage-a-out <
> GPIO2 | | |
| | > enable-in voltage-b-out <
| | | |
| | +--------------------------+
| |
| | +--------------------------+
| | | REGULATOR-1 |
| | | |
| +-- > vsel-in voltage-a-out <
| | |
| > enable-in voltage-b-out <
| | |
| +--------------------------+
|
The 'vsel-in' and 'enable-in' regulator inputs must be routed to the
PMIC GPIOs which must be configured as input. If this is a pinmux in
your opinion, then yes we need to do that. IMHO it isn't a pinmux
because from the regulator point of view it is just a GPIO which comes
from our own gpio-dev (da9062-gpio). So the abstraction is vald. Anyway
I'm with you that this isn't the typical use-case.
Regards,
Marco
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 10 December 2019 09:42, Marco Felsch wrote:
> Hi Mark,
>
> On 19-12-04 13:46, Mark Brown wrote:
> > On Fri, Nov 29, 2019 at 06:25:34PM +0100, Marco Felsch wrote:
> >
> > > + Optional regulator device-specific properties:
> > > + - dlg,vsel-sense-gpios : A GPIO reference to a local general purpose input,
> > > + the datasheet calls it GPI. The regulator sense the input signal and select
> > > + the active or suspend voltage settings. If the signal is active the
> > > + active-settings are applied else the suspend-settings are applied.
> > > + Attention: Sharing the same GPI for other purposes or across multiple
> > > + regulators is possible but the polarity setting must equal.
> >
> > I'm really confused by this. As far as I understand it it seems
> > to be doing pinmuxing on the chip using the GPIO bindings which
> > is itself a bit odd and I don't see anything here that configures
> > whatever sets the state of the pins. Don't we need another GPIO
> > to set the vsel-sense inputs on the PMIC?
>
> Yes the PMIC is very configurable and it took a while till I understand
> it.. @Adam please correct me if I'm wrong.
>
> The PMIC regulators regardless of the type: ldo or buck can be
> simplified drawn as:
>
>
>
> da9062-gpio da9062-regulator
>
> +-------------------------------------------------------
> | PMIC
> |
> > GPIO0 +--------------------------+
> | | REGULATOR-0 |
> > GPIO1 -------+ | |
> | +-- > vsel-in voltage-a-out <
> > GPIO2 | | |
> | | > enable-in voltage-b-out <
> | | | |
> | | +--------------------------+
> | |
> | | +--------------------------+
> | | | REGULATOR-1 |
> | | | |
> | +-- > vsel-in voltage-a-out <
> | | |
> | > enable-in voltage-b-out <
> | | |
> | +--------------------------+
> |
>
> The 'vsel-in' and 'enable-in' regulator inputs must be routed to the
> PMIC GPIOs which must be configured as input. If this is a pinmux in
> your opinion, then yes we need to do that. IMHO it isn't a pinmux
> because from the regulator point of view it is just a GPIO which comes
> from our own gpio-dev (da9062-gpio). So the abstraction is vald. Anyway
> I'm with you that this isn't the typical use-case.
We've had this discussion before and to me it felt more like pinmux than GPIO
although I understand we're configuring the GPIO pin as input before then
configuring a regulator to take that specific internal GPIO as the control
signal. We're defining a specific role to this pin in HW rather than it being a
general software handled GPI so it feels like this would be neater under pinmux.
There does still need to be a mapping between that pin and the regulator which I
guess would be served by passing the pin to the regulator through generic pinmux
bindings and then in the regulator code you're simply just enabling the
regulator to be controlled from that pin. The HW lets you control multiple
regulators from the same input pin so there's a flexibility there to be
captured, as you mention.
>
> Regards,
> Marco
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Adam,
On 19-12-11 16:14, Adam Thomson wrote:
> On 10 December 2019 09:42, Marco Felsch wrote:
>
> > Hi Mark,
> >
> > On 19-12-04 13:46, Mark Brown wrote:
> > > On Fri, Nov 29, 2019 at 06:25:34PM +0100, Marco Felsch wrote:
> > >
> > > > + Optional regulator device-specific properties:
> > > > + - dlg,vsel-sense-gpios : A GPIO reference to a local general purpose input,
> > > > + the datasheet calls it GPI. The regulator sense the input signal and select
> > > > + the active or suspend voltage settings. If the signal is active the
> > > > + active-settings are applied else the suspend-settings are applied.
> > > > + Attention: Sharing the same GPI for other purposes or across multiple
> > > > + regulators is possible but the polarity setting must equal.
> > >
> > > I'm really confused by this. As far as I understand it it seems
> > > to be doing pinmuxing on the chip using the GPIO bindings which
> > > is itself a bit odd and I don't see anything here that configures
> > > whatever sets the state of the pins. Don't we need another GPIO
> > > to set the vsel-sense inputs on the PMIC?
> >
> > Yes the PMIC is very configurable and it took a while till I understand
> > it.. @Adam please correct me if I'm wrong.
> >
> > The PMIC regulators regardless of the type: ldo or buck can be
> > simplified drawn as:
> >
> >
> >
> > da9062-gpio da9062-regulator
> >
> > +-------------------------------------------------------
> > | PMIC
> > |
> > > GPIO0 +--------------------------+
> > | | REGULATOR-0 |
> > > GPIO1 -------+ | |
> > | +-- > vsel-in voltage-a-out <
> > > GPIO2 | | |
> > | | > enable-in voltage-b-out <
> > | | | |
> > | | +--------------------------+
> > | |
> > | | +--------------------------+
> > | | | REGULATOR-1 |
> > | | | |
> > | +-- > vsel-in voltage-a-out <
> > | | |
> > | > enable-in voltage-b-out <
> > | | |
> > | +--------------------------+
> > |
> >
> > The 'vsel-in' and 'enable-in' regulator inputs must be routed to the
> > PMIC GPIOs which must be configured as input. If this is a pinmux in
> > your opinion, then yes we need to do that. IMHO it isn't a pinmux
> > because from the regulator point of view it is just a GPIO which comes
> > from our own gpio-dev (da9062-gpio). So the abstraction is vald. Anyway
> > I'm with you that this isn't the typical use-case.
>
> We've had this discussion before and to me it felt more like pinmux than GPIO
> although I understand we're configuring the GPIO pin as input before then
> configuring a regulator to take that specific internal GPIO as the control
> signal. We're defining a specific role to this pin in HW rather than it being a
> general software handled GPI so it feels like this would be neater under pinmux.
> There does still need to be a mapping between that pin and the regulator which I
> guess would be served by passing the pin to the regulator through generic pinmux
> bindings and then in the regulator code you're simply just enabling the
> regulator to be controlled from that pin. The HW lets you control multiple
> regulators from the same input pin so there's a flexibility there to be
> captured, as you mention.
I know that we already had this discussion but the result was to wait
for the maintainers input. Since Linus is the pinctrl/gpio maintainer
and Mark the regulator maintainer we now have some input so we can move
forward. Linus made some comments on the dt-bindings and on the code but
he didn't pointed out that this usage is wrong. So I guessed it would be
fine for him. Mark did his first comments now and I explained the
current state..
I discussed it with a colleague again and he mentioned that pinctrl
should be named pinctrl instead it should be named padctrl. We don't
reconfigure the pad to a other function it is still a device general
purpose input pad. The hw-signal flow goes always trough the gpio block
so one argument more for my solution. Also we don't configure the "pad"
to be a vsel/ena-pin. The hw-pad can only be a gpio or has an alternate
function (WDKICK for GPIO0, Seq. SYS_EN for GPIO2, Seq. PWR_EN for GPIO4).
Instead we tell the regulator to use _this_ GPIO e.g. for voltage
selection so we go the other way around. My last argument why pinctrl
isn't the correct place is that the GPIO1 can be used for
regulator-0:vsel-in and for regulator-1:enable-in. So this pad would
have different states which is invalid IMHO.
Regards,
Marco
> > Regards,
> > Marco
> >
> > --
> > Pengutronix e.K. | |
> > Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Wed, Dec 11, 2019 at 6:09 PM Marco Felsch <[email protected]> wrote:
> I discussed it with a colleague again and he mentioned that pinctrl
> should be named pinctrl instead it should be named padctrl.
Quoting Documentation/driver-api/pinctl.rst:
(...)
Definition of PIN:
- PINS are equal to pads, fingers, balls or whatever packaging input or
output line you want to control and these are denoted by unsigned integers
in the range 0..maxpin.
(...)
> We don't
> reconfigure the pad to a other function it is still a device general
> purpose input pad. The hw-signal flow goes always trough the gpio block
> so one argument more for my solution. Also we don't configure the "pad"
> to be a vsel/ena-pin. The hw-pad can only be a gpio or has an alternate
> function (WDKICK for GPIO0, Seq. SYS_EN for GPIO2, Seq. PWR_EN for GPIO4).
> Instead we tell the regulator to use _this_ GPIO e.g. for voltage
> selection so we go the other way around. My last argument why pinctrl
> isn't the correct place is that the GPIO1 can be used for
> regulator-0:vsel-in and for regulator-1:enable-in. So this pad would
> have different states which is invalid IMHO.
Yeah it is just one of these cases where the silicon designer pulled
a line of polysilicone over to the regulator enable signal and put a
switch on it and say "so you can also enable the regulator
with a signal from here", it can be used in parallel with anything
else, which is especially messy.
Special cases require special handling, since the electronic design
of this thing is a bit Rube Goldberg.
Yours,
Linus Walleij
Hi,
On 19-12-12 16:08, Linus Walleij wrote:
> On Wed, Dec 11, 2019 at 6:09 PM Marco Felsch <[email protected]> wrote:
>
> > I discussed it with a colleague again and he mentioned that pinctrl
> > should be named pinctrl instead it should be named padctrl.
>
> Quoting Documentation/driver-api/pinctl.rst:
>
> (...)
> Definition of PIN:
>
> - PINS are equal to pads, fingers, balls or whatever packaging input or
> output line you want to control and these are denoted by unsigned integers
> in the range 0..maxpin.
> (...)
Okay there is the definition.
> > We don't
> > reconfigure the pad to a other function it is still a device general
> > purpose input pad. The hw-signal flow goes always trough the gpio block
> > so one argument more for my solution. Also we don't configure the "pad"
> > to be a vsel/ena-pin. The hw-pad can only be a gpio or has an alternate
> > function (WDKICK for GPIO0, Seq. SYS_EN for GPIO2, Seq. PWR_EN for GPIO4).
> > Instead we tell the regulator to use _this_ GPIO e.g. for voltage
> > selection so we go the other way around. My last argument why pinctrl
> > isn't the correct place is that the GPIO1 can be used for
> > regulator-0:vsel-in and for regulator-1:enable-in. So this pad would
> > have different states which is invalid IMHO.
>
> Yeah it is just one of these cases where the silicon designer pulled
> a line of polysilicone over to the regulator enable signal and put a
> switch on it and say "so you can also enable the regulator
> with a signal from here", it can be used in parallel with anything
> else, which is especially messy.
I didn't say that the design isn't messy ;) I just wanna make the right
abstraction and IMHO this is the correct abstraction.
Regards,
Marco
> Special cases require special handling, since the electronic design
> of this thing is a bit Rube Goldberg.
>
> Yours,
> Linus Walleij
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Wed, Dec 11, 2019 at 06:09:18PM +0100, Marco Felsch wrote:
> so one argument more for my solution. Also we don't configure the "pad"
> to be a vsel/ena-pin. The hw-pad can only be a gpio or has an alternate
> function (WDKICK for GPIO0, Seq. SYS_EN for GPIO2, Seq. PWR_EN for GPIO4).
> Instead we tell the regulator to use _this_ GPIO e.g. for voltage
> selection so we go the other way around. My last argument why pinctrl
> isn't the correct place is that the GPIO1 can be used for
> regulator-0:vsel-in and for regulator-1:enable-in. So this pad would
> have different states which is invalid IMHO.
Note that there's two bits to my concern - one is if we should be using
gpiolib or pinctrl, the other is what's driving the input (whichever API
it's configured through) which didn't seem to be mentioned.
On 19-12-12 16:10, Mark Brown wrote:
> On Wed, Dec 11, 2019 at 06:09:18PM +0100, Marco Felsch wrote:
>
> > so one argument more for my solution. Also we don't configure the "pad"
> > to be a vsel/ena-pin. The hw-pad can only be a gpio or has an alternate
> > function (WDKICK for GPIO0, Seq. SYS_EN for GPIO2, Seq. PWR_EN for GPIO4).
> > Instead we tell the regulator to use _this_ GPIO e.g. for voltage
> > selection so we go the other way around. My last argument why pinctrl
> > isn't the correct place is that the GPIO1 can be used for
> > regulator-0:vsel-in and for regulator-1:enable-in. So this pad would
> > have different states which is invalid IMHO.
>
> Note that there's two bits to my concern - one is if we should be using
> gpiolib or pinctrl, the other is what's driving the input (whichever API
> it's configured through) which didn't seem to be mentioned.
gpiolib or pinctrl:
I pointed out all my arguments above so I think it is time for Linus.
"... what's driving the input ..":
Sorry I didn't get you here. What did you mean? The input is driven by
the host. This can be any gpio line and in my case it is a gpio line
driven by the soc-hw during a suspend operation.
Regards,
Marco
On Thu, Dec 12, 2019 at 05:21:53PM +0100, Marco Felsch wrote:
> "... what's driving the input ..":
> Sorry I didn't get you here. What did you mean? The input is driven by
> the host. This can be any gpio line and in my case it is a gpio line
> driven by the soc-hw during a suspend operation.
Something needs to say what that thing is, especially if it's runtime
controllable. In your case from the point of view of software there is
actually no enable control so we shouldn't be providing an enable
operation to the framework.
Hi Mark,
On 19-12-17 12:58, Mark Brown wrote:
> On Tue, Dec 17, 2019 at 08:35:33AM +0100, Marco Felsch wrote:
> > On 19-12-16 11:44, Mark Brown wrote:
>
> > > What I'm saying is that I think the binding needs to explicitly talk
> > > about that since at the minute it's really confusing reading it as it
> > > is, it sounds very much like it's trying to override that in a chip
> > > specific fashion as using gpiolib and the GPIO bindings for pinmuxing is
> > > really quite unusual.
>
> > Hm.. I still think that we don't mux the pin to some special function.
> > It is still a gpio input pin and if we don't request the pin we could
> > read the input from user-space too and get a 'valid' value. Muxing would
> > happen if we change the pad to so called _alternate_ function. Anyway,
> > lets find a binding description:
>
> I don't think any of this makes much difference from a user point of
> view.
>
> > IMHO this is very descriptive and needs no update.
>
> > description:
> > - A GPIO reference to a local general purpose input, [1] calls it GPI.
> > The DA9062 regulators can select between voltage-a/-b settings.
> > Each regulator has a VBUCK*_GPI or VLDO*_GPI input to determine the
> > active setting. In front of the VBUCK*_GPI/VLDO*_GPI input is a mux
> > to select between different signal sources, valid sources are: the
> > internal sequencer, GPI1, GPI2 and GPI3. See [1] table 63 for more
> > information. Most the time the internal sequencer is fine but
> > sometimes it is necessary to use the signal from the DA9062 GPI
> > pads. This binding covers the second use case.
> > Attention: Sharing the same GPI for other purposes or across multiple
> > regulators is possible but the polarity setting must equal.
>
> This doesn't say anything about how the GPIO input is expected to be
> controlled, for voltage setting any runtime control would need to be
> done by the driver and it sounds like that's all that can be controlled.
> The way this reads I'd expect one use of this to be for fast voltage
> setting for example (you could even combine that with suspend sequencing
> using the internal sequencer if you mux back to the sequencer during
> suspend).
The input signal is routed trough the da9062 gpio block to the
regualtors. You can't set any voltage value using a gpio instead you
decide which voltage setting is applied. The voltage values for
runtime/suspend comes from the dt-data. No it's not just a fast
switching option imagine the system suspend case where the cpu and soc
voltage can be reduced to a very low value. Older soc's like the imx6
signaling this state by a hard wired gpio line because the soc and
cpu cores don't work properly on such low voltage values. This is
my use case and I can't use the sequencer.
Regards,
Marco
On Tue, Jan 07, 2020 at 09:36:54AM +0100, Marco Felsch wrote:
> On 19-12-17 12:58, Mark Brown wrote:
> > This doesn't say anything about how the GPIO input is expected to be
> > controlled, for voltage setting any runtime control would need to be
> > done by the driver and it sounds like that's all that can be controlled.
> > The way this reads I'd expect one use of this to be for fast voltage
> > setting for example (you could even combine that with suspend sequencing
> > using the internal sequencer if you mux back to the sequencer during
> > suspend).
> The input signal is routed trough the da9062 gpio block to the
> regualtors. You can't set any voltage value using a gpio instead you
> decide which voltage setting is applied. The voltage values for
> runtime/suspend comes from the dt-data. No it's not just a fast
> switching option imagine the system suspend case where the cpu and soc
> voltage can be reduced to a very low value. Older soc's like the imx6
> signaling this state by a hard wired gpio line because the soc and
> cpu cores don't work properly on such low voltage values. This is
> my use case and I can't use the sequencer.
My point is that I can't tell any of this from the description.
On 20-01-07 13:09, Mark Brown wrote:
> On Tue, Jan 07, 2020 at 09:36:54AM +0100, Marco Felsch wrote:
> > On 19-12-17 12:58, Mark Brown wrote:
>
> > > This doesn't say anything about how the GPIO input is expected to be
> > > controlled, for voltage setting any runtime control would need to be
> > > done by the driver and it sounds like that's all that can be controlled.
> > > The way this reads I'd expect one use of this to be for fast voltage
> > > setting for example (you could even combine that with suspend sequencing
> > > using the internal sequencer if you mux back to the sequencer during
> > > suspend).
>
> > The input signal is routed trough the da9062 gpio block to the
> > regualtors. You can't set any voltage value using a gpio instead you
> > decide which voltage setting is applied. The voltage values for
> > runtime/suspend comes from the dt-data. No it's not just a fast
> > switching option imagine the system suspend case where the cpu and soc
> > voltage can be reduced to a very low value. Older soc's like the imx6
> > signaling this state by a hard wired gpio line because the soc and
> > cpu cores don't work properly on such low voltage values. This is
> > my use case and I can't use the sequencer.
>
> My point is that I can't tell any of this from the description.
Therefore I want to discuss the dt-binding documentation with you and
the others to get this done. Is the above description better to
understand the dt-binding?
Regards,
Marco
On Tue, Jan 07, 2020 at 02:38:11PM +0100, Marco Felsch wrote:
> On 20-01-07 13:09, Mark Brown wrote:
> > On Tue, Jan 07, 2020 at 09:36:54AM +0100, Marco Felsch wrote:
> > > The input signal is routed trough the da9062 gpio block to the
> > > regualtors. You can't set any voltage value using a gpio instead you
> > > decide which voltage setting is applied. The voltage values for
> > > runtime/suspend comes from the dt-data. No it's not just a fast
> > > switching option imagine the system suspend case where the cpu and soc
> > > voltage can be reduced to a very low value. Older soc's like the imx6
> > > signaling this state by a hard wired gpio line because the soc and
> > > cpu cores don't work properly on such low voltage values. This is
> > > my use case and I can't use the sequencer.
> > My point is that I can't tell any of this from the description.
> Therefore I want to discuss the dt-binding documentation with you and
> the others to get this done. Is the above description better to
> understand the dt-binding?
That text really doesn't feel like text that'd be idiomatic
directly in a binding document but some of those ideas probably
do need to be in the text I think.