[PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Sergei Shtylyov]

Define the generic R8A777x part of the Ether device node. 

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
===================================================================
--- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
+++ renesas/arch/arm/boot/dts/r8a7778.dtsi
@@ -98,4 +98,15 @@
 		reg = <0xfffc000 0x118>;
 		#gpio-range-cells = <3>;
 	};
+
+	ether: ethernet@fde00000 {
+		device_type = "network";
+		compatible = "renesas,ether-r8a7779";
+		reg = <0xfde00000 0x400>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 105 0x4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };

[PATCH 3/4] ARM: shmobile: bockw-reference: add Ether DT support

[Sergei Shtylyov]

Define the BOCK-W board dependent part of the Ether device node. 

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 arch/arm/boot/dts/r8a7778-bockw-reference.dts |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: renesas/arch/arm/boot/dts/r8a7778-bockw-reference.dts
===================================================================
--- renesas.orig/arch/arm/boot/dts/r8a7778-bockw-reference.dts
+++ renesas/arch/arm/boot/dts/r8a7778-bockw-reference.dts
@@ -30,3 +30,14 @@
 		reg = <0x60000000 0x10000000>;
 	};
 };
+
+&ether {
+	status = "ok";
+	phy-mode = "rmii";
+	phy-handle = <&phy1>;
+	renesas,no-ether-link;
+
+	phy1: ethernet-phy@1 {
+		reg = <1>;
+	};
+};

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Sunday 01 September 2013 03:15:48 Sergei Shtylyov wrote:
> Define the generic R8A777x part of the Ether device node.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> ===================================================================
> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> @@ -98,4 +98,15 @@
>  		reg = <0xfffc000 0x118>;
>  		#gpio-range-cells = <3>;
>  	};
> +
> +	ether: ethernet@fde00000 {
> +		device_type = "network";

Documentation/devicetree/bindings/net/sh_eth.txt from "[PATCH 2/2] sh_eth: add 
device tree support" doesn't document this property, should you update it ?

> +		compatible = "renesas,ether-r8a7779";
> +		reg = <0xfde00000 0x400>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 105 0x4>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		status = "disabled";
> +	};
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Sergei Shtylyov]

On 09/02/2013 05:14 PM, Laurent Pinchart wrote:

>> Define the generic R8A777x part of the Ether device node.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>> ---
>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>>   1 file changed, 11 insertions(+)

>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
>> ===================================================================
>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
>> @@ -98,4 +98,15 @@
>>   		reg = <0xfffc000 0x118>;
>>   		#gpio-range-cells = <3>;
>>   	};
>> +
>> +	ether: ethernet@fde00000 {
>> +		device_type = "network";

> Documentation/devicetree/bindings/net/sh_eth.txt from "[PATCH 2/2] sh_eth: add
> device tree support" doesn't document this property, should you update it ?

    It's a standard property but completely optional for non-OF systems. I've 
not seen anybody document it but I should grep for it...

WBR, Sergei

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Laurent Pinchart]

On Monday 02 September 2013 18:05:10 Sergei Shtylyov wrote:
> On 09/02/2013 05:14 PM, Laurent Pinchart wrote:
> >> Define the generic R8A777x part of the Ether device node.
> >> 
> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >> 
> >> ---
> >> 
> >>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
> >>   1 file changed, 11 insertions(+)
> >> 
> >> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> >> ===================================================================
> >> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> >> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> >> @@ -98,4 +98,15 @@
> >> 
> >>   		reg = <0xfffc000 0x118>;
> >>   		#gpio-range-cells = <3>;
> >>   	
> >>   	};
> >> 
> >> +
> >> +	ether: ethernet@fde00000 {
> >> +		device_type = "network";
> > 
> > Documentation/devicetree/bindings/net/sh_eth.txt from "[PATCH 2/2] sh_eth:
> > add device tree support" doesn't document this property, should you
> > update it ?
>     It's a standard property but completely optional for non-OF systems.
> I've not seen anybody document it but I should grep for it...

Documentation/devicetree/booting-without-of.txt mentions it, as well as a 
couple of other DT bindings document, but there's no clear documentation of 
what device_type should be for network device, and whether the property is 
mandatory or optional. Clarifying the documentation would be great.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Pawel Moll]

