diff mbox

[U-Boot,v9,04/12] dm: exynos: dts: Adjust device tree files for U-Boot

Message ID 1413362282-25451-5-git-send-email-sjg@chromium.org
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass Oct. 15, 2014, 8:37 a.m. UTC
The pinctrl bindings used by Linux are an incomplete description of the
hardware. It is possible in most cases to determine the register address
of each, but not in all cases. By adding an additional property we can
fix this, and avoid adding a table to U-Boot for every single Exynos
SOC.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v9: None
Changes in v8:
- Add missing special case reg property for exynos5420 GPX0

Changes in v7: None
Changes in v6:
- Move U-Boot changes into their own file
- Use exynos54xx everywhere instead of exynos5420

Changes in v5: None
Changes in v4: None

 arch/arm/dts/exynos4210-pinctrl-uboot.dtsi | 27 ++++++++++++++++++
 arch/arm/dts/exynos4210-pinctrl.dtsi       |  2 ++
 arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi | 46 ++++++++++++++++++++++++++++++
 arch/arm/dts/exynos4x12-pinctrl.dtsi       |  2 ++
 arch/arm/dts/exynos5250-pinctrl-uboot.dtsi | 40 ++++++++++++++++++++++++++
 arch/arm/dts/exynos5250-pinctrl.dtsi       |  2 ++
 arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi | 40 ++++++++++++++++++++++++++
 arch/arm/dts/exynos54xx-pinctrl.dtsi       |  2 ++
 arch/arm/dts/exynos54xx.dtsi               |  1 +
 9 files changed, 162 insertions(+)
 create mode 100644 arch/arm/dts/exynos4210-pinctrl-uboot.dtsi
 create mode 100644 arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
 create mode 100644 arch/arm/dts/exynos5250-pinctrl-uboot.dtsi
 create mode 100644 arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi

Comments

Przemyslaw Marczak Oct. 20, 2014, 2:55 p.m. UTC | #1
Hello Simon,

On 10/15/2014 10:37 AM, Simon Glass wrote:
> The pinctrl bindings used by Linux are an incomplete description of the
> hardware. It is possible in most cases to determine the register address
> of each, but not in all cases. By adding an additional property we can
> fix this, and avoid adding a table to U-Boot for every single Exynos
> SOC.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v9: None
> Changes in v8:
> - Add missing special case reg property for exynos5420 GPX0
>
> Changes in v7: None
> Changes in v6:
> - Move U-Boot changes into their own file
> - Use exynos54xx everywhere instead of exynos5420
>
> Changes in v5: None
> Changes in v4: None
>
>   arch/arm/dts/exynos4210-pinctrl-uboot.dtsi | 27 ++++++++++++++++++
>   arch/arm/dts/exynos4210-pinctrl.dtsi       |  2 ++
>   arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi | 46 ++++++++++++++++++++++++++++++
>   arch/arm/dts/exynos4x12-pinctrl.dtsi       |  2 ++
>   arch/arm/dts/exynos5250-pinctrl-uboot.dtsi | 40 ++++++++++++++++++++++++++
>   arch/arm/dts/exynos5250-pinctrl.dtsi       |  2 ++
>   arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi | 40 ++++++++++++++++++++++++++
>   arch/arm/dts/exynos54xx-pinctrl.dtsi       |  2 ++
>   arch/arm/dts/exynos54xx.dtsi               |  1 +
>   9 files changed, 162 insertions(+)
>   create mode 100644 arch/arm/dts/exynos4210-pinctrl-uboot.dtsi
>   create mode 100644 arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
>   create mode 100644 arch/arm/dts/exynos5250-pinctrl-uboot.dtsi
>   create mode 100644 arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi
>

... snip ...

