| Message ID | 20190405234514.6183-8-megous@megous.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Orange Pi 3 | expand |
On Sat, Apr 06, 2019 at 01:45:09AM +0200, megous@megous.com wrote: > From: Ondrej Jirman <megous@megous.com> > > Orange Pi 3 has two regulators that power the Realtek RTL8211E. > According to the phy datasheet, both regulators need to be enabled > at the same time, but we can only specify a single phy-supply in > the DT. > > This can be achieved by making one regulator depedning on the > other via vin-supply. While it's not a technically correct > description of the hardware, it achieves the purpose. > > All values of RX/TX delay were tested exhaustively and a middle > one of the working values was chosen. > > Signed-off-by: Ondrej Jirman <megous@megous.com> > --- > .../dts/allwinner/sun50i-h6-orangepi-3.dts | 44 +++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > index 644946749088..5270142527f5 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > @@ -15,6 +15,7 @@ > > aliases { > serial0 = &uart0; > + ethernet0 = &emac; > }; > > chosen { > @@ -64,6 +65,27 @@ > 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-3V3 (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>; /* PD6 */ Is enable-active-high still needed? It's redundant with the GPIO_ACTIVE_HIGH flag. The indentation is wrong here as well. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Mon, Apr 08, 2019 at 11:41:38AM +0530, Jagan Teki wrote: > On Sat, Apr 6, 2019 at 5:15 AM <megous@megous.com> wrote: > > > > From: Ondrej Jirman <megous@megous.com> > > > > Orange Pi 3 has two regulators that power the Realtek RTL8211E. > > According to the phy datasheet, both regulators need to be enabled > > at the same time, but we can only specify a single phy-supply in > > the DT. > > > > This can be achieved by making one regulator depedning on the > > other via vin-supply. While it's not a technically correct > > description of the hardware, it achieves the purpose. > > > > All values of RX/TX delay were tested exhaustively and a middle > > one of the working values was chosen. > > > > Signed-off-by: Ondrej Jirman <megous@megous.com> > > --- > > .../dts/allwinner/sun50i-h6-orangepi-3.dts | 44 +++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > This was in ML[1], I thought this change would already merged. I > remember Chen-Yu applied and revert due to emac node not mainlined at > that time. > > [1] https://patchwork.kernel.org/patch/10558281/ The patch was not merged at the time, and bitrotted a bit. Armbian folks were applying the bitortted patch out-of-the mainline and were experiencing NPEs. I fixed the patch, and it is also part of this series. It's patch 5. o.
On Mon, Apr 08, 2019 at 09:40:42AM +0200, Maxime Ripard wrote: > On Sat, Apr 06, 2019 at 01:45:09AM +0200, megous@megous.com wrote: > > From: Ondrej Jirman <megous@megous.com> > > > > Orange Pi 3 has two regulators that power the Realtek RTL8211E. > > According to the phy datasheet, both regulators need to be enabled > > at the same time, but we can only specify a single phy-supply in > > the DT. > > > > This can be achieved by making one regulator depedning on the > > other via vin-supply. While it's not a technically correct > > description of the hardware, it achieves the purpose. > > > > All values of RX/TX delay were tested exhaustively and a middle > > one of the working values was chosen. > > > > Signed-off-by: Ondrej Jirman <megous@megous.com> > > --- > > .../dts/allwinner/sun50i-h6-orangepi-3.dts | 44 +++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > index 644946749088..5270142527f5 100644 > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > @@ -15,6 +15,7 @@ > > > > aliases { > > serial0 = &uart0; > > + ethernet0 = &emac; > > }; > > > > chosen { > > @@ -64,6 +65,27 @@ > > 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-3V3 (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>; /* PD6 */ > > Is enable-active-high still needed? It's redundant with the > GPIO_ACTIVE_HIGH flag. Looking at the code, use/non-use of enable-active-high inhibits flags specified in gpio property. So the GPIO_ACTIVE_HIGH flag is ignored here (had GPIO_ACTIVE_LOW been used, the kernel would ignore it too and print a warning). So enable-active-high is still necessary here. See comment in gpiolib-of.c where this is handled: /* * The regulator GPIO handles are specified such that the * presence or absence of "enable-active-high" solely controls * the polarity of the GPIO line. Any phandle flags must * be actively ignored. */ regards, o. > The indentation is wrong here as well. > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On Tue, Apr 09, 2019 at 01:22:32AM +0200, Ondřej Jirman wrote: > On Mon, Apr 08, 2019 at 09:40:42AM +0200, Maxime Ripard wrote: > > On Sat, Apr 06, 2019 at 01:45:09AM +0200, megous@megous.com wrote: > > > From: Ondrej Jirman <megous@megous.com> > > > > > > Orange Pi 3 has two regulators that power the Realtek RTL8211E. > > > According to the phy datasheet, both regulators need to be enabled > > > at the same time, but we can only specify a single phy-supply in > > > the DT. > > > > > > This can be achieved by making one regulator depedning on the > > > other via vin-supply. While it's not a technically correct > > > description of the hardware, it achieves the purpose. > > > > > > All values of RX/TX delay were tested exhaustively and a middle > > > one of the working values was chosen. > > > > > > Signed-off-by: Ondrej Jirman <megous@megous.com> > > > --- > > > .../dts/allwinner/sun50i-h6-orangepi-3.dts | 44 +++++++++++++++++++ > > > 1 file changed, 44 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > > index 644946749088..5270142527f5 100644 > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts > > > @@ -15,6 +15,7 @@ > > > > > > aliases { > > > serial0 = &uart0; > > > + ethernet0 = &emac; > > > }; > > > > > > chosen { > > > @@ -64,6 +65,27 @@ > > > 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-3V3 (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>; /* PD6 */ > > > > Is enable-active-high still needed? It's redundant with the > > GPIO_ACTIVE_HIGH flag. > > Looking at the code, use/non-use of enable-active-high inhibits > flags specified in gpio property. So the GPIO_ACTIVE_HIGH flag > is ignored here (had GPIO_ACTIVE_LOW been used, the kernel would > ignore it too and print a warning). > > So enable-active-high is still necessary here. Too bad :/ > See comment in gpiolib-of.c where this is handled: > > /* > * The regulator GPIO handles are specified such that the > * presence or absence of "enable-active-high" solely controls > * the polarity of the GPIO line. Any phandle flags must > * be actively ignored. > */ Thanks for digging this out Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Tue, Apr 9, 2019 at 1:22 AM Ondřej Jirman <megous@megous.com> wrote: > > > + enable-active-high; > > > + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */ > > > > Is enable-active-high still needed? It's redundant with the > > GPIO_ACTIVE_HIGH flag. > > Looking at the code, use/non-use of enable-active-high inhibits > flags specified in gpio property. So the GPIO_ACTIVE_HIGH flag > is ignored here (had GPIO_ACTIVE_LOW been used, the kernel would > ignore it too and print a warning). > > So enable-active-high is still necessary here. > > See comment in gpiolib-of.c where this is handled: > > /* > * The regulator GPIO handles are specified such that the > * presence or absence of "enable-active-high" solely controls > * the polarity of the GPIO line. Any phandle flags must > * be actively ignored. > */ Yeah this caused me special headache in the current merge window because of buggy code on my part. This is an effect of this flag being defined for powerpc ages before we properly implemented generic GPIO bindings. We just have to respect it. See: https://marc.info/?l=linux-gpio&m=155417774822532&w=2 Yours, Linus Walleij
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts index 644946749088..5270142527f5 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts @@ -15,6 +15,7 @@ aliases { serial0 = &uart0; + ethernet0 = &emac; }; chosen { @@ -64,6 +65,27 @@ 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-3V3 (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>; /* 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>; /* GMAC-3V3 */ + }; }; &cpu0 { @@ -82,6 +104,17 @@ status = "okay"; }; +&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>; + status = "okay"; +}; + &hdmi { ddc-supply = <®_ddc>; status = "okay"; @@ -93,6 +126,17 @@ }; }; +&mdio { + ext_rgmii_phy: ethernet-phy@1 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <1>; + + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */ + reset-assert-us = <15000>; + reset-deassert-us = <40000>; + }; +}; + &mmc0 { pinctrl-names = "default"; pinctrl-0 = <&mmc0_pins>;