| Message ID | 1432126385-27402-1-git-send-email-antoine.tenart@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 20.05.2015 14:53, Antoine Tenart wrote: > The BG2Q SoC has two SPI controllers. Add the corresponding nodes. > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> > --- > > Based on top of the Berlin clock rework series. > > arch/arm/boot/dts/berlin2q.dtsi | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi > index 187d056f7ad2..c25ee86b2bfa 100644 > --- a/arch/arm/boot/dts/berlin2q.dtsi > +++ b/arch/arm/boot/dts/berlin2q.dtsi > @@ -286,6 +286,20 @@ > status = "disabled"; > }; > > + spi0: spi@1c00 { > + compatible = "snps,dw-apb-ssi"; > + reg = <0x1c00 0x100>; > + interrupt-parrent = <&aic>; Antoine, the same question as for the ADC node patch: IIRC you don't have to repeat the interrupt-parent property as long as any node upstream will have it already. > + interrupts = <7>; > + clocks = <&chip_clk CLKID_CFG>; > + pinctrl-0 = <&spi0_pmux>; > + pinctrl-names = "default"; > + #address-cells = <1>; > + #size-cells = <0>; > + num-cs = <4>; > + status = "disabled"; > + }; > + > timer0: timer@2c00 { > compatible = "snps,dw-apb-timer"; > reg = <0x2c00 0x14>; > @@ -383,6 +397,11 @@ > groups = "G7"; > function = "twsi1"; > }; > + > + spi0_pmux: spi0-pmux { > + groups = "G8", "G9", "G10", "G11"; > + function = "spi1"; Hmm, "spi0_pmux" but "spi1" function? > + }; > }; > > chip_rst: reset { > @@ -473,6 +492,20 @@ > }; > }; > > + spi1: spi@5000 { > + compatible = "snps,dw-apb-ssi"; > + reg = <0x6000 0x100>; > + interrupt-parent = <&sic>; > + interrupts = <5>; > + clocks = <&refclk>; > + pinctrl-0 = <&spi1_pmux>; > + pinctrl-names = "default"; > + #address-cells = <1>; > + #size-cells = <0>; > + num-cs = <4>; > + status = "disabled"; > + }; > + > i2c2: i2c@7000 { > compatible = "snps,designware-i2c"; > #address-cells = <1>; > @@ -564,6 +597,11 @@ > groups = "GSM14"; > function = "twsi3"; > }; > + > + spi1_pmux: spi1-pmux { > + groups = "GSM0", "GSM1", "GSM2", "GSM3"; > + function = "spi2"; ditto. I know the internal numbering scheme on BG-SoCs is weird, but it looks like that either you are missing the third SPI or there is only 2 and numbering starts with 1 *sigh* ;) Anyway, the numbering should be consistent with pinctrl function names although I would have preferred to start counting with 0. Sebastian > + }; > }; > }; > >
On 20.05.2015 14:53, Antoine Tenart wrote: > The BG2Q SoC has two SPI controllers. Add the corresponding nodes. > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> > --- [...] Some more I noticed while looking at BG2's spi controllers. > diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi > index 187d056f7ad2..c25ee86b2bfa 100644 > --- a/arch/arm/boot/dts/berlin2q.dtsi > +++ b/arch/arm/boot/dts/berlin2q.dtsi > @@ -286,6 +286,20 @@ > status = "disabled"; > }; > > + spi0: spi@1c00 { > + compatible = "snps,dw-apb-ssi"; > + reg = <0x1c00 0x100>; > + interrupt-parrent = <&aic>; > + interrupts = <7>; > + clocks = <&chip_clk CLKID_CFG>; > + pinctrl-0 = <&spi0_pmux>; > + pinctrl-names = "default"; > + #address-cells = <1>; > + #size-cells = <0>; > + num-cs = <4>; As you are adding this node to the SoC dtsi, I doubt that all boards will use the _up to_ 4 available CSn lines. Most likely only one (or even none) CSn will be used. Anyway, you can leave the num-cs at 4 but... > + status = "disabled"; > + }; > + > timer0: timer@2c00 { > compatible = "snps,dw-apb-timer"; > reg = <0x2c00 0x14>; > @@ -383,6 +397,11 @@ > groups = "G7"; > function = "twsi1"; > }; > + > + spi0_pmux: spi0-pmux { > + groups = "G8", "G9", "G10", "G11"; > + function = "spi1"; > + }; ... can you check which of G8-G11 are actually clock/data and which are CSn lines? CSn lines should all be optional and per-board pinmux - same for the other spi pinmux. > }; > > chip_rst: reset { > @@ -473,6 +492,20 @@ > }; > }; > > + spi1: spi@5000 { > + compatible = "snps,dw-apb-ssi"; > + reg = <0x6000 0x100>; s/spi@5000/spi@6000/ Sebastian
Sebastian, On Wed, May 20, 2015 at 06:58:35PM +0200, Sebastian Hesselbarth wrote: > On 20.05.2015 14:53, Antoine Tenart wrote: > >The BG2Q SoC has two SPI controllers. Add the corresponding nodes. > > > >Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> > >--- > > > >Based on top of the Berlin clock rework series. > > > > arch/arm/boot/dts/berlin2q.dtsi | 38 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > >diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi > >index 187d056f7ad2..c25ee86b2bfa 100644 > >--- a/arch/arm/boot/dts/berlin2q.dtsi > >+++ b/arch/arm/boot/dts/berlin2q.dtsi > >@@ -286,6 +286,20 @@ > > status = "disabled"; > > }; > > > >+ spi0: spi@1c00 { > >+ compatible = "snps,dw-apb-ssi"; > >+ reg = <0x1c00 0x100>; > >+ interrupt-parrent = <&aic>; > > the same question as for the ADC node patch: IIRC you don't have > to repeat the interrupt-parent property as long as any node upstream > will have it already. You're right, I'll remove this. And we have other nodes doing the same thing, I'll cook up a patch to remove non mandatory interrupt-parent properties. > > >+ interrupts = <7>; > >+ clocks = <&chip_clk CLKID_CFG>; > >+ pinctrl-0 = <&spi0_pmux>; > >+ pinctrl-names = "default"; > >+ #address-cells = <1>; > >+ #size-cells = <0>; > >+ num-cs = <4>; > >+ status = "disabled"; > >+ }; > >+ > > timer0: timer@2c00 { > > compatible = "snps,dw-apb-timer"; > > reg = <0x2c00 0x14>; > >@@ -383,6 +397,11 @@ > > groups = "G7"; > > function = "twsi1"; > > }; > >+ > >+ spi0_pmux: spi0-pmux { > >+ groups = "G8", "G9", "G10", "G11"; > >+ function = "spi1"; > > Hmm, "spi0_pmux" but "spi1" function? Yep. See below. > >+ }; > > }; > > > > chip_rst: reset { > >@@ -473,6 +492,20 @@ > > }; > > }; > > > >+ spi1: spi@5000 { > >+ compatible = "snps,dw-apb-ssi"; > >+ reg = <0x6000 0x100>; > >+ interrupt-parent = <&sic>; > >+ interrupts = <5>; > >+ clocks = <&refclk>; > >+ pinctrl-0 = <&spi1_pmux>; > >+ pinctrl-names = "default"; > >+ #address-cells = <1>; > >+ #size-cells = <0>; > >+ num-cs = <4>; > >+ status = "disabled"; > >+ }; > >+ > > i2c2: i2c@7000 { > > compatible = "snps,designware-i2c"; > > #address-cells = <1>; > >@@ -564,6 +597,11 @@ > > groups = "GSM14"; > > function = "twsi3"; > > }; > >+ > >+ spi1_pmux: spi1-pmux { > >+ groups = "GSM0", "GSM1", "GSM2", "GSM3"; > >+ function = "spi2"; > > ditto. > > I know the internal numbering scheme on BG-SoCs is weird, but it looks like > that either you are missing the third SPI or there is only 2 and > numbering starts with 1 *sigh* ;) There are 2 SPI, starting at 1... :) > Anyway, the numbering should be consistent with pinctrl function names > although I would have preferred to start counting with 0. OK, so we'll have spi1_pmux in the spi0 node. I'll update. Antoine
On Thu, May 21, 2015 at 01:21:42AM +0200, Sebastian Hesselbarth wrote: > On 20.05.2015 14:53, Antoine Tenart wrote: > >+ > >+ spi0_pmux: spi0-pmux { > >+ groups = "G8", "G9", "G10", "G11"; > >+ function = "spi1"; > >+ }; > > ... can you check which of G8-G11 are actually clock/data and which > are CSn lines? > > CSn lines should all be optional and per-board pinmux - same for the > other spi pinmux. G8 and GSM3 are for clock/data, the other groups (G9-11 and GSM0-2) control the CSn lines. I'll update. > > }; > > > > chip_rst: reset { > >@@ -473,6 +492,20 @@ > > }; > > }; > > > >+ spi1: spi@5000 { > >+ compatible = "snps,dw-apb-ssi"; > >+ reg = <0x6000 0x100>; > > s/spi@5000/spi@6000/ Oops. Antoine
On 25.05.2015 11:01, Antoine Tenart wrote: > On Thu, May 21, 2015 at 01:21:42AM +0200, Sebastian Hesselbarth wrote: >> On 20.05.2015 14:53, Antoine Tenart wrote: >>> + >>> + spi0_pmux: spi0-pmux { >>> + groups = "G8", "G9", "G10", "G11"; >>> + function = "spi1"; >>> + }; >> >> ... can you check which of G8-G11 are actually clock/data and which >> are CSn lines? >> >> CSn lines should all be optional and per-board pinmux - same for the >> other spi pinmux. > > G8 and GSM3 are for clock/data, the other groups (G9-11 and GSM0-2) > control the CSn lines. I'll update. Re-reading this mail after you published v2, I thought it would be a good idea to add this information to the pinmux driver. We already have some /* comments */ about the actual function, why not add the above, too? Also, I guess G8/GSM3 are for clk/data _and_ cs0n while G9-G11/GSM0-GSM2 add cs{1,2,3}n ? Sebastian
On Wed, May 27, 2015 at 10:36:25AM +0200, Sebastian Hesselbarth wrote: > On 25.05.2015 11:01, Antoine Tenart wrote: > >On Thu, May 21, 2015 at 01:21:42AM +0200, Sebastian Hesselbarth wrote: > >>On 20.05.2015 14:53, Antoine Tenart wrote: > >>>+ > >>>+ spi0_pmux: spi0-pmux { > >>>+ groups = "G8", "G9", "G10", "G11"; > >>>+ function = "spi1"; > >>>+ }; > >> > >>... can you check which of G8-G11 are actually clock/data and which > >>are CSn lines? > >> > >>CSn lines should all be optional and per-board pinmux - same for the > >>other spi pinmux. > > > >G8 and GSM3 are for clock/data, the other groups (G9-11 and GSM0-2) > >control the CSn lines. I'll update. > > Re-reading this mail after you published v2, I thought it would > be a good idea to add this information to the pinmux driver. We > already have some /* comments */ about the actual function, why > not add the above, too? Good idea, I'll prepare a patch. > Also, I guess G8/GSM3 are for clk/data _and_ cs0n while > G9-G11/GSM0-GSM2 add cs{1,2,3}n ? After re-reading carefully the documentation, G8 -> CLK, SDI, SDO G9 -> S0n, S1n G10 -> S2n G10 -> S3n GSM0 -> S0n GSM1 -> S1n GSM2 -> S2n, S3n GSM3 -> CLK, SDO and have no entry for the SPI2 SDI pinmux in my documentation. Antoine
diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi index 187d056f7ad2..c25ee86b2bfa 100644 --- a/arch/arm/boot/dts/berlin2q.dtsi +++ b/arch/arm/boot/dts/berlin2q.dtsi @@ -286,6 +286,20 @@ status = "disabled"; }; + spi0: spi@1c00 { + compatible = "snps,dw-apb-ssi"; + reg = <0x1c00 0x100>; + interrupt-parrent = <&aic>; + interrupts = <7>; + clocks = <&chip_clk CLKID_CFG>; + pinctrl-0 = <&spi0_pmux>; + pinctrl-names = "default"; + #address-cells = <1>; + #size-cells = <0>; + num-cs = <4>; + status = "disabled"; + }; + timer0: timer@2c00 { compatible = "snps,dw-apb-timer"; reg = <0x2c00 0x14>; @@ -383,6 +397,11 @@ groups = "G7"; function = "twsi1"; }; + + spi0_pmux: spi0-pmux { + groups = "G8", "G9", "G10", "G11"; + function = "spi1"; + }; }; chip_rst: reset { @@ -473,6 +492,20 @@ }; }; + spi1: spi@5000 { + compatible = "snps,dw-apb-ssi"; + reg = <0x6000 0x100>; + interrupt-parent = <&sic>; + interrupts = <5>; + clocks = <&refclk>; + pinctrl-0 = <&spi1_pmux>; + pinctrl-names = "default"; + #address-cells = <1>; + #size-cells = <0>; + num-cs = <4>; + status = "disabled"; + }; + i2c2: i2c@7000 { compatible = "snps,designware-i2c"; #address-cells = <1>; @@ -564,6 +597,11 @@ groups = "GSM14"; function = "twsi3"; }; + + spi1_pmux: spi1-pmux { + groups = "GSM0", "GSM1", "GSM2", "GSM3"; + function = "spi2"; + }; }; };
The BG2Q SoC has two SPI controllers. Add the corresponding nodes. Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com> --- Based on top of the Berlin clock rework series. arch/arm/boot/dts/berlin2q.dtsi | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)