diff mbox

[1/2] ARM: shmobile: r8a7778/r8a7779 dtsi: Improve and correct HSPI bindings

Message ID 1392109008-29941-1-git-send-email-geert@linux-m68k.org (mailing list archive)
State Not Applicable
Headers show

Commit Message

Geert Uytterhoeven Feb. 11, 2014, 8:56 a.m. UTC
From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Binding documentation:
  - Add future-proof "renesas,hspi-<soctype>" compatible values,
  - Add "interrupt-parent", "#address-cells" and "#size-cells" properties,
  - Add reference to pinctrl documentation,
  - Add example bindings.

r8a7778 and r8a7779 dtsi:
  - Add "renesas,hspi-r8a7778" resp. "renesas,hspi-r8a7779" compatible
    values,
  - Correct reference to parent interrupt controller
    (use "interrupt-parent" instead of "interrupt-controller"),
  - Add missing "#address-cells" and "#size-cells" properties, which are
    needed when populating the SPI buses.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Cc: Mark Brown <broonie@linaro.org>
---
Untested due to lack of hardware

 Documentation/devicetree/bindings/spi/sh-hspi.txt |   27 ++++++++++++++++++---
 arch/arm/boot/dts/r8a7778.dtsi                    |   18 +++++++++-----
 arch/arm/boot/dts/r8a7779.dtsi                    |   18 +++++++++-----
 3 files changed, 48 insertions(+), 15 deletions(-)

Comments

Simon Horman Feb. 13, 2014, 5:56 a.m. UTC | #1
Hi Morimoto-san,

could you test this series as Geert does not have access to a bockw board?