On Mon, 2013-09-02 at 15:18 +0100, Laurent Pinchart wrote:
> Documentation/devicetree/booting-without-of.txt mentions it, as well as a 
> couple of other DT bindings document, but there's no clear documentation of 
> what device_type should be for network device, and whether the property is 
> mandatory or optional. Clarifying the documentation would be great.

It's documented in the ePAPR spec:

"The device_type property was used in IEEE 1275 to describe the device’s
FCode programming model. Because ePAPR does not have FCode, new use of
the property is deprecated, and it should be included only on cpu and
memory nodes for compatibility with IEEE 1275–derived device trees."

The bottom line is - don't use it, unless you've good good reasons?

Paweł

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Laurent Pinchart]

Hi Paweł,

On Monday 02 September 2013 16:32:34 Pawel Moll wrote:
> On Mon, 2013-09-02 at 15:18 +0100, Laurent Pinchart wrote:
> > Documentation/devicetree/booting-without-of.txt mentions it, as well as a
> > couple of other DT bindings document, but there's no clear documentation
> > of what device_type should be for network device, and whether the property
> > is mandatory or optional. Clarifying the documentation would be great.
> 
> It's documented in the ePAPR spec:
> 
> "The device_type property was used in IEEE 1275 to describe the device’s
> FCode programming model. Because ePAPR does not have FCode, new use of
> the property is deprecated, and it should be included only on cpu and
> memory nodes for compatibility with IEEE 1275–derived device trees."

Thank you for the clarification.

> The bottom line is - don't use it, unless you've good good reasons?

I'll let Sergei share his opinion on whether we've got good reasons :-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Sergei Shtylyov]

Hello.

On 03-09-2013 16:11, Laurent Pinchart wrote:

>>> Documentation/devicetree/booting-without-of.txt mentions it, as well as a
>>> couple of other DT bindings document, but there's no clear documentation
>>> of what device_type should be for network device, and whether the property
>>> is mandatory or optional. Clarifying the documentation would be great.

>> It's documented in the ePAPR spec:

>> "The device_type property was used in IEEE 1275 to describe the device’s
>> FCode programming model. Because ePAPR does not have FCode, new use of
>> the property is deprecated, and it should be included only on cpu and
>> memory nodes for compatibility with IEEE 1275–derived device trees."

> Thank you for the clarification.

>> The bottom line is - don't use it, unless you've good good reasons?

> I'll let Sergei share his opinion on whether we've got good reasons :-)

    No, we don't, I'll remove the prop.

WBR, Sergei

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Magnus Damm]

On Sun, Sep 1, 2013 at 8:15 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Define the generic R8A777x part of the Ether device node.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
>  arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> ===================================================================
> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> @@ -98,4 +98,15 @@
>                 reg = <0xfffc000 0x118>;
>                 #gpio-range-cells = <3>;
>         };
> +
> +       ether: ethernet@fde00000 {
> +               device_type = "network";
> +               compatible = "renesas,ether-r8a7779";

Hi Sergei,

Thanks for your patch. What's the reason behind the r8a7778 SoC using
a compatible string for r8a7779 like "renesas,ether-r8a7779"?

It seems that you assume that the r8a7778 ethernet controller is 100%
compatible with r8a7779. Is that really true? For earlier versions the
sh_eth hardware documentation was anything but accurate, so it seems
to me that it must be more safe that r8a7778 would be using
"renesas,ether-r8a7778". What do you think?

Thanks,

/ magnus

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Sergei Shtylyov]

Hello.

On 09/03/2013 07:17 PM, Magnus Damm wrote:

>> Define the generic R8A777x part of the Ether device node.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>> ---
>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
>> ===================================================================
>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
>> @@ -98,4 +98,15 @@
>>                  reg = <0xfffc000 0x118>;
>>                  #gpio-range-cells = <3>;
>>          };
>> +
>> +       ether: ethernet@fde00000 {
>> +               device_type = "network";
>> +               compatible = "renesas,ether-r8a7779";

> Hi Sergei,

> Thanks for your patch. What's the reason behind the r8a7778 SoC using
> a compatible string for r8a7779 like "renesas,ether-r8a7779"?

    R8A7779 support has appeared first in Linux and as R8A7778 Ether is 
identical to R8A7779 and no wildcards are allowed in the device tree, I 
decided to use this "compatible" prop.

> It seems that you assume that the r8a7778 ethernet controller is 100%
> compatible with r8a7779. Is that really true? For earlier versions the
> sh_eth hardware documentation was anything but accurate, so it seems
> to me that it must be more safe that r8a7778 would be using
> "renesas,ether-r8a7778". What do you think?

    I think R8A7778 and R8A7779 EtherMACs are identical. I've cross checked 
the documentation at the start of the development and the registers appeared 
to be the same.

> Thanks,

> / magnus

WBR, Sergei

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Magnus Damm]

