| Message ID | 1397135274-10764-6-git-send-email-antoine.tenart@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 10, 2014 at 03:07:54PM +0200, Antoine Ténart wrote: > The uart0 pinmux configuration is in the dtsi because uart0 will always > use uart0-pmux to work, no other possibility. Same thing for uart1. > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > --- > arch/arm/boot/dts/berlin2.dtsi | 20 ++++++++++++++++++++ > arch/arm/boot/dts/berlin2cd.dtsi | 13 +++++++++++++ > arch/arm/boot/dts/berlin2q.dtsi | 20 ++++++++++++++++++++ > 3 files changed, 53 insertions(+) > > diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi > index 56a1af2f1052..43eb90c36050 100644 > --- a/arch/arm/boot/dts/berlin2.dtsi > +++ b/arch/arm/boot/dts/berlin2.dtsi > @@ -176,6 +176,22 @@ > }; > }; > > + pinctrl: pinctrl@0 { > + compatible = "marvell,berlin2-pinctrl"; > + reg = <0xea0000 0x08>, <0xfc0000 0x44>; > + reg-names = "global_base", "apb_base"; > + > + uart0_pmux: uart0-pmux { > + berlin,group = "GSM4"; > + berlin,function = <0>; > + }; > + > + uart1_pmux: uart1-pmux { > + berlin,group = "GSM5"; > + berlin,function = <1>; Hi Antoine This very much looks like black magic. I assume the data sheet is not available? So i think you need to document all possible combinations of values of group and function. Maybe you can add a file in arch/arm/boot/dts/include/dt-bindings/pinctrl/ with something like #define GSM4_UART0 0 #define GSM5_UART1 1 #define GSM12_UART0 1 #define GSM12_IrDA0 1 #define GSM12_GPIO 2 Andrew
Hi Andrew, On Fri, Apr 11, 2014 at 10:18:59AM +0200, Andrew Lunn wrote: > On Thu, Apr 10, 2014 at 03:07:54PM +0200, Antoine Ténart wrote: > > The uart0 pinmux configuration is in the dtsi because uart0 will always > > use uart0-pmux to work, no other possibility. Same thing for uart1. > > > > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> > > --- > > arch/arm/boot/dts/berlin2.dtsi | 20 ++++++++++++++++++++ > > arch/arm/boot/dts/berlin2cd.dtsi | 13 +++++++++++++ > > arch/arm/boot/dts/berlin2q.dtsi | 20 ++++++++++++++++++++ > > 3 files changed, 53 insertions(+) > > > > diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi > > index 56a1af2f1052..43eb90c36050 100644 > > --- a/arch/arm/boot/dts/berlin2.dtsi > > +++ b/arch/arm/boot/dts/berlin2.dtsi > > @@ -176,6 +176,22 @@ > > }; > > }; > > > > + pinctrl: pinctrl@0 { > > + compatible = "marvell,berlin2-pinctrl"; > > + reg = <0xea0000 0x08>, <0xfc0000 0x44>; > > + reg-names = "global_base", "apb_base"; > > + > > + uart0_pmux: uart0-pmux { > > + berlin,group = "GSM4"; > > + berlin,function = <0>; > > + }; > > + > > + uart1_pmux: uart1-pmux { > > + berlin,group = "GSM5"; > > + berlin,function = <1>; > > Hi Antoine > > This very much looks like black magic. > > I assume the data sheet is not available? So i think you need to > document all possible combinations of values of group and function. > Maybe you can add a file in > > arch/arm/boot/dts/include/dt-bindings/pinctrl/ > > with something like > > #define GSM4_UART0 0 > #define GSM5_UART1 1 > #define GSM12_UART0 1 > #define GSM12_IrDA0 1 > #define GSM12_GPIO 2 Groups' functions are not the same between BG2/BG2CD and BG2Q so multiple headers would be needed which may not be a good thing. Some platforms use black magic, and we can find a pinmux configuration for OMAP looking like: 0x128 (PIN_INPUT_PULLUP | MUX_MODE0) Weel, we could explicitly define the functions in the driver itself and use in the dt something like: berlin,group = "GSM4"; berlin,function = "uart0"; This is what's done on the sunxi's pinctrl. Antoine
On 04/11/2014 11:09 AM, Antoine Ténart wrote: > Hi Andrew, > > On Fri, Apr 11, 2014 at 10:18:59AM +0200, Andrew Lunn wrote: >> On Thu, Apr 10, 2014 at 03:07:54PM +0200, Antoine Ténart wrote: >>> The uart0 pinmux configuration is in the dtsi because uart0 will always >>> use uart0-pmux to work, no other possibility. Same thing for uart1. >>> >>> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> >>> --- >>> arch/arm/boot/dts/berlin2.dtsi | 20 ++++++++++++++++++++ >>> arch/arm/boot/dts/berlin2cd.dtsi | 13 +++++++++++++ >>> arch/arm/boot/dts/berlin2q.dtsi | 20 ++++++++++++++++++++ >>> 3 files changed, 53 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi >>> index 56a1af2f1052..43eb90c36050 100644 >>> --- a/arch/arm/boot/dts/berlin2.dtsi >>> +++ b/arch/arm/boot/dts/berlin2.dtsi >>> @@ -176,6 +176,22 @@ >>> }; >>> }; >>> >>> + pinctrl: pinctrl@0 { >>> + compatible = "marvell,berlin2-pinctrl"; >>> + reg = <0xea0000 0x08>, <0xfc0000 0x44>; >>> + reg-names = "global_base", "apb_base"; >>> + >>> + uart0_pmux: uart0-pmux { >>> + berlin,group = "GSM4"; >>> + berlin,function = <0>; >>> + }; >>> + >>> + uart1_pmux: uart1-pmux { >>> + berlin,group = "GSM5"; >>> + berlin,function = <1>; >> >> This very much looks like black magic. >> >> I assume the data sheet is not available? So i think you need to >> document all possible combinations of values of group and function. >> Maybe you can add a file in >> >> arch/arm/boot/dts/include/dt-bindings/pinctrl/ >> >> with something like >> >> #define GSM4_UART0 0 >> #define GSM5_UART1 1 >> #define GSM12_UART0 1 >> #define GSM12_IrDA0 1 >> #define GSM12_GPIO 2 > > Groups' functions are not the same between BG2/BG2CD and BG2Q so multiple > headers would be needed which may not be a good thing. Some platforms use black > magic, and we can find a pinmux configuration for OMAP looking like: > > 0x128 (PIN_INPUT_PULLUP | MUX_MODE0) And it is a constant pain-in-the-ass to understand it at all without looking it up in different places. For Berlin, the only publically available option is BSP code, and you don't want to look it up in there even once more than necessary. ;) I don't see a problem in having one binding for each (different) SoC at all. > Weel, we could explicitly define the functions in the driver itself and use in > the dt something like: > > berlin,group = "GSM4"; > berlin,function = "uart0"; > > This is what's done on the sunxi's pinctrl. We also have function strings in mvebu pinctrl. The name is very useful for /sys/kernel/debug/pinctrl/ but nothing more. Matching numbers is easier than matching string, so I suggest to stick with binding includes as Andrew suggested. Sebastian
diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi index 56a1af2f1052..43eb90c36050 100644 --- a/arch/arm/boot/dts/berlin2.dtsi +++ b/arch/arm/boot/dts/berlin2.dtsi @@ -176,6 +176,22 @@ }; }; + pinctrl: pinctrl@0 { + compatible = "marvell,berlin2-pinctrl"; + reg = <0xea0000 0x08>, <0xfc0000 0x44>; + reg-names = "global_base", "apb_base"; + + uart0_pmux: uart0-pmux { + berlin,group = "GSM4"; + berlin,function = <0>; + }; + + uart1_pmux: uart1-pmux { + berlin,group = "GSM5"; + berlin,function = <1>; + }; + }; + apb@fc0000 { compatible = "simple-bus"; #address-cells = <1>; @@ -191,6 +207,8 @@ reg-io-width = <1>; interrupts = <8>; clocks = <&smclk>; + pinctrl-0 = <&uart0_pmux>; + pinctrl-names = "default"; status = "disabled"; }; @@ -201,6 +219,8 @@ reg-io-width = <1>; interrupts = <9>; clocks = <&smclk>; + pinctrl-0 = <&uart1_pmux>; + pinctrl-names = "default"; status = "disabled"; }; diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi index 094968c27533..4c04cff89226 100644 --- a/arch/arm/boot/dts/berlin2cd.dtsi +++ b/arch/arm/boot/dts/berlin2cd.dtsi @@ -169,6 +169,17 @@ }; }; + pinctrl: pinctrl@0 { + compatible = "marvell,berlin2cd-pinctrl"; + reg = <0xea0000 0x08>, <0xfc0000 0x44>; + reg-names = "global_base", "apb_base"; + + uart0_pmux: uart0-pmux { + berlin,group = "G6"; + berlin,function = <0>; + }; + }; + apb@fc0000 { compatible = "simple-bus"; #address-cells = <1>; @@ -184,6 +195,8 @@ reg-io-width = <1>; interrupts = <8>; clocks = <&smclk>; + pinctrl-0 = <&uart0_pmux>; + pinctrl-names = "default"; status = "disabled"; }; diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi index 07452a7483fa..e6e556055dfc 100644 --- a/arch/arm/boot/dts/berlin2q.dtsi +++ b/arch/arm/boot/dts/berlin2q.dtsi @@ -183,6 +183,22 @@ }; }; + pinctrl: pinctrl@0 { + compatible = "marvell,berlin2q-pinctrl"; + reg = <0xea0000 0x4c>, <0xfcd000 0x44>; + reg-names = "global_base", "sm_base"; + + uart0_pmux: uart0-pmux { + berlin,group = "GSM12"; + berlin,function = <0>; + }; + + uart1_pmux: uart1-pmux { + berlin,group = "GSM14"; + berlin,function = <1>; + }; + }; + apb@fc0000 { compatible = "simple-bus"; #address-cells = <1>; @@ -198,6 +214,8 @@ interrupts = <8>; clocks = <&smclk>; reg-shift = <2>; + pinctrl-0 = <&uart0_pmux>; + pinctrl-names = "default"; status = "disabled"; }; @@ -208,6 +226,8 @@ interrupts = <9>; clocks = <&smclk>; reg-shift = <2>; + pinctrl-0 = <&uart1_pmux>; + pinctrl-names = "default"; status = "disabled"; };
The uart0 pinmux configuration is in the dtsi because uart0 will always use uart0-pmux to work, no other possibility. Same thing for uart1. Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com> --- arch/arm/boot/dts/berlin2.dtsi | 20 ++++++++++++++++++++ arch/arm/boot/dts/berlin2cd.dtsi | 13 +++++++++++++ arch/arm/boot/dts/berlin2q.dtsi | 20 ++++++++++++++++++++ 3 files changed, 53 insertions(+)