[PATCH] net: fec: Fix dual ethernet issue in VFxx

[PATCH] net: fec: Fix dual ethernet issue in VFxx

[Bhuvanchandra DV]

On i.MX28, the MDIO bus is shared between the
two RMII interfaces. However, in newer designs,
such as Vybrid, this is not the case. This patch
adds a quirk for the single MDIO case. This allows
to use both FEC interfaces working independently
on Vybird.

Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
---
 drivers/net/ethernet/freescale/fec.h      |    3 +++
 drivers/net/ethernet/freescale/fec_main.c |    7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 469691a..c9515bc 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -425,6 +425,9 @@ struct bufdesc_ex {
  */
 #define FEC_QUIRK_BUG_CAPTURE		(1 << 10)
 
+/* Controller has only one MDIO bus for interfacing external PHY's */
+#define FEC_QUIRK_SINGLE_MDIO		(1 << 11)
+
 struct fec_enet_priv_tx_q {
 	int index;
 	unsigned char *tx_bounce[TX_RING_SIZE];
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5ebdf8d..22b7748 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -91,7 +91,8 @@ static struct platform_device_id fec_devtype[] = {
 		.driver_data = 0,
 	}, {
 		.name = "imx28-fec",
-		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME,
+		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME |
+				FEC_QUIRK_SINGLE_MDIO,
 	}, {
 		.name = "imx6q-fec",
 		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
@@ -1952,7 +1953,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
 	 * mdio interface in board design, and need to be configured by
 	 * fec0 mii_bus.
 	 */
-	if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
+	if ((fep->quirks & FEC_QUIRK_SINGLE_MDIO) && fep->dev_id > 0) {
 		/* fec1 uses fec0 mii_bus */
 		if (mii_cnt && fec0_mii_bus) {
 			fep->mii_bus = fec0_mii_bus;
@@ -2015,7 +2016,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
 	mii_cnt++;
 
 	/* save fec0 mii_bus */
-	if (fep->quirks & FEC_QUIRK_ENET_MAC)
+	if (fep->quirks & FEC_QUIRK_SINGLE_MDIO)
 		fec0_mii_bus = fep->mii_bus;
 
 	return 0;
-- 
1.7.9.5

Re: [PATCH] net: fec: Fix dual ethernet issue in VFxx

[Stefan Agner]

Fwiw, this patch really touches many devices and disables the quirk for
some devices, but this is done by intent.

The quirk FEC_QUIRK_ENET_MAC was active for i.MX28, i.MX6Q, Vybrid
(mvf600-fec) and i.MX6SX. However, the new quirk is only enabled for
i.MX28. i.MX6Q doesn't need the quirk since there is one FEC instance
only anyway. Vybrid and i.MX6SX have a MDIO bus for each instance.

Acked-by: Stefan Agner <stefan@agner.ch>

On 2015-01-06 14:47, Bhuvanchandra DV wrote:
> On i.MX28, the MDIO bus is shared between the
> two RMII interfaces. However, in newer designs,
> such as Vybrid, this is not the case. This patch
> adds a quirk for the single MDIO case. This allows
> to use both FEC interfaces working independently
> on Vybird.
> 
> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> ---
>  drivers/net/ethernet/freescale/fec.h      |    3 +++
>  drivers/net/ethernet/freescale/fec_main.c |    7 ++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index 469691a..c9515bc 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -425,6 +425,9 @@ struct bufdesc_ex {
>   */
>  #define FEC_QUIRK_BUG_CAPTURE		(1 << 10)
>  
> +/* Controller has only one MDIO bus for interfacing external PHY's */
> +#define FEC_QUIRK_SINGLE_MDIO		(1 << 11)
> +
>  struct fec_enet_priv_tx_q {
>  	int index;
>  	unsigned char *tx_bounce[TX_RING_SIZE];
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 5ebdf8d..22b7748 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -91,7 +91,8 @@ static struct platform_device_id fec_devtype[] = {
>  		.driver_data = 0,
>  	}, {
>  		.name = "imx28-fec",
> -		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME,
> +		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME |
> +				FEC_QUIRK_SINGLE_MDIO,
>  	}, {
>  		.name = "imx6q-fec",
>  		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
> @@ -1952,7 +1953,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
>  	 * mdio interface in board design, and need to be configured by
>  	 * fec0 mii_bus.
>  	 */
> -	if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
> +	if ((fep->quirks & FEC_QUIRK_SINGLE_MDIO) && fep->dev_id > 0) {
>  		/* fec1 uses fec0 mii_bus */
>  		if (mii_cnt && fec0_mii_bus) {
>  			fep->mii_bus = fec0_mii_bus;
> @@ -2015,7 +2016,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
>  	mii_cnt++;
>  
>  	/* save fec0 mii_bus */
> -	if (fep->quirks & FEC_QUIRK_ENET_MAC)
> +	if (fep->quirks & FEC_QUIRK_SINGLE_MDIO)
>  		fec0_mii_bus = fep->mii_bus;
>  
>  	return 0;

RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx

[fugang.duan]

From: Stefan Agner <stefan@agner.ch> Sent: Tuesday, January 06, 2015 10:52 PM
> To: Bhuvanchandra DV
> Cc: linux-kernel@vger.kernel.org; Zhou Luwei-B45643; LW@karo-
> electronics.de; Li Frank-B20596; Duan Fugang-B38611; davem@davemloft.net
> Subject: Re: [PATCH] net: fec: Fix dual ethernet issue in VFxx
> 
> Fwiw, this patch really touches many devices and disables the quirk for
> some devices, but this is done by intent.
> 
> The quirk FEC_QUIRK_ENET_MAC was active for i.MX28, i.MX6Q, Vybrid
> (mvf600-fec) and i.MX6SX. However, the new quirk is only enabled for
> i.MX28. i.MX6Q doesn't need the quirk since there is one FEC instance
> only anyway. Vybrid and i.MX6SX have a MDIO bus for each instance.
> 
> Acked-by: Stefan Agner <stefan@agner.ch>

We cannot do it by adding a quirk.
For Vybrid and i.MX6SX and later i.MX7 serial,  there have their own MDIO bus for each MAC.
But, for board design, to save two pin (MDIO, MDC), MAC0 and MAC1 share the MDIO bus. For example, i.MX6SX sdb/sabreai/arm2 boards did like this.
So we must add one dts property to distinguish it, not a quirk. 

Thanks,
Andy
> 
> On 2015-01-06 14:47, Bhuvanchandra DV wrote:
> > On i.MX28, the MDIO bus is shared between the two RMII interfaces.
> > However, in newer designs, such as Vybrid, this is not the case. This
> > patch adds a quirk for the single MDIO case. This allows to use both
> > FEC interfaces working independently on Vybird.
> >
> > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> > ---
> >  drivers/net/ethernet/freescale/fec.h      |    3 +++
> >  drivers/net/ethernet/freescale/fec_main.c |    7 ++++---
> >  2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec.h
> > b/drivers/net/ethernet/freescale/fec.h
> > index 469691a..c9515bc 100644
> > --- a/drivers/net/ethernet/freescale/fec.h
> > +++ b/drivers/net/ethernet/freescale/fec.h
> > @@ -425,6 +425,9 @@ struct bufdesc_ex {
> >   */
> >  #define FEC_QUIRK_BUG_CAPTURE		(1 << 10)
> >
> > +/* Controller has only one MDIO bus for interfacing external PHY's */
> > +#define FEC_QUIRK_SINGLE_MDIO		(1 << 11)
> > +
> >  struct fec_enet_priv_tx_q {
> >  	int index;
> >  	unsigned char *tx_bounce[TX_RING_SIZE]; diff --git
> > a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index 5ebdf8d..22b7748 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -91,7 +91,8 @@ static struct platform_device_id fec_devtype[] = {
> >  		.driver_data = 0,
> >  	}, {
> >  		.name = "imx28-fec",
> > -		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME,
> > +		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME |
> > +				FEC_QUIRK_SINGLE_MDIO,
> >  	}, {
> >  		.name = "imx6q-fec",
> >  		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT | @@ -
> 1952,7
> > +1953,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
> >  	 * mdio interface in board design, and need to be configured by
> >  	 * fec0 mii_bus.
> >  	 */
> > -	if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
> > +	if ((fep->quirks & FEC_QUIRK_SINGLE_MDIO) && fep->dev_id > 0) {
> >  		/* fec1 uses fec0 mii_bus */
> >  		if (mii_cnt && fec0_mii_bus) {
> >  			fep->mii_bus = fec0_mii_bus;
> > @@ -2015,7 +2016,7 @@ static int fec_enet_mii_init(struct
> platform_device *pdev)
> >  	mii_cnt++;
> >
> >  	/* save fec0 mii_bus */
> > -	if (fep->quirks & FEC_QUIRK_ENET_MAC)
> > +	if (fep->quirks & FEC_QUIRK_SINGLE_MDIO)
> >  		fec0_mii_bus = fep->mii_bus;
> >
> >  	return 0;

RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx

[Stefan Agner]

On 2015-01-07 03:11, fugang.duan@freescale.com wrote:
> From: Stefan Agner <stefan@agner.ch> Sent: Tuesday, January 06, 2015 10:52 PM
>> To: Bhuvanchandra DV
>> Cc: linux-kernel@vger.kernel.org; Zhou Luwei-B45643; LW@karo-
>> electronics.de; Li Frank-B20596; Duan Fugang-B38611; davem@davemloft.net
>> Subject: Re: [PATCH] net: fec: Fix dual ethernet issue in VFxx
>>
>> Fwiw, this patch really touches many devices and disables the quirk for
>> some devices, but this is done by intent.
>>
>> The quirk FEC_QUIRK_ENET_MAC was active for i.MX28, i.MX6Q, Vybrid
>> (mvf600-fec) and i.MX6SX. However, the new quirk is only enabled for
>> i.MX28. i.MX6Q doesn't need the quirk since there is one FEC instance
>> only anyway. Vybrid and i.MX6SX have a MDIO bus for each instance.
>>
>> Acked-by: Stefan Agner <stefan@agner.ch>
> 
> We cannot do it by adding a quirk.
> For Vybrid and i.MX6SX and later i.MX7 serial,  there have their own
> MDIO bus for each MAC.
> But, for board design, to save two pin (MDIO, MDC), MAC0 and MAC1
> share the MDIO bus. For example, i.MX6SX sdb/sabreai/arm2 boards did
> like this.

Hm, so those board use a circumstance which was SoC specific back at
i.MX28 time. IMHO, "Out of luck" the shared MDIO bus is the first one
even for those boards, hence this SoC specific work around still works.
So I still think for the i.MX28 case, the quirk would be a viable
solution, but not for those boards, I agree.

> So we must add one dts property to distinguish it, not a quirk. 

Just adding a property to the FEC instance who's MDIO bus is in use
seems somewhat archaic. There is a MDIO bus description for other SoC,
do you have in mind how this should look like for fec?

--
Stefan


>>
>> On 2015-01-06 14:47, Bhuvanchandra DV wrote:
>> > On i.MX28, the MDIO bus is shared between the two RMII interfaces.
>> > However, in newer designs, such as Vybrid, this is not the case. This
>> > patch adds a quirk for the single MDIO case. This allows to use both
>> > FEC interfaces working independently on Vybird.
>> >
>> > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
>> > ---
>> >  drivers/net/ethernet/freescale/fec.h      |    3 +++
>> >  drivers/net/ethernet/freescale/fec_main.c |    7 ++++---
>> >  2 files changed, 7 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/net/ethernet/freescale/fec.h
>> > b/drivers/net/ethernet/freescale/fec.h
>> > index 469691a..c9515bc 100644
>> > --- a/drivers/net/ethernet/freescale/fec.h
>> > +++ b/drivers/net/ethernet/freescale/fec.h
>> > @@ -425,6 +425,9 @@ struct bufdesc_ex {
>> >   */
>> >  #define FEC_QUIRK_BUG_CAPTURE		(1 << 10)
>> >
>> > +/* Controller has only one MDIO bus for interfacing external PHY's */
>> > +#define FEC_QUIRK_SINGLE_MDIO		(1 << 11)
>> > +
>> >  struct fec_enet_priv_tx_q {
>> >  	int index;
>> >  	unsigned char *tx_bounce[TX_RING_SIZE]; diff --git
>> > a/drivers/net/ethernet/freescale/fec_main.c
>> > b/drivers/net/ethernet/freescale/fec_main.c
>> > index 5ebdf8d..22b7748 100644
>> > --- a/drivers/net/ethernet/freescale/fec_main.c
>> > +++ b/drivers/net/ethernet/freescale/fec_main.c
>> > @@ -91,7 +91,8 @@ static struct platform_device_id fec_devtype[] = {
>> >  		.driver_data = 0,
>> >  	}, {
>> >  		.name = "imx28-fec",
>> > -		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME,
>> > +		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME |
>> > +				FEC_QUIRK_SINGLE_MDIO,
>> >  	}, {
>> >  		.name = "imx6q-fec",
>> >  		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT | @@ -
>> 1952,7
>> > +1953,7 @@ static int fec_enet_mii_init(struct platform_device *pdev)
>> >  	 * mdio interface in board design, and need to be configured by
>> >  	 * fec0 mii_bus.
>> >  	 */
>> > -	if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
>> > +	if ((fep->quirks & FEC_QUIRK_SINGLE_MDIO) && fep->dev_id > 0) {
>> >  		/* fec1 uses fec0 mii_bus */
>> >  		if (mii_cnt && fec0_mii_bus) {
>> >  			fep->mii_bus = fec0_mii_bus;
>> > @@ -2015,7 +2016,7 @@ static int fec_enet_mii_init(struct
>> platform_device *pdev)
>> >  	mii_cnt++;
>> >
>> >  	/* save fec0 mii_bus */
>> > -	if (fep->quirks & FEC_QUIRK_ENET_MAC)
>> > +	if (fep->quirks & FEC_QUIRK_SINGLE_MDIO)
>> >  		fec0_mii_bus = fep->mii_bus;
>> >
>> >  	return 0;

RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx

[fugang.duan]

From: Stefan Agner <stefan@agner.ch> Sent: Wednesday, January 07, 2015 6:30 PM
> To: Duan Fugang-B38611
> Cc: Bhuvanchandra DV; linux-kernel@vger.kernel.org; Zhou Luwei-B45643;
> LW@karo-electronics.de; Li Frank-B20596; davem@davemloft.net
> Subject: RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx
> 
> On 2015-01-07 03:11, fugang.duan@freescale.com wrote:
> > From: Stefan Agner <stefan@agner.ch> Sent: Tuesday, January 06, 2015
> > 10:52 PM
> >> To: Bhuvanchandra DV
> >> Cc: linux-kernel@vger.kernel.org; Zhou Luwei-B45643; LW@karo-
> >> electronics.de; Li Frank-B20596; Duan Fugang-B38611;
> >> davem@davemloft.net
> >> Subject: Re: [PATCH] net: fec: Fix dual ethernet issue in VFxx
> >>
> >> Fwiw, this patch really touches many devices and disables the quirk
> >> for some devices, but this is done by intent.
> >>
> >> The quirk FEC_QUIRK_ENET_MAC was active for i.MX28, i.MX6Q, Vybrid
> >> (mvf600-fec) and i.MX6SX. However, the new quirk is only enabled for
> >> i.MX28. i.MX6Q doesn't need the quirk since there is one FEC instance
> >> only anyway. Vybrid and i.MX6SX have a MDIO bus for each instance.
> >>
> >> Acked-by: Stefan Agner <stefan@agner.ch>
> >
> > We cannot do it by adding a quirk.
> > For Vybrid and i.MX6SX and later i.MX7 serial,  there have their own
> > MDIO bus for each MAC.
> > But, for board design, to save two pin (MDIO, MDC), MAC0 and MAC1
> > share the MDIO bus. For example, i.MX6SX sdb/sabreai/arm2 boards did
> > like this.
> 
> Hm, so those board use a circumstance which was SoC specific back at
> i.MX28 time. IMHO, "Out of luck" the shared MDIO bus is the first one
> even for those boards, hence this SoC specific work around still works.
> So I still think for the i.MX28 case, the quirk would be a viable
> solution, but not for those boards, I agree.
> 
> > So we must add one dts property to distinguish it, not a quirk.
> 
> Just adding a property to the FEC instance who's MDIO bus is in use seems
> somewhat archaic. There is a MDIO bus description for other SoC, do you
> have in mind how this should look like for fec?
> 
> --
> Stefan

In general,  MAC2 use MAC1 MDIO bus. Because MAC1 is registered firstly, then register MAC2.
We can invent one property like "shared-mii-bus" for MAC1 to tell driver there have other MACs use itself mii bus.
Of course, the property needs to add for MAC2 device node to tell driver it will share the MAC1 mii bus.

Whether add "shared-mii-bus" property depends on customer boards design.
The problem is related to boards design, not related to SOC itself, so I think add property is feasible. 

I don't know whether there have better solution for this.

Regards,
Andy

RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx

[Stefan Agner]

On 2015-01-08 04:33, fugang.duan@freescale.com wrote:
> From: Stefan Agner <stefan@agner.ch> Sent: Wednesday, January 07, 2015 6:30 PM
>> To: Duan Fugang-B38611
>> Cc: Bhuvanchandra DV; linux-kernel@vger.kernel.org; Zhou Luwei-B45643;
>> LW@karo-electronics.de; Li Frank-B20596; davem@davemloft.net
>> Subject: RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx
>>
>> On 2015-01-07 03:11, fugang.duan@freescale.com wrote:
>> > From: Stefan Agner <stefan@agner.ch> Sent: Tuesday, January 06, 2015
>> > 10:52 PM
>> >> To: Bhuvanchandra DV
>> >> Cc: linux-kernel@vger.kernel.org; Zhou Luwei-B45643; LW@karo-
>> >> electronics.de; Li Frank-B20596; Duan Fugang-B38611;
>> >> davem@davemloft.net
>> >> Subject: Re: [PATCH] net: fec: Fix dual ethernet issue in VFxx
>> >>
>> >> Fwiw, this patch really touches many devices and disables the quirk
>> >> for some devices, but this is done by intent.
>> >>
>> >> The quirk FEC_QUIRK_ENET_MAC was active for i.MX28, i.MX6Q, Vybrid
>> >> (mvf600-fec) and i.MX6SX. However, the new quirk is only enabled for
>> >> i.MX28. i.MX6Q doesn't need the quirk since there is one FEC instance
>> >> only anyway. Vybrid and i.MX6SX have a MDIO bus for each instance.
>> >>
>> >> Acked-by: Stefan Agner <stefan@agner.ch>
>> >
>> > We cannot do it by adding a quirk.
>> > For Vybrid and i.MX6SX and later i.MX7 serial,  there have their own
>> > MDIO bus for each MAC.
>> > But, for board design, to save two pin (MDIO, MDC), MAC0 and MAC1
>> > share the MDIO bus. For example, i.MX6SX sdb/sabreai/arm2 boards did
>> > like this.
>>
>> Hm, so those board use a circumstance which was SoC specific back at
>> i.MX28 time. IMHO, "Out of luck" the shared MDIO bus is the first one
>> even for those boards, hence this SoC specific work around still works.
>> So I still think for the i.MX28 case, the quirk would be a viable
>> solution, but not for those boards, I agree.
>>
>> > So we must add one dts property to distinguish it, not a quirk.
>>
>> Just adding a property to the FEC instance who's MDIO bus is in use seems
>> somewhat archaic. There is a MDIO bus description for other SoC, do you
>> have in mind how this should look like for fec?
>>
>> --
>> Stefan

(added Uwe to CC since he added the device tree support for MDIO bus)

> In general,  MAC2 use MAC1 MDIO bus. Because MAC1 is registered
> firstly, then register MAC2.
> We can invent one property like "shared-mii-bus" for MAC1 to tell
> driver there have other MACs use itself mii bus.
> Of course, the property needs to add for MAC2 device node to tell
> driver it will share the MAC1 mii bus.

There is actually already lot of device tree support for MDIO bus
description in place (see of_mdiobus_register call in the fec_main
driver), just the device tree's do not make a lot use of it currently.

The vf610-twr.dts has both FEC enabled as well as the quirk is enabled
in the driver for Vybrid. I tested the Tower System Module TWR-SER2 with
Vybrid. This module has two ethernet PHY's, but _both_ are connected to
the first RMII's MDIO bus (FEC0). Thanks to the quirk, both ports work
fine.

As a side note: Don't be fooled that there are both MDIO buses available
on the elevator: The module makes use of the MDIO of the first RMII
signals only, see TWR-SER2 schematic.

I then disabled the quirk and altered the device tree (patch courtesy of
Bhuvan):

diff --git a/arch/arm/boot/dts/vf610-twr.dts
b/arch/arm/boot/dts/vf610-twr.dts
index a0f7621..876c80a 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -129,13 +129,28 @@
 
 &fec0 {
        phy-mode = "rmii";
+       phy-handle = <&ethphy0>;
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_fec0>;
        status = "okay";
+
+       fec0mdio: mdio {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               ethphy0: ethernet-phy@0 {
+                       reg = <0>;
+               };
+
+               ethphy1: ethernet-phy@1 {
+                       reg = <1>;
+               };
+       };
 };
 
 &fec1 {
        phy-mode = "rmii";
+       phy-handle = <&ethphy1>;
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_fec1>;
        status = "okay";

This worked also fine! Hence we actually already have all device tree
support needed. For the case we try to solve (using the MDIO bus of each
RMII), a device tree like this should be sufficient:

&fec0 {
	phy-mode = "rmii";
	phy-handle = <&ethphy0>;
...

	fec0mdio: mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		ethphy0: ethernet-phy@0 {
			reg = <0>;
		};
	};
};

&fec1 {
...
	phy-handle = <&ethphy1>;
...

	fec1mdio: mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		ethphy1: ethernet-phy@1 {
			reg = <1>;
		};
	};
};

Unfortunately, the quirk FEC_QUIRK_ENET_MAC leads to fec_enet_mii_init
return early for the _second_ FEC. Even if the PHY's would be properly
described in dt, parsing is prevented by that quirk.

Hence my suggestion is this:
- Implement the FEC_QUIRK_SINGLE_MDIO and add it to as in this patch,
which makes the quirk always work for i.MX28 even without dt changes...
(backward compatible)
- Fix the Tower device tree using the patch above to make the Tower work
again
- i.MX6SX is similar to Vybrid in that regard, adding a patch similar to
the one above should make the SX boards work too
- i.MX6S/D/Q is not affected since only one FEC instance is available
- i.MX7 is not yet merged, so a proper device tree description can be
used upfront

This would break old device trees with newer kernels for i.MX6SX and
Vybrid.. The first is quite new, and the latter is not very widespread,
IMHO this would be acceptable. Also, the device tree just did not
describe the hardware completely. "Out of a luck" (just because the MDIO
quirk for the i.MX28 SoC is active for all SoC's)..., it currently
works...

Alternatively, we can add the FEC_QUIRK_SINGLE_MDIO quirk to driver_data
of imx6sx-fec and mvf600-fec. Then, check early if a MDIO bus
description is available. This would keep the current device tree's
working while having support to describe other boards in device tree's
in the future (something along these lines):

diff --git a/drivers/net/ethernet/freescale/fec_main.c
b/drivers/net/ethernet/freescale/fec_main.c
index 5ebdf8d..527eb3e 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1952,7 +1952,8 @@ static int fec_enet_mii_init(struct
platform_device *pdev)
         * mdio interface in board design, and need to be configured by
         * fec0 mii_bus.
         */
-       if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
+       node = of_get_child_by_name(pdev->dev.of_node, "mdio");
+       if (!node && fep->quirks & FEC_QUIRK_SINGLE_MDIO && fep->dev_id
> 0) {
                /* fec1 uses fec0 mii_bus */
                if (mii_cnt && fec0_mii_bus) {
                        fep->mii_bus = fec0_mii_bus;
@@ -2001,7 +2002,6 @@ static int fec_enet_mii_init(struct
platform_device *pdev)
        for (i = 0; i < PHY_MAX_ADDR; i++)
                fep->mii_bus->irq[i] = PHY_POLL;
 
-       node = of_get_child_by_name(pdev->dev.of_node, "mdio");
        if (node) {
                err = of_mdiobus_register(fep->mii_bus, node);
                of_node_put(node);


However, IMHO this would add a quirk to SoC's which actually are not
affected... Hence I would prefer to not add that quirk on those SoC's
and just break the (uncomplete) device tree's....

--
Stefan

RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx

[fugang.duan]

From: Stefan Agner <stefan@agner.ch> Sent: Friday, January 09, 2015 2:59 AM
> To: Duan Fugang-B38611
> Cc: Bhuvanchandra DV; linux-kernel@vger.kernel.org; Zhou Luwei-B45643;
> LW@karo-electronics.de; Li Frank-B20596; davem@davemloft.net; u.kleine-
> koenig@pengutronix.de; shawn.guo@linaro.org
> Subject: RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx
> 
> On 2015-01-08 04:33, fugang.duan@freescale.com wrote:
> > From: Stefan Agner <stefan@agner.ch> Sent: Wednesday, January 07, 2015
> > 6:30 PM
> >> To: Duan Fugang-B38611
> >> Cc: Bhuvanchandra DV; linux-kernel@vger.kernel.org; Zhou
> >> Luwei-B45643; LW@karo-electronics.de; Li Frank-B20596;
> >> davem@davemloft.net
> >> Subject: RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx
> >>
> >> On 2015-01-07 03:11, fugang.duan@freescale.com wrote:
> >> > From: Stefan Agner <stefan@agner.ch> Sent: Tuesday, January 06,
> >> > 2015
> >> > 10:52 PM
> >> >> To: Bhuvanchandra DV
> >> >> Cc: linux-kernel@vger.kernel.org; Zhou Luwei-B45643; LW@karo-
> >> >> electronics.de; Li Frank-B20596; Duan Fugang-B38611;
> >> >> davem@davemloft.net
> >> >> Subject: Re: [PATCH] net: fec: Fix dual ethernet issue in VFxx
> >> >>
> >> >> Fwiw, this patch really touches many devices and disables the
> >> >> quirk for some devices, but this is done by intent.
> >> >>
> >> >> The quirk FEC_QUIRK_ENET_MAC was active for i.MX28, i.MX6Q, Vybrid
> >> >> (mvf600-fec) and i.MX6SX. However, the new quirk is only enabled
> >> >> for i.MX28. i.MX6Q doesn't need the quirk since there is one FEC
> >> >> instance only anyway. Vybrid and i.MX6SX have a MDIO bus for each
> instance.
> >> >>
> >> >> Acked-by: Stefan Agner <stefan@agner.ch>
> >> >
> >> > We cannot do it by adding a quirk.
> >> > For Vybrid and i.MX6SX and later i.MX7 serial,  there have their
> >> > own MDIO bus for each MAC.
> >> > But, for board design, to save two pin (MDIO, MDC), MAC0 and MAC1
> >> > share the MDIO bus. For example, i.MX6SX sdb/sabreai/arm2 boards
> >> > did like this.
> >>
> >> Hm, so those board use a circumstance which was SoC specific back at
> >> i.MX28 time. IMHO, "Out of luck" the shared MDIO bus is the first one
> >> even for those boards, hence this SoC specific work around still works.
> >> So I still think for the i.MX28 case, the quirk would be a viable
> >> solution, but not for those boards, I agree.
> >>
> >> > So we must add one dts property to distinguish it, not a quirk.
> >>
> >> Just adding a property to the FEC instance who's MDIO bus is in use
> >> seems somewhat archaic. There is a MDIO bus description for other
> >> SoC, do you have in mind how this should look like for fec?
> >>
> >> --
> >> Stefan
> 
> (added Uwe to CC since he added the device tree support for MDIO bus)
> 
> > In general,  MAC2 use MAC1 MDIO bus. Because MAC1 is registered
> > firstly, then register MAC2.
> > We can invent one property like "shared-mii-bus" for MAC1 to tell
> > driver there have other MACs use itself mii bus.
> > Of course, the property needs to add for MAC2 device node to tell
> > driver it will share the MAC1 mii bus.
> 
> There is actually already lot of device tree support for MDIO bus
> description in place (see of_mdiobus_register call in the fec_main
> driver), just the device tree's do not make a lot use of it currently.
> 
> The vf610-twr.dts has both FEC enabled as well as the quirk is enabled in
> the driver for Vybrid. I tested the Tower System Module TWR-SER2 with
> Vybrid. This module has two ethernet PHY's, but _both_ are connected to
> the first RMII's MDIO bus (FEC0). Thanks to the quirk, both ports work
> fine.
> 
> As a side note: Don't be fooled that there are both MDIO buses available
> on the elevator: The module makes use of the MDIO of the first RMII
> signals only, see TWR-SER2 schematic.
> 
> I then disabled the quirk and altered the device tree (patch courtesy of
> Bhuvan):
> 
> diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-
> twr.dts index a0f7621..876c80a 100644
> --- a/arch/arm/boot/dts/vf610-twr.dts
> +++ b/arch/arm/boot/dts/vf610-twr.dts
> @@ -129,13 +129,28 @@
> 
>  &fec0 {
>         phy-mode = "rmii";
> +       phy-handle = <&ethphy0>;
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_fec0>;
>         status = "okay";
> +
> +       fec0mdio: mdio {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               ethphy0: ethernet-phy@0 {
> +                       reg = <0>;
> +               };
> +
> +               ethphy1: ethernet-phy@1 {
> +                       reg = <1>;
> +               };
> +       };
>  };
> 
>  &fec1 {
>         phy-mode = "rmii";
> +       phy-handle = <&ethphy1>;
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_fec1>;
>         status = "okay";
> 
> This worked also fine! Hence we actually already have all device tree
> support needed. For the case we try to solve (using the MDIO bus of each
> RMII), a device tree like this should be sufficient:
> 
> &fec0 {
> 	phy-mode = "rmii";
> 	phy-handle = <&ethphy0>;
> ...
> 
> 	fec0mdio: mdio {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		ethphy0: ethernet-phy@0 {
> 			reg = <0>;
> 		};
> 	};
> };
> 
> &fec1 {
> ...
> 	phy-handle = <&ethphy1>;
> ...
> 
> 	fec1mdio: mdio {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		ethphy1: ethernet-phy@1 {
> 			reg = <1>;
> 		};
> 	};
> };
> 
> Unfortunately, the quirk FEC_QUIRK_ENET_MAC leads to fec_enet_mii_init
> return early for the _second_ FEC. Even if the PHY's would be properly
> described in dt, parsing is prevented by that quirk.
> 
> Hence my suggestion is this:
> - Implement the FEC_QUIRK_SINGLE_MDIO and add it to as in this patch,
> which makes the quirk always work for i.MX28 even without dt changes...
> (backward compatible)
> - Fix the Tower device tree using the patch above to make the Tower work
> again
> - i.MX6SX is similar to Vybrid in that regard, adding a patch similar to
> the one above should make the SX boards work too
> - i.MX6S/D/Q is not affected since only one FEC instance is available
> - i.MX7 is not yet merged, so a proper device tree description can be
> used upfront
> 
> This would break old device trees with newer kernels for i.MX6SX and
> Vybrid.. The first is quite new, and the latter is not very widespread,
> IMHO this would be acceptable. Also, the device tree just did not
> describe the hardware completely. "Out of a luck" (just because the MDIO
> quirk for the i.MX28 SoC is active for all SoC's)..., it currently
> works...
> 
> Alternatively, we can add the FEC_QUIRK_SINGLE_MDIO quirk to driver_data
> of imx6sx-fec and mvf600-fec. Then, check early if a MDIO bus description
> is available. This would keep the current device tree's working while
> having support to describe other boards in device tree's in the future
> (something along these lines):
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 5ebdf8d..527eb3e 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1952,7 +1952,8 @@ static int fec_enet_mii_init(struct platform_device
> *pdev)
>          * mdio interface in board design, and need to be configured by
>          * fec0 mii_bus.
>          */
> -       if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
> +       node = of_get_child_by_name(pdev->dev.of_node, "mdio");
> +       if (!node && fep->quirks & FEC_QUIRK_SINGLE_MDIO && fep->dev_id
> > 0) {
>                 /* fec1 uses fec0 mii_bus */
>                 if (mii_cnt && fec0_mii_bus) {
>                         fep->mii_bus = fec0_mii_bus; @@ -2001,7 +2002,6
> @@ static int fec_enet_mii_init(struct platform_device *pdev)
>         for (i = 0; i < PHY_MAX_ADDR; i++)
>                 fep->mii_bus->irq[i] = PHY_POLL;
> 
> -       node = of_get_child_by_name(pdev->dev.of_node, "mdio");
>         if (node) {
>                 err = of_mdiobus_register(fep->mii_bus, node);
>                 of_node_put(node);
> 
> 
> However, IMHO this would add a quirk to SoC's which actually are not
> affected... Hence I would prefer to not add that quirk on those SoC's and
> just break the (uncomplete) device tree's....
> 

I suggest using one patch to fix all SOCs and boards issue. Don't leave the issue for other boards.

i.MX serial with two MACs always have two MDIO bus, whether sharing MDIO bus only depend on boards design.
For example, i.MX6SX silicon, freescale reference boards use sharing MDIO bus, maybe customer's boards use independent MDIO bus (two MII bus).

There have four combination for i.MX28, i.MX6SX, Vybird, and the i.MX7....
1. MAC1 share MAC0 mii bus,  and no "phy_node" property and no "mdio" sub-node in dts. [legacy]
2. MAC1 share MAC0 mii bus,  and have "phy_node" property in both MAC node,  and only MAC0 node has "mdio" sub-node in dts.
3. MAC0 and MAC1 use independent MII bus,  and no "phy_node" property and no "mdio" sub-node in dts. [legacy]
4. MAC0 and MAC1 use independent MII bus,   and have "phy_node" property in both MAC node,  and only MAC0 node has "mdio" sub-node in dts.

For all these cases, your above patches cannot handle these.
So I still insist to add another property for this to avoid the existing dts files change.

Regards,
Andy

RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx

[Stefan Agner]

On 2015-01-09 04:00, fugang.duan@freescale.com wrote:
> From: Stefan Agner <stefan@agner.ch> Sent: Friday, January 09, 2015 2:59 AM
>> To: Duan Fugang-B38611
>> Cc: Bhuvanchandra DV; linux-kernel@vger.kernel.org; Zhou Luwei-B45643;
>> LW@karo-electronics.de; Li Frank-B20596; davem@davemloft.net; u.kleine-
>> koenig@pengutronix.de; shawn.guo@linaro.org
>> Subject: RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx
>>
>> On 2015-01-08 04:33, fugang.duan@freescale.com wrote:
>> > From: Stefan Agner <stefan@agner.ch> Sent: Wednesday, January 07, 2015
>> > 6:30 PM
>> >> To: Duan Fugang-B38611
>> >> Cc: Bhuvanchandra DV; linux-kernel@vger.kernel.org; Zhou
>> >> Luwei-B45643; LW@karo-electronics.de; Li Frank-B20596;
>> >> davem@davemloft.net
>> >> Subject: RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx
>> >>
>> >> On 2015-01-07 03:11, fugang.duan@freescale.com wrote:
>> >> > From: Stefan Agner <stefan@agner.ch> Sent: Tuesday, January 06,
>> >> > 2015
>> >> > 10:52 PM
>> >> >> To: Bhuvanchandra DV
>> >> >> Cc: linux-kernel@vger.kernel.org; Zhou Luwei-B45643; LW@karo-
>> >> >> electronics.de; Li Frank-B20596; Duan Fugang-B38611;
>> >> >> davem@davemloft.net
>> >> >> Subject: Re: [PATCH] net: fec: Fix dual ethernet issue in VFxx
>> >> >>
>> >> >> Fwiw, this patch really touches many devices and disables the
>> >> >> quirk for some devices, but this is done by intent.
>> >> >>
>> >> >> The quirk FEC_QUIRK_ENET_MAC was active for i.MX28, i.MX6Q, Vybrid
>> >> >> (mvf600-fec) and i.MX6SX. However, the new quirk is only enabled
>> >> >> for i.MX28. i.MX6Q doesn't need the quirk since there is one FEC
>> >> >> instance only anyway. Vybrid and i.MX6SX have a MDIO bus for each
>> instance.
>> >> >>
>> >> >> Acked-by: Stefan Agner <stefan@agner.ch>
>> >> >
>> >> > We cannot do it by adding a quirk.
>> >> > For Vybrid and i.MX6SX and later i.MX7 serial,  there have their
>> >> > own MDIO bus for each MAC.
>> >> > But, for board design, to save two pin (MDIO, MDC), MAC0 and MAC1
>> >> > share the MDIO bus. For example, i.MX6SX sdb/sabreai/arm2 boards
>> >> > did like this.
>> >>
>> >> Hm, so those board use a circumstance which was SoC specific back at
>> >> i.MX28 time. IMHO, "Out of luck" the shared MDIO bus is the first one
>> >> even for those boards, hence this SoC specific work around still works.
>> >> So I still think for the i.MX28 case, the quirk would be a viable
>> >> solution, but not for those boards, I agree.
>> >>
>> >> > So we must add one dts property to distinguish it, not a quirk.
>> >>
>> >> Just adding a property to the FEC instance who's MDIO bus is in use
>> >> seems somewhat archaic. There is a MDIO bus description for other
>> >> SoC, do you have in mind how this should look like for fec?
>> >>
>> >> --
>> >> Stefan
>>
>> (added Uwe to CC since he added the device tree support for MDIO bus)
>>
>> > In general,  MAC2 use MAC1 MDIO bus. Because MAC1 is registered
>> > firstly, then register MAC2.
>> > We can invent one property like "shared-mii-bus" for MAC1 to tell
>> > driver there have other MACs use itself mii bus.
>> > Of course, the property needs to add for MAC2 device node to tell
>> > driver it will share the MAC1 mii bus.
>>
>> There is actually already lot of device tree support for MDIO bus
>> description in place (see of_mdiobus_register call in the fec_main
>> driver), just the device tree's do not make a lot use of it currently.
>>
>> The vf610-twr.dts has both FEC enabled as well as the quirk is enabled in
>> the driver for Vybrid. I tested the Tower System Module TWR-SER2 with
>> Vybrid. This module has two ethernet PHY's, but _both_ are connected to
>> the first RMII's MDIO bus (FEC0). Thanks to the quirk, both ports work
>> fine.
>>
>> As a side note: Don't be fooled that there are both MDIO buses available
>> on the elevator: The module makes use of the MDIO of the first RMII
>> signals only, see TWR-SER2 schematic.
>>
>> I then disabled the quirk and altered the device tree (patch courtesy of
>> Bhuvan):
>>
>> diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-
>> twr.dts index a0f7621..876c80a 100644
>> --- a/arch/arm/boot/dts/vf610-twr.dts
>> +++ b/arch/arm/boot/dts/vf610-twr.dts
>> @@ -129,13 +129,28 @@
>>
>>  &fec0 {
>>         phy-mode = "rmii";
>> +       phy-handle = <&ethphy0>;
>>         pinctrl-names = "default";
>>         pinctrl-0 = <&pinctrl_fec0>;
>>         status = "okay";
>> +
>> +       fec0mdio: mdio {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               ethphy0: ethernet-phy@0 {
>> +                       reg = <0>;
>> +               };
>> +
>> +               ethphy1: ethernet-phy@1 {
>> +                       reg = <1>;
>> +               };
>> +       };
>>  };
>>
>>  &fec1 {
>>         phy-mode = "rmii";
>> +       phy-handle = <&ethphy1>;
>>         pinctrl-names = "default";
>>         pinctrl-0 = <&pinctrl_fec1>;
>>         status = "okay";
>>
>> This worked also fine! Hence we actually already have all device tree
>> support needed. For the case we try to solve (using the MDIO bus of each
>> RMII), a device tree like this should be sufficient:
>>
>> &fec0 {
>> 	phy-mode = "rmii";
>> 	phy-handle = <&ethphy0>;
>> ...
>>
>> 	fec0mdio: mdio {
>> 		#address-cells = <1>;
>> 		#size-cells = <0>;
>>
>> 		ethphy0: ethernet-phy@0 {
>> 			reg = <0>;
>> 		};
>> 	};
>> };
>>
>> &fec1 {
>> ...
>> 	phy-handle = <&ethphy1>;
>> ...
>>
>> 	fec1mdio: mdio {
>> 		#address-cells = <1>;
>> 		#size-cells = <0>;
>>
>> 		ethphy1: ethernet-phy@1 {
>> 			reg = <1>;
>> 		};
>> 	};
>> };
>>
>> Unfortunately, the quirk FEC_QUIRK_ENET_MAC leads to fec_enet_mii_init
>> return early for the _second_ FEC. Even if the PHY's would be properly
>> described in dt, parsing is prevented by that quirk.
>>
>> Hence my suggestion is this:
>> - Implement the FEC_QUIRK_SINGLE_MDIO and add it to as in this patch,
>> which makes the quirk always work for i.MX28 even without dt changes...
>> (backward compatible)
>> - Fix the Tower device tree using the patch above to make the Tower work
>> again
>> - i.MX6SX is similar to Vybrid in that regard, adding a patch similar to
>> the one above should make the SX boards work too
>> - i.MX6S/D/Q is not affected since only one FEC instance is available
>> - i.MX7 is not yet merged, so a proper device tree description can be
>> used upfront
>>
>> This would break old device trees with newer kernels for i.MX6SX and
>> Vybrid.. The first is quite new, and the latter is not very widespread,
>> IMHO this would be acceptable. Also, the device tree just did not
>> describe the hardware completely. "Out of a luck" (just because the MDIO
>> quirk for the i.MX28 SoC is active for all SoC's)..., it currently
>> works...
>>
>> Alternatively, we can add the FEC_QUIRK_SINGLE_MDIO quirk to driver_data
>> of imx6sx-fec and mvf600-fec. Then, check early if a MDIO bus description
>> is available. This would keep the current device tree's working while
>> having support to describe other boards in device tree's in the future
>> (something along these lines):
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>> b/drivers/net/ethernet/freescale/fec_main.c
>> index 5ebdf8d..527eb3e 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -1952,7 +1952,8 @@ static int fec_enet_mii_init(struct platform_device
>> *pdev)
>>          * mdio interface in board design, and need to be configured by
>>          * fec0 mii_bus.
>>          */
>> -       if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) {
>> +       node = of_get_child_by_name(pdev->dev.of_node, "mdio");
>> +       if (!node && fep->quirks & FEC_QUIRK_SINGLE_MDIO && fep->dev_id
>> > 0) {
>>                 /* fec1 uses fec0 mii_bus */
>>                 if (mii_cnt && fec0_mii_bus) {
>>                         fep->mii_bus = fec0_mii_bus; @@ -2001,7 +2002,6
>> @@ static int fec_enet_mii_init(struct platform_device *pdev)
>>         for (i = 0; i < PHY_MAX_ADDR; i++)
>>                 fep->mii_bus->irq[i] = PHY_POLL;
>>
>> -       node = of_get_child_by_name(pdev->dev.of_node, "mdio");
>>         if (node) {
>>                 err = of_mdiobus_register(fep->mii_bus, node);
>>                 of_node_put(node);
>>
>>
>> However, IMHO this would add a quirk to SoC's which actually are not
>> affected... Hence I would prefer to not add that quirk on those SoC's and
>> just break the (uncomplete) device tree's....
>>
> 
> I suggest using one patch to fix all SOCs and boards issue. Don't
> leave the issue for other boards.

I agree. I never suggested to leave the issue for some boards.

> 
> i.MX serial with two MACs always have two MDIO bus, whether sharing
> MDIO bus only depend on boards design.
> For example, i.MX6SX silicon, freescale reference boards use sharing
> MDIO bus, maybe customer's boards use independent MDIO bus (two MII
> bus).


Yes, I have understood that.

> 
> There have four combination for i.MX28, i.MX6SX, Vybird, and the i.MX7....
> 1. MAC1 share MAC0 mii bus,  and no "phy_node" property and no "mdio"
> sub-node in dts. [legacy]
> 2. MAC1 share MAC0 mii bus,  and have "phy_node" property in both MAC
> node,  and only MAC0 node has "mdio" sub-node in dts.
> 3. MAC0 and MAC1 use independent MII bus,  and no "phy_node" property
> and no "mdio" sub-node in dts. [legacy]
> 4. MAC0 and MAC1 use independent MII bus,   and have "phy_node"
> property in both MAC node,  and only MAC0 node has "mdio" sub-node in
> dts.

Currently, MAC1 is broken in case 3 on Vybrid (as well as i.MX6XS and
i.MX7, because the FEC_QUIRK_ENET_MAC is enabled for all of them).

> For all these cases, your above patches cannot handle these.

Yes, in it's current state, I agree. A new revision is required.

> So I still insist to add another property for this to avoid the
> existing dts files change.

But I don't think adding another device tree property is helping here.
There is already a device tree standard how to describe the MDIO buses,
why not just make use of them?

I will create a v2 patchset, so we have a new base to discuss this
further.

--
Stefan

RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx

[fugang.duan]

From: Stefan Agner <stefan@agner.ch> Sent: Friday, January 09, 2015 4:22 PM
> To: Duan Fugang-B38611
> Cc: Bhuvanchandra DV; linux-kernel@vger.kernel.org; Zhou Luwei-B45643;
> LW@karo-electronics.de; Li Frank-B20596; davem@davemloft.net; u.kleine-
> koenig@pengutronix.de; shawn.guo@linaro.org
> Subject: RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx
> 
> On 2015-01-09 04:00, fugang.duan@freescale.com wrote:
> > From: Stefan Agner <stefan@agner.ch> Sent: Friday, January 09, 2015
> > 2:59 AM
> >> To: Duan Fugang-B38611
> >> Cc: Bhuvanchandra DV; linux-kernel@vger.kernel.org; Zhou
> >> Luwei-B45643; LW@karo-electronics.de; Li Frank-B20596;
> >> davem@davemloft.net; u.kleine- koenig@pengutronix.de;
> >> shawn.guo@linaro.org
> >> Subject: RE: [PATCH] net: fec: Fix dual ethernet issue in VFxx
> >>
[snip]
> 
> Currently, MAC1 is broken in case 3 on Vybrid (as well as i.MX6XS and
> i.MX7, because the FEC_QUIRK_ENET_MAC is enabled for all of them).
> 
> > For all these cases, your above patches cannot handle these.
> 
> Yes, in it's current state, I agree. A new revision is required.
> 
> > So I still insist to add another property for this to avoid the
> > existing dts files change.
> 
> But I don't think adding another device tree property is helping here.
> There is already a device tree standard how to describe the MDIO buses,
> why not just make use of them?
> 
> I will create a v2 patchset, so we have a new base to discuss this
> further.
> 
Don't invent new property, and compatible with above four cases (avoid existing dts changing), it is difficult.
Looking forward to your next patchset.

Regards,
Andy