* [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support @ 2019-05-03 11:59 Jagan Teki 2019-05-03 14:46 ` Maxime Ripard 0 siblings, 1 reply; 8+ messages in thread From: Jagan Teki @ 2019-05-03 11:59 UTC (permalink / raw) To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland Cc: Michael Trimarchi, devicetree, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi, Jagan Teki Add Ethernet support for orangepi-one-plus board, - Ethernet port connected via RTL8211E PHY - PHY suppiled with GMAC-2V5, fixed regulator with GMAC_EN pin via PD6 GMAC-3V, which is supplied by VCC3V3-MAC via aldo2 - RGMII-RESET pin connected via PD14 Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v2: - emac changes on top of https://patchwork.kernel.org/cover/10899529/ series .../allwinner/sun50i-h6-orangepi-one-plus.dts | 8 ++++ .../dts/allwinner/sun50i-h6-orangepi.dtsi | 42 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts index 12e17567ab56..9e8ed1053715 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts @@ -9,4 +9,12 @@ / { model = "OrangePi One Plus"; compatible = "xunlong,orangepi-one-plus", "allwinner,sun50i-h6"; + + aliases { + ethernet0 = &emac; + }; +}; + +&emac { + status = "okay"; }; diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi index 62e27948a3fa..c48e24acaf8a 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi @@ -45,6 +45,48 @@ regulator-max-microvolt = <5000000>; regulator-always-on; }; + + /* + * The board uses 2.5V RGMII signalling. Power sequence to enable + * the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails + * at the same time and to wait 100ms. + */ + reg_gmac_2v5: gmac-2v5 { + compatible = "regulator-fixed"; + regulator-name = "gmac-2v5"; + regulator-min-microvolt = <2500000>; + regulator-max-microvolt = <2500000>; + startup-delay-us = <100000>; + enable-active-high; + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* GMAC_EN: PD6 */ + + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to + * enable two regulators to power the phy. This is one way + * to achieve that. + */ + vin-supply = <®_aldo2>; /* VCC3V3-MAC: GMAC-3V */ + }; +}; + +&emac { + pinctrl-names = "default"; + pinctrl-0 = <&ext_rgmii_pins>; + phy-mode = "rgmii"; + phy-handle = <&ext_rgmii_phy>; + phy-supply = <®_gmac_2v5>; + allwinner,rx-delay-ps = <1500>; + allwinner,tx-delay-ps = <700>; +}; + +&mdio { + ext_rgmii_phy: ethernet-phy@1 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <1>; + + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* RGMII-RESET: PD14 */ + reset-assert-us = <15000>; + reset-deassert-us = <40000>; + }; }; &ehci0 { -- 2.18.0.321.gffc6fa0e3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support 2019-05-03 11:59 [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support Jagan Teki @ 2019-05-03 14:46 ` Maxime Ripard 2019-05-06 9:33 ` Jagan Teki 0 siblings, 1 reply; 8+ messages in thread From: Maxime Ripard @ 2019-05-03 14:46 UTC (permalink / raw) To: Jagan Teki Cc: Chen-Yu Tsai, Rob Herring, Mark Rutland, Michael Trimarchi, devicetree, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 3224 bytes --] On Fri, May 03, 2019 at 05:29:28PM +0530, Jagan Teki wrote: > Add Ethernet support for orangepi-one-plus board, > > - Ethernet port connected via RTL8211E PHY > - PHY suppiled with > GMAC-2V5, fixed regulator with GMAC_EN pin via PD6 > GMAC-3V, which is supplied by VCC3V3-MAC via aldo2 > - RGMII-RESET pin connected via PD14 > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> Your commit log should be improved. We can get those informations from the patch itself... > --- > Changes for v2: > - emac changes on top of https://patchwork.kernel.org/cover/10899529/ > series > > .../allwinner/sun50i-h6-orangepi-one-plus.dts | 8 ++++ > .../dts/allwinner/sun50i-h6-orangepi.dtsi | 42 +++++++++++++++++++ > 2 files changed, 50 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > index 12e17567ab56..9e8ed1053715 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > @@ -9,4 +9,12 @@ > / { > model = "OrangePi One Plus"; > compatible = "xunlong,orangepi-one-plus", "allwinner,sun50i-h6"; > + > + aliases { > + ethernet0 = &emac; > + }; > +}; > + > +&emac { > + status = "okay"; > }; > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > index 62e27948a3fa..c48e24acaf8a 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > @@ -45,6 +45,48 @@ > regulator-max-microvolt = <5000000>; > regulator-always-on; > }; > + > + /* > + * The board uses 2.5V RGMII signalling. Power sequence to enable > + * the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails > + * at the same time and to wait 100ms. > + */ > + reg_gmac_2v5: gmac-2v5 { > + compatible = "regulator-fixed"; > + regulator-name = "gmac-2v5"; > + regulator-min-microvolt = <2500000>; > + regulator-max-microvolt = <2500000>; > + startup-delay-us = <100000>; > + enable-active-high; > + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* GMAC_EN: PD6 */ > + > + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to > + * enable two regulators to power the phy. This is one way > + * to achieve that. > + */ > + vin-supply = <®_aldo2>; /* VCC3V3-MAC: GMAC-3V */ > + }; > +}; > + > +&emac { > + pinctrl-names = "default"; > + pinctrl-0 = <&ext_rgmii_pins>; > + phy-mode = "rgmii"; > + phy-handle = <&ext_rgmii_phy>; > + phy-supply = <®_gmac_2v5>; > + allwinner,rx-delay-ps = <1500>; > + allwinner,tx-delay-ps = <700>; > +}; > + > +&mdio { > + ext_rgmii_phy: ethernet-phy@1 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <1>; > + > + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* RGMII-RESET: PD14 */ > + reset-assert-us = <15000>; > + reset-deassert-us = <40000>; > + }; > }; ... however, at no point in time you explain why you made that switch, and while most of the definition of the EMAC nodes is in the DTSI, you only enable it in one DTS. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support 2019-05-03 14:46 ` Maxime Ripard @ 2019-05-06 9:33 ` Jagan Teki 2019-05-06 13:31 ` [linux-sunxi] " Ondřej Jirman 2019-05-07 7:06 ` Maxime Ripard 0 siblings, 2 replies; 8+ messages in thread From: Jagan Teki @ 2019-05-06 9:33 UTC (permalink / raw) To: Maxime Ripard Cc: Chen-Yu Tsai, Rob Herring, Mark Rutland, Michael Trimarchi, devicetree, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi On Fri, May 3, 2019 at 8:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Fri, May 03, 2019 at 05:29:28PM +0530, Jagan Teki wrote: > > Add Ethernet support for orangepi-one-plus board, > > > > - Ethernet port connected via RTL8211E PHY > > - PHY suppiled with > > GMAC-2V5, fixed regulator with GMAC_EN pin via PD6 > > GMAC-3V, which is supplied by VCC3V3-MAC via aldo2 > > - RGMII-RESET pin connected via PD14 > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > Your commit log should be improved. We can get those informations from > the patch itself... Thought it was a clear commit log :) will update anyway. > > > --- > > Changes for v2: > > - emac changes on top of https://patchwork.kernel.org/cover/10899529/ > > series > > > > .../allwinner/sun50i-h6-orangepi-one-plus.dts | 8 ++++ > > .../dts/allwinner/sun50i-h6-orangepi.dtsi | 42 +++++++++++++++++++ > > 2 files changed, 50 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > index 12e17567ab56..9e8ed1053715 100644 > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > @@ -9,4 +9,12 @@ > > / { > > model = "OrangePi One Plus"; > > compatible = "xunlong,orangepi-one-plus", "allwinner,sun50i-h6"; > > + > > + aliases { > > + ethernet0 = &emac; > > + }; > > +}; > > + > > +&emac { > > + status = "okay"; > > }; > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > index 62e27948a3fa..c48e24acaf8a 100644 > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > @@ -45,6 +45,48 @@ > > regulator-max-microvolt = <5000000>; > > regulator-always-on; > > }; > > + > > + /* > > + * The board uses 2.5V RGMII signalling. Power sequence to enable > > + * the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails > > + * at the same time and to wait 100ms. > > + */ > > + reg_gmac_2v5: gmac-2v5 { > > + compatible = "regulator-fixed"; > > + regulator-name = "gmac-2v5"; > > + regulator-min-microvolt = <2500000>; > > + regulator-max-microvolt = <2500000>; > > + startup-delay-us = <100000>; > > + enable-active-high; > > + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* GMAC_EN: PD6 */ > > + > > + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to > > + * enable two regulators to power the phy. This is one way > > + * to achieve that. > > + */ > > + vin-supply = <®_aldo2>; /* VCC3V3-MAC: GMAC-3V */ > > + }; > > +}; > > + > > +&emac { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&ext_rgmii_pins>; > > + phy-mode = "rgmii"; > > + phy-handle = <&ext_rgmii_phy>; > > + phy-supply = <®_gmac_2v5>; > > + allwinner,rx-delay-ps = <1500>; > > + allwinner,tx-delay-ps = <700>; > > +}; > > + > > +&mdio { > > + ext_rgmii_phy: ethernet-phy@1 { > > + compatible = "ethernet-phy-ieee802.3-c22"; > > + reg = <1>; > > + > > + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* RGMII-RESET: PD14 */ > > + reset-assert-us = <15000>; > > + reset-deassert-us = <40000>; > > + }; > > }; > > ... however, at no point in time you explain why you made that switch, > and while most of the definition of the EMAC nodes is in the DTSI, you > only enable it in one DTS. The dtsi is shared b/w 1+ and lite2 and 1+ has emac, so I enabled the status directly on dts and keeping the relevant nodes on dtsi just like SoC dtsi does. do I need to mention this in commit log? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support 2019-05-06 9:33 ` Jagan Teki @ 2019-05-06 13:31 ` Ondřej Jirman 2019-05-07 7:06 ` Maxime Ripard 2019-05-07 12:57 ` Jagan Teki 2019-05-07 7:06 ` Maxime Ripard 1 sibling, 2 replies; 8+ messages in thread From: Ondřej Jirman @ 2019-05-06 13:31 UTC (permalink / raw) To: Jagan Teki Cc: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland, Michael Trimarchi, devicetree, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi On Mon, May 06, 2019 at 03:03:15PM +0530, Jagan Teki wrote: > On Fri, May 3, 2019 at 8:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > On Fri, May 03, 2019 at 05:29:28PM +0530, Jagan Teki wrote: > > > Add Ethernet support for orangepi-one-plus board, > > > > > > - Ethernet port connected via RTL8211E PHY > > > - PHY suppiled with > > > GMAC-2V5, fixed regulator with GMAC_EN pin via PD6 > > > GMAC-3V, which is supplied by VCC3V3-MAC via aldo2 > > > - RGMII-RESET pin connected via PD14 > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > > Your commit log should be improved. We can get those informations from > > the patch itself... > > Thought it was a clear commit log :) will update anyway. > > > > > > --- > > > Changes for v2: > > > - emac changes on top of https://patchwork.kernel.org/cover/10899529/ > > > series > > > > > > .../allwinner/sun50i-h6-orangepi-one-plus.dts | 8 ++++ > > > .../dts/allwinner/sun50i-h6-orangepi.dtsi | 42 +++++++++++++++++++ > > > 2 files changed, 50 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > index 12e17567ab56..9e8ed1053715 100644 > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > @@ -9,4 +9,12 @@ > > > / { > > > model = "OrangePi One Plus"; > > > compatible = "xunlong,orangepi-one-plus", "allwinner,sun50i-h6"; > > > + > > > + aliases { > > > + ethernet0 = &emac; > > > + }; > > > +}; > > > + > > > +&emac { > > > + status = "okay"; > > > }; > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > index 62e27948a3fa..c48e24acaf8a 100644 > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > @@ -45,6 +45,48 @@ > > > regulator-max-microvolt = <5000000>; > > > regulator-always-on; > > > }; > > > + > > > + /* > > > + * The board uses 2.5V RGMII signalling. Power sequence to enable > > > + * the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails > > > + * at the same time and to wait 100ms. > > > + */ > > > + reg_gmac_2v5: gmac-2v5 { > > > + compatible = "regulator-fixed"; > > > + regulator-name = "gmac-2v5"; > > > + regulator-min-microvolt = <2500000>; > > > + regulator-max-microvolt = <2500000>; > > > + startup-delay-us = <100000>; > > > + enable-active-high; > > > + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* GMAC_EN: PD6 */ > > > + > > > + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to > > > + * enable two regulators to power the phy. This is one way > > > + * to achieve that. > > > + */ > > > + vin-supply = <®_aldo2>; /* VCC3V3-MAC: GMAC-3V */ > > > + }; > > > +}; > > > + > > > +&emac { > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&ext_rgmii_pins>; > > > + phy-mode = "rgmii"; > > > + phy-handle = <&ext_rgmii_phy>; > > > + phy-supply = <®_gmac_2v5>; > > > + allwinner,rx-delay-ps = <1500>; > > > + allwinner,tx-delay-ps = <700>; > > > +}; > > > + > > > +&mdio { > > > + ext_rgmii_phy: ethernet-phy@1 { > > > + compatible = "ethernet-phy-ieee802.3-c22"; > > > + reg = <1>; > > > + > > > + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* RGMII-RESET: PD14 */ > > > + reset-assert-us = <15000>; > > > + reset-deassert-us = <40000>; > > > + }; > > > }; > > > > ... however, at no point in time you explain why you made that switch, > > and while most of the definition of the EMAC nodes is in the DTSI, you > > only enable it in one DTS. > > The dtsi is shared b/w 1+ and lite2 and 1+ has emac, so I enabled the > status directly on dts and keeping the relevant nodes on dtsi just > like SoC dtsi does. do I need to mention this in commit log? Lite 2 doesn't have reg_gmac_2v5 and it also doesn't have the external phy. But with this patch, reg_gmac_2v5 will also show up in the Lite 2's final DTB. Comapred to SoC dtsi, the SoC always has things that are in the dtsi, they are just not enabled/used by the board, but they are present on the chip. So this comes down to what the meaning of board-level dtsi should be. I doubt we want it to mean "a collection of stuff that may or may not be present on the boards that depend on it". regards, o. > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support 2019-05-06 13:31 ` [linux-sunxi] " Ondřej Jirman @ 2019-05-07 7:06 ` Maxime Ripard 2019-05-07 12:57 ` Jagan Teki 1 sibling, 0 replies; 8+ messages in thread From: Maxime Ripard @ 2019-05-07 7:06 UTC (permalink / raw) To: Jagan Teki, Chen-Yu Tsai, Rob Herring, Mark Rutland, Michael Trimarchi, devicetree, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 4439 bytes --] On Mon, May 06, 2019 at 03:31:01PM +0200, Ondřej Jirman wrote: > > > > --- > > > > Changes for v2: > > > > - emac changes on top of https://patchwork.kernel.org/cover/10899529/ > > > > series > > > > > > > > .../allwinner/sun50i-h6-orangepi-one-plus.dts | 8 ++++ > > > > .../dts/allwinner/sun50i-h6-orangepi.dtsi | 42 +++++++++++++++++++ > > > > 2 files changed, 50 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > > index 12e17567ab56..9e8ed1053715 100644 > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > > @@ -9,4 +9,12 @@ > > > > / { > > > > model = "OrangePi One Plus"; > > > > compatible = "xunlong,orangepi-one-plus", "allwinner,sun50i-h6"; > > > > + > > > > + aliases { > > > > + ethernet0 = &emac; > > > > + }; > > > > +}; > > > > + > > > > +&emac { > > > > + status = "okay"; > > > > }; > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > > index 62e27948a3fa..c48e24acaf8a 100644 > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > > @@ -45,6 +45,48 @@ > > > > regulator-max-microvolt = <5000000>; > > > > regulator-always-on; > > > > }; > > > > + > > > > + /* > > > > + * The board uses 2.5V RGMII signalling. Power sequence to enable > > > > + * the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails > > > > + * at the same time and to wait 100ms. > > > > + */ > > > > + reg_gmac_2v5: gmac-2v5 { > > > > + compatible = "regulator-fixed"; > > > > + regulator-name = "gmac-2v5"; > > > > + regulator-min-microvolt = <2500000>; > > > > + regulator-max-microvolt = <2500000>; > > > > + startup-delay-us = <100000>; > > > > + enable-active-high; > > > > + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* GMAC_EN: PD6 */ > > > > + > > > > + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to > > > > + * enable two regulators to power the phy. This is one way > > > > + * to achieve that. > > > > + */ > > > > + vin-supply = <®_aldo2>; /* VCC3V3-MAC: GMAC-3V */ > > > > + }; > > > > +}; > > > > + > > > > +&emac { > > > > + pinctrl-names = "default"; > > > > + pinctrl-0 = <&ext_rgmii_pins>; > > > > + phy-mode = "rgmii"; > > > > + phy-handle = <&ext_rgmii_phy>; > > > > + phy-supply = <®_gmac_2v5>; > > > > + allwinner,rx-delay-ps = <1500>; > > > > + allwinner,tx-delay-ps = <700>; > > > > +}; > > > > + > > > > +&mdio { > > > > + ext_rgmii_phy: ethernet-phy@1 { > > > > + compatible = "ethernet-phy-ieee802.3-c22"; > > > > + reg = <1>; > > > > + > > > > + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* RGMII-RESET: PD14 */ > > > > + reset-assert-us = <15000>; > > > > + reset-deassert-us = <40000>; > > > > + }; > > > > }; > > > > > > ... however, at no point in time you explain why you made that switch, > > > and while most of the definition of the EMAC nodes is in the DTSI, you > > > only enable it in one DTS. > > > > The dtsi is shared b/w 1+ and lite2 and 1+ has emac, so I enabled the > > status directly on dts and keeping the relevant nodes on dtsi just > > like SoC dtsi does. do I need to mention this in commit log? > > Lite 2 doesn't have reg_gmac_2v5 and it also doesn't have the external phy. > But with this patch, reg_gmac_2v5 will also show up in the Lite 2's final > DTB. > > Comapred to SoC dtsi, the SoC always has things that are in the dtsi, they > are just not enabled/used by the board, but they are present on the chip. > > So this comes down to what the meaning of board-level dtsi should be. I > doubt we want it to mean "a collection of stuff that may or may not be > present on the boards that depend on it". Agreed. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support 2019-05-06 13:31 ` [linux-sunxi] " Ondřej Jirman 2019-05-07 7:06 ` Maxime Ripard @ 2019-05-07 12:57 ` Jagan Teki 1 sibling, 0 replies; 8+ messages in thread From: Jagan Teki @ 2019-05-07 12:57 UTC (permalink / raw) To: Jagan Teki, Maxime Ripard, Chen-Yu Tsai, Rob Herring, Mark Rutland, Michael Trimarchi, devicetree, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi On Mon, May 6, 2019 at 7:01 PM Ondřej Jirman <megous@megous.com> wrote: > > On Mon, May 06, 2019 at 03:03:15PM +0530, Jagan Teki wrote: > > On Fri, May 3, 2019 at 8:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > On Fri, May 03, 2019 at 05:29:28PM +0530, Jagan Teki wrote: > > > > Add Ethernet support for orangepi-one-plus board, > > > > > > > > - Ethernet port connected via RTL8211E PHY > > > > - PHY suppiled with > > > > GMAC-2V5, fixed regulator with GMAC_EN pin via PD6 > > > > GMAC-3V, which is supplied by VCC3V3-MAC via aldo2 > > > > - RGMII-RESET pin connected via PD14 > > > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > > > > Your commit log should be improved. We can get those informations from > > > the patch itself... > > > > Thought it was a clear commit log :) will update anyway. > > > > > > > > > --- > > > > Changes for v2: > > > > - emac changes on top of https://patchwork.kernel.org/cover/10899529/ > > > > series > > > > > > > > .../allwinner/sun50i-h6-orangepi-one-plus.dts | 8 ++++ > > > > .../dts/allwinner/sun50i-h6-orangepi.dtsi | 42 +++++++++++++++++++ > > > > 2 files changed, 50 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > > index 12e17567ab56..9e8ed1053715 100644 > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-one-plus.dts > > > > @@ -9,4 +9,12 @@ > > > > / { > > > > model = "OrangePi One Plus"; > > > > compatible = "xunlong,orangepi-one-plus", "allwinner,sun50i-h6"; > > > > + > > > > + aliases { > > > > + ethernet0 = &emac; > > > > + }; > > > > +}; > > > > + > > > > +&emac { > > > > + status = "okay"; > > > > }; > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > > index 62e27948a3fa..c48e24acaf8a 100644 > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi > > > > @@ -45,6 +45,48 @@ > > > > regulator-max-microvolt = <5000000>; > > > > regulator-always-on; > > > > }; > > > > + > > > > + /* > > > > + * The board uses 2.5V RGMII signalling. Power sequence to enable > > > > + * the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails > > > > + * at the same time and to wait 100ms. > > > > + */ > > > > + reg_gmac_2v5: gmac-2v5 { > > > > + compatible = "regulator-fixed"; > > > > + regulator-name = "gmac-2v5"; > > > > + regulator-min-microvolt = <2500000>; > > > > + regulator-max-microvolt = <2500000>; > > > > + startup-delay-us = <100000>; > > > > + enable-active-high; > > > > + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* GMAC_EN: PD6 */ > > > > + > > > > + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to > > > > + * enable two regulators to power the phy. This is one way > > > > + * to achieve that. > > > > + */ > > > > + vin-supply = <®_aldo2>; /* VCC3V3-MAC: GMAC-3V */ > > > > + }; > > > > +}; > > > > + > > > > +&emac { > > > > + pinctrl-names = "default"; > > > > + pinctrl-0 = <&ext_rgmii_pins>; > > > > + phy-mode = "rgmii"; > > > > + phy-handle = <&ext_rgmii_phy>; > > > > + phy-supply = <®_gmac_2v5>; > > > > + allwinner,rx-delay-ps = <1500>; > > > > + allwinner,tx-delay-ps = <700>; > > > > +}; > > > > + > > > > +&mdio { > > > > + ext_rgmii_phy: ethernet-phy@1 { > > > > + compatible = "ethernet-phy-ieee802.3-c22"; > > > > + reg = <1>; > > > > + > > > > + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* RGMII-RESET: PD14 */ > > > > + reset-assert-us = <15000>; > > > > + reset-deassert-us = <40000>; > > > > + }; > > > > }; > > > > > > ... however, at no point in time you explain why you made that switch, > > > and while most of the definition of the EMAC nodes is in the DTSI, you > > > only enable it in one DTS. > > > > The dtsi is shared b/w 1+ and lite2 and 1+ has emac, so I enabled the > > status directly on dts and keeping the relevant nodes on dtsi just > > like SoC dtsi does. do I need to mention this in commit log? > > Lite 2 doesn't have reg_gmac_2v5 and it also doesn't have the external phy. > But with this patch, reg_gmac_2v5 will also show up in the Lite 2's final > DTB. > > Comapred to SoC dtsi, the SoC always has things that are in the dtsi, they > are just not enabled/used by the board, but they are present on the chip. > > So this comes down to what the meaning of board-level dtsi should be. I > doubt we want it to mean "a collection of stuff that may or may not be > present on the boards that depend on it". Was thinking in another direction, thanks for the info. agreed. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support 2019-05-06 9:33 ` Jagan Teki 2019-05-06 13:31 ` [linux-sunxi] " Ondřej Jirman @ 2019-05-07 7:06 ` Maxime Ripard 2019-05-07 12:58 ` Jagan Teki 1 sibling, 1 reply; 8+ messages in thread From: Maxime Ripard @ 2019-05-07 7:06 UTC (permalink / raw) To: Jagan Teki Cc: Chen-Yu Tsai, Rob Herring, Mark Rutland, Michael Trimarchi, devicetree, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi [-- Attachment #1: Type: text/plain, Size: 1121 bytes --] On Mon, May 06, 2019 at 03:03:15PM +0530, Jagan Teki wrote: > On Fri, May 3, 2019 at 8:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > On Fri, May 03, 2019 at 05:29:28PM +0530, Jagan Teki wrote: > > > Add Ethernet support for orangepi-one-plus board, > > > > > > - Ethernet port connected via RTL8211E PHY > > > - PHY suppiled with > > > GMAC-2V5, fixed regulator with GMAC_EN pin via PD6 > > > GMAC-3V, which is supplied by VCC3V3-MAC via aldo2 > > > - RGMII-RESET pin connected via PD14 > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > > Your commit log should be improved. We can get those informations from > > the patch itself... > > Thought it was a clear commit log :) will update anyway. Well, yes and no. The commit log is clear indeed, but it doesn't provide what it's supposed to provide. You shouldn't put *what* is being done by the patch. That's pretty easy to figure out by reading the patch itself. You have to explain why and how you did it, which is lacking in that case. -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support 2019-05-07 7:06 ` Maxime Ripard @ 2019-05-07 12:58 ` Jagan Teki 0 siblings, 0 replies; 8+ messages in thread From: Jagan Teki @ 2019-05-07 12:58 UTC (permalink / raw) To: Maxime Ripard Cc: Chen-Yu Tsai, Rob Herring, Mark Rutland, Michael Trimarchi, devicetree, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi On Tue, May 7, 2019 at 12:36 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > On Mon, May 06, 2019 at 03:03:15PM +0530, Jagan Teki wrote: > > On Fri, May 3, 2019 at 8:16 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > On Fri, May 03, 2019 at 05:29:28PM +0530, Jagan Teki wrote: > > > > Add Ethernet support for orangepi-one-plus board, > > > > > > > > - Ethernet port connected via RTL8211E PHY > > > > - PHY suppiled with > > > > GMAC-2V5, fixed regulator with GMAC_EN pin via PD6 > > > > GMAC-3V, which is supplied by VCC3V3-MAC via aldo2 > > > > - RGMII-RESET pin connected via PD14 > > > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > > > > Your commit log should be improved. We can get those informations from > > > the patch itself... > > > > Thought it was a clear commit log :) will update anyway. > > Well, yes and no. The commit log is clear indeed, but it doesn't > provide what it's supposed to provide. > > You shouldn't put *what* is being done by the patch. That's pretty > easy to figure out by reading the patch itself. You have to explain > why and how you did it, which is lacking in that case. Make sense, will rework. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-05-07 12:58 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-03 11:59 [PATCH v2] arm64: allwinner: h6: orangepi-one-plus: Add Ethernet support Jagan Teki 2019-05-03 14:46 ` Maxime Ripard 2019-05-06 9:33 ` Jagan Teki 2019-05-06 13:31 ` [linux-sunxi] " Ondřej Jirman 2019-05-07 7:06 ` Maxime Ripard 2019-05-07 12:57 ` Jagan Teki 2019-05-07 7:06 ` Maxime Ripard 2019-05-07 12:58 ` Jagan Teki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).