[PATCH 1/2] ARM: dts: Fix compatible value of pinctrl module on EXYNOS5440

[PATCH 1/2] ARM: dts: Fix compatible value of pinctrl module on EXYNOS5440

[Kukjin Kim]

From: Thomas Abraham <thomas.ab@samsung.com>

Fix the incorrect compatible property value of pin-controller module
EXYNOS5440 SoC.

Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
[kgene.kim@samsung.com: fixed it in gpio together for exynos5440]
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Cc: devicetree-discuss@lists.ozlabs.org
---
 arch/arm/boot/dts/exynos5440.dtsi |    2 +-
 drivers/gpio/gpio-samsung.c       |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
index 024269d..25f0fa6 100644
--- a/arch/arm/boot/dts/exynos5440.dtsi
+++ b/arch/arm/boot/dts/exynos5440.dtsi
@@ -86,7 +86,7 @@
 	};
 
 	pinctrl {
-		compatible = "samsung,pinctrl-exynos5440";
+		compatible = "samsung,exynos5440-pinctrl";
 		reg = <0xE0000 0x1000>;
 		interrupt-controller;
 		#interrupt-cells = <2>;
diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
index 953737d..cac4b45 100644
--- a/drivers/gpio/gpio-samsung.c
+++ b/drivers/gpio/gpio-samsung.c
@@ -3026,7 +3026,7 @@ static __init int samsung_gpiolib_init(void)
 	static const struct of_device_id exynos_pinctrl_ids[] = {
 		{ .compatible = "samsung,pinctrl-exynos4210", },
 		{ .compatible = "samsung,pinctrl-exynos4x12", },
-		{ .compatible = "samsung,pinctrl-exynos5440", },
+		{ .compatible = "samsung,exynos5440-pinctrl", },
 	};
 	for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
 		if (pctrl_np && of_device_is_available(pctrl_np))
-- 
1.7.10.4

[PATCH 2/2] ARM: dts: fix compatible value for exynos pinctrl

[Kukjin Kim]

Fix the incorrect compatible property value of pinctrl for EXYNOS4 SoCs.

Cc: Thomas Abraham <thomas.ab@samsung.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Cc: devicetree-discuss@lists.ozlabs.org
---
 .../devicetree/bindings/pinctrl/samsung-pinctrl.txt        |   12 ++++++------
 arch/arm/boot/dts/exynos4210.dtsi                          |    6 +++---
 arch/arm/boot/dts/exynos4x12.dtsi                          |    8 ++++----
 arch/arm/mach-exynos/common.c                              |    4 ++--
 drivers/gpio/gpio-samsung.c                                |    4 ++--
 drivers/pinctrl/pinctrl-samsung.c                          |    4 ++--
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index e97a278..4598a47 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -7,9 +7,9 @@ on-chip controllers onto these pads.
 
 Required Properties:
 - compatible: should be one of the following.
-  - "samsung,pinctrl-exynos4210": for Exynos4210 compatible pin-controller.
-  - "samsung,pinctrl-exynos4x12": for Exynos4x12 compatible pin-controller.
-  - "samsung,pinctrl-exynos5250": for Exynos5250 compatible pin-controller.
+  - "samsung,exynos4210-pinctrl": for Exynos4210 compatible pin-controller.
+  - "samsung,exynos4x12-pinctrl": for Exynos4x12 compatible pin-controller.
+  - "samsung,exynos5250-pinctrl": for Exynos5250 compatible pin-controller.
 
 - reg: Base address of the pin controller hardware module and length of
   the address space it occupies.
@@ -142,7 +142,7 @@ the following format 'pinctrl{n}' where n is a unique number for the alias.
 Example: A pin-controller node with pin banks:
 
 	pinctrl_0: pinctrl@11400000 {
-		compatible = "samsung,pinctrl-exynos4210";
+		compatible = "samsung,exynos4210-pinctrl";
 		reg = <0x11400000 0x1000>;
 		interrupts = <0 47 0>;
 
@@ -185,7 +185,7 @@ Example: A pin-controller node with pin banks:
 Example 1: A pin-controller node with pin groups.
 
 	pinctrl_0: pinctrl@11400000 {
-		compatible = "samsung,pinctrl-exynos4210";
+		compatible = "samsung,exynos4210-pinctrl";
 		reg = <0x11400000 0x1000>;
 		interrupts = <0 47 0>;
 
@@ -230,7 +230,7 @@ Example 1: A pin-controller node with pin groups.
 Example 2: A pin-controller node with external wakeup interrupt controller node.
 
 	pinctrl_1: pinctrl@11000000 {
-		compatible = "samsung,pinctrl-exynos4210";
+		compatible = "samsung,exynos4210-pinctrl";
 		reg = <0x11000000 0x1000>;
 		interrupts = <0 46 0>
 
diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index e31bfc4..2feffc7 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -48,13 +48,13 @@
 	};
 
 	pinctrl_0: pinctrl@11400000 {
-		compatible = "samsung,pinctrl-exynos4210";
+		compatible = "samsung,exynos4210-pinctrl";
 		reg = <0x11400000 0x1000>;
 		interrupts = <0 47 0>;
 	};
 
 	pinctrl_1: pinctrl@11000000 {
-		compatible = "samsung,pinctrl-exynos4210";
+		compatible = "samsung,exynos4210-pinctrl";
 		reg = <0x11000000 0x1000>;
 		interrupts = <0 46 0>;
 
@@ -66,7 +66,7 @@
 	};
 
 	pinctrl_2: pinctrl@03860000 {
-		compatible = "samsung,pinctrl-exynos4210";
+		compatible = "samsung,exynos4210-pinctrl";
 		reg = <0x03860000 0x1000>;
 	};
 
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index 179a62e..9a87806 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -37,13 +37,13 @@
 	};
 
 	pinctrl_0: pinctrl@11400000 {
-		compatible = "samsung,pinctrl-exynos4x12";
+		compatible = "samsung,exynos4x12-pinctrl";
 		reg = <0x11400000 0x1000>;
 		interrupts = <0 47 0>;
 	};
 
 	pinctrl_1: pinctrl@11000000 {
-		compatible = "samsung,pinctrl-exynos4x12";
+		compatible = "samsung,exynos4x12-pinctrl";
 		reg = <0x11000000 0x1000>;
 		interrupts = <0 46 0>;
 
@@ -55,14 +55,14 @@
 	};
 
 	pinctrl_2: pinctrl@03860000 {
-		compatible = "samsung,pinctrl-exynos4x12";
+		compatible = "samsung,exynos4x12-pinctrl";
 		reg = <0x03860000 0x1000>;
 		interrupt-parent = <&combiner>;
 		interrupts = <10 0>;
 	};
 
 	pinctrl_3: pinctrl@106E0000 {
-		compatible = "samsung,pinctrl-exynos4x12";
+		compatible = "samsung,exynos4x12-pinctrl";
 		reg = <0x106E0000 0x1000>;
 		interrupts = <0 72 0>;
 	};
diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index d6d0dc6..3777a41 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -1024,8 +1024,8 @@ static int __init exynos_init_irq_eint(void)
 	 * interrupt support code here can be completely removed.
 	 */
 	static const struct of_device_id exynos_pinctrl_ids[] = {
-		{ .compatible = "samsung,pinctrl-exynos4210", },
-		{ .compatible = "samsung,pinctrl-exynos4x12", },
+		{ .compatible = "samsung,exynos4210-pinctrl", },
+		{ .compatible = "samsung,exynos4x12-pinctrl", },
 	};
 	struct device_node *pctrl_np, *wkup_np;
 	const char *wkup_compat = "samsung,exynos4210-wakeup-eint";
diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
index cac4b45..8c00f7d 100644
--- a/drivers/gpio/gpio-samsung.c
+++ b/drivers/gpio/gpio-samsung.c
@@ -3024,8 +3024,8 @@ static __init int samsung_gpiolib_init(void)
 	*/
 	struct device_node *pctrl_np;
 	static const struct of_device_id exynos_pinctrl_ids[] = {
-		{ .compatible = "samsung,pinctrl-exynos4210", },
-		{ .compatible = "samsung,pinctrl-exynos4x12", },
+		{ .compatible = "samsung,exynos4210-pinctrl", },
+		{ .compatible = "samsung,exynos4x12-pinctrl", },
 		{ .compatible = "samsung,exynos5440-pinctrl", },
 	};
 	for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
index 864fed8..602b30b 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -945,9 +945,9 @@ static int __devinit samsung_pinctrl_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id samsung_pinctrl_dt_match[] = {
-	{ .compatible = "samsung,pinctrl-exynos4210",
+	{ .compatible = "samsung,exynos4210-pinctrl",
 		.data = (void *)exynos4210_pin_ctrl },
-	{ .compatible = "samsung,pinctrl-exynos4x12",
+	{ .compatible = "samsung,exynos4x12-pinctrl",
 		.data = (void *)exynos4x12_pin_ctrl },
 	{},
 };
-- 
1.7.10.4

Re: [PATCH 1/2] ARM: dts: Fix compatible value of pinctrl module on EXYNOS5440

[Olof Johansson]

On Wed, Jan 2, 2013 at 4:20 PM, Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> From: Thomas Abraham <thomas.ab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
> Fix the incorrect compatible property value of pin-controller module
> EXYNOS5440 SoC.
>
> Signed-off-by: Thomas Abraham <thomas.ab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> [kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org: fixed it in gpio together for exynos5440]
> Signed-off-by: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> ---
>  arch/arm/boot/dts/exynos5440.dtsi |    2 +-
>  drivers/gpio/gpio-samsung.c       |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5440.dtsi b/arch/arm/boot/dts/exynos5440.dtsi
> index 024269d..25f0fa6 100644
> --- a/arch/arm/boot/dts/exynos5440.dtsi
> +++ b/arch/arm/boot/dts/exynos5440.dtsi
> @@ -86,7 +86,7 @@
>         };
>
>         pinctrl {
> -               compatible = "samsung,pinctrl-exynos5440";
> +               compatible = "samsung,exynos5440-pinctrl";
>                 reg = <0xE0000 0x1000>;
>                 interrupt-controller;
>                 #interrupt-cells = <2>;
> diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
> index 953737d..cac4b45 100644
> --- a/drivers/gpio/gpio-samsung.c
> +++ b/drivers/gpio/gpio-samsung.c
> @@ -3026,7 +3026,7 @@ static __init int samsung_gpiolib_init(void)
>         static const struct of_device_id exynos_pinctrl_ids[] = {
>                 { .compatible = "samsung,pinctrl-exynos4210", },
>                 { .compatible = "samsung,pinctrl-exynos4x12", },
> -               { .compatible = "samsung,pinctrl-exynos5440", },
> +               { .compatible = "samsung,exynos5440-pinctrl", },

Uugh. There's something to be said about consistency, even if it's
possibly not 100% correct. Is it really worth it to have 5440 be the
only one with the "right" naming here?


-Olof

Re: [PATCH 2/2] ARM: dts: fix compatible value for exynos pinctrl

[Olof Johansson]

On Wed, Jan 2, 2013 at 4:20 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Fix the incorrect compatible property value of pinctrl for EXYNOS4 SoCs.

Ah, this answers my question from the previous patch. So, why do 5450
as a separate patch?


> diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> index e97a278..4598a47 100644
> --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> @@ -7,9 +7,9 @@ on-chip controllers onto these pads.
>
>  Required Properties:
>  - compatible: should be one of the following.
> -  - "samsung,pinctrl-exynos4210": for Exynos4210 compatible pin-controller.
> -  - "samsung,pinctrl-exynos4x12": for Exynos4x12 compatible pin-controller.
> -  - "samsung,pinctrl-exynos5250": for Exynos5250 compatible pin-controller.
> +  - "samsung,exynos4210-pinctrl": for Exynos4210 compatible pin-controller.
> +  - "samsung,exynos4x12-pinctrl": for Exynos4x12 compatible pin-controller.
> +  - "samsung,exynos5250-pinctrl": for Exynos5250 compatible pin-controller.

Do we care about backwards compatibility for ARM device trees yet?
It's becoming time to start caring soon, if we don't. So while this
might be OK for this time around, we should start requiring some
backwards compatibility for bindings that have been used in at least
one released kernel.

Once we do care, then you can update the dts/dtsi files, but you'll
need to keep both the old and the new bindings in the C file below.


> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index d6d0dc6..3777a41 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -1024,8 +1024,8 @@ static int __init exynos_init_irq_eint(void)
>          * interrupt support code here can be completely removed.
>          */
>         static const struct of_device_id exynos_pinctrl_ids[] = {
> -               { .compatible = "samsung,pinctrl-exynos4210", },
> -               { .compatible = "samsung,pinctrl-exynos4x12", },
> +               { .compatible = "samsung,exynos4210-pinctrl", },
> +               { .compatible = "samsung,exynos4x12-pinctrl", },
>         };
>         struct device_node *pctrl_np, *wkup_np;
>         const char *wkup_compat = "samsung,exynos4210-wakeup-eint";
> diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
> index cac4b45..8c00f7d 100644
> --- a/drivers/gpio/gpio-samsung.c
> +++ b/drivers/gpio/gpio-samsung.c
> @@ -3024,8 +3024,8 @@ static __init int samsung_gpiolib_init(void)
>         */
>         struct device_node *pctrl_np;
>         static const struct of_device_id exynos_pinctrl_ids[] = {
> -               { .compatible = "samsung,pinctrl-exynos4210", },
> -               { .compatible = "samsung,pinctrl-exynos4x12", },
> +               { .compatible = "samsung,exynos4210-pinctrl", },
> +               { .compatible = "samsung,exynos4x12-pinctrl", },
>                 { .compatible = "samsung,exynos5440-pinctrl", },
>         };
>         for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
> diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
> index 864fed8..602b30b 100644
> --- a/drivers/pinctrl/pinctrl-samsung.c
> +++ b/drivers/pinctrl/pinctrl-samsung.c
> @@ -945,9 +945,9 @@ static int __devinit samsung_pinctrl_probe(struct platform_device *pdev)
>  }
>
>  static const struct of_device_id samsung_pinctrl_dt_match[] = {
> -       { .compatible = "samsung,pinctrl-exynos4210",
> +       { .compatible = "samsung,exynos4210-pinctrl",
>                 .data = (void *)exynos4210_pin_ctrl },
> -       { .compatible = "samsung,pinctrl-exynos4x12",
> +       { .compatible = "samsung,exynos4x12-pinctrl",
>                 .data = (void *)exynos4x12_pin_ctrl },
>         {},
>  };


-Olof

RE: [PATCH 2/2] ARM: dts: fix compatible value for exynos pinctrl

[Kukjin Kim]

Olof Johansson wrote:
> 
> On Wed, Jan 2, 2013 at 4:20 PM, Kukjin Kim <kgene.kim@samsung.com>
> wrote:
> > Fix the incorrect compatible property value of pinctrl for EXYNOS4 SoCs.
> 
> Ah, this answers my question from the previous patch. So, why do 5450
> as a separate patch?
> 
OK, let me create one patch to fix it :-)

> 
> > diff --git
a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > index e97a278..4598a47 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > @@ -7,9 +7,9 @@ on-chip controllers onto these pads.
> >
> >  Required Properties:
> >  - compatible: should be one of the following.
> > -  - "samsung,pinctrl-exynos4210": for Exynos4210 compatible pin-
> controller.
> > -  - "samsung,pinctrl-exynos4x12": for Exynos4x12 compatible pin-
> controller.
> > -  - "samsung,pinctrl-exynos5250": for Exynos5250 compatible pin-
> controller.
> > +  - "samsung,exynos4210-pinctrl": for Exynos4210 compatible pin-
> controller.
> > +  - "samsung,exynos4x12-pinctrl": for Exynos4x12 compatible pin-
> controller.
> > +  - "samsung,exynos5250-pinctrl": for Exynos5250 compatible pin-
> controller.
> 
> Do we care about backwards compatibility for ARM device trees yet?
> It's becoming time to start caring soon, if we don't. So while this
> might be OK for this time around, we should start requiring some
> backwards compatibility for bindings that have been used in at least
> one released kernel.
> 
Agree, let me check again.

> Once we do care, then you can update the dts/dtsi files, but you'll
> need to keep both the old and the new bindings in the C file below.
> 
OK, I see.

Thanks.

[...]

- Kukjin

Re: [PATCH 1/2] ARM: dts: Fix compatible value of pinctrl module on EXYNOS5440

[Linus Walleij]

On Thu, Jan 3, 2013 at 1:20 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:

> From: Thomas Abraham <thomas.ab@samsung.com>
>
> Fix the incorrect compatible property value of pin-controller module
> EXYNOS5440 SoC.
>
(...)
>
>         pinctrl {
> -               compatible = "samsung,pinctrl-exynos5440";
> +               compatible = "samsung,exynos5440-pinctrl";
>                 reg = <0xE0000 0x1000>;
>                 interrupt-controller;
>                 #interrupt-cells = <2>;
> diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
> index 953737d..cac4b45 100644
> --- a/drivers/gpio/gpio-samsung.c
> +++ b/drivers/gpio/gpio-samsung.c
> @@ -3026,7 +3026,7 @@ static __init int samsung_gpiolib_init(void)
>         static const struct of_device_id exynos_pinctrl_ids[] = {
>                 { .compatible = "samsung,pinctrl-exynos4210", },
>                 { .compatible = "samsung,pinctrl-exynos4x12", },
> -               { .compatible = "samsung,pinctrl-exynos5440", },
> +               { .compatible = "samsung,exynos5440-pinctrl", },
>         };
>         for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
>                 if (pctrl_np && of_device_is_available(pctrl_np))

I am tempted to apply this patch just to annoy people with
obsessive-compulsive disorder (but that's a bit mean).
Reference:
http://izismile.com/2011/08/16/obnoxious_ways_to_drive_people_with_ocd_nuts_16_pics.html

But I'd like a word from Rob, Grant or Stephen so I have some
guidance here.

Yours,
Linus Walleij

Re: [PATCH 2/2] ARM: dts: fix compatible value for exynos pinctrl

[Tomasz Figa]

Hi Olof,

On Saturday 05 of January 2013 14:57:54 Olof Johansson wrote:
> On Wed, Jan 2, 2013 at 4:20 PM, Kukjin Kim <kgene.kim@samsung.com> 
wrote:
> > Fix the incorrect compatible property value of pinctrl for EXYNOS4
> > SoCs.
> Ah, this answers my question from the previous patch. So, why do 5450
> as a separate patch?
> 
> > diff --git
> > a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt index
> > e97a278..4598a47 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > @@ -7,9 +7,9 @@ on-chip controllers onto these pads.
> > 
> >  Required Properties:
> >  - compatible: should be one of the following.
> > 
> > -  - "samsung,pinctrl-exynos4210": for Exynos4210 compatible
> > pin-controller. -  - "samsung,pinctrl-exynos4x12": for Exynos4x12
> > compatible pin-controller. -  - "samsung,pinctrl-exynos5250": for
> > Exynos5250 compatible pin-controller. +  -
> > "samsung,exynos4210-pinctrl": for Exynos4210 compatible
> > pin-controller. +  - "samsung,exynos4x12-pinctrl": for Exynos4x12
> > compatible pin-controller. +  - "samsung,exynos5250-pinctrl": for
> > Exynos5250 compatible pin-controller.
> Do we care about backwards compatibility for ARM device trees yet?
> It's becoming time to start caring soon, if we don't. So while this
> might be OK for this time around, we should start requiring some
> backwards compatibility for bindings that have been used in at least
> one released kernel.

This also brings the general problem of dts verification.

As of now, we don't have any way of checking dts files for compliance with 
defined bindings. Moreover we don't even have a way of defining those 
bindings other than plain text documentation.

Now, the problem is that anyone can put anything in their device tree 
sources and we can't check if the information is specified correctly by 
any other means that someone reading it and confirming with documentation 
and/or driver code (the documentation isn't always up to date...).

I remember there was some effort going towards encoding device tree 
bindings using something like XML schemas, but I'm not sure to what point 
it got and what was the conclusion. (Personally I think that it was a bit 
overcomplicated, but I never had time to polish and implement some of my 
ideas, eventually letting the problem fade off from my mind.)

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

Re: [PATCH 2/2] ARM: dts: fix compatible value for exynos pinctrl

[Tomasz Figa]

Hi Kukjin,

On Wednesday 02 of January 2013 16:20:23 Kukjin Kim wrote:
> Fix the incorrect compatible property value of pinctrl for EXYNOS4 SoCs.
> 
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> Cc: devicetree-discuss@lists.ozlabs.org
> ---
>  .../devicetree/bindings/pinctrl/samsung-pinctrl.txt        |   12
> ++++++------ arch/arm/boot/dts/exynos4210.dtsi                         
> |    6 +++--- arch/arm/boot/dts/exynos4x12.dtsi                        
>  |    8 ++++---- arch/arm/mach-exynos/common.c                         
>     |    4 ++-- drivers/gpio/gpio-samsung.c                            
>    |    4 ++-- drivers/pinctrl/pinctrl-samsung.c                       
>   |    4 ++-- 6 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt index
> e97a278..4598a47 100644
> --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> @@ -7,9 +7,9 @@ on-chip controllers onto these pads.
> 
>  Required Properties:
>  - compatible: should be one of the following.
> -  - "samsung,pinctrl-exynos4210": for Exynos4210 compatible
> pin-controller. -  - "samsung,pinctrl-exynos4x12": for Exynos4x12
> compatible pin-controller. -  - "samsung,pinctrl-exynos5250": for
> Exynos5250 compatible pin-controller. +  -
> "samsung,exynos4210-pinctrl": for Exynos4210 compatible pin-controller.
> +  - "samsung,exynos4x12-pinctrl": for Exynos4x12 compatible
> pin-controller. +  - "samsung,exynos5250-pinctrl": for Exynos5250
> compatible pin-controller.
> 
>  - reg: Base address of the pin controller hardware module and length of
> the address space it occupies.
> @@ -142,7 +142,7 @@ the following format 'pinctrl{n}' where n is a
> unique number for the alias. Example: A pin-controller node with pin
> banks:
> 
>  	pinctrl_0: pinctrl@11400000 {
> -		compatible = "samsung,pinctrl-exynos4210";
> +		compatible = "samsung,exynos4210-pinctrl";
>  		reg = <0x11400000 0x1000>;
>  		interrupts = <0 47 0>;
> 
> @@ -185,7 +185,7 @@ Example: A pin-controller node with pin banks:
>  Example 1: A pin-controller node with pin groups.
> 
>  	pinctrl_0: pinctrl@11400000 {
> -		compatible = "samsung,pinctrl-exynos4210";
> +		compatible = "samsung,exynos4210-pinctrl";
>  		reg = <0x11400000 0x1000>;
>  		interrupts = <0 47 0>;
> 
> @@ -230,7 +230,7 @@ Example 1: A pin-controller node with pin groups.
>  Example 2: A pin-controller node with external wakeup interrupt
> controller node.
> 
>  	pinctrl_1: pinctrl@11000000 {
> -		compatible = "samsung,pinctrl-exynos4210";
> +		compatible = "samsung,exynos4210-pinctrl";
>  		reg = <0x11000000 0x1000>;
>  		interrupts = <0 46 0>
> 
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi
> b/arch/arm/boot/dts/exynos4210.dtsi index e31bfc4..2feffc7 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -48,13 +48,13 @@
>  	};
> 
>  	pinctrl_0: pinctrl@11400000 {
> -		compatible = "samsung,pinctrl-exynos4210";
> +		compatible = "samsung,exynos4210-pinctrl";
>  		reg = <0x11400000 0x1000>;
>  		interrupts = <0 47 0>;
>  	};
> 
>  	pinctrl_1: pinctrl@11000000 {
> -		compatible = "samsung,pinctrl-exynos4210";
> +		compatible = "samsung,exynos4210-pinctrl";
>  		reg = <0x11000000 0x1000>;
>  		interrupts = <0 46 0>;
> 
> @@ -66,7 +66,7 @@
>  	};
> 
>  	pinctrl_2: pinctrl@03860000 {
> -		compatible = "samsung,pinctrl-exynos4210";
> +		compatible = "samsung,exynos4210-pinctrl";
>  		reg = <0x03860000 0x1000>;
>  	};
> 
> diff --git a/arch/arm/boot/dts/exynos4x12.dtsi
> b/arch/arm/boot/dts/exynos4x12.dtsi index 179a62e..9a87806 100644
> --- a/arch/arm/boot/dts/exynos4x12.dtsi
> +++ b/arch/arm/boot/dts/exynos4x12.dtsi
> @@ -37,13 +37,13 @@
>  	};
> 
>  	pinctrl_0: pinctrl@11400000 {
> -		compatible = "samsung,pinctrl-exynos4x12";
> +		compatible = "samsung,exynos4x12-pinctrl";
>  		reg = <0x11400000 0x1000>;
>  		interrupts = <0 47 0>;
>  	};
> 
>  	pinctrl_1: pinctrl@11000000 {
> -		compatible = "samsung,pinctrl-exynos4x12";
> +		compatible = "samsung,exynos4x12-pinctrl";
>  		reg = <0x11000000 0x1000>;
>  		interrupts = <0 46 0>;
> 
> @@ -55,14 +55,14 @@
>  	};
> 
>  	pinctrl_2: pinctrl@03860000 {
> -		compatible = "samsung,pinctrl-exynos4x12";
> +		compatible = "samsung,exynos4x12-pinctrl";
>  		reg = <0x03860000 0x1000>;
>  		interrupt-parent = <&combiner>;
>  		interrupts = <10 0>;
>  	};
> 
>  	pinctrl_3: pinctrl@106E0000 {
> -		compatible = "samsung,pinctrl-exynos4x12";
> +		compatible = "samsung,exynos4x12-pinctrl";
>  		reg = <0x106E0000 0x1000>;
>  		interrupts = <0 72 0>;
>  	};
> diff --git a/arch/arm/mach-exynos/common.c
> b/arch/arm/mach-exynos/common.c index d6d0dc6..3777a41 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -1024,8 +1024,8 @@ static int __init exynos_init_irq_eint(void)
>  	 * interrupt support code here can be completely removed.
>  	 */
>  	static const struct of_device_id exynos_pinctrl_ids[] = {
> -		{ .compatible = "samsung,pinctrl-exynos4210", },
> -		{ .compatible = "samsung,pinctrl-exynos4x12", },
> +		{ .compatible = "samsung,exynos4210-pinctrl", },
> +		{ .compatible = "samsung,exynos4x12-pinctrl", },

Since you are around these arrays of of_device_id, could you also add the 
missing terminators?

There was a patch floating on the list some time ago, but it doesn't seem 
to be merged.

>  	};
>  	struct device_node *pctrl_np, *wkup_np;
>  	const char *wkup_compat = "samsung,exynos4210-wakeup-eint";
> diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
> index cac4b45..8c00f7d 100644
> --- a/drivers/gpio/gpio-samsung.c
> +++ b/drivers/gpio/gpio-samsung.c
> @@ -3024,8 +3024,8 @@ static __init int samsung_gpiolib_init(void)
>  	*/
>  	struct device_node *pctrl_np;
>  	static const struct of_device_id exynos_pinctrl_ids[] = {
> -		{ .compatible = "samsung,pinctrl-exynos4210", },
> -		{ .compatible = "samsung,pinctrl-exynos4x12", },
> +		{ .compatible = "samsung,exynos4210-pinctrl", },
> +		{ .compatible = "samsung,exynos4x12-pinctrl", },
>  		{ .compatible = "samsung,exynos5440-pinctrl", },

Also here.

Best regards,
-- 
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

>  	};
>  	for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
> diff --git a/drivers/pinctrl/pinctrl-samsung.c
> b/drivers/pinctrl/pinctrl-samsung.c index 864fed8..602b30b 100644
> --- a/drivers/pinctrl/pinctrl-samsung.c
> +++ b/drivers/pinctrl/pinctrl-samsung.c
> @@ -945,9 +945,9 @@ static int __devinit samsung_pinctrl_probe(struct
> platform_device *pdev) }
> 
>  static const struct of_device_id samsung_pinctrl_dt_match[] = {
> -	{ .compatible = "samsung,pinctrl-exynos4210",
> +	{ .compatible = "samsung,exynos4210-pinctrl",
>  		.data = (void *)exynos4210_pin_ctrl },
> -	{ .compatible = "samsung,pinctrl-exynos4x12",
> +	{ .compatible = "samsung,exynos4x12-pinctrl",
>  		.data = (void *)exynos4x12_pin_ctrl },
>  	{},
>  };

Re: [PATCH 2/2] ARM: dts: fix compatible value for exynos pinctrl

[Linus Walleij]

On Thu, Jan 3, 2013 at 1:20 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:

> Fix the incorrect compatible property value of pinctrl for EXYNOS4 SoCs.
>
> Cc: Thomas Abraham <thomas.ab@samsung.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> Cc: devicetree-discuss@lists.ozlabs.org

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks!
Linus Walleij

Re: [PATCH 1/2] ARM: dts: Fix compatible value of pinctrl module on EXYNOS5440

[Stephen Warren]

On 01/17/2013 01:34 AM, Linus Walleij wrote:
> On Thu, Jan 3, 2013 at 1:20 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> 
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> Fix the incorrect compatible property value of pin-controller module
>> EXYNOS5440 SoC.
>>
> (...)
>>
>>         pinctrl {
>> -               compatible = "samsung,pinctrl-exynos5440";
>> +               compatible = "samsung,exynos5440-pinctrl";
>>                 reg = <0xE0000 0x1000>;
>>                 interrupt-controller;
>>                 #interrupt-cells = <2>;
>> diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
>> index 953737d..cac4b45 100644
>> --- a/drivers/gpio/gpio-samsung.c
>> +++ b/drivers/gpio/gpio-samsung.c
>> @@ -3026,7 +3026,7 @@ static __init int samsung_gpiolib_init(void)
>>         static const struct of_device_id exynos_pinctrl_ids[] = {
>>                 { .compatible = "samsung,pinctrl-exynos4210", },
>>                 { .compatible = "samsung,pinctrl-exynos4x12", },
>> -               { .compatible = "samsung,pinctrl-exynos5440", },
>> +               { .compatible = "samsung,exynos5440-pinctrl", },
>>         };
>>         for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
>>                 if (pctrl_np && of_device_is_available(pctrl_np))
> 
> I am tempted to apply this patch just to annoy people with
> obsessive-compulsive disorder (but that's a bit mean).
> Reference:
> http://izismile.com/2011/08/16/obnoxious_ways_to_drive_people_with_ocd_nuts_16_pics.html

Aargh. Those pencils are wrong:-)

> But I'd like a word from Rob, Grant or Stephen so I have some
> guidance here.

Yes, the existing order looks odd c.f. existing practice.

I notice that only one of the strangely ordered entries in
exynos_pinctrl_ids[] is fixed. Surely they should all be fixed?

It's probably worth adding the new names to exynos_pinctrl_ids[] rather
than replacing them. That allows the driver to be backwards-compatible
with older DTBs, and partially decouples the driver change from the
.dts/.dtsi file. Still, the whole backwards-compatible DTB thing seems a
little irrelevant when the .dts files are in the kernel source tree anyway.

Re: [PATCH 1/2] ARM: dts: Fix compatible value of pinctrl module on EXYNOS5440

[Rob Herring]

On 01/17/2013 02:34 AM, Linus Walleij wrote:
> On Thu, Jan 3, 2013 at 1:20 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> 
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> Fix the incorrect compatible property value of pin-controller module
>> EXYNOS5440 SoC.
>>
> (...)
>>
>>         pinctrl {
>> -               compatible = "samsung,pinctrl-exynos5440";
>> +               compatible = "samsung,exynos5440-pinctrl";
>>                 reg = <0xE0000 0x1000>;
>>                 interrupt-controller;
>>                 #interrupt-cells = <2>;
>> diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
>> index 953737d..cac4b45 100644
>> --- a/drivers/gpio/gpio-samsung.c
>> +++ b/drivers/gpio/gpio-samsung.c
>> @@ -3026,7 +3026,7 @@ static __init int samsung_gpiolib_init(void)
>>         static const struct of_device_id exynos_pinctrl_ids[] = {
>>                 { .compatible = "samsung,pinctrl-exynos4210", },
>>                 { .compatible = "samsung,pinctrl-exynos4x12", },
>> -               { .compatible = "samsung,pinctrl-exynos5440", },
>> +               { .compatible = "samsung,exynos5440-pinctrl", },
>>         };
>>         for_each_matching_node(pctrl_np, exynos_pinctrl_ids)
>>                 if (pctrl_np && of_device_is_available(pctrl_np))
> 
> I am tempted to apply this patch just to annoy people with
> obsessive-compulsive disorder (but that's a bit mean).
> Reference:
> http://izismile.com/2011/08/16/obnoxious_ways_to_drive_people_with_ocd_nuts_16_pics.html
> 
> But I'd like a word from Rob, Grant or Stephen so I have some
> guidance here.

As long as we're consistently inconsistent... What about 4210 and 4x12?
Hopefully, the reason was to avoid breaking them.

This creates horrible incompatibilities between dts and kernel versions
by changing both. Any prior kernel will not work with this dts or later.
And more importantly this kernel version or newer will not work with
existing dtb's. We've been pretty lax on this as DT support has been
under heavy development, but things need to start tightening up. That
being said, if this isn't yet a concern for this platform, then I don't
really care.

Rob

RE: [PATCH 1/2] ARM: dts: Fix compatible value of pinctrl module on EXYNOS5440

[Kukjin Kim]

Rob Herring wrote:

[...]

> 
> As long as we're consistently inconsistent... What about 4210 and 4x12?
> Hopefully, the reason was to avoid breaking them.
> 
> This creates horrible incompatibilities between dts and kernel versions
> by changing both. Any prior kernel will not work with this dts or later.
> And more importantly this kernel version or newer will not work with
> existing dtb's. We've been pretty lax on this as DT support has been
> under heavy development, but things need to start tightening up. That
> being said, if this isn't yet a concern for this platform, then I don't
> really care.
> 

I think, this can be applied for v3.8 and this two fixes should be harmless
for DT compatibility of Samsung stuff.

If there is no other concerns, let me pick up with Linus Walleij's ack.

Thanks.

- Kukjin