diff mbox

[4/6] ARM: mvebu: Add support for NAND controller in Armada 38x SoC

Message ID 1394637404-7651-5-git-send-email-ezequiel.garcia@free-electrons.com
State Not Applicable
Headers show

Commit Message

Ezequiel Garcia March 12, 2014, 3:16 p.m. UTC
The Armada 38x SoC family has a NAND controller, compatible
with the controller in Armada 370/375/XP SoCs. Add support for
it in the devicetree file.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
 arch/arm/boot/dts/armada-38x.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Ezequiel Garcia March 12, 2014, 8:30 p.m. UTC | #1
On Mar 13, Sergei Shtylyov wrote:
> On 03/12/2014 06:16 PM, Ezequiel Garcia wrote:
> 
> >The Armada 38x SoC family has a NAND controller, compatible
> >with the controller in Armada 370/375/XP SoCs. Add support for
> >it in the devicetree file.
> 
> >Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >---
> >  arch/arm/boot/dts/armada-38x.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> 
> >diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> >index 76cc27e..18d8f80 100644
> >--- a/arch/arm/boot/dts/armada-38x.dtsi
> >+++ b/arch/arm/boot/dts/armada-38x.dtsi
> >@@ -345,6 +345,16 @@
> >  				clocks = <&mainpll>;
> >  				clock-output-names = "nand";
> >  			};
> >+
> >+			nand@d0000 {
> 
>    ePAPR standard [1] tells us:
> 
> The name of a node should be somewhat generic, reflecting the function of
> the device and not its precise programming model. If appropriate, the name
> should be one of the following choices:
> 
> [...]
> • flash
> 

I think 'nand' is generic enough, isn't it?

In any case, it seems sane to distinguish a NAND flash from a NOR flash,
from a SPI flash.

FWIW, quite a few other SoCs have chosen 'nand' for the node name, including
the other Armada variants. Was this a wrong choice?
Sergei Shtylyov March 12, 2014, 9:17 p.m. UTC | #2
Hello.

On 03/12/2014 06:16 PM, Ezequiel Garcia wrote:

> The Armada 38x SoC family has a NAND controller, compatible
> with the controller in Armada 370/375/XP SoCs. Add support for
> it in the devicetree file.

> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
>   arch/arm/boot/dts/armada-38x.dtsi | 10 ++++++++++
>   1 file changed, 10 insertions(+)

> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> index 76cc27e..18d8f80 100644
> --- a/arch/arm/boot/dts/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/armada-38x.dtsi
> @@ -345,6 +345,16 @@
>   				clocks = <&mainpll>;
>   				clock-output-names = "nand";
>   			};
> +
> +			nand@d0000 {

    ePAPR standard [1] tells us:

The name of a node should be somewhat generic, reflecting the function of the 
device and not its precise programming model. If appropriate, the name should 
be one of the following choices:

[...]
• flash

[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf

WBR, Sergei
Ezequiel Garcia March 12, 2014, 9:29 p.m. UTC | #3
On Mar 13, Sergei Shtylyov wrote:
> On 03/12/2014 11:30 PM, Ezequiel Garcia wrote:
> 
> >>>The Armada 38x SoC family has a NAND controller, compatible
> >>>with the controller in Armada 370/375/XP SoCs. Add support for
> >>>it in the devicetree file.
> 
> >>>Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >>>---
> >>>  arch/arm/boot/dts/armada-38x.dtsi | 10 ++++++++++
> >>>  1 file changed, 10 insertions(+)
> 
> >>>diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> >>>index 76cc27e..18d8f80 100644
> >>>--- a/arch/arm/boot/dts/armada-38x.dtsi
> >>>+++ b/arch/arm/boot/dts/armada-38x.dtsi
> >>>@@ -345,6 +345,16 @@
> >>>  				clocks = <&mainpll>;
> >>>  				clock-output-names = "nand";
> >>>  			};
> >>>+
> >>>+			nand@d0000 {
> 
> >>    ePAPR standard [1] tells us:
> 
> >>The name of a node should be somewhat generic, reflecting the function of
> >>the device and not its precise programming model. If appropriate, the name
> >>should be one of the following choices:
> 
> >>[...]
> >>• flash
> 
> >I think 'nand' is generic enough, isn't it?
> 
>    It is but not more generic than "flash". :-)
> 

Right.

> >FWIW, quite a few other SoCs have chosen 'nand' for the node name, including
> >the other Armada variants. Was this a wrong choice?
> 
>    I guess. There's a lot of wrong choices now all over the
> arch/arm/boot/dts/ because people are probably not aware of the necessary
> documentation such as http://devicetree.org/Device_Tree_Usage (pointing to
> ePAPR and having a passage on the generic device names too).
> 

OK, I guess it's fine. Let's try to do things from now on, at least. I'll fix
this and send a new series.
Sergei Shtylyov March 12, 2014, 9:39 p.m. UTC | #4
On 03/12/2014 11:30 PM, Ezequiel Garcia wrote:

>>> The Armada 38x SoC family has a NAND controller, compatible
>>> with the controller in Armada 370/375/XP SoCs. Add support for
>>> it in the devicetree file.

>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>>> ---
>>>   arch/arm/boot/dts/armada-38x.dtsi | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)

>>> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
>>> index 76cc27e..18d8f80 100644
>>> --- a/arch/arm/boot/dts/armada-38x.dtsi
>>> +++ b/arch/arm/boot/dts/armada-38x.dtsi
>>> @@ -345,6 +345,16 @@
>>>   				clocks = <&mainpll>;
>>>   				clock-output-names = "nand";
>>>   			};
>>> +
>>> +			nand@d0000 {

>>     ePAPR standard [1] tells us:

>> The name of a node should be somewhat generic, reflecting the function of
>> the device and not its precise programming model. If appropriate, the name
>> should be one of the following choices:

>> [...]
>> • flash

> I think 'nand' is generic enough, isn't it?

    It is but not more generic than "flash". :-)

> In any case, it seems sane to distinguish a NAND flash from a NOR flash,
> from a SPI flash.

    I don't know enough about the SPI flashes but this is only a node name, no 
more, so I think we can afford to be really generic...

> FWIW, quite a few other SoCs have chosen 'nand' for the node name, including
> the other Armada variants. Was this a wrong choice?

    I guess. There's a lot of wrong choices now all over the 
arch/arm/boot/dts/ because people are probably not aware of the necessary 
documentation such as http://devicetree.org/Device_Tree_Usage (pointing to 
ePAPR and having a passage on the generic device names too).

WBR, Sergei
diff mbox

Patch

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 76cc27e..18d8f80 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -345,6 +345,16 @@ 
 				clocks = <&mainpll>;
 				clock-output-names = "nand";
 			};
+
+			nand@d0000 {
+				compatible = "marvell,armada370-nand";
+				reg = <0xd0000 0x54>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+				interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&coredivclk 0>;
+				status = "disabled";
+			};
 		};
 	};