> diff --git a/arch/arm/dts/exynos4210-pinctrl.dtsi b/arch/arm/dts/exynos4210-pinctrl.dtsi
> index bda17f7..87f162b 100644
> --- a/arch/arm/dts/exynos4210-pinctrl.dtsi
> +++ b/arch/arm/dts/exynos4210-pinctrl.dtsi
> @@ -14,6 +14,8 @@
>    * published by the Free Software Foundation.
>   */
>
> +#include "exynos4210-pinctrl-uboot.dtsi"
> +
>   / {
>   	pinctrl@11400000 {
>   		gpa0: gpa0 {
> diff --git a/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi b/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
> new file mode 100644
> index 0000000..c02796d
> --- /dev/null
> +++ b/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
> @@ -0,0 +1,46 @@
> +/*
> + * U-Boot additions to enable a generic Exynos GPIO driver
> + *
> + * Copyright (c) 2014 Google, Inc
> + */
> +
> +/{
> +	pinctrl_0: pinctrl@11400000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;

The first issue with Exynos GPIO driver starts here. If you put pinctrl* 
node data in a separated file, then the order of nodes in dtb is 
changed. So for the pinctrl 0, the first subnode is gpf0, instead of 
gpa0. And the same is for other pinctrls.

This means that function gpio_exynos_bind(), which expects proper 
subnodes order assign wrong base addresses to some GPIO pins.

Move "reg" properties into arch/arm/dts/exynos4x12-pinctrl.dtsi fixes 
this issue. But in this case, the file *-pinctrl-uboot.dtsi is quite 
useless - since the cells size can be moved to pinctrl.dtsi too.

This probably will touch all *pinctrl-uboot.dtsi files.
Simon Glass Oct. 20, 2014, 3:29 p.m. UTC | #2
Hi,

On 20 October 2014 08:55, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> Hello Simon,
>
>
> On 10/15/2014 10:37 AM, Simon Glass wrote:
>>
>> The pinctrl bindings used by Linux are an incomplete description of the
>> hardware. It is possible in most cases to determine the register address
>> of each, but not in all cases. By adding an additional property we can
>> fix this, and avoid adding a table to U-Boot for every single Exynos
>> SOC.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v9: None
>> Changes in v8:
>> - Add missing special case reg property for exynos5420 GPX0
>>
>> Changes in v7: None
>> Changes in v6:
>> - Move U-Boot changes into their own file
>> - Use exynos54xx everywhere instead of exynos5420
>>
>> Changes in v5: None
>> Changes in v4: None
>>
>>   arch/arm/dts/exynos4210-pinctrl-uboot.dtsi | 27 ++++++++++++++++++
>>   arch/arm/dts/exynos4210-pinctrl.dtsi       |  2 ++
>>   arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi | 46
>> ++++++++++++++++++++++++++++++
>>   arch/arm/dts/exynos4x12-pinctrl.dtsi       |  2 ++
>>   arch/arm/dts/exynos5250-pinctrl-uboot.dtsi | 40
>> ++++++++++++++++++++++++++
>>   arch/arm/dts/exynos5250-pinctrl.dtsi       |  2 ++
>>   arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi | 40
>> ++++++++++++++++++++++++++
>>   arch/arm/dts/exynos54xx-pinctrl.dtsi       |  2 ++
>>   arch/arm/dts/exynos54xx.dtsi               |  1 +
>>   9 files changed, 162 insertions(+)
>>   create mode 100644 arch/arm/dts/exynos4210-pinctrl-uboot.dtsi
>>   create mode 100644 arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
>>   create mode 100644 arch/arm/dts/exynos5250-pinctrl-uboot.dtsi
>>   create mode 100644 arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi
>>
>
> ... snip ...
>
>
>> diff --git a/arch/arm/dts/exynos4210-pinctrl.dtsi
>> b/arch/arm/dts/exynos4210-pinctrl.dtsi
>> index bda17f7..87f162b 100644
>> --- a/arch/arm/dts/exynos4210-pinctrl.dtsi
>> +++ b/arch/arm/dts/exynos4210-pinctrl.dtsi
>> @@ -14,6 +14,8 @@
>>    * published by the Free Software Foundation.
>>   */
>>
>> +#include "exynos4210-pinctrl-uboot.dtsi"
>> +
>>   / {
>>         pinctrl@11400000 {
>>                 gpa0: gpa0 {
>> diff --git a/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
>> b/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
>> new file mode 100644
>> index 0000000..c02796d
>> --- /dev/null
>> +++ b/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
>> @@ -0,0 +1,46 @@
>> +/*
>> + * U-Boot additions to enable a generic Exynos GPIO driver
>> + *
>> + * Copyright (c) 2014 Google, Inc
>> + */
>> +
>> +/{
>> +       pinctrl_0: pinctrl@11400000 {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>
>
> The first issue with Exynos GPIO driver starts here. If you put pinctrl*
> node data in a separated file, then the order of nodes in dtb is changed. So
> for the pinctrl 0, the first subnode is gpf0, instead of gpa0. And the same
> is for other pinctrls.
>
> This means that function gpio_exynos_bind(), which expects proper subnodes
> order assign wrong base addresses to some GPIO pins.
>
> Move "reg" properties into arch/arm/dts/exynos4x12-pinctrl.dtsi fixes this
> issue. But in this case, the file *-pinctrl-uboot.dtsi is quite useless -
> since the cells size can be moved to pinctrl.dtsi too.
>
> This probably will touch all *pinctrl-uboot.dtsi files.

Yes this was a mistake in the series. I have pushed an updated series
to u-boot-dm/next. I will go through your comments as well before I
see if a new series is warranted, or just a few patch updates.

Regards,
Simon
diff mbox

Patch

diff --git a/arch/arm/dts/exynos4210-pinctrl-uboot.dtsi b/arch/arm/dts/exynos4210-pinctrl-uboot.dtsi
new file mode 100644
index 0000000..ee071c1
--- /dev/null
+++ b/arch/arm/dts/exynos4210-pinctrl-uboot.dtsi
@@ -0,0 +1,27 @@ 
+/*
+ * U-Boot additions to enable a generic Exynos GPIO driver
+ *
+ * Copyright (c) 2014 Google, Inc
+ */
+
+/{
+	pinctrl_0: pinctrl@11400000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "samsung,exynos4210-pinctrl";
+	};
+
+	pinctrl_1: pinctrl@11000000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		gpy0: gpy0 {
+			reg = <0xc00>;
+		};
+	};
+
+	pinctrl_2: pinctrl@03860000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+};
diff --git a/arch/arm/dts/exynos4210-pinctrl.dtsi b/arch/arm/dts/exynos4210-pinctrl.dtsi
index bda17f7..87f162b 100644
--- a/arch/arm/dts/exynos4210-pinctrl.dtsi
+++ b/arch/arm/dts/exynos4210-pinctrl.dtsi
@@ -14,6 +14,8 @@ 
  * published by the Free Software Foundation.
 */
 
+#include "exynos4210-pinctrl-uboot.dtsi"
+
 / {
 	pinctrl@11400000 {
 		gpa0: gpa0 {
diff --git a/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi b/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
new file mode 100644
index 0000000..c02796d
--- /dev/null
+++ b/arch/arm/dts/exynos4x12-pinctrl-uboot.dtsi
@@ -0,0 +1,46 @@ 
+/*
+ * U-Boot additions to enable a generic Exynos GPIO driver
+ *
+ * Copyright (c) 2014 Google, Inc
+ */
+
+/{
+	pinctrl_0: pinctrl@11400000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		gpf0: gpf0 {
+			reg = <0xc180>;
+		};
+		gpj0: gpj0 {
+			reg = <0x240>;
+		};
+	};
+
+	pinctrl_1: pinctrl@11000000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		gpk0: gpk0 {
+			reg = <0x40>;
+		};
+		gpm0: gpm0 {
+			reg = <0x260>;
+		};
+		gpy0: gpy0 {
+			reg = <0x120>;
+		};
+		gpx0: gpx0 {
+			reg = <0xc00>;
+		};
+	};
+
+	pinctrl_2: pinctrl@03860000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	pinctrl_3: pinctrl@106E0000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+};
diff --git a/arch/arm/dts/exynos4x12-pinctrl.dtsi b/arch/arm/dts/exynos4x12-pinctrl.dtsi
index 93f3998..f40de1f 100644
--- a/arch/arm/dts/exynos4x12-pinctrl.dtsi
+++ b/arch/arm/dts/exynos4x12-pinctrl.dtsi
@@ -12,6 +12,8 @@ 
  * published by the Free Software Foundation.
 */
 
+#include "exynos4x12-pinctrl-uboot.dtsi"
+
 / {
 	pinctrl@11400000 {
 		gpa0: gpa0 {
diff --git a/arch/arm/dts/exynos5250-pinctrl-uboot.dtsi b/arch/arm/dts/exynos5250-pinctrl-uboot.dtsi
new file mode 100644
index 0000000..7edb0ca
--- /dev/null
+++ b/arch/arm/dts/exynos5250-pinctrl-uboot.dtsi
@@ -0,0 +1,40 @@ 
+/*
+ * U-Boot additions to enable a generic Exynos GPIO driver
+ *
+ * Copyright (c) 2014 Google, Inc
+ */
+
+/{
+	pinctrl_0: pinctrl@11400000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		gpc4: gpc4 {
+			reg = <0x2e0>;
+		};
+		gpx0: gpx0 {
+			reg = <0xc00>;
+		};
+	};
+
+	pinctrl_1: pinctrl@13400000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+	pinctrl_2: pinctrl@10d10000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		gpv2: gpv2 {
+			reg = <0x060>;
+		};
+		gpv4: gpv4 {
+			reg = <0xc0>;
+		};
+	};
+
+	pinctrl_3: pinctrl@03860000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+};
diff --git a/arch/arm/dts/exynos5250-pinctrl.dtsi b/arch/arm/dts/exynos5250-pinctrl.dtsi
index 67755a1..706b888 100644
--- a/arch/arm/dts/exynos5250-pinctrl.dtsi
+++ b/arch/arm/dts/exynos5250-pinctrl.dtsi
@@ -12,6 +12,8 @@ 
  * published by the Free Software Foundation.
 */
 
+#include "exynos5250-pinctrl-uboot.dtsi"
+
 / {
 	pinctrl@11400000 {
 		gpa0: gpa0 {
diff --git a/arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi b/arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi
new file mode 100644
index 0000000..5a86211
--- /dev/null
+++ b/arch/arm/dts/exynos54xx-pinctrl-uboot.dtsi
@@ -0,0 +1,40 @@ 
+/*
+ * U-Boot additions to enable a generic Exynos GPIO driver
+ *
+ * Copyright (c) 2014 Google, Inc
+ */
+
+/{
+	/*
+	 * Replicate the ordering of arch/arm/include/asm/arch-exynos/gpio.h
+	 * TODO(sjg@chromium.org): This ordering ceases to matter once GPIO
+	 * numbers are not needed in U-Boot for exynos.
+	 */
+	pinctrl@14010000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+	pinctrl@13400000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		gpy7 {
+		};
+
+		gpx0 {
+			reg = <0xc00>;
+		};
+	};
+	pinctrl@13410000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+	pinctrl@14000000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+	pinctrl@03860000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
+
+};
diff --git a/arch/arm/dts/exynos54xx-pinctrl.dtsi b/arch/arm/dts/exynos54xx-pinctrl.dtsi
index b3e63d1..775d956 100644
--- a/arch/arm/dts/exynos54xx-pinctrl.dtsi
+++ b/arch/arm/dts/exynos54xx-pinctrl.dtsi
@@ -12,6 +12,8 @@ 
  * published by the Free Software Foundation.
 */
 
+#include "exynos54xx-pinctrl-uboot.dtsi"
+
 / {
 	pinctrl@13400000 {
 		gpy7: gpy7 {
diff --git a/arch/arm/dts/exynos54xx.dtsi b/arch/arm/dts/exynos54xx.dtsi
index 887b034..916cf3a 100644
--- a/arch/arm/dts/exynos54xx.dtsi
+++ b/arch/arm/dts/exynos54xx.dtsi
@@ -6,6 +6,7 @@ 
  */
 
 #include "exynos5.dtsi"
+#include "exynos54xx-pinctrl.dtsi"
 
 / {
 	config {