Hi Sergei,

[CC Laurent]

On Wed, Sep 4, 2013 at 3:27 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
>
> On 09/03/2013 07:17 PM, Magnus Damm wrote:
>
>>> Define the generic R8A777x part of the Ether device node.
>
>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>
>>> ---
>>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
>>>   1 file changed, 11 insertions(+)
>>>
>>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
>>> ===================================================================
>>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
>>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
>>> @@ -98,4 +98,15 @@
>>>                  reg = <0xfffc000 0x118>;
>>>                  #gpio-range-cells = <3>;
>>>          };
>>> +
>>> +       ether: ethernet@fde00000 {
>>> +               device_type = "network";
>>> +               compatible = "renesas,ether-r8a7779";
>
>
>> Hi Sergei,
>
>
>> Thanks for your patch. What's the reason behind the r8a7778 SoC using
>> a compatible string for r8a7779 like "renesas,ether-r8a7779"?
>
>
>    R8A7779 support has appeared first in Linux and as R8A7778 Ether is
> identical to R8A7779 and no wildcards are allowed in the device tree, I
> decided to use this "compatible" prop.

Thanks for your reply, I see.

>> It seems that you assume that the r8a7778 ethernet controller is 100%
>> compatible with r8a7779. Is that really true? For earlier versions the
>> sh_eth hardware documentation was anything but accurate, so it seems
>> to me that it must be more safe that r8a7778 would be using
>> "renesas,ether-r8a7778". What do you think?
>
>
>    I think R8A7778 and R8A7779 EtherMACs are identical. I've cross checked
> the documentation at the start of the development and the registers appeared
> to be the same.

But even if the current version of the documentation happens to be
similar in it still doesn't guarantee that the IP is the same. And
using the "correct" SoC compatible value doesn't really hurt in any
way, does it?

My feeling is that using the identical SoC as compatible value must be
the best option - unless we know for sure they are identical that is.

So unless we're 100% certain about IP compatibility I'm trying to
enforce that we either use a strict matching for exactly the same SoC
version or IP block. Using a different maybe-compatible SoC string
seems to be begging for future trouble IMO.

Laurent, any opinion?

Cheers,

/ manus

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Laurent Pinchart]

On Wednesday 04 September 2013 14:35:14 Magnus Damm wrote:
> On Wed, Sep 4, 2013 at 3:27 AM, Sergei Shtylyov wrote:
> > On 09/03/2013 07:17 PM, Magnus Damm wrote:
> >>> Define the generic R8A777x part of the Ether device node.
> >>> 
> >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >>> 
> >>> ---
> >>> 
> >>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
> >>>   1 file changed, 11 insertions(+)
> >>> 
> >>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> >>> ===================================================================
> >>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> >>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> >>> @@ -98,4 +98,15 @@
> >>>                  reg = <0xfffc000 0x118>;
> >>>                  #gpio-range-cells = <3>;
> >>>          };
> >>> +
> >>> +       ether: ethernet@fde00000 {
> >>> +               device_type = "network";
> >>> +               compatible = "renesas,ether-r8a7779";
> >> 
> >> Hi Sergei,
> >> 
> >> Thanks for your patch. What's the reason behind the r8a7778 SoC using
> >> a compatible string for r8a7779 like "renesas,ether-r8a7779"?
> >
> > R8A7779 support has appeared first in Linux and as R8A7778 Ether is
> > identical to R8A7779 and no wildcards are allowed in the device tree, I
> > decided to use this "compatible" prop.
> 
> Thanks for your reply, I see.
> 
> >> It seems that you assume that the r8a7778 ethernet controller is 100%
> >> compatible with r8a7779. Is that really true? For earlier versions the
> >> sh_eth hardware documentation was anything but accurate, so it seems
> >> to me that it must be more safe that r8a7778 would be using
> >> "renesas,ether-r8a7778". What do you think?
> >
> > I think R8A7778 and R8A7779 EtherMACs are identical. I've cross checked
> > the documentation at the start of the development and the registers
> > appeared to be the same.
> 
> But even if the current version of the documentation happens to be
> similar in it still doesn't guarantee that the IP is the same. And
> using the "correct" SoC compatible value doesn't really hurt in any
> way, does it?
> 
> My feeling is that using the identical SoC as compatible value must be
> the best option - unless we know for sure they are identical that is.
> 
> So unless we're 100% certain about IP compatibility I'm trying to
> enforce that we either use a strict matching for exactly the same SoC
> version or IP block. Using a different maybe-compatible SoC string
> seems to be begging for future trouble IMO.
> 
> Laurent, any opinion?

I agree. If we had access to IP version information we could use that to 
construct the compatible name. As we don't, the SoC name is the best option, 
and to ensure future compatibility we should use the exact SoC version.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/4] ARM: shmobile: r8a7778: add Ether DT support