On Tue, Feb 11, 2014 at 09:56:47AM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Binding documentation:
>   - Add future-proof "renesas,hspi-<soctype>" compatible values,
>   - Add "interrupt-parent", "#address-cells" and "#size-cells" properties,
>   - Add reference to pinctrl documentation,
>   - Add example bindings.
> 
> r8a7778 and r8a7779 dtsi:
>   - Add "renesas,hspi-r8a7778" resp. "renesas,hspi-r8a7779" compatible
>     values,
>   - Correct reference to parent interrupt controller
>     (use "interrupt-parent" instead of "interrupt-controller"),
>   - Add missing "#address-cells" and "#size-cells" properties, which are
>     needed when populating the SPI buses.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> Cc: Mark Brown <broonie@linaro.org>
> ---
> Untested due to lack of hardware
> 
>  Documentation/devicetree/bindings/spi/sh-hspi.txt |   27 ++++++++++++++++++---
>  arch/arm/boot/dts/r8a7778.dtsi                    |   18 +++++++++-----
>  arch/arm/boot/dts/r8a7779.dtsi                    |   18 +++++++++-----
>  3 files changed, 48 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/sh-hspi.txt b/Documentation/devicetree/bindings/spi/sh-hspi.txt
> index 30b57b1c8a13..d43080eb6b3a 100644
> --- a/Documentation/devicetree/bindings/spi/sh-hspi.txt
> +++ b/Documentation/devicetree/bindings/spi/sh-hspi.txt
> @@ -1,7 +1,28 @@
>  Renesas HSPI.
>  
>  Required properties:
> -- compatible : 	"renesas,hspi"
> -- reg : Offset and length of the register set for the device
> -- interrupts : interrupt line used by HSPI
> +- compatible       : "renesas,hspi-<soctype>", "renesas,hspi" as fallback.
> +		     Examples of valid soctypes are "r8a7778" (R-Car M1),
> +		     and "r8a7779" (R-Car H1)
> +- reg              : Offset and length of the register set for the device
> +- interrupt-parent : The phandle for the interrupt controller that
> +		     services interrupts for this device
> +- interrupts       : Interrupt specifier
> +- #address-cells   : Must be <1>
> +- #size-cells      : Must be <0>
> +
> +Pinctrl properties might be needed, too.  See
> +Documentation/devicetree/bindings/pinctrl/renesas,*.
> +
> +Example:
> +
> +	hspi0: spi@fffc7000 {
> +		compatible = "renesas,hspi-r8a7778", "renesas,hspi";
> +		reg = <0xfffc7000 0x18>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 63 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "disabled";
> +	};
>  
> diff --git a/arch/arm/boot/dts/r8a7778.dtsi b/arch/arm/boot/dts/r8a7778.dtsi
> index 85c5b3b99f5e..3c6fab5c9702 100644
> --- a/arch/arm/boot/dts/r8a7778.dtsi
> +++ b/arch/arm/boot/dts/r8a7778.dtsi
> @@ -204,26 +204,32 @@
>  	};
>  
>  	hspi0: spi@fffc7000 {
> -		compatible = "renesas,hspi";
> +		compatible = "renesas,hspi-r8a7778", "renesas,hspi";
>  		reg = <0xfffc7000 0x18>;
> -		interrupt-controller = <&gic>;
> +		interrupt-parent = <&gic>;
>  		interrupts = <0 63 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		status = "disabled";
>  	};
>  
>  	hspi1: spi@fffc8000 {
> -		compatible = "renesas,hspi";
> +		compatible = "renesas,hspi-r8a7778", "renesas,hspi";
>  		reg = <0xfffc8000 0x18>;
> -		interrupt-controller = <&gic>;
> +		interrupt-parent = <&gic>;
>  		interrupts = <0 84 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		status = "disabled";
>  	};
>  
>  	hspi2: spi@fffc6000 {
> -		compatible = "renesas,hspi";
> +		compatible = "renesas,hspi-r8a7778", "renesas,hspi";
>  		reg = <0xfffc6000 0x18>;
> -		interrupt-controller = <&gic>;
> +		interrupt-parent = <&gic>;
>  		interrupts = <0 85 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		status = "disabled";
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi
> index d0561d4c7c46..8b1a336ee401 100644
> --- a/arch/arm/boot/dts/r8a7779.dtsi
> +++ b/arch/arm/boot/dts/r8a7779.dtsi
> @@ -256,26 +256,32 @@
>  	};
>  
>  	hspi0: spi@fffc7000 {
> -		compatible = "renesas,hspi";
> +		compatible = "renesas,hspi-r8a7779", "renesas,hspi";
>  		reg = <0xfffc7000 0x18>;
> -		interrupt-controller = <&gic>;
> +		interrupt-parent = <&gic>;
>  		interrupts = <0 73 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		status = "disabled";
>  	};
>  
>  	hspi1: spi@fffc8000 {
> -		compatible = "renesas,hspi";
> +		compatible = "renesas,hspi-r8a7779", "renesas,hspi";
>  		reg = <0xfffc8000 0x18>;
> -		interrupt-controller = <&gic>;
> +		interrupt-parent = <&gic>;
>  		interrupts = <0 74 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		status = "disabled";
>  	};
>  
>  	hspi2: spi@fffc6000 {
> -		compatible = "renesas,hspi";
> +		compatible = "renesas,hspi-r8a7779", "renesas,hspi";
>  		reg = <0xfffc6000 0x18>;
> -		interrupt-controller = <&gic>;
> +		interrupt-parent = <&gic>;
>  		interrupts = <0 75 IRQ_TYPE_LEVEL_HIGH>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
>  		status = "disabled";
>  	};
>  };
> -- 
> 1.7.9.5
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kuninori Morimoto Feb. 13, 2014, 7:11 a.m. UTC | #2
Hi Simon, Geert
Cc Mark

> could you test this series as Geert does not have access to a bockw board?
> 
> On Tue, Feb 11, 2014 at 09:56:47AM +0100, Geert Uytterhoeven wrote:
> > From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> > 
> > Binding documentation:
> >   - Add future-proof "renesas,hspi-<soctype>" compatible values,
> >   - Add "interrupt-parent", "#address-cells" and "#size-cells" properties,
> >   - Add reference to pinctrl documentation,
> >   - Add example bindings.
> > 
> > r8a7778 and r8a7779 dtsi:
> >   - Add "renesas,hspi-r8a7778" resp. "renesas,hspi-r8a7779" compatible
> >     values,
> >   - Correct reference to parent interrupt controller
> >     (use "interrupt-parent" instead of "interrupt-controller"),
> >   - Add missing "#address-cells" and "#size-cells" properties, which are
> >     needed when populating the SPI buses.
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> > Cc: Mark Brown <broonie@linaro.org>
> > ---

I tested this patch, and it works well.
But there are some notes

