diff mbox

[4/5] ARM: dts: imx6sx-sabreauto: add fec support

Message ID 1524455219-6736-4-git-send-email-Anson.Huang@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Anson Huang April 23, 2018, 3:46 a.m. UTC
Add FEC support on i.MX6SX Sabre Auto board.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm/boot/dts/imx6sx-sabreauto.dts | 70 ++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

Comments

Fabio Estevam April 23, 2018, 11:08 a.m. UTC | #1
Hi Anson,

On Mon, Apr 23, 2018 at 12:46 AM, Anson Huang <Anson.Huang@nxp.com> wrote:
> Add FEC support on i.MX6SX Sabre Auto board.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  arch/arm/boot/dts/imx6sx-sabreauto.dts | 70 ++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> index 812f40b..eadd483 100644
> --- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> +++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> @@ -41,6 +41,40 @@
>         clock-frequency = <24576000>;
>  };
>
> +&fec1 {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_enet1_1>;
> +       phy-mode = "rgmii";
> +       phy-handle = <&ethphy1>;
> +       pinctrl-assert-gpios = <&max7322 0 GPIO_ACTIVE_HIGH>;

This property does not exist in mainline, only in the NXP vendor kernel.
Anson Huang April 24, 2018, 2:09 a.m. UTC | #2
Hi, Fabio

Anson Huang
Best Regards!


> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: Monday, April 23, 2018 7:09 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> dl-linux-imx <linux-imx@nxp.com>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; open list:OPEN
> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
> 
> Hi Anson,
> 
> On Mon, Apr 23, 2018 at 12:46 AM, Anson Huang <Anson.Huang@nxp.com>
> wrote:
> > Add FEC support on i.MX6SX Sabre Auto board.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  arch/arm/boot/dts/imx6sx-sabreauto.dts | 70
> ++++++++++++++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> > index 812f40b..eadd483 100644
> > --- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> > +++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> > @@ -41,6 +41,40 @@
> >         clock-frequency = <24576000>;
> >  };
> >
> > +&fec1 {
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_enet1_1>;
> > +       phy-mode = "rgmii";
> > +       phy-handle = <&ethphy1>;
> > +       pinctrl-assert-gpios = <&max7322 0 GPIO_ACTIVE_HIGH>;
> 
> This property does not exist in mainline, only in the NXP vendor kernel.

Ah, yes, thanks for pointing out this issue, I just removed it and the function is still working,
already sent out V2 patch set, thanks.

Anson.
Fabio Estevam April 24, 2018, 12:22 p.m. UTC | #3
Hi Anson,

On Mon, Apr 23, 2018 at 11:09 PM, Anson Huang <anson.huang@nxp.com> wrote:

> Ah, yes, thanks for pointing out this issue, I just removed it and the function is still working,
> already sent out V2 patch set, thanks.

So now maybe it is working only because the bootloader activated this
GPIO, which is not good.

I don't have access to the mx6sx sabreauto schematics to verify  where
&max7322 0 connects to, but it would be better to make sure that you
activate this pin in dts if it is needed for Ethernet, without relying
on the bootloader.
Anson Huang April 25, 2018, 3:30 a.m. UTC | #4
Hi, Fabio

Anson Huang
Best Regards!


> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: Tuesday, April 24, 2018 8:23 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> dl-linux-imx <linux-imx@nxp.com>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; open list:OPEN
> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
> 
> Hi Anson,
> 
> On Mon, Apr 23, 2018 at 11:09 PM, Anson Huang <anson.huang@nxp.com>
> wrote:
> 
> > Ah, yes, thanks for pointing out this issue, I just removed it and the
> > function is still working, already sent out V2 patch set, thanks.
> 
> So now maybe it is working only because the bootloader activated this GPIO,
> which is not good.
> 
> I don't have access to the mx6sx sabreauto schematics to verify  where
> &max7322 0 connects to, but it would be better to make sure that you activate
> this pin in dts if it is needed for Ethernet, without relying on the bootloader.

It is working by default hardware settings, but I agree we should do it in dts,
I found it has a lot of dependency if we want to enable the gpio reset for FEC1,
many gpio reset patch missed in upstream kernel, need patch work for MAX7322 first, so
I plan to remove FEC support in this patch set, and will upstream the MAX7322
reset patch first, then will add FEC support after MAX7322 patch done. Will send
out V3 patch to drop fec support for now, thanks.

Anson.
Anson Huang April 25, 2018, 5:36 a.m. UTC | #5
Hi, Fabio

Anson Huang
Best Regards!


> -----Original Message-----
> From: Anson Huang
> Sent: Wednesday, April 25, 2018 11:30 AM
> To: 'Fabio Estevam' <festevam@gmail.com>
> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> dl-linux-imx <linux-imx@nxp.com>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; open list:OPEN
> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>
> Subject: RE: [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
> 
> Hi, Fabio
> 
> Anson Huang
> Best Regards!
> 
> 
> > -----Original Message-----
> > From: Fabio Estevam [mailto:festevam@gmail.com]
> > Sent: Tuesday, April 24, 2018 8:23 PM
> > To: Anson Huang <anson.huang@nxp.com>
> > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>; Rob
> > Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> > dl-linux-imx <linux-imx@nxp.com>; moderated list:ARM/FREESCALE IMX /
> > MXC ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; open
> > list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> > <devicetree@vger.kernel.org>; linux-kernel
> > <linux-kernel@vger.kernel.org>
> > Subject: Re: [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
> >
> > Hi Anson,
> >
> > On Mon, Apr 23, 2018 at 11:09 PM, Anson Huang <anson.huang@nxp.com>
> > wrote:
> >
> > > Ah, yes, thanks for pointing out this issue, I just removed it and
> > > the function is still working, already sent out V2 patch set, thanks.
> >
> > So now maybe it is working only because the bootloader activated this
> > GPIO, which is not good.
> >
> > I don't have access to the mx6sx sabreauto schematics to verify  where
> > &max7322 0 connects to, but it would be better to make sure that you
> > activate this pin in dts if it is needed for Ethernet, without relying on the
> bootloader.
> 
> It is working by default hardware settings, but I agree we should do it in dts, I
> found it has a lot of dependency if we want to enable the gpio reset for FEC1,
> many gpio reset patch missed in upstream kernel, need patch work for
> MAX7322 first, so I plan to remove FEC support in this patch set, and will
> upstream the MAX7322 reset patch first, then will add FEC support after
> MAX7322 patch done. Will send out V3 patch to drop fec support for now,
> thanks.
> 
> Anson.

Sorry, I made a mistake here, the MAX7320 IO0 is for adjusting FEC1's voltage,
NOT reset, and when I tested MAX7320 driver, I did NOT notice that the
CONFIG_GPIO_MAX732X is NOT enabled in imx_v6_v7_defconfig, so I thought
it is NOT working, after enabling MAX7320 driver, max7320 is working as expected,
will send out another patch set including fec driver and MAX7320 config, thanks for your patience.

Anson.

> 
> 
> 
>
Fabio Estevam April 25, 2018, 1:46 p.m. UTC | #6
Hi Anson,

On Wed, Apr 25, 2018 at 2:36 AM, Anson Huang <anson.huang@nxp.com> wrote:

> Sorry, I made a mistake here, the MAX7320 IO0 is for adjusting FEC1's voltage,

In this case you need to pass the 'phy-supply' property inside the fec
node and add a regulator that is controlled via MAX7320 IO0 pin.
Anson Huang April 26, 2018, 6:57 a.m. UTC | #7
Hi, Fabio

Anson Huang
Best Regards!


> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: Wednesday, April 25, 2018 9:47 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> dl-linux-imx <linux-imx@nxp.com>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; open list:OPEN
> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
> 
> Hi Anson,
> 
> On Wed, Apr 25, 2018 at 2:36 AM, Anson Huang <anson.huang@nxp.com>
> wrote:
> 
> > Sorry, I made a mistake here, the MAX7320 IO0 is for adjusting FEC1's
> > voltage,
> 
> In this case you need to pass the 'phy-supply' property inside the fec node and
> add a regulator that is controlled via MAX7320 IO0 pin.

The 'phy-supply' is for enabling/disabling phy regulator, but here the MAX7322 IO0 is NOT for
enabling/disabling PHY regulator, it is for IO voltage switch between 1.5V and 1.8V, our ENET
IO can work with both 1.5V and 1.8V, so any config is OK for ENET function.

The 1.5V/1.8V selection is a one time setting thing, that means we only need to
config it once during boot up, most of i.MX platforms does NOT provide such voltage
switch function for ENET IO, on this 6SX sabre auto board, it is more like a backup or
validation purpose. With default settings, ENET's function is NOT impacted at all.

I think we can add a gpio regulator for it and let the regulator initialization set the GPIO
Level for fec, such below, with " enable-active-high " present, GPIO will be at LOW and voltage
is 1.5V, without this property, GPIO will be HIGH and voltage will be 1.8V.
+               reg_fec: fec_io_supply {
+                       compatible = "regulator-gpio";
+                       regulator-name = "1.8V_1.5V_FEC";
+                       regulator-min-microvolt = <1500000>;
+                       regulator-max-microvolt = <1800000>;
+                       states = <1500000 0x0 1800000 0x1>;
+                       enable-gpio = <&max7322 0 GPIO_ACTIVE_HIGH>;
+                       vin-supply = <&sw2_reg>;
+                       enable-active-high;
+               };
+

Anson.
Fabio Estevam April 26, 2018, 5:28 p.m. UTC | #8
On Thu, Apr 26, 2018 at 3:57 AM, Anson Huang <anson.huang@nxp.com> wrote:

> The 'phy-supply' is for enabling/disabling phy regulator, but here the MAX7322 IO0 is NOT for
> enabling/disabling PHY regulator, it is for IO voltage switch between 1.5V and 1.8V, our ENET
> IO can work with both 1.5V and 1.8V, so any config is OK for ENET function.

Thanks for the clarification.

> The 1.5V/1.8V selection is a one time setting thing, that means we only need to
> config it once during boot up, most of i.MX platforms does NOT provide such voltage
> switch function for ENET IO, on this 6SX sabre auto board, it is more like a backup or
> validation purpose. With default settings, ENET's function is NOT impacted at all.
>
> I think we can add a gpio regulator for it and let the regulator initialization set the GPIO
> Level for fec, such below, with " enable-active-high " present, GPIO will be at LOW and voltage
> is 1.5V, without this property, GPIO will be HIGH and voltage will be 1.8V.
> +               reg_fec: fec_io_supply {
> +                       compatible = "regulator-gpio";
> +                       regulator-name = "1.8V_1.5V_FEC";
> +                       regulator-min-microvolt = <1500000>;
> +                       regulator-max-microvolt = <1800000>;
> +                       states = <1500000 0x0 1800000 0x1>;
> +                       enable-gpio = <&max7322 0 GPIO_ACTIVE_HIGH>;
> +                       vin-supply = <&sw2_reg>;
> +                       enable-active-high;

If there is no consumer for this regulator, the regulator API will disable it.

It seems you need a 'regulator-always-on;'.


> +               };
> +
>
> Anson.
>
Anson Huang April 27, 2018, 1:37 a.m. UTC | #9
Anson Huang
Best Regards!


> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: Friday, April 27, 2018 1:29 AM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> dl-linux-imx <linux-imx@nxp.com>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; open list:OPEN
> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>;
> Andy Duan <fugang.duan@nxp.com>
> Subject: Re: [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
> 
> On Thu, Apr 26, 2018 at 3:57 AM, Anson Huang <anson.huang@nxp.com>
> wrote:
> 
> > The 'phy-supply' is for enabling/disabling phy regulator, but here the
> > MAX7322 IO0 is NOT for enabling/disabling PHY regulator, it is for IO
> > voltage switch between 1.5V and 1.8V, our ENET IO can work with both 1.5V
> and 1.8V, so any config is OK for ENET function.
> 
> Thanks for the clarification.
> 
> > The 1.5V/1.8V selection is a one time setting thing, that means we
> > only need to config it once during boot up, most of i.MX platforms
> > does NOT provide such voltage switch function for ENET IO, on this 6SX
> > sabre auto board, it is more like a backup or validation purpose. With default
> settings, ENET's function is NOT impacted at all.
> >
> > I think we can add a gpio regulator for it and let the regulator
> > initialization set the GPIO Level for fec, such below, with "
> > enable-active-high " present, GPIO will be at LOW and voltage is 1.5V, without
> this property, GPIO will be HIGH and voltage will be 1.8V.
> > +               reg_fec: fec_io_supply {
> > +                       compatible = "regulator-gpio";
> > +                       regulator-name = "1.8V_1.5V_FEC";
> > +                       regulator-min-microvolt = <1500000>;
> > +                       regulator-max-microvolt = <1800000>;
> > +                       states = <1500000 0x0 1800000 0x1>;
> > +                       enable-gpio = <&max7322 0
> GPIO_ACTIVE_HIGH>;
> > +                       vin-supply = <&sw2_reg>;
> > +                       enable-active-high;
> 
> If there is no consumer for this regulator, the regulator API will disable it.
> 
> It seems you need a 'regulator-always-on;'.

GPIO regulator is slight different when regulator framework try to disable
those unused regulator in late phase, in _regulator_do_disable, the GPIO
regulator only got disabled when it is explicitly enabled before (ena_gpio_state
is set when regulator is enabled), but this reg_fec is NOT getting enabled, since
it only has voltage switch function. But I agree that we can anyway add 'regulator-always-on'
to avoid any confusion, will send out new patch, thanks!
 
         if (rdev->ena_pin) {
                 if (rdev->ena_gpio_state) {
                         ret = regulator_ena_gpio_ctrl(rdev, false);


Anson.

> 
> 
> > +               };
> > +
> >
> > Anson.
> >
Anson Huang April 27, 2018, 2:12 a.m. UTC | #10
Anson Huang
Best Regards!


> -----Original Message-----
> From: Anson Huang
> Sent: Friday, April 27, 2018 9:37 AM
> To: 'Fabio Estevam' <festevam@gmail.com>
> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> dl-linux-imx <linux-imx@nxp.com>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; open list:OPEN
> FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; linux-kernel <linux-kernel@vger.kernel.org>;
> Andy Duan <fugang.duan@nxp.com>
> Subject: RE: [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
> 
> 
> 
> Anson Huang
> Best Regards!
> 
> 
> > -----Original Message-----
> > From: Fabio Estevam [mailto:festevam@gmail.com]
> > Sent: Friday, April 27, 2018 1:29 AM
> > To: Anson Huang <anson.huang@nxp.com>
> > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > <kernel@pengutronix.de>; Fabio Estevam <fabio.estevam@nxp.com>; Rob
> > Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> > dl-linux-imx <linux-imx@nxp.com>; moderated list:ARM/FREESCALE IMX /
> > MXC ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; open
> > list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> > <devicetree@vger.kernel.org>; linux-kernel
> > <linux-kernel@vger.kernel.org>; Andy Duan <fugang.duan@nxp.com>
> > Subject: Re: [PATCH 4/5] ARM: dts: imx6sx-sabreauto: add fec support
> >
> > On Thu, Apr 26, 2018 at 3:57 AM, Anson Huang <anson.huang@nxp.com>
> > wrote:
> >
> > > The 'phy-supply' is for enabling/disabling phy regulator, but here
> > > the
> > > MAX7322 IO0 is NOT for enabling/disabling PHY regulator, it is for
> > > IO voltage switch between 1.5V and 1.8V, our ENET IO can work with
> > > both 1.5V
> > and 1.8V, so any config is OK for ENET function.
> >
> > Thanks for the clarification.
> >
> > > The 1.5V/1.8V selection is a one time setting thing, that means we
> > > only need to config it once during boot up, most of i.MX platforms
> > > does NOT provide such voltage switch function for ENET IO, on this
> > > 6SX sabre auto board, it is more like a backup or validation
> > > purpose. With default
> > settings, ENET's function is NOT impacted at all.
> > >
> > > I think we can add a gpio regulator for it and let the regulator
> > > initialization set the GPIO Level for fec, such below, with "
> > > enable-active-high " present, GPIO will be at LOW and voltage is
> > > 1.5V, without
> > this property, GPIO will be HIGH and voltage will be 1.8V.
> > > +               reg_fec: fec_io_supply {
> > > +                       compatible = "regulator-gpio";
> > > +                       regulator-name = "1.8V_1.5V_FEC";
> > > +                       regulator-min-microvolt = <1500000>;
> > > +                       regulator-max-microvolt = <1800000>;
> > > +                       states = <1500000 0x0 1800000 0x1>;
> > > +                       enable-gpio = <&max7322 0
> > GPIO_ACTIVE_HIGH>;
> > > +                       vin-supply = <&sw2_reg>;
> > > +                       enable-active-high;
> >
> > If there is no consumer for this regulator, the regulator API will disable it.
> >
> > It seems you need a 'regulator-always-on;'.
> 
> GPIO regulator is slight different when regulator framework try to disable those
> unused regulator in late phase, in _regulator_do_disable, the GPIO regulator
> only got disabled when it is explicitly enabled before (ena_gpio_state is set when
> regulator is enabled), but this reg_fec is NOT getting enabled, since it only has
> voltage switch function. But I agree that we can anyway add
> 'regulator-always-on'
> to avoid any confusion, will send out new patch, thanks!
> 
>          if (rdev->ena_pin) {
>                  if (rdev->ena_gpio_state) {
>                          ret = regulator_ena_gpio_ctrl(rdev, false);
> 
> 
> Anson.

I found if I add ' regulator-always-on ', the regulator core will explicitly enable this GPIO
regulator and cause MAX7322 IO0 to be HIGH and the FEC IO will be 1.8V, since the GPIO regulator
state 1 means 1.8V, but what we want for now is 1.5V, so I think we should NOT add ' regulator-always-on ',
if anyone wants to use 1.8V IO voltage, they can enable this GPIO regulator to get 1.8V. So I think V5 patch set
is just what we want, we want to use the OFF state's voltage (1.5V) of this MAX7322 IO0 GPIO regulator. Thanks.

Anson.


> 
> >
> >
> > > +               };
> > > +
> > >
> > > Anson.
> > >
Shawn Guo May 4, 2018, 8:45 a.m. UTC | #11
On Thu, Apr 26, 2018 at 06:57:07AM +0000, Anson Huang wrote:
> > > Sorry, I made a mistake here, the MAX7320 IO0 is for adjusting FEC1's
> > > voltage,
> > 
> > In this case you need to pass the 'phy-supply' property inside the fec node and
> > add a regulator that is controlled via MAX7320 IO0 pin.
> 
> The 'phy-supply' is for enabling/disabling phy regulator, but here the MAX7322 IO0 is NOT for
> enabling/disabling PHY regulator, it is for IO voltage switch between 1.5V and 1.8V, our ENET
> IO can work with both 1.5V and 1.8V, so any config is OK for ENET function.
> 
> The 1.5V/1.8V selection is a one time setting thing, that means we only need to
> config it once during boot up, most of i.MX platforms does NOT provide such voltage
> switch function for ENET IO, on this 6SX sabre auto board, it is more like a backup or
> validation purpose. With default settings, ENET's function is NOT impacted at all.
> 
> I think we can add a gpio regulator for it and let the regulator initialization set the GPIO
> Level for fec, such below, with " enable-active-high " present, GPIO will be at LOW and voltage
> is 1.5V, without this property, GPIO will be HIGH and voltage will be 1.8V.
> +               reg_fec: fec_io_supply {
> +                       compatible = "regulator-gpio";
> +                       regulator-name = "1.8V_1.5V_FEC";
> +                       regulator-min-microvolt = <1500000>;
> +                       regulator-max-microvolt = <1800000>;
> +                       states = <1500000 0x0 1800000 0x1>;
> +                       enable-gpio = <&max7322 0 GPIO_ACTIVE_HIGH>;
> +                       vin-supply = <&sw2_reg>;
> +                       enable-active-high;
> +               };

Looking at gpio-regulator bindings doc, I feel that property 'gpios'
rather than 'enable-gpio' should be used to specify the MAX7320 IO0 pin.

- enable-gpio           : GPIO to use to enable/disable the regulator.
- gpios                 : GPIO group used to control voltage.

Shawn
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
index 812f40b..eadd483 100644
--- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
+++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
@@ -41,6 +41,40 @@ 
 	clock-frequency = <24576000>;
 };
 
+&fec1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_enet1_1>;
+	phy-mode = "rgmii";
+	phy-handle = <&ethphy1>;
+	pinctrl-assert-gpios = <&max7322 0 GPIO_ACTIVE_HIGH>;
+	fsl,magic-packet;
+	status = "okay";
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ethphy0: ethernet-phy@0 {
+			compatible = "ethernet-phy-ieee802.3-c22";
+			reg = <0>;
+		};
+
+		ethphy1: ethernet-phy@1 {
+			compatible = "ethernet-phy-ieee802.3-c22";
+			reg = <1>;
+		};
+	};
+};
+
+&fec2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_enet2_1>;
+	phy-mode = "rgmii";
+	phy-handle = <&ethphy0>;
+	fsl,magic-packet;
+	status = "okay";
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
@@ -75,6 +109,42 @@ 
 &iomuxc {
 	imx6x-sabreauto {
 
+		pinctrl_enet1_1: enet1grp-1 {
+			fsl,pins = <
+				MX6SX_PAD_ENET1_MDIO__ENET1_MDIO        0xa0b1
+				MX6SX_PAD_ENET1_MDC__ENET1_MDC          0xa0b1
+				MX6SX_PAD_RGMII1_TXC__ENET1_RGMII_TXC   0xa0b9
+				MX6SX_PAD_RGMII1_TD0__ENET1_TX_DATA_0   0xa0b1
+				MX6SX_PAD_RGMII1_TD1__ENET1_TX_DATA_1   0xa0b1
+				MX6SX_PAD_RGMII1_TD2__ENET1_TX_DATA_2   0xa0b1
+				MX6SX_PAD_RGMII1_TD3__ENET1_TX_DATA_3   0xa0b1
+				MX6SX_PAD_RGMII1_TX_CTL__ENET1_TX_EN    0xa0b1
+				MX6SX_PAD_RGMII1_RXC__ENET1_RX_CLK      0x3081
+				MX6SX_PAD_RGMII1_RD0__ENET1_RX_DATA_0   0x3081
+				MX6SX_PAD_RGMII1_RD1__ENET1_RX_DATA_1   0x3081
+				MX6SX_PAD_RGMII1_RD2__ENET1_RX_DATA_2   0x3081
+				MX6SX_PAD_RGMII1_RD3__ENET1_RX_DATA_3   0x3081
+				MX6SX_PAD_RGMII1_RX_CTL__ENET1_RX_EN    0x3081
+			>;
+		};
+
+		pinctrl_enet2_1: enet2grp-1 {
+			fsl,pins = <
+				MX6SX_PAD_RGMII2_TXC__ENET2_RGMII_TXC   0xa0b9
+				MX6SX_PAD_RGMII2_TD0__ENET2_TX_DATA_0   0xa0b1
+				MX6SX_PAD_RGMII2_TD1__ENET2_TX_DATA_1   0xa0b1
+				MX6SX_PAD_RGMII2_TD2__ENET2_TX_DATA_2   0xa0b1
+				MX6SX_PAD_RGMII2_TD3__ENET2_TX_DATA_3   0xa0b1
+				MX6SX_PAD_RGMII2_TX_CTL__ENET2_TX_EN    0xa0b1
+				MX6SX_PAD_RGMII2_RXC__ENET2_RX_CLK      0x3081
+				MX6SX_PAD_RGMII2_RD0__ENET2_RX_DATA_0   0x3081
+				MX6SX_PAD_RGMII2_RD1__ENET2_RX_DATA_1   0x3081
+				MX6SX_PAD_RGMII2_RD2__ENET2_RX_DATA_2   0x3081
+				MX6SX_PAD_RGMII2_RD3__ENET2_RX_DATA_3   0x3081
+				MX6SX_PAD_RGMII2_RX_CTL__ENET2_RX_EN    0x3081
+			>;
+		};
+
 		pinctrl_i2c2_1: i2c2grp-1 {
 			fsl,pins = <
 				MX6SX_PAD_GPIO1_IO03__I2C2_SDA          0x4001b8b1