Re: [PATCH 07/12] arm64: dts: allwinner: orange-pi-3: Enable ethernet
On Mon, Apr 08, 2019 at 09:40:42AM +0200, Maxime Ripard wrote:
> On Sat, Apr 06, 2019 at 01:45:09AM +0200, meg...@megous.com wrote:
> > From: Ondrej Jirman
> >
> > 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
> > ---
> > .../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 = <500>;
> > 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 = <250>;
> > +regulator-max-microvolt = <250>;
> > +startup-delay-us = <10>;
> > +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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/12] arm64: dts: allwinner: orange-pi-3: Enable ethernet
On Mon, Apr 08, 2019 at 11:41:38AM +0530, Jagan Teki wrote: > On Sat, Apr 6, 2019 at 5:15 AM wrote: > > > > From: Ondrej Jirman > > > > 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 > > --- > > .../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. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/12] arm64: dts: allwinner: orange-pi-3: Enable ethernet
On Tue, Apr 9, 2019 at 1:22 AM Ondřej Jirman 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 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/12] arm64: dts: allwinner: orange-pi-3: Enable ethernet
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, meg...@megous.com wrote:
> > > From: Ondrej Jirman
> > >
> > > 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
> > > ---
> > > .../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 = <500>;
> > > 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 = <250>;
> > > +regulator-max-microvolt = <250>;
> > > +startup-delay-us = <10>;
> > > +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
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/12] arm64: dts: allwinner: orange-pi-3: Enable ethernet
On Sat, Apr 06, 2019 at 01:45:09AM +0200, meg...@megous.com wrote:
> From: Ondrej Jirman
>
> 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
> ---
> .../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 = <500>;
> 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 = <250>;
> +regulator-max-microvolt = <250>;
> +startup-delay-us = <10>;
> +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
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/12] arm64: dts: allwinner: orange-pi-3: Enable ethernet
From: Ondrej Jirman
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
---
.../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 = <500>;
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 = <250>;
+regulator-max-microvolt = <250>;
+startup-delay-us = <10>;
+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 = <4>;
+ };
+};
+
&mmc0 {
pinctrl-names = "default";
pinctrl-0 = <&mmc0_pins>;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/12] arm64: dts: allwinner: orange-pi-3: Enable ethernet
On Sat, Apr 6, 2019 at 5:15 AM wrote: > > From: Ondrej Jirman > > 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 > --- > .../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/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
