Message ID | 1479656309-21772-5-git-send-email-kostap@marvell.com |
---|---|
State | Superseded |
Delegated to: | Stefan Roese |
Headers | show |
Hi Kosta, On 20.11.2016 16:38, kostap@marvell.com wrote: > From: Konstantin Porotchkin <kostap@marvell.com> > > Add pin control nodes to APN806, CP-master, CP-slave and > Armada-7040 and Armada-8040 boards DTS files > > Change-Id: I51522f33f23e3f9e94c6f5a5f9af19f5dc86e6b7 > Signed-off-by: Konstantin Porotchkin <kostap@marvell.com> > Cc: Stefan Roese <sr@denx.de> > Cc: Nadav Haklai <nadavh@marvell.com> > Cc: Neta Zur Hershkovits <neta@marvell.com> > Cc: Omri Itach <omrii@marvell.com> > Cc: Igal Liberman <igall@marvell.com> > Cc: Haim Boot <hayim@marvell.com> > Cc: Hanna Hawa <hannah@marvell.com> > --- > arch/arm/dts/armada-7040-db.dts | 38 +++++++++++++++++++++++ > arch/arm/dts/armada-8040-db.dts | 57 +++++++++++++++++++++++++++++++++++ > arch/arm/dts/armada-ap806.dtsi | 18 +++++++++++ > arch/arm/dts/armada-cp110-master.dtsi | 32 ++++++++++++++++++++ > arch/arm/dts/armada-cp110-slave.dtsi | 19 ++++++++++++ > 5 files changed, 164 insertions(+) > > diff --git a/arch/arm/dts/armada-7040-db.dts b/arch/arm/dts/armada-7040-db.dts > index b8fe5a9..1bfb5a9 100644 > --- a/arch/arm/dts/armada-7040-db.dts > +++ b/arch/arm/dts/armada-7040-db.dts > @@ -67,6 +67,8 @@ > }; > > &i2c0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&cpm_i2c0_pins>; > status = "okay"; > clock-frequency = <100000>; > }; > @@ -98,6 +100,17 @@ > }; > }; > > +&ap_pinctl { > + /* MPP Bus: > + SDIO [0-10] > + UART0 [11,19] > + */ Please use the common multiline comment instead: /* * MPP Bus: * SDIO [0-10] * UART0 [11,19] */ > + /* 0 1 2 3 4 5 6 7 8 9 */ > + pin-func = < 1 1 1 1 1 1 1 1 1 1 > + 1 3 0 0 0 0 0 0 0 3>; Is there any chance to not use those numeric values but some macros instead to make it clearer, which function is selected? > + status = "okay"; > +}; > + > &uart0 { > status = "okay"; > }; > @@ -112,8 +125,33 @@ > clock-frequency = <100000>; > }; > > +&cpm_pinctl { > + /* MPP Bus: > + TDM [0-11] > + SPI [13-16] > + SATA1 [28] > + UART0 [29-30] > + SMI [32,34] > + XSMI [35-36] > + I2C [37-38] > + RGMII1[44-55] > + SD [56-62] > + */ Again, comment style please. > + /* 0 1 2 3 4 5 6 7 8 9 */ > + pin-func = < 4 4 4 4 4 4 4 4 4 4 > + 4 4 0 3 3 3 3 0 0 0 > + 0 0 0 0 0 0 0 0 9 0xA > + 0xA 0 7 0 7 7 7 2 2 0 > + 0 0 0 0 1 1 1 1 1 1 > + 1 1 1 1 1 1 0xE 0xE 0xE 0xE > + 0xE 0xE 0xE>; Lower case hex values please (globally). > + status = "okay"; > +}; > + > &cpm_spi1 { > status = "okay"; > + pinctrl-names = "default"; > + pinctrl-0 = <&cpm_spi0_pins>; > > spi-flash@0 { > #address-cells = <0x1>; > diff --git a/arch/arm/dts/armada-8040-db.dts b/arch/arm/dts/armada-8040-db.dts > index 86666a1..30fd364 100644 > --- a/arch/arm/dts/armada-8040-db.dts > +++ b/arch/arm/dts/armada-8040-db.dts > @@ -71,6 +71,42 @@ > status = "okay"; > }; > > +&ap_pinctl { > + /* MPP Bus: > + SPI0 [0-3] > + I2C0 [4-5] > + UART0 [11,19] > + */ > + /* 0 1 2 3 4 5 6 7 8 9 */ > + pin-func = < 3 3 3 3 3 3 0 0 0 0 > + 0 3 0 0 0 0 0 0 0 3>; > +}; > + > +&cpm_pinctl { > + /* MPP Bus: > + [0-31] = 0xff: Keep default CP0_shared_pins: > + [11] CLKOUT_MPP_11 (out) > + [23] LINK_RD_IN_CP2CP (in) > + [25] CLKOUT_MPP_25 (out) > + [29] AVS_FB_IN_CP2CP (in) > + [32,34] SMI > + [31] GPIO: push button/Wake > + [35-36] GPIO > + [37-38] I2C > + [40-41] SATA[0/1]_PRESENT_ACTIVEn > + [42-43] XSMI > + [44-55] RGMII1 > + [56-62] SD > + */ > + /* 0 1 2 3 4 5 6 7 8 9 */ > + pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff > + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff > + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff > + 0xff 0 7 0 7 0 0 2 2 0 > + 0 0 8 8 1 1 1 1 1 1 > + 1 1 1 1 1 1 0xE 0xE 0xE 0xE > + 0xE 0xE 0xE>; > +}; > > /* CON5 on CP0 expansion */ > &cpm_pcie2 { > @@ -78,6 +114,8 @@ > }; > > &cpm_i2c0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&cpm_i2c0_pins>; > status = "okay"; > clock-frequency = <100000>; > }; > @@ -97,12 +135,31 @@ > status = "okay"; > }; > > +&cps_pinctl { > + /* MPP Bus: > + [0-11] RGMII0 > + [13-16] SPI1 > + [27,31] GE_MDIO/MDC > + [32-62] = 0xff: Keep default CP1_shared_pins: > + */ > + /* 0 1 2 3 4 5 6 7 8 9 */ > + pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 > + 0x3 0x3 0xff 0x3 0x3 0x3 0x3 0xff 0xff 0xff > + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff > + 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff > + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff > + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff > + 0xff 0xff 0xff>; > +}; > + > /* CON5 on CP1 expansion */ > &cps_pcie2 { > status = "okay"; > }; > > &cps_spi1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&cps_spi1_pins>; > status = "okay"; > > spi-flash@0 { > diff --git a/arch/arm/dts/armada-ap806.dtsi b/arch/arm/dts/armada-ap806.dtsi > index d315b29..efb383b 100644 > --- a/arch/arm/dts/armada-ap806.dtsi > +++ b/arch/arm/dts/armada-ap806.dtsi > @@ -140,6 +140,24 @@ > marvell,spi-base = <128>, <136>, <144>, <152>; > }; > > + ap_pinctl: ap-pinctl@6F4000 { > + compatible = "marvell,armada-ap806-pinctrl"; > + bank-name ="apn-806"; > + reg = <0x6F4000 0x10>; > + pin-count = <20>; > + max-func = <3>; > + > + ap_i2c0_pins: i2c-pins-0 { > + marvell,pins = < 4 5 >; > + marvell,function = <3>; So what are these marvell,pins/functions? They are not listed in the DT bindings documentation. > + }; > + ap_emmc_pins: emmc-pins-0 { > + marvell,pins = < 0 1 2 3 4 5 6 7 > + 8 9 10 >; > + marvell,function = <1>; > + }; > + }; > + > xor@400000 { > compatible = "marvell,mv-xor-v2"; > reg = <0x400000 0x1000>, > diff --git a/arch/arm/dts/armada-cp110-master.dtsi b/arch/arm/dts/armada-cp110-master.dtsi > index 422d754..d637867 100644 > --- a/arch/arm/dts/armada-cp110-master.dtsi > +++ b/arch/arm/dts/armada-cp110-master.dtsi > @@ -81,6 +81,38 @@ > "cpm-usb3dev", "cpm-eip150", "cpm-eip197"; > }; > > + cpm_pinctl: cpm-pinctl@440000 { > + compatible = "marvell,mvebu-pinctrl", > + "marvell,a70x0-pinctrl", > + "marvell,a80x0-cp0-pinctrl"; > + bank-name ="cp0-110"; > + reg = <0x440000 0x20>; > + pin-count = <63>; > + max-func = <0xf>; > + > + cpm_i2c0_pins: cpm-i2c-pins-0 { > + marvell,pins = < 37 38 >; > + marvell,function = <2>; > + }; > + cpm_ge2_rgmii_pins: cpm-ge-rgmii-pins-0 { > + marvell,pins = < 44 45 46 47 48 49 50 51 > + 52 53 54 55 >; > + marvell,function = <1>; > + }; > + pca0_pins: cpm-pca0_pins { > + marvell,pins = <62>; > + marvell,function = <0>; > + }; > + cpm_sdhci_pins: cpm-sdhi-pins-0 { > + marvell,pins = < 56 57 58 59 60 61 >; > + marvell,function = <14>; > + }; > + cpm_spi0_pins: cpm-spi-pins-0 { > + marvell,pins = < 13 14 15 16 >; > + marvell,function = <3>; > + }; > + }; > + > cpm_sata0: sata@540000 { > compatible = "marvell,armada-8k-ahci"; > reg = <0x540000 0x30000>; > diff --git a/arch/arm/dts/armada-cp110-slave.dtsi b/arch/arm/dts/armada-cp110-slave.dtsi > index a7f77b9..92ef55c 100644 > --- a/arch/arm/dts/armada-cp110-slave.dtsi > +++ b/arch/arm/dts/armada-cp110-slave.dtsi > @@ -81,6 +81,25 @@ > "cps-usb3dev", "cps-eip150", "cps-eip197"; > }; > > + cps_pinctl: cps-pinctl@440000 { > + compatible = "marvell,mvebu-pinctrl", > + "marvell,a80x0-cp1-pinctrl"; > + bank-name ="cp1-110"; > + reg = <0x440000 0x20>; > + pin-count = <63>; > + max-func = <0xf>; > + > + cps_ge1_rgmii_pins: cps-ge-rgmii-pins-0 { > + marvell,pins = < 0 1 2 3 4 5 6 7 > + 8 9 10 11 >; > + marvell,function = <3>; > + }; > + cps_spi1_pins: cps-spi-pins-1 { > + marvell,pins = < 13 14 15 16 >; > + marvell,function = <3>; > + }; > + }; > + > cps_sata0: sata@540000 { > compatible = "marvell,armada-8k-ahci"; > reg = <0x540000 0x30000>; > Thanks, Stefan
Hi Kosta, On 24.11.2016 15:09, Kostya Porotchkin wrote: > Thank you for your review! > I will put all required changes into second patch version. Thanks. > Regarding the symbolic names for the pin controller functions > and lack of documentation. > The problem is that same function number does not have the > same meaning for different pins. > So if I want to put symbolic names instead of numbers, I have > to add large structures defining symbolic names for each > function on every pin as a platform data. > I think in this case I will loose the driver code compactness > provided by the FDT usage. I suspected that something like this might be the reason for the "plain numbers". But I also suspect that other SoCs might suffer from the same problem. Did you take a look at other pinctrl implementation (not only in U-Boot but also in Linux). How is this solved for other SoCs (if this problem exists there as well)? > I can create a documentation file with all pin function values > taken from SoC HW manual and keep the numeric function IDs for > the DTS usage. Is this something that you will create manually? Or can this be created automatically from some documentation of internal source? I'm asking, since manual creation always has the potential problem of erroneous values. > Will it be good enough? This will help of course. Thanks, Stefan
diff --git a/arch/arm/dts/armada-7040-db.dts b/arch/arm/dts/armada-7040-db.dts index b8fe5a9..1bfb5a9 100644 --- a/arch/arm/dts/armada-7040-db.dts +++ b/arch/arm/dts/armada-7040-db.dts @@ -67,6 +67,8 @@ }; &i2c0 { + pinctrl-names = "default"; + pinctrl-0 = <&cpm_i2c0_pins>; status = "okay"; clock-frequency = <100000>; }; @@ -98,6 +100,17 @@ }; }; +&ap_pinctl { + /* MPP Bus: + SDIO [0-10] + UART0 [11,19] + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 1 1 1 1 1 1 1 1 1 1 + 1 3 0 0 0 0 0 0 0 3>; + status = "okay"; +}; + &uart0 { status = "okay"; }; @@ -112,8 +125,33 @@ clock-frequency = <100000>; }; +&cpm_pinctl { + /* MPP Bus: + TDM [0-11] + SPI [13-16] + SATA1 [28] + UART0 [29-30] + SMI [32,34] + XSMI [35-36] + I2C [37-38] + RGMII1[44-55] + SD [56-62] + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 4 4 4 4 4 4 4 4 4 4 + 4 4 0 3 3 3 3 0 0 0 + 0 0 0 0 0 0 0 0 9 0xA + 0xA 0 7 0 7 7 7 2 2 0 + 0 0 0 0 1 1 1 1 1 1 + 1 1 1 1 1 1 0xE 0xE 0xE 0xE + 0xE 0xE 0xE>; + status = "okay"; +}; + &cpm_spi1 { status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&cpm_spi0_pins>; spi-flash@0 { #address-cells = <0x1>; diff --git a/arch/arm/dts/armada-8040-db.dts b/arch/arm/dts/armada-8040-db.dts index 86666a1..30fd364 100644 --- a/arch/arm/dts/armada-8040-db.dts +++ b/arch/arm/dts/armada-8040-db.dts @@ -71,6 +71,42 @@ status = "okay"; }; +&ap_pinctl { + /* MPP Bus: + SPI0 [0-3] + I2C0 [4-5] + UART0 [11,19] + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 3 3 3 3 3 3 0 0 0 0 + 0 3 0 0 0 0 0 0 0 3>; +}; + +&cpm_pinctl { + /* MPP Bus: + [0-31] = 0xff: Keep default CP0_shared_pins: + [11] CLKOUT_MPP_11 (out) + [23] LINK_RD_IN_CP2CP (in) + [25] CLKOUT_MPP_25 (out) + [29] AVS_FB_IN_CP2CP (in) + [32,34] SMI + [31] GPIO: push button/Wake + [35-36] GPIO + [37-38] I2C + [40-41] SATA[0/1]_PRESENT_ACTIVEn + [42-43] XSMI + [44-55] RGMII1 + [56-62] SD + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0 7 0 7 0 0 2 2 0 + 0 0 8 8 1 1 1 1 1 1 + 1 1 1 1 1 1 0xE 0xE 0xE 0xE + 0xE 0xE 0xE>; +}; /* CON5 on CP0 expansion */ &cpm_pcie2 { @@ -78,6 +114,8 @@ }; &cpm_i2c0 { + pinctrl-names = "default"; + pinctrl-0 = <&cpm_i2c0_pins>; status = "okay"; clock-frequency = <100000>; }; @@ -97,12 +135,31 @@ status = "okay"; }; +&cps_pinctl { + /* MPP Bus: + [0-11] RGMII0 + [13-16] SPI1 + [27,31] GE_MDIO/MDC + [32-62] = 0xff: Keep default CP1_shared_pins: + */ + /* 0 1 2 3 4 5 6 7 8 9 */ + pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 + 0x3 0x3 0xff 0x3 0x3 0x3 0x3 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff + 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff + 0xff 0xff 0xff>; +}; + /* CON5 on CP1 expansion */ &cps_pcie2 { status = "okay"; }; &cps_spi1 { + pinctrl-names = "default"; + pinctrl-0 = <&cps_spi1_pins>; status = "okay"; spi-flash@0 { diff --git a/arch/arm/dts/armada-ap806.dtsi b/arch/arm/dts/armada-ap806.dtsi index d315b29..efb383b 100644 --- a/arch/arm/dts/armada-ap806.dtsi +++ b/arch/arm/dts/armada-ap806.dtsi @@ -140,6 +140,24 @@ marvell,spi-base = <128>, <136>, <144>, <152>; }; + ap_pinctl: ap-pinctl@6F4000 { + compatible = "marvell,armada-ap806-pinctrl"; + bank-name ="apn-806"; + reg = <0x6F4000 0x10>; + pin-count = <20>; + max-func = <3>; + + ap_i2c0_pins: i2c-pins-0 { + marvell,pins = < 4 5 >; + marvell,function = <3>; + }; + ap_emmc_pins: emmc-pins-0 { + marvell,pins = < 0 1 2 3 4 5 6 7 + 8 9 10 >; + marvell,function = <1>; + }; + }; + xor@400000 { compatible = "marvell,mv-xor-v2"; reg = <0x400000 0x1000>, diff --git a/arch/arm/dts/armada-cp110-master.dtsi b/arch/arm/dts/armada-cp110-master.dtsi index 422d754..d637867 100644 --- a/arch/arm/dts/armada-cp110-master.dtsi +++ b/arch/arm/dts/armada-cp110-master.dtsi @@ -81,6 +81,38 @@ "cpm-usb3dev", "cpm-eip150", "cpm-eip197"; }; + cpm_pinctl: cpm-pinctl@440000 { + compatible = "marvell,mvebu-pinctrl", + "marvell,a70x0-pinctrl", + "marvell,a80x0-cp0-pinctrl"; + bank-name ="cp0-110"; + reg = <0x440000 0x20>; + pin-count = <63>; + max-func = <0xf>; + + cpm_i2c0_pins: cpm-i2c-pins-0 { + marvell,pins = < 37 38 >; + marvell,function = <2>; + }; + cpm_ge2_rgmii_pins: cpm-ge-rgmii-pins-0 { + marvell,pins = < 44 45 46 47 48 49 50 51 + 52 53 54 55 >; + marvell,function = <1>; + }; + pca0_pins: cpm-pca0_pins { + marvell,pins = <62>; + marvell,function = <0>; + }; + cpm_sdhci_pins: cpm-sdhi-pins-0 { + marvell,pins = < 56 57 58 59 60 61 >; + marvell,function = <14>; + }; + cpm_spi0_pins: cpm-spi-pins-0 { + marvell,pins = < 13 14 15 16 >; + marvell,function = <3>; + }; + }; + cpm_sata0: sata@540000 { compatible = "marvell,armada-8k-ahci"; reg = <0x540000 0x30000>; diff --git a/arch/arm/dts/armada-cp110-slave.dtsi b/arch/arm/dts/armada-cp110-slave.dtsi index a7f77b9..92ef55c 100644 --- a/arch/arm/dts/armada-cp110-slave.dtsi +++ b/arch/arm/dts/armada-cp110-slave.dtsi @@ -81,6 +81,25 @@ "cps-usb3dev", "cps-eip150", "cps-eip197"; }; + cps_pinctl: cps-pinctl@440000 { + compatible = "marvell,mvebu-pinctrl", + "marvell,a80x0-cp1-pinctrl"; + bank-name ="cp1-110"; + reg = <0x440000 0x20>; + pin-count = <63>; + max-func = <0xf>; + + cps_ge1_rgmii_pins: cps-ge-rgmii-pins-0 { + marvell,pins = < 0 1 2 3 4 5 6 7 + 8 9 10 11 >; + marvell,function = <3>; + }; + cps_spi1_pins: cps-spi-pins-1 { + marvell,pins = < 13 14 15 16 >; + marvell,function = <3>; + }; + }; + cps_sata0: sata@540000 { compatible = "marvell,armada-8k-ahci"; reg = <0x540000 0x30000>;