1) m25p80 driver DT support seems strange ??

   ${LINUX}/Documentation/devicetree/bindings/mtd/m25p80.txt
   has explain about DT of m25p80 driver, but,
   ${LINUX}/drivers/mtd/devices/m25p80.c
   doesn't have driver :: of_match_table.
   but, it is using of_property_read_bool() on probe.
   Is it out-of-tree support ?? I'm not sure,
   anyway, m25p80 driver didn't probe.
   So, I quick-hacked this issue in my local environment.

2) it needs Geert's this patch

   Subject: [PATCH] mtd: m25p80: add support for the Spansion s25fl008k chip
   Date:	Tue, 11 Feb 2014 09:51:18 +0100

   Kernel will hung-up without this patch

For HSPI / BockW point of view

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Feb. 13, 2014, 8:31 a.m. UTC | #3
Hi Morimoto-san,

On Thu, Feb 13, 2014 at 8:11 AM, Kuninori Morimoto
<kuninori.morimoto.gx@gmail.com> wrote:
>> could you test this series as Geert does not have access to a bockw board?
>>
>> On Tue, Feb 11, 2014 at 09:56:47AM +0100, Geert Uytterhoeven wrote:
>> > From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>> >
>> > Binding documentation:
>> >   - Add future-proof "renesas,hspi-<soctype>" compatible values,
>> >   - Add "interrupt-parent", "#address-cells" and "#size-cells" properties,
>> >   - Add reference to pinctrl documentation,
>> >   - Add example bindings.
>> >
>> > r8a7778 and r8a7779 dtsi:
>> >   - Add "renesas,hspi-r8a7778" resp. "renesas,hspi-r8a7779" compatible
>> >     values,
>> >   - Correct reference to parent interrupt controller
>> >     (use "interrupt-parent" instead of "interrupt-controller"),
>> >   - Add missing "#address-cells" and "#size-cells" properties, which are
>> >     needed when populating the SPI buses.
>> >
>> > Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>> > Cc: Mark Brown <broonie@linaro.org>
>> > ---
>
> I tested this patch, and it works well.
> But there are some notes
>
> 1) m25p80 driver DT support seems strange ??
>
>    ${LINUX}/Documentation/devicetree/bindings/mtd/m25p80.txt
>    has explain about DT of m25p80 driver, but,
>    ${LINUX}/drivers/mtd/devices/m25p80.c
>    doesn't have driver :: of_match_table.
>    but, it is using of_property_read_bool() on probe.
>    Is it out-of-tree support ?? I'm not sure,
>    anyway, m25p80 driver didn't probe.

It probes based on the values in m25p_ids[].

See drivers/of/base.c:of_modalias_node():

 * Based on the value of the compatible property, this routine will attempt
 * to choose an appropriate modalias value for a particular device tree node.
 * It does this by stripping the manufacturer prefix (as delimited by a ',')
 * from the first entry in the compatible list property.

Which is used by drivers/spi/spi.c:of_register_spi_devices().

>    So, I quick-hacked this issue in my local environment.

Was there anything else you needed to do, besides adding support for
s25fl008k to m25p80.c?

> 2) it needs Geert's this patch
>
>    Subject: [PATCH] mtd: m25p80: add support for the Spansion s25fl008k chip
>    Date:        Tue, 11 Feb 2014 09:51:18 +0100
>
>    Kernel will hung-up without this patch

Hang up? Not just ignoring the device?

> For HSPI / BockW point of view
>
> Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kuninori Morimoto Feb. 13, 2014, 9:02 a.m. UTC | #4
Hi Geert
CC Simon

Thank you for your help

>  * Based on the value of the compatible property, this routine will attempt
>  * to choose an appropriate modalias value for a particular device tree node.
>  * It does this by stripping the manufacturer prefix (as delimited by a ',')
>  * from the first entry in the compatible list property.
> 
> Which is used by drivers/spi/spi.c:of_register_spi_devices().
> 
> >    So, I quick-hacked this issue in my local environment.
> 
> Was there anything else you needed to do, besides adding support for
> s25fl008k to m25p80.c?

I re-tested cleanly this patch.
Yes, indeed, my local-hack was not needed.
It was my fault.

> > 2) it needs Geert's this patch
> >
> >    Subject: [PATCH] mtd: m25p80: add support for the Spansion s25fl008k chip
> >    Date:        Tue, 11 Feb 2014 09:51:18 +0100
> >
> >    Kernel will hung-up without this patch
> 
> Hang up? Not just ignoring the device?

