| Message ID | d550094896837ac921e5d2a69fc5aae9b287a98a.1416158990.git.arno@natisbad.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Nov 16, 2014 at 06:37:33PM +0100, Arnaud Ebalard wrote: > > pinctrl entries for uart0 using MPP0-1 and spi0 using MPP33-36 are > common configurations. Instead of replicating them in each .dts file, > put those in armada-370.dtsi file so that they can be referenced. > > Suggested-by: Andrew Lunn <andrew@lunn.ch> > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > --- > arch/arm/boot/dts/armada-370.dtsi | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi > index 6b3c23b1e138..d9f5d59e463e 100644 > --- a/arch/arm/boot/dts/armada-370.dtsi > +++ b/arch/arm/boot/dts/armada-370.dtsi > @@ -115,6 +115,17 @@ > compatible = "marvell,mv88f6710-pinctrl"; > reg = <0x18000 0x38>; > > + uart0_pins: uart0-pins { > + marvell,pins = "mpp0", "mpp1"; > + marvell,function = "uart0"; > + }; Thanks for these. We can go one stage further. kirkwood.dts has: uart0: serial@12000 { compatible = "ns16550a"; reg = <0x12000 0x100>; reg-shift = <2>; interrupts = <33>; clocks = <&gate_clk 7>; pinctrl-0 = <&pmx_uart0>; pinctrl-names = "default"; status = "disabled"; }; i.e actually references them. This is safe because a board .dts file can override the pins if needed. We should do the same here, both for 370 and XP. Andrew > + > + spi0_pins: spi0-pins { > + marvell,pins = "mpp33", "mpp34", > + "mpp35", "mpp36"; > + marvell,function = "spi0"; > + }; > + > sdio_pins1: sdio-pins1 { > marvell,pins = "mpp9", "mpp11", "mpp12", > "mpp13", "mpp14", "mpp15"; > -- > 2.1.1 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi, Andrew Lunn <andrew@lunn.ch> writes: > On Sun, Nov 16, 2014 at 06:37:33PM +0100, Arnaud Ebalard wrote: >> >> pinctrl entries for uart0 using MPP0-1 and spi0 using MPP33-36 are >> common configurations. Instead of replicating them in each .dts file, >> put those in armada-370.dtsi file so that they can be referenced. >> >> Suggested-by: Andrew Lunn <andrew@lunn.ch> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org> >> --- >> arch/arm/boot/dts/armada-370.dtsi | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi >> index 6b3c23b1e138..d9f5d59e463e 100644 >> --- a/arch/arm/boot/dts/armada-370.dtsi >> +++ b/arch/arm/boot/dts/armada-370.dtsi >> @@ -115,6 +115,17 @@ >> compatible = "marvell,mv88f6710-pinctrl"; >> reg = <0x18000 0x38>; >> >> + uart0_pins: uart0-pins { >> + marvell,pins = "mpp0", "mpp1"; >> + marvell,function = "uart0"; >> + }; > > Thanks for these. > > We can go one stage further. kirkwood.dts has: > > uart0: serial@12000 { > compatible = "ns16550a"; > reg = <0x12000 0x100>; > reg-shift = <2>; > interrupts = <33>; > clocks = <&gate_clk 7>; > pinctrl-0 = <&pmx_uart0>; > pinctrl-names = "default"; > status = "disabled"; > }; > > i.e actually references them. This is safe because a board .dts file > can override the pins if needed. > > We should do the same here, both for 370 and XP. Just to be sure I understand: for uart0 and uart1, I think this can only be done for Armada 370 (on XP, uart0/1 rx/tx are not mpp). This only a matter of adding the following in armada-370.dtsi for uart0: /* * Default UART pinctrl setting without RTS/CTS, can * be overwritten on board level. */ uart0: serial@12000 { pinctrl-0 = <&uart0_pins>; pinctrl-names = "default"; }; For uart1, it's not obvious because there are 6 different MPP which can be configured to act as uart1 TXD. Same for RXD. If I remove the ones overlapping w/ RGMII pins for GbE interfaces, which one should be selected as default: 41/42, 55/57, 60/61? Now, regarding armada XP, uart2 and uart3 txd/rxd MPP are 42/43 and 44/45. Best we can do is put the same as above in each specific armada-xp-mv78XXX.dtsi file, where pinctrl node is, after adding uart2-pins and uart3-pins. Unless I am missing something, this cannot be done in armada-xp.dtsi. Cheers, a+
On Sun, Nov 16, 2014 at 11:29:00PM +0100, Arnaud Ebalard wrote: > Hi, > > Andrew Lunn <andrew@lunn.ch> writes: > > > On Sun, Nov 16, 2014 at 06:37:33PM +0100, Arnaud Ebalard wrote: > >> > >> pinctrl entries for uart0 using MPP0-1 and spi0 using MPP33-36 are > >> common configurations. Instead of replicating them in each .dts file, > >> put those in armada-370.dtsi file so that they can be referenced. > >> > >> Suggested-by: Andrew Lunn <andrew@lunn.ch> > >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > >> --- > >> arch/arm/boot/dts/armada-370.dtsi | 11 +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >> diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi > >> index 6b3c23b1e138..d9f5d59e463e 100644 > >> --- a/arch/arm/boot/dts/armada-370.dtsi > >> +++ b/arch/arm/boot/dts/armada-370.dtsi > >> @@ -115,6 +115,17 @@ > >> compatible = "marvell,mv88f6710-pinctrl"; > >> reg = <0x18000 0x38>; > >> > >> + uart0_pins: uart0-pins { > >> + marvell,pins = "mpp0", "mpp1"; > >> + marvell,function = "uart0"; > >> + }; > > > > Thanks for these. > > > > We can go one stage further. kirkwood.dts has: > > > > uart0: serial@12000 { > > compatible = "ns16550a"; > > reg = <0x12000 0x100>; > > reg-shift = <2>; > > interrupts = <33>; > > clocks = <&gate_clk 7>; > > pinctrl-0 = <&pmx_uart0>; > > pinctrl-names = "default"; > > status = "disabled"; > > }; > > > > i.e actually references them. This is safe because a board .dts file > > can override the pins if needed. > > > > We should do the same here, both for 370 and XP. > > Just to be sure I understand: for uart0 and uart1, I think this can only > be done for Armada 370 (on XP, uart0/1 rx/tx are not mpp). I must admit, i did not look at the datasheet for XP. So you can ignore XP. > This only a matter of adding the following in armada-370.dtsi for uart0: > > /* > * Default UART pinctrl setting without RTS/CTS, can > * be overwritten on board level. > */ > uart0: serial@12000 { > pinctrl-0 = <&uart0_pins>; > pinctrl-names = "default"; > }; Looks good. > For uart1, it's not obvious because there are 6 different MPP which can > be configured to act as uart1 TXD. That is too many configurations. So lets leave it per board. > Now, regarding armada XP, uart2 and uart3 txd/rxd MPP are 42/43 and > 44/45. Best we can do is put the same as above in each specific > armada-xp-mv78XXX.dtsi file, where pinctrl node is, after adding > uart2-pins and uart3-pins. Unless I am missing something, this cannot > be done in armada-xp.dtsi. Actually, i think it can. But i could be wrong. armada-370-xp.dtsi declares uart0 and uart1. armada-xp.dtsi then defined another two uart's, uart2 and uart3. You should be able to define the pinctrl nodes for the serial ports there, even if the main pinctrl node is in the SoC specific .dtsi file. Andrew
On 11/17/2014 01:55 AM, Andrew Lunn wrote: > On Sun, Nov 16, 2014 at 11:29:00PM +0100, Arnaud Ebalard wrote: >> Hi, >> >> Andrew Lunn <andrew@lunn.ch> writes: >> >>> On Sun, Nov 16, 2014 at 06:37:33PM +0100, Arnaud Ebalard wrote: >>>> >>>> pinctrl entries for uart0 using MPP0-1 and spi0 using MPP33-36 are >>>> common configurations. Instead of replicating them in each .dts file, >>>> put those in armada-370.dtsi file so that they can be referenced. >>>> >>>> Suggested-by: Andrew Lunn <andrew@lunn.ch> >>>> Signed-off-by: Arnaud Ebalard <arno@natisbad.org> >>>> --- >>>> arch/arm/boot/dts/armada-370.dtsi | 11 +++++++++++ >>>> 1 file changed, 11 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi >>>> index 6b3c23b1e138..d9f5d59e463e 100644 >>>> --- a/arch/arm/boot/dts/armada-370.dtsi >>>> +++ b/arch/arm/boot/dts/armada-370.dtsi >>>> @@ -115,6 +115,17 @@ >>>> compatible = "marvell,mv88f6710-pinctrl"; >>>> reg = <0x18000 0x38>; >>>> >>>> + uart0_pins: uart0-pins { >>>> + marvell,pins = "mpp0", "mpp1"; >>>> + marvell,function = "uart0"; >>>> + }; >>> >>> Thanks for these. >>> >>> We can go one stage further. kirkwood.dts has: >>> >>> uart0: serial@12000 { >>> compatible = "ns16550a"; >>> reg = <0x12000 0x100>; >>> reg-shift = <2>; >>> interrupts = <33>; >>> clocks = <&gate_clk 7>; >>> pinctrl-0 = <&pmx_uart0>; >>> pinctrl-names = "default"; >>> status = "disabled"; >>> }; >>> >>> i.e actually references them. This is safe because a board .dts file >>> can override the pins if needed. >>> >>> We should do the same here, both for 370 and XP. >> >> Just to be sure I understand: for uart0 and uart1, I think this can only >> be done for Armada 370 (on XP, uart0/1 rx/tx are not mpp). > > I must admit, i did not look at the datasheet for XP. So you can > ignore XP. > > >> This only a matter of adding the following in armada-370.dtsi for uart0: >> >> /* >> * Default UART pinctrl setting without RTS/CTS, can >> * be overwritten on board level. >> */ >> uart0: serial@12000 { >> pinctrl-0 = <&uart0_pins>; >> pinctrl-names = "default"; >> }; > > Looks good. > >> For uart1, it's not obvious because there are 6 different MPP which can >> be configured to act as uart1 TXD. > > That is too many configurations. So lets leave it per board. > >> Now, regarding armada XP, uart2 and uart3 txd/rxd MPP are 42/43 and >> 44/45. Best we can do is put the same as above in each specific >> armada-xp-mv78XXX.dtsi file, where pinctrl node is, after adding >> uart2-pins and uart3-pins. Unless I am missing something, this cannot >> be done in armada-xp.dtsi. It would make sense to have a basic pinctrl node in armada-370-xp.dtsi already. It will only have the reg property set, which is IIRC the same for all SoCs. Each of the more specific dtsi will set what is common below that, i.e. armada-370.dtsi will have the compatible + 370 specific pinmux settings. For XP, the common pinmux settings will be in armada-xp.dtsi while the compatible is set later in each of the XP variant dtsi files. Sebastian > Actually, i think it can. But i could be wrong. > > armada-370-xp.dtsi declares uart0 and uart1. armada-xp.dtsi then > defined another two uart's, uart2 and uart3. You should be able to > define the pinctrl nodes for the serial ports there, even if the main > pinctrl node is in the SoC specific .dtsi file.
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi index 6b3c23b1e138..d9f5d59e463e 100644 --- a/arch/arm/boot/dts/armada-370.dtsi +++ b/arch/arm/boot/dts/armada-370.dtsi @@ -115,6 +115,17 @@ compatible = "marvell,mv88f6710-pinctrl"; reg = <0x18000 0x38>; + uart0_pins: uart0-pins { + marvell,pins = "mpp0", "mpp1"; + marvell,function = "uart0"; + }; + + spi0_pins: spi0-pins { + marvell,pins = "mpp33", "mpp34", + "mpp35", "mpp36"; + marvell,function = "spi0"; + }; + sdio_pins1: sdio-pins1 { marvell,pins = "mpp9", "mpp11", "mpp12", "mpp13", "mpp14", "mpp15";
pinctrl entries for uart0 using MPP0-1 and spi0 using MPP33-36 are common configurations. Instead of replicating them in each .dts file, put those in armada-370.dtsi file so that they can be referenced. Suggested-by: Andrew Lunn <andrew@lunn.ch> Signed-off-by: Arnaud Ebalard <arno@natisbad.org> --- arch/arm/boot/dts/armada-370.dtsi | 11 +++++++++++ 1 file changed, 11 insertions(+)