[Simon Horman]

On Wed, Sep 04, 2013 at 11:39:59AM +0200, Laurent Pinchart wrote:
> On Wednesday 04 September 2013 14:35:14 Magnus Damm wrote:
> > On Wed, Sep 4, 2013 at 3:27 AM, Sergei Shtylyov wrote:
> > > On 09/03/2013 07:17 PM, Magnus Damm wrote:
> > >>> Define the generic R8A777x part of the Ether device node.
> > >>> 
> > >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> > >>> 
> > >>> ---
> > >>> 
> > >>>   arch/arm/boot/dts/r8a7778.dtsi |   11 +++++++++++
> > >>>   1 file changed, 11 insertions(+)
> > >>> 
> > >>> Index: renesas/arch/arm/boot/dts/r8a7778.dtsi
> > >>> ===================================================================
> > >>> --- renesas.orig/arch/arm/boot/dts/r8a7778.dtsi
> > >>> +++ renesas/arch/arm/boot/dts/r8a7778.dtsi
> > >>> @@ -98,4 +98,15 @@
> > >>>                  reg = <0xfffc000 0x118>;
> > >>>                  #gpio-range-cells = <3>;
> > >>>          };
> > >>> +
> > >>> +       ether: ethernet@fde00000 {
> > >>> +               device_type = "network";
> > >>> +               compatible = "renesas,ether-r8a7779";
> > >> 
> > >> Hi Sergei,
> > >> 
> > >> Thanks for your patch. What's the reason behind the r8a7778 SoC using
> > >> a compatible string for r8a7779 like "renesas,ether-r8a7779"?
> > >
> > > R8A7779 support has appeared first in Linux and as R8A7778 Ether is
> > > identical to R8A7779 and no wildcards are allowed in the device tree, I
> > > decided to use this "compatible" prop.
> > 
> > Thanks for your reply, I see.
> > 
> > >> It seems that you assume that the r8a7778 ethernet controller is 100%
> > >> compatible with r8a7779. Is that really true? For earlier versions the
> > >> sh_eth hardware documentation was anything but accurate, so it seems
> > >> to me that it must be more safe that r8a7778 would be using
> > >> "renesas,ether-r8a7778". What do you think?
> > >
> > > I think R8A7778 and R8A7779 EtherMACs are identical. I've cross checked
> > > the documentation at the start of the development and the registers
> > > appeared to be the same.
> > 
> > But even if the current version of the documentation happens to be
> > similar in it still doesn't guarantee that the IP is the same. And
> > using the "correct" SoC compatible value doesn't really hurt in any
> > way, does it?
> > 
> > My feeling is that using the identical SoC as compatible value must be
> > the best option - unless we know for sure they are identical that is.
> > 
> > So unless we're 100% certain about IP compatibility I'm trying to
> > enforce that we either use a strict matching for exactly the same SoC
> > version or IP block. Using a different maybe-compatible SoC string
> > seems to be begging for future trouble IMO.
> > 
> > Laurent, any opinion?
> 
> I agree. If we had access to IP version information we could use that to 
> construct the compatible name. As we don't, the SoC name is the best option, 
> and to ensure future compatibility we should use the exact SoC version.

Using the SoC name is in keeping with recent discussions that
we have had about compat string names for shmobile. So I agree
with both Magnus and Laurent here (no surprise!).