It works without Hang-up in my re-test now.
My previous test seems something wrong.
Thank you

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/spi/sh-hspi.txt b/Documentation/devicetree/bindings/spi/sh-hspi.txt
index 30b57b1c8a13..d43080eb6b3a 100644
--- a/Documentation/devicetree/bindings/spi/sh-hspi.txt
+++ b/Documentation/devicetree/bindings/spi/sh-hspi.txt
@@ -1,7 +1,28 @@ 
 Renesas HSPI.
 
 Required properties:
-- compatible : 	"renesas,hspi"
-- reg : Offset and length of the register set for the device
-- interrupts : interrupt line used by HSPI
+- compatible       : "renesas,hspi-<soctype>", "renesas,hspi" as fallback.
+		     Examples of valid soctypes are "r8a7778" (R-Car M1),
+		     and "r8a7779" (R-Car H1)
+- reg              : Offset and length of the register set for the device
+- interrupt-parent : The phandle for the interrupt controller that
+		     services interrupts for this device
+- interrupts       : Interrupt specifier
+- #address-cells   : Must be <1>
+- #size-cells      : Must be <0>
+
+Pinctrl properties might be needed, too.  See
+Documentation/devicetree/bindings/pinctrl/renesas,*.
+
+Example:
+
+	hspi0: spi@fffc7000 {
+		compatible = "renesas,hspi-r8a7778", "renesas,hspi";
+		reg = <0xfffc7000 0x18>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 63 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 
diff --git a/arch/arm/boot/dts/r8a7778.dtsi b/arch/arm/boot/dts/r8a7778.dtsi
index 85c5b3b99f5e..3c6fab5c9702 100644
--- a/arch/arm/boot/dts/r8a7778.dtsi
+++ b/arch/arm/boot/dts/r8a7778.dtsi
@@ -204,26 +204,32 @@ 
 	};
 
 	hspi0: spi@fffc7000 {
-		compatible = "renesas,hspi";
+		compatible = "renesas,hspi-r8a7778", "renesas,hspi";
 		reg = <0xfffc7000 0x18>;
-		interrupt-controller = <&gic>;
+		interrupt-parent = <&gic>;
 		interrupts = <0 63 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 		status = "disabled";
 	};
 
 	hspi1: spi@fffc8000 {
-		compatible = "renesas,hspi";
+		compatible = "renesas,hspi-r8a7778", "renesas,hspi";
 		reg = <0xfffc8000 0x18>;
-		interrupt-controller = <&gic>;
+		interrupt-parent = <&gic>;
 		interrupts = <0 84 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 		status = "disabled";
 	};
 
 	hspi2: spi@fffc6000 {
-		compatible = "renesas,hspi";
+		compatible = "renesas,hspi-r8a7778", "renesas,hspi";
 		reg = <0xfffc6000 0x18>;
-		interrupt-controller = <&gic>;
+		interrupt-parent = <&gic>;
 		interrupts = <0 85 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 		status = "disabled";
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi
index d0561d4c7c46..8b1a336ee401 100644
--- a/arch/arm/boot/dts/r8a7779.dtsi
+++ b/arch/arm/boot/dts/r8a7779.dtsi
@@ -256,26 +256,32 @@ 
 	};
 
 	hspi0: spi@fffc7000 {
-		compatible = "renesas,hspi";
+		compatible = "renesas,hspi-r8a7779", "renesas,hspi";
 		reg = <0xfffc7000 0x18>;
-		interrupt-controller = <&gic>;
+		interrupt-parent = <&gic>;
 		interrupts = <0 73 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 		status = "disabled";
 	};
 
 	hspi1: spi@fffc8000 {
-		compatible = "renesas,hspi";
+		compatible = "renesas,hspi-r8a7779", "renesas,hspi";
 		reg = <0xfffc8000 0x18>;
-		interrupt-controller = <&gic>;
+		interrupt-parent = <&gic>;
 		interrupts = <0 74 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 		status = "disabled";
 	};
 
 	hspi2: spi@fffc6000 {
-		compatible = "renesas,hspi";
+		compatible = "renesas,hspi-r8a7779", "renesas,hspi";
 		reg = <0xfffc6000 0x18>;
-		interrupt-controller = <&gic>;
+		interrupt-parent = <&gic>;
 		interrupts = <0 75 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
 		status = "disabled";
 	};
 };