Message ID | 1413362282-25451-5-git-send-email-sjg@chromium.org |
---|---|
State | Superseded |
Delegated to: | Simon Glass |
Headers | show |
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.
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 --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 {
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