| Message ID | 20171002120854.5212-13-quentin.schulz@free-electrons.com |
|---|---|
| State | Accepted |
| Commit | 38ecd22004e850c6b927bc7e085feb87032935af |
| Headers | show |
| Series | None | expand |
On Mon, Oct 2, 2017 at 2:08 PM, Quentin Schulz <quentin.schulz@free-electrons.com> wrote: > On AXP813/818, GPIO0 and GPIO1 can be used as LDO as (respectively) > ldo_io0 and ldo_io1. (...) > + gpio0_ldo: gpio0_ldo { > + pins = "GPIO0"; > + function = "ldo"; > + }; (...) > + pinctrl-names = "default"; > + pinctrl-0 = <&gpio0_ldo>; > /* Disable by default to avoid conflicts with GPIO */ > status = "disabled"; So this is still by default disabled, but you make the default mode something called "ldo". And I think that is to be understood as a low-dropout regulator? So is the idea that this should be represented as a regulator in the end? Then I think the state name should not be "default" rather something like "regulator" and "default" should be the GPIO mode, as I guess something like that exists. Activating a regulator using pin control "default" mode is not very pretty. It would probably be unintuitive and end up wasting power because people will get confused about what is going on. Instead, call this state "regulator" and when using, in Linux create a regulator device that set the pin into "regulator" state to start using it as a LDO, and "default" to deactivate it as LDO, if that is how the usage is intended. Yours, Linus Walleij
On Tue, Oct 03, 2017 at 09:18:37AM +0000, Russell King - ARM Linux wrote: > On Tue, Oct 03, 2017 at 10:06:29AM +0800, Chen-Yu Tsai wrote: > > On Tue, Oct 3, 2017 at 4:42 AM, Maxime Ripard > > <maxime.ripard@free-electrons.com> wrote: > > > On Mon, Oct 02, 2017 at 12:08:54PM +0000, Quentin Schulz wrote: > > >> On AXP813/818, GPIO0 and GPIO1 can be used as LDO as (respectively) > > >> ldo_io0 and ldo_io1. > > >> > > >> Let's add the pinctrl properties to the said regulators. > > >> > > >> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com> > > >> --- > > >> arch/arm/boot/dts/axp81x.dtsi | 14 ++++++++++++++ > > >> 1 file changed, 14 insertions(+) > > >> > > >> diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi > > >> index f90f257130d5..099b0ddc1bbb 100644 > > >> --- a/arch/arm/boot/dts/axp81x.dtsi > > >> +++ b/arch/arm/boot/dts/axp81x.dtsi > > >> @@ -52,6 +52,16 @@ > > >> compatible = "x-powers,axp813-gpio"; > > >> gpio-controller; > > >> #gpio-cells = <2>; > > >> + > > >> + gpio0_ldo: gpio0_ldo { > > >> + pins = "GPIO0"; > > >> + function = "ldo"; > > >> + }; > > >> + > > >> + gpio1_ldo: gpio1_ldo { > > >> + pins = "GPIO1"; > > >> + function = "ldo"; > > >> + }; > > > > > > The node names are not supposed to contain any hyphens. > > > > Hmm, I was under the impression that hyphens were preferred in > > node names, and a warning would be added to dtc later on. > > I might be wrong though. > > I think there's a terminology issue here. > > "-" is a hyphen or minus sign. > "_" is an underscore. > > Underscores are not supposed to be used for node names, instead hyphens > are preferred. I think Maxime means "underscore". > > Here's the list from dtc/checks.c: > > #define LOWERCASE "abcdefghijklmnopqrstuvwxyz" > #define UPPERCASE "ABCDEFGHIJKLMNOPQRSTUVWXYZ" > #define DIGITS "0123456789" > #define PROPNODECHARS LOWERCASE UPPERCASE DIGITS ",._+*#?-" > #define PROPNODECHARSSTRICT LOWERCASE UPPERCASE DIGITS ",-" > > If strict mode is enabled, use of any of "._+#?" in the node name will > produce a warning. Right, sorry, I meant underscores.. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi index f90f257130d5..099b0ddc1bbb 100644 --- a/arch/arm/boot/dts/axp81x.dtsi +++ b/arch/arm/boot/dts/axp81x.dtsi @@ -52,6 +52,16 @@ compatible = "x-powers,axp813-gpio"; gpio-controller; #gpio-cells = <2>; + + gpio0_ldo: gpio0_ldo { + pins = "GPIO0"; + function = "ldo"; + }; + + gpio1_ldo: gpio1_ldo { + pins = "GPIO1"; + function = "ldo"; + }; }; regulators { @@ -119,11 +129,15 @@ }; reg_ldo_io0: ldo-io0 { + pinctrl-names = "default"; + pinctrl-0 = <&gpio0_ldo>; /* Disable by default to avoid conflicts with GPIO */ status = "disabled"; }; reg_ldo_io1: ldo-io1 { + pinctrl-names = "default"; + pinctrl-0 = <&gpio1_ldo>; /* Disable by default to avoid conflicts with GPIO */ status = "disabled"; };
On AXP813/818, GPIO0 and GPIO1 can be used as LDO as (respectively) ldo_io0 and ldo_io1. Let's add the pinctrl properties to the said regulators. Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com> --- arch/arm/boot/dts/axp81x.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+) -- 2.11.0