Re: [PATCH 1/2] arm64: dts: renesas: r8a77980: add GEther support

[Simon Horman]

On Wed, May 23, 2018 at 12:05:30PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 5/23/2018 11:41 AM, Simon Horman wrote:
> 
> > > > > Define the generic R8A77980 part of the GEther device node.
> > > > > 
> > > > > Based on the original (and large) patch by Vladimir Barinov.
> > > > > 
> > > > > Signed-off-by: Vladimir Barinov 
> > > > > Signed-off-by: Sergei Shtylyov 
> > > > 
> > > > Thanks for your patch!
> > > > 
> > > > With the below addressed:
> > > > Reviewed-by: Geert Uytterhoeven 
> > > 
> > > Thanks!
> > > 
> > > > > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > > > > +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > > > > @@ -417,6 +417,17 @@
> > > > >  dma-channels = <16>;
> > > > >  };
> > > > > 
> > > > > +   gether: ethernet@e740 {
> > > > > +   compatible = "renesas,gether-r8a77980";
> > > > > +   reg = <0 0xe740 0 0x1000>;
> > > > > +   interrupts = ;
> > > > > +   clocks = < CPG_MOD 813>;
> > > > > +   power-domains = < R8A77980_PD_ALWAYS_ON>;
> > > > 
> > > > resets = < 813>;
> > > 
> > > As usual...
> > > 
> > > > 
> > > > > +   #address-cells = <1>;
> > > > > +   #size-cells = <0>;
> > > > > +   status = "disabled";
> > > > 
> > > > Any default phy-mode needed?
> > > 
> > > A default "phy-mode" IMO make sense when the MAC supports a single
> > > PHY interface mode. In this case, both RMII and RGMII are supported, so
> > > I coulsn't choose a default...
> > 
> > I would think making an arbitrary choice is better than no choice.
> > How does the driver behave in the absence of a default?
> 
>The board DT *must* assign some "phy-mode" -- it's a required prop.
>In this particular case, iff the mode is still unspecified, the driver
> will select the MII mode for the RMII_MII reg (which is a reserved value for
> this GEther)...

Thanks, if its required that boards provide this property then
this makes sense to me.

I have applied the patch after adding the resets property.
The result is as follows:

From: Sergei Shtylyov 
Subject: [PATCH] arm64: dts: renesas: r8a77980: add GEther support

Define the generic R8A77980 part of the GEther device node.

Based on the original (and large) patch by Vladimir Barinov.

Signed-off-by: Vladimir Barinov 
Signed-off-by: Sergei Shtylyov 
Reviewed-by: Geert Uytterhoeven 
[simon: add resets property]
Signed-off-by: Simon Horman 
---
 arch/arm64/boot/dts/renesas/r8a77980.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77980.dtsi 
b/arch/arm64/boot/dts/renesas/r8a77980.dtsi
index 6d2b61d83caf..c797db59ae18 100644
--- a/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -417,6 +417,18 @@
dma-channels = <16>;
};
 
+   gether: ethernet@e740 {
+   compatible = "renesas,gether-r8a77980";
+   reg = <0 0xe740 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 813>;
+   power-domains = < R8A77980_PD_ALWAYS_ON>;
+   resets = < 813>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   status = "disabled";
+   };
+
mmc0: mmc@ee14 {
compatible = "renesas,sdhi-r8a77980",
 "renesas,rcar-gen3-sdhi";
-- 
2.11.0

Re: [PATCH 1/2] arm64: dts: renesas: r8a77980: add GEther support

[Sergei Shtylyov]

Hello!

On 5/23/2018 11:41 AM, Simon Horman wrote:


Define the generic R8A77980 part of the GEther device node.

Based on the original (and large) patch by Vladimir Barinov.

Signed-off-by: Vladimir Barinov 
Signed-off-by: Sergei Shtylyov 


Thanks for your patch!

With the below addressed:
Reviewed-by: Geert Uytterhoeven 


Thanks!


--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -417,6 +417,17 @@
 dma-channels = <16>;
 };

+   gether: ethernet@e740 {
+   compatible = "renesas,gether-r8a77980";
+   reg = <0 0xe740 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 813>;
+   power-domains = < R8A77980_PD_ALWAYS_ON>;


resets = < 813>;


As usual...




+   #address-cells = <1>;
+   #size-cells = <0>;
+   status = "disabled";


Any default phy-mode needed?


A default "phy-mode" IMO make sense when the MAC supports a single
PHY interface mode. In this case, both RMII and RGMII are supported, so
I coulsn't choose a default...


I would think making an arbitrary choice is better than no choice.
How does the driver behave in the absence of a default?


   The board DT *must* assign some "phy-mode" -- it's a required prop.
   In this particular case, iff the mode is still unspecified, the driver 
will select the MII mode for the RMII_MII reg (which is a reserved value for 
this GEther)...


[...]

MBR, Sergei

Re: [PATCH 1/2] arm64: dts: renesas: r8a77980: add GEther support

[Sergei Shtylyov]

On 05/22/2018 02:48 PM, Geert Uytterhoeven wrote:

>> Define the generic R8A77980 part of the GEther device node.
>>
>> Based on the original (and large) patch by Vladimir Barinov.
>>
>> Signed-off-by: Vladimir Barinov 
>> Signed-off-by: Sergei Shtylyov 
> 
> Thanks for your patch!
> 
> With the below addressed:
> Reviewed-by: Geert Uytterhoeven 

   Thanks!

>> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
>> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
>> @@ -417,6 +417,17 @@
>> dma-channels = <16>;
>> };
>>
>> +   gether: ethernet@e740 {
>> +   compatible = "renesas,gether-r8a77980";
>> +   reg = <0 0xe740 0 0x1000>;
>> +   interrupts = ;
>> +   clocks = < CPG_MOD 813>;
>> +   power-domains = < R8A77980_PD_ALWAYS_ON>;
> 
> resets = < 813>;

   As usual...

> 
>> +   #address-cells = <1>;
>> +   #size-cells = <0>;
>> +   status = "disabled";
> 
> Any default phy-mode needed?

   A default "phy-mode" IMO make sense when the MAC supports a single 
PHY interface mode. In this case, both RMII and RGMII are supported, so
I coulsn't choose a default...

>> +   };
>> +
>> mmc0: mmc@ee14 {
>> compatible = "renesas,sdhi-r8a77980",
>>  "renesas,rcar-gen3-sdhi";
> 
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 

Re: [PATCH 1/2] arm64: dts: renesas: r8a77980: add GEther support

[Geert Uytterhoeven]

Hi Sergei,

On Fri, May 18, 2018 at 9:45 PM, Sergei Shtylyov
 wrote:
> Define the generic R8A77980 part of the GEther device node.
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov 
> Signed-off-by: Sergei Shtylyov 

Thanks for your patch!

With the below addressed:
Reviewed-by: Geert Uytterhoeven 

> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -417,6 +417,17 @@
> dma-channels = <16>;
> };
>
> +   gether: ethernet@e740 {
> +   compatible = "renesas,gether-r8a77980";
> +   reg = <0 0xe740 0 0x1000>;
> +   interrupts = ;
> +   clocks = < CPG_MOD 813>;
> +   power-domains = < R8A77980_PD_ALWAYS_ON>;

resets = < 813>;

> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   status = "disabled";

Any default phy-mode needed?

> +   };
> +
> mmc0: mmc@ee14 {
> compatible = "renesas,sdhi-r8a77980",
>  "renesas,rcar-gen3-sdhi";


Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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

Re: [PATCH 1/2] arm64: dts: renesas: r8a77980: add GEther support

[Simon Horman]

On Fri, May 18, 2018 at 10:45:36PM +0300, Sergei Shtylyov wrote:
> Define the generic R8A77980 part of the GEther device node.
> 
> Based on the original (and large) patch by Vladimir Barinov.
> 
> Signed-off-by: Vladimir Barinov 
> Signed-off-by: Sergei Shtylyov 

This looks fine but I will wait to see if there are other reviews before
applying.

Reviewed-by: Simon Horman 

[PATCH 1/2] arm64: dts: renesas: r8a77980: add GEther support

[Sergei Shtylyov]

Define the generic R8A77980 part of the GEther device node.

Based on the original (and large) patch by Vladimir Barinov.

Signed-off-by: Vladimir Barinov 
Signed-off-by: Sergei Shtylyov 

---
 arch/arm64/boot/dts/renesas/r8a77980.dtsi |   11 +++
 1 file changed, 11 insertions(+)

Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
===
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -417,6 +417,17 @@
dma-channels = <16>;
};
 
+   gether: ethernet@e740 {
+   compatible = "renesas,gether-r8a77980";
+   reg = <0 0xe740 0 0x1000>;
+   interrupts = ;
+   clocks = < CPG_MOD 813>;
+   power-domains = < R8A77980_PD_ALWAYS_ON>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   status = "disabled";
+   };
+
mmc0: mmc@ee14 {
compatible = "renesas,sdhi-r8a77980",
 "renesas,rcar-gen3-sdhi";