[PATCH] pinctrl: Enable "power-source" to be extracted from DT files

[PATCH] pinctrl: Enable "power-source" to be extracted from DT files

[Ivan T. Ivanov]

From: "Ivan T. Ivanov" <iivanov@mm-sol.com>

Add "power-source" property to generic options used for DT parsing files.
This  enables drivers, which use generic pin configurations, to get the
value passed to this property.

Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
---
 drivers/pinctrl/pinconf-generic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 3d9a999..c7ead91 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -166,6 +166,7 @@ static struct pinconf_generic_dt_params dt_params[] = {
 	{ "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 },
 	{ "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 },
 	{ "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 },
+	{ "power-source", PIN_CONFIG_POWER_SOURCE, 0 },
 	{ "low-power-enable", PIN_CONFIG_LOW_POWER_MODE, 1 },
 	{ "low-power-disable", PIN_CONFIG_LOW_POWER_MODE, 0 },
 	{ "output-low", PIN_CONFIG_OUTPUT, 0, },
-- 
1.8.3.2

Re: [PATCH] pinctrl: Enable "power-source" to be extracted from DT files

[Linus Walleij]

On Wed, May 21, 2014 at 5:14 PM, Ivan T. Ivanov <iivanov@mm-sol.com> wrote:

> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
>
> Add "power-source" property to generic options used for DT parsing files.
> This  enables drivers, which use generic pin configurations, to get the
> value passed to this property.
>
> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>

Great! But please also patch:
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

In this same patch to add the binding, and respin.

Yours,
Linus Walleij

Re: [PATCH] pinctrl: Enable "power-source" to be extracted from DT files

[Ivan T. Ivanov]

On Fri, 2014-05-23 at 15:58 +0200, Linus Walleij wrote:
> On Wed, May 21, 2014 at 5:14 PM, Ivan T. Ivanov <iivanov@mm-sol.com> wrote:
> 
> > From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> >
> > Add "power-source" property to generic options used for DT parsing files.
> > This  enables drivers, which use generic pin configurations, to get the
> > value passed to this property.
> >
> > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> 
> Great! But please also patch:
> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> 
> In this same patch to add the binding, and respin.


Sure. Thanks

Ivan

> 
> Yours,
> Linus Walleij

[PATCH v2] pinctrl: Enable "power-source" to be extracted from DT files

[Ivan T. Ivanov]

From: "Ivan T. Ivanov" <iivanov@mm-sol.com>

Add "power-source" property to generic options used for DT parsing files.
This  enables drivers, which use generic pin configurations, to get the
value passed to this property.

Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
---
 Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt | 1 +
 drivers/pinctrl/pinconf-generic.c                              | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index 4414163..fa40a17 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -156,6 +156,7 @@ input-disable		- disable input on pin (no effect on output)
 input-schmitt-enable	- enable schmitt-trigger mode
 input-schmitt-disable	- disable schmitt-trigger mode
 input-debounce		- debounce mode with debound time X
+power-source		- select between different power supplies
 low-power-enable	- enable low power mode
 low-power-disable	- disable low power mode
 output-low		- set the pin to output mode with low level
diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 3d9a999..c7ead91 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -166,6 +166,7 @@ static struct pinconf_generic_dt_params dt_params[] = {
 	{ "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 },
 	{ "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 },
 	{ "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 },
+	{ "power-source", PIN_CONFIG_POWER_SOURCE, 0 },
 	{ "low-power-enable", PIN_CONFIG_LOW_POWER_MODE, 1 },
 	{ "low-power-disable", PIN_CONFIG_LOW_POWER_MODE, 0 },
 	{ "output-low", PIN_CONFIG_OUTPUT, 0, },
--
1.8.3.2

Re: [PATCH v2] pinctrl: Enable "power-source" to be extracted from DT files

[Heiko Stübner]

Am Dienstag, 27. Mai 2014, 09:27:36 schrieb Ivan T. Ivanov:
> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> 
> Add "power-source" property to generic options used for DT parsing files.
> This  enables drivers, which use generic pin configurations, to get the
> value passed to this property.

I think the main problem here is, that pinconf-generic.h defines the power-
source as having a "custom format". With DT as a hardware description, 
implementaton specific values do not work well - instead it should have a 
regular unit-value.

For the power-source I think volts could work well - as this is the main use-
case for pinctrl I know. The regulator-binding uses microvolts, maybe it would 
be good use a similar unit.

So something along

--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -156,6 +156,7 @@ input-disable		- disable input on pin (no effect on
output) input-schmitt-enable	- enable schmitt-trigger mode
 input-schmitt-disable	- disable schmitt-trigger mode
 input-debounce		- debounce mode with debound time X
+power-source		- set power supply to X microvolts
 low-power-enable	- enable low power mode
 low-power-disable	- disable low power mode
 output-low		- set the pin to output mode with low level

and in the dt itself:

	pcfg_1v8 {
		power-source = <1800000>;
	};

And then the pinctrl driver being responsible to select an appropriate source 
for this voltage.

But I guess Linus knows more pinctrl corner-cases than me, so this may be 
entire rubbish :-) .


Heiko

> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> ---
>  Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt | 1 +
>  drivers/pinctrl/pinconf-generic.c                              | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt index
> 4414163..fa40a17 100644
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> @@ -156,6 +156,7 @@ input-disable		- disable input on pin (no effect on
> output) input-schmitt-enable	- enable schmitt-trigger mode
>  input-schmitt-disable	- disable schmitt-trigger mode
>  input-debounce		- debounce mode with debound time X
> +power-source		- select between different power supplies
>  low-power-enable	- enable low power mode
>  low-power-disable	- disable low power mode
>  output-low		- set the pin to output mode with low level
> diff --git a/drivers/pinctrl/pinconf-generic.c
> b/drivers/pinctrl/pinconf-generic.c index 3d9a999..c7ead91 100644
> --- a/drivers/pinctrl/pinconf-generic.c
> +++ b/drivers/pinctrl/pinconf-generic.c
> @@ -166,6 +166,7 @@ static struct pinconf_generic_dt_params dt_params[] = {
>  	{ "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 },
>  	{ "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 },
>  	{ "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 },
> +	{ "power-source", PIN_CONFIG_POWER_SOURCE, 0 },
>  	{ "low-power-enable", PIN_CONFIG_LOW_POWER_MODE, 1 },
>  	{ "low-power-disable", PIN_CONFIG_LOW_POWER_MODE, 0 },
>  	{ "output-low", PIN_CONFIG_OUTPUT, 0, },
> --
> 1.8.3.2

Re: [PATCH v2] pinctrl: Enable "power-source" to be extracted from DT files

[Linus Walleij]

On Tue, May 27, 2014 at 3:09 PM, Heiko Stübner <heiko@sntech.de> wrote:
> Am Dienstag, 27. Mai 2014, 09:27:36 schrieb Ivan T. Ivanov:
>> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
>>
>> Add "power-source" property to generic options used for DT parsing files.
>> This  enables drivers, which use generic pin configurations, to get the
>> value passed to this property.
>
> I think the main problem here is, that pinconf-generic.h defines the power-
> source as having a "custom format". With DT as a hardware description,
> implementaton specific values do not work well - instead it should have a
> regular unit-value.
>
> For the power-source I think volts could work well - as this is the main use-
> case for pinctrl I know. The regulator-binding uses microvolts, maybe it would
> be good use a similar unit.
(...)
>         pcfg_1v8 {
>                 power-source = <1800000>;
>         };

I don't think this is what it's meant for actually. In this case the thing would
be modeled as a regulator rather than some pin control option I guess?

I think it's more like a selector, such as found on old amplifiers,
input source A, B or C? So it's just some discrete number.

Then we don't know which voltage comes from power source A, B, C.

Yours,
Linus Walleij

Re: [PATCH v2] pinctrl: Enable "power-source" to be extracted from DT files

[Heiko Stübner]

Am Mittwoch, 28. Mai 2014, 10:15:43 schrieb Linus Walleij:
> On Tue, May 27, 2014 at 3:09 PM, Heiko Stübner <heiko@sntech.de> wrote:
> > Am Dienstag, 27. Mai 2014, 09:27:36 schrieb Ivan T. Ivanov:
> >> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> >> 
> >> Add "power-source" property to generic options used for DT parsing files.
> >> This  enables drivers, which use generic pin configurations, to get the
> >> value passed to this property.
> > 
> > I think the main problem here is, that pinconf-generic.h defines the
> > power-
> > source as having a "custom format". With DT as a hardware description,
> > implementaton specific values do not work well - instead it should have a
> > regular unit-value.
> > 
> > For the power-source I think volts could work well - as this is the main
> > use- case for pinctrl I know. The regulator-binding uses microvolts,
> > maybe it would be good use a similar unit.
> 
> (...)
> 
> >         pcfg_1v8 {
> >         
> >                 power-source = <1800000>;
> >         
> >         };
> 
> I don't think this is what it's meant for actually. In this case the thing
> would be modeled as a regulator rather than some pin control option I
> guess?
> 
> I think it's more like a selector, such as found on old amplifiers,
> input source A, B or C? So it's just some discrete number.

as I said, you know this better than me ;-)

I was thinking more of these io-voltages, where one can select between say 1.8 
and 3.3V for the pins.

I have this somewhere on my todo list for the rockchips too.

So essentially the pinctrl driver [as it is a pin-group specific setting] 
should just export regulators for these voltage settings, without touching the 
pinconfig itself, right?


Thanks
Heiko

Re: [PATCH v2] pinctrl: Enable "power-source" to be extracted from DT files

[Linus Walleij]

On Wed, May 28, 2014 at 10:36 AM, Heiko Stübner <heiko@sntech.de> wrote:
> Am Mittwoch, 28. Mai 2014, 10:15:43 schrieb Linus Walleij:
>> On Tue, May 27, 2014 at 3:09 PM, Heiko Stübner <heiko@sntech.de> wrote:
>> > Am Dienstag, 27. Mai 2014, 09:27:36 schrieb Ivan T. Ivanov:
>> >> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
>> >>
>> >> Add "power-source" property to generic options used for DT parsing files.
>> >> This  enables drivers, which use generic pin configurations, to get the
>> >> value passed to this property.
>> >
>> > I think the main problem here is, that pinconf-generic.h defines the
>> > power-
>> > source as having a "custom format". With DT as a hardware description,
>> > implementaton specific values do not work well - instead it should have a
>> > regular unit-value.
>> >
>> > For the power-source I think volts could work well - as this is the main
>> > use- case for pinctrl I know. The regulator-binding uses microvolts,
>> > maybe it would be good use a similar unit.
>>
>> (...)
>>
>> >         pcfg_1v8 {
>> >
>> >                 power-source = <1800000>;
>> >
>> >         };
>>
>> I don't think this is what it's meant for actually. In this case the thing
>> would be modeled as a regulator rather than some pin control option I
>> guess?
>>
>> I think it's more like a selector, such as found on old amplifiers,
>> input source A, B or C? So it's just some discrete number.
>
> as I said, you know this better than me ;-)
>
> I was thinking more of these io-voltages, where one can select between say 1.8
> and 3.3V for the pins.
>
> I have this somewhere on my todo list for the rockchips too.
>
> So essentially the pinctrl driver [as it is a pin-group specific setting]
> should just export regulators for these voltage settings, without touching the
> pinconfig itself, right?

Hm hm hm.

I am a bit in the blue actually.

I think we need to have a discussion with Mark Brown on how to
handle this.

We have previously had the case of MMC/SD level-shifters, where
a certain setting gives a certain level of signals out, and another setting
gives another level. Like two discrete levels.

So we modeled that as a regulator provider inside the pin control
driver eventually, see sh-pfc/pfc-sh73a0.c

But this is different: it is the power of the pin itself.

I can think of a "padring regulator" hooking in as part of the pin state
but have no clear idea on how to actually do that. We need some
coding I think.

Yours,
Linus Walleij

Re: [PATCH v2] pinctrl: Enable "power-source" to be extracted from DT files

[Ivan T. Ivanov]

On Wed, 2014-05-28 at 10:48 +0200, Linus Walleij wrote:
> On Wed, May 28, 2014 at 10:36 AM, Heiko Stübner <heiko@sntech.de> wrote:
> > Am Mittwoch, 28. Mai 2014, 10:15:43 schrieb Linus Walleij:
> >> On Tue, May 27, 2014 at 3:09 PM, Heiko Stübner <heiko@sntech.de> wrote:
> >> > Am Dienstag, 27. Mai 2014, 09:27:36 schrieb Ivan T. Ivanov:
> >> >> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> >> >>
> >> >> Add "power-source" property to generic options used for DT parsing files.
> >> >> This  enables drivers, which use generic pin configurations, to get the
> >> >> value passed to this property.
> >> >
> >> > I think the main problem here is, that pinconf-generic.h defines the
> >> > power-
> >> > source as having a "custom format". With DT as a hardware description,
> >> > implementaton specific values do not work well - instead it should have a
> >> > regular unit-value.
> >> >
> >> > For the power-source I think volts could work well - as this is the main
> >> > use- case for pinctrl I know. The regulator-binding uses microvolts,
> >> > maybe it would be good use a similar unit.
> >>
> >> (...)
> >>
> >> >         pcfg_1v8 {
> >> >
> >> >                 power-source = <1800000>;
> >> >
> >> >         };
> >>
> >> I don't think this is what it's meant for actually. In this case the thing
> >> would be modeled as a regulator rather than some pin control option I
> >> guess?
> >>
> >> I think it's more like a selector, such as found on old amplifiers,
> >> input source A, B or C? So it's just some discrete number.
> >
> > as I said, you know this better than me ;-)
> >
> > I was thinking more of these io-voltages, where one can select between say 1.8
> > and 3.3V for the pins.
> >
> > I have this somewhere on my todo list for the rockchips too.
> >
> > So essentially the pinctrl driver [as it is a pin-group specific setting]
> > should just export regulators for these voltage settings, without touching the
> > pinconfig itself, right?
> 
> Hm hm hm.
> 
> I am a bit in the blue actually.
> 
> I think we need to have a discussion with Mark Brown on how to
> handle this.
> 
> We have previously had the case of MMC/SD level-shifters, where
> a certain setting gives a certain level of signals out, and another setting
> gives another level. Like two discrete levels.
> 
> So we modeled that as a regulator provider inside the pin control
> driver eventually, see sh-pfc/pfc-sh73a0.c
> 
> But this is different: it is the power of the pin itself.
> 
> I can think of a "padring regulator" hooking in as part of the pin state
> but have no clear idea on how to actually do that. We need some
> coding I think.
> 

In my case this is just a switch, which can select from different power
sources and these sources are external from the chip. So I can't know
a priory what the levels are and hard code them inside the driver.

Thanks,
Ivan

Re: [PATCH v2] pinctrl: Enable "power-source" to be extracted from DT files

[Heiko Stübner]

Am Freitag, 30. Mai 2014, 10:41:31 schrieb Ivan T. Ivanov:
> On Wed, 2014-05-28 at 10:48 +0200, Linus Walleij wrote:
> > On Wed, May 28, 2014 at 10:36 AM, Heiko Stübner <heiko@sntech.de> wrote:
> > > Am Mittwoch, 28. Mai 2014, 10:15:43 schrieb Linus Walleij:
> > >> On Tue, May 27, 2014 at 3:09 PM, Heiko Stübner <heiko@sntech.de> wrote:
> > >> > Am Dienstag, 27. Mai 2014, 09:27:36 schrieb Ivan T. Ivanov:
> > >> >> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> > >> >> 
> > >> >> Add "power-source" property to generic options used for DT parsing
> > >> >> files.
> > >> >> This  enables drivers, which use generic pin configurations, to get
> > >> >> the
> > >> >> value passed to this property.
> > >> > 
> > >> > I think the main problem here is, that pinconf-generic.h defines the
> > >> > power-
> > >> > source as having a "custom format". With DT as a hardware
> > >> > description,
> > >> > implementaton specific values do not work well - instead it should
> > >> > have a
> > >> > regular unit-value.
> > >> > 
> > >> > For the power-source I think volts could work well - as this is the
> > >> > main
> > >> > use- case for pinctrl I know. The regulator-binding uses microvolts,
> > >> > maybe it would be good use a similar unit.
> > >> 
> > >> (...)
> > >> 
> > >> >         pcfg_1v8 {
> > >> >         
> > >> >                 power-source = <1800000>;
> > >> >         
> > >> >         };
> > >> 
> > >> I don't think this is what it's meant for actually. In this case the
> > >> thing
> > >> would be modeled as a regulator rather than some pin control option I
> > >> guess?
> > >> 
> > >> I think it's more like a selector, such as found on old amplifiers,
> > >> input source A, B or C? So it's just some discrete number.
> > > 
> > > as I said, you know this better than me ;-)
> > > 
> > > I was thinking more of these io-voltages, where one can select between
> > > say 1.8 and 3.3V for the pins.
> > > 
> > > I have this somewhere on my todo list for the rockchips too.
> > > 
> > > So essentially the pinctrl driver [as it is a pin-group specific
> > > setting]
> > > should just export regulators for these voltage settings, without
> > > touching the pinconfig itself, right?
> > 
> > Hm hm hm.
> > 
> > I am a bit in the blue actually.
> > 
> > I think we need to have a discussion with Mark Brown on how to
> > handle this.
> > 
> > We have previously had the case of MMC/SD level-shifters, where
> > a certain setting gives a certain level of signals out, and another
> > setting
> > gives another level. Like two discrete levels.
> > 
> > So we modeled that as a regulator provider inside the pin control
> > driver eventually, see sh-pfc/pfc-sh73a0.c
> > 
> > But this is different: it is the power of the pin itself.
> > 
> > I can think of a "padring regulator" hooking in as part of the pin state
> > but have no clear idea on how to actually do that. We need some
> > coding I think.
> 
> In my case this is just a switch, which can select from different power
> sources and these sources are external from the chip. So I can't know
> a priory what the levels are and hard code them inside the driver.

Yep, that is exactly what Linus described as use-case too ... I was simply 
thinking in the wrong direction :-)

Heiko

Re: [PATCH v2] pinctrl: Enable "power-source" to be extracted from DT files

[Linus Walleij]

On Tue, May 27, 2014 at 8:27 AM, Ivan T. Ivanov <iivanov@mm-sol.com> wrote:

> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
>
> Add "power-source" property to generic options used for DT parsing files.
> This  enables drivers, which use generic pin configurations, to get the
> value passed to this property.
>
> Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>

Patch applied. Cannot think of anything better anyway.

Yours,
Linus Walleij