From: Kostya Porotchkin <kostap@marvell.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files
Date: Thu, 24 Nov 2016 14:09:08 +0000 [thread overview]
Message-ID: <1479996649610.82419@marvell.com> (raw)
In-Reply-To: <420c04d6-b507-6d82-d187-e1295f2e451c@denx.de>
Hi, Stefan,
Thank you for your review!
I will put all required changes into second patch version.
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 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.
Will it be good enough?
Best Regards
Konstantin
________________________________________
From: Stefan Roese <sr@denx.de>
Sent: Thursday, November 24, 2016 11:13
To: Kostya Porotchkin; u-boot at lists.denx.de
Cc: Nadav Haklai; Neta Zur Hershkovits; Omri Itach; Igal Liberman; Haim Boot; Hanna Hawa
Subject: Re: [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files
Hi Kosta,
On 20.11.2016 16:38, kostap at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 540000 {
> compatible = "marvell,armada-8k-ahci";
> reg = <0x540000 0x30000>;
>
Thanks,
Stefan
next prev parent reply other threads:[~2016-11-24 14:09 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-20 15:38 [U-Boot] [PATCH 0/6] arm64: mvebu: Armada-8K family patches kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS kostap at marvell.com
2016-11-24 9:02 ` Stefan Roese
2016-11-24 9:22 ` Kostya Porotchkin
2016-11-24 12:05 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 2/6] arm64: mvebu: Add bubt command for flash image burn kostap at marvell.com
2016-11-24 8:58 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family kostap at marvell.com
2016-11-24 2:20 ` Simon Glass
2016-11-24 8:28 ` Kostya Porotchkin
2016-11-20 15:38 ` [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files kostap at marvell.com
2016-11-24 9:13 ` Stefan Roese
2016-11-24 14:09 ` Kostya Porotchkin [this message]
2016-11-24 16:00 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 5/6] arm64: mvebu: Enable BUBT command support in A8K default config kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 6/6] arm64: mvebu: Enable pin control " kostap at marvell.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1479996649610.82419@marvell.com \
--to=kostap@marvell.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox