diff mbox

[V6,4/7] ARM: dts: imx6sx-sabreauto: add fec support

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

Commit Message

Anson Huang May 5, 2018, 8:29 a.m. UTC 
Add FEC support on i.MX6SX Sabre Auto board.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
changes since V5:
	use "gpios" instead of "enable-gpio".
 arch/arm/boot/dts/imx6sx-sabreauto.dts | 80 ++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

Comments

Fabio Estevam May 5, 2018, 12:10 p.m. UTC | #1 
On Sat, May 5, 2018 at 5:29 AM, Anson Huang <Anson.Huang@nxp.com> wrote:
> Add FEC support on i.MX6SX Sabre Auto board.
>
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>

Again, it is not clear who is the author here. Is it Fugang or yourself?

> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> changes since V5:
>         use "gpios" instead of "enable-gpio".
>  arch/arm/boot/dts/imx6sx-sabreauto.dts | 80 ++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> index 4d41b4d..7dda741 100644
> --- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> +++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> @@ -18,6 +18,17 @@
>                 reg = <0x80000000 0x80000000>;
>         };
>
> +       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>;
> +               gpios = <&max7322 0 GPIO_ACTIVE_HIGH>;
> +               vin-supply = <&sw2_reg>;
> +               enable-active-high;
> +       };

I still find this confusing.

There is no consumer for reg_fec in, so it seems you are relying on
the fact that the kernel regulator core will disable reg_fec to put
the regulator in the state you require.
Anson Huang May 6, 2018, 6:08 a.m. UTC | #2 
Hi, Fabio

Anson Huang
Best Regards!


> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: Saturday, May 5, 2018 8:11 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>;
> Haibo Chen <haibo.chen@freescale.com>; Andy Duan
> <fugang.duan@nxp.com>; A.s. Dong <aisheng.dong@nxp.com>; Robin Gong
> <yibin.gong@nxp.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 V6 4/7] ARM: dts: imx6sx-sabreauto: add fec support
> 
> On Sat, May 5, 2018 at 5:29 AM, Anson Huang <Anson.Huang@nxp.com>
> wrote:
> > Add FEC support on i.MX6SX Sabre Auto board.
> >
> > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> 
> Again, it is not clear who is the author here. Is it Fugang or yourself?
 
Same story explained in patch V6 7/7.

> 
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > changes since V5:
> >         use "gpios" instead of "enable-gpio".
> >  arch/arm/boot/dts/imx6sx-sabreauto.dts | 80
> > ++++++++++++++++++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> > b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> > index 4d41b4d..7dda741 100644
> > --- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
> > +++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
> > @@ -18,6 +18,17 @@
> >                 reg = <0x80000000 0x80000000>;
> >         };
> >
> > +       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>;
> > +               gpios = <&max7322 0 GPIO_ACTIVE_HIGH>;
> > +               vin-supply = <&sw2_reg>;
> > +               enable-active-high;
> > +       };
> 
> I still find this confusing.
> 
> There is no consumer for reg_fec in, so it seems you are relying on the fact that
> the kernel regulator core will disable reg_fec to put the regulator in the state
> you require.

Adding consumer for reg_fec in NOT available in this patch, as FEC driver itself
does NOT support setting IO voltage based on setting of dtb, so if want to add
consumer, need to patch FEC driver as well.

As I explained before, this reg is for adjusting IO voltage between 1.5V and 1.8V,
and FEC driver can work on both of them, current FEC driver can work well no matter
if it is 1.5V or 1.8V, to avoid confusion, I think I can remove this reg_fec in this patch series,
let FEC driver work with default setting of this GPIO regulator, we can add reg_fec support
after FEC driver supports adjusting IO voltage. Thanks.

Anson.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx6sx-sabreauto.dts b/arch/arm/boot/dts/imx6sx-sabreauto.dts
index 4d41b4d..7dda741 100644
--- a/arch/arm/boot/dts/imx6sx-sabreauto.dts
+++ b/arch/arm/boot/dts/imx6sx-sabreauto.dts
@@ -18,6 +18,17 @@ 
 		reg = <0x80000000 0x80000000>;
 	};
 
+	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>;
+		gpios = <&max7322 0 GPIO_ACTIVE_HIGH>;
+		vin-supply = <&sw2_reg>;
+		enable-active-high;
+	};
+
 	vcc_sd3: regulator-vcc-sd3 {
 		compatible = "regulator-fixed";
 		pinctrl-names = "default";
@@ -34,6 +45,39 @@ 
 	clock-frequency = <24576000>;
 };
 
+&fec1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_enet1>;
+	phy-mode = "rgmii";
+	phy-handle = <&ethphy1>;
+	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>;
+	phy-mode = "rgmii";
+	phy-handle = <&ethphy0>;
+	fsl,magic-packet;
+	status = "okay";
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
@@ -66,6 +110,42 @@ 
 };
 
 &iomuxc {
+	pinctrl_enet1: enet1grp {
+		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: enet2grp {
+		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: i2c2grp {
 		fsl,pins = <
 			MX6SX_PAD_GPIO1_IO03__I2C2_SDA          0x4001b8b1