| Message ID | 201309010315.49763.sergei.shtylyov@cogentembedded.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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"; > + }; > };
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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? -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 :-)
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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!). -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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"; + }; };
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(+) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html