From: Stefan Agner <stefan@xxxxxxxx> Sent: Friday, January 09, 2015 10:02 PM > To: davem@xxxxxxxxxxxxx > Cc: shawn.guo@xxxxxxxxxx; u.kleine-koenig@xxxxxxxxxxxxxx; Duan Fugang- > B38611; mark.rutland@xxxxxxx; robh+dt@xxxxxxxxxx; pawel.moll@xxxxxxx; > ijc+devicetree@xxxxxxxxxxxxxx; galak@xxxxxxxxxxxxxx; Duan Fugang-B38611; > LW@xxxxxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; stefan@xxxxxxxx; > Bhuvanchandra DV > Subject: [PATCH v2] net: fec: fix MDIO bus assignement for dual fec SoC's > > On i.MX28, the MDIO bus is shared between the two FEC instances. > The driver makes sure that the second FEC uses the MDIO bus of the first > FEC. This is done conditionally if FEC_QUIRK_ENET_MAC is set. > However, in newer designs, such as Vybrid or i.MX6SX, each FEC MAC has > its own MDIO bus. Simply removing the quirk FEC_QUIRK_ENET_MAC is not an > option since other logic, triggered by this quirk, is still needed. > > Furthermore, there are board designs which use the same MDIO bus for both > PHY's even though the second bus would be available on the SoC side. Such > layout are popular since it saves pins on SoC side. > Due to the above quirk, those boards currently do work fine. The boards > in the mainline tree with such a layout are: > - Freescale Vybrid Tower with TWR-SER2 (vf610-twr.dts) > - Freescale i.MX6 SoloX SDB Board (imx6sx-sdb.dts) > > This patch adds a new quirk FEC_QUIRK_SINGLE_MDIO for i.MX28, which makes > sure that the MDIO bus of the first FEC is used in any case. > > However, the boards above do have a SoC with a MDIO bus for each FEC > instance. But the PHY's are not connected in a 1:1 configuration. A > proper device tree description is needed to allow the driver to figure > out where to find its PHY. This patch fixes that shortcoming by adding a > MDIO bus child node to the first FEC instance, along with the two PHY's > on that bus, and making use of the phy-handle property to add a reference > to the PHY's. > > Signed-off-by: Stefan Agner <stefan@xxxxxxxx> > Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@xxxxxxxxxxx> > --- > Yes, this breaks existing device trees, but it does this because those > device trees were lacking proper description of the HW... > IMHO, in this case, this is acceptable. We also do this in other cases, > e.g. in the gic_arch_extn killing patchset: > http://archive.arm.linux.org.uk/lurker/message/20150107.174235.3fc3a92f.e > n.html > > Also, the two boards we are breaking are not very widespread: > The Vybrid Tower is generally not very widespread and there is only the > TWR-VF65GS10-PRO variant with TWR-SER2 affected. And the SoloX SDB board > is not even generally available yet... > > If we don't want to break the board, we could add the > FEC_QUIRK_SINGLE_MDIO to Vybrid or/and i.MX6SX too. But then, we need to > make the quirk conditional: If a MDIO node or phy- handle is specified, > we should rely on the device tree information. > However, this solution adds new code and complexity. Using the quirk for > those SoC's just feels wrong. So I strongly advocate for the breaking > variant. > > Changes since v1 (https://lkml.org/lkml/2015/1/6/284): > - Add MDIO/phy-handle device tree nodes for dual FEC boards > > arch/arm/boot/dts/imx6sx-sdb.dts | 15 +++++++++++++++ > arch/arm/boot/dts/vf610-twr.dts | 15 +++++++++++++++ > drivers/net/ethernet/freescale/fec.h | 2 ++ > drivers/net/ethernet/freescale/fec_main.c | 9 +++++---- > 4 files changed, 37 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx- > sdb.dts > index 1e6e5cc..8c1febd 100644 > --- a/arch/arm/boot/dts/imx6sx-sdb.dts > +++ b/arch/arm/boot/dts/imx6sx-sdb.dts > @@ -159,13 +159,28 @@ > pinctrl-0 = <&pinctrl_enet1>; > phy-supply = <®_enet_3v3>; > phy-mode = "rgmii"; > + phy-handle = <ðphy1>; > status = "okay"; > + > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethphy1: ethernet-phy@0 { > + reg = <0>; > + }; > + > + ethphy2: ethernet-phy@1 { > + reg = <1>; > + }; > + }; > }; > > &fec2 { > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_enet2>; > phy-mode = "rgmii"; > + phy-handle = <ðphy2>; > status = "okay"; > }; > > diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610- > twr.dts index a0f7621..f2b64b1 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 = <ðphy0>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_fec0>; > status = "okay"; > + > + 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 = <ðphy1>; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_fec1>; > status = "okay"; > diff --git a/drivers/net/ethernet/freescale/fec.h > b/drivers/net/ethernet/freescale/fec.h > index 469691a..4013292 100644 > --- a/drivers/net/ethernet/freescale/fec.h > +++ b/drivers/net/ethernet/freescale/fec.h > @@ -424,6 +424,8 @@ struct bufdesc_ex { > * (40ns * 6). > */ > #define FEC_QUIRK_BUG_CAPTURE (1 << 10) > +/* Controller has only one MDIO bus */ > +#define FEC_QUIRK_SINGLE_MDIO (1 << 11) > > struct fec_enet_priv_tx_q { > int index; > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index 5ebdf8d..8dc0391 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 | @@ - > 1937,7 +1938,7 @@ static int fec_enet_mii_init(struct platform_device > *pdev) > int err = -ENXIO, i; > > /* > - * The dual fec interfaces are not equivalent with enet-mac. > + * The i.MX28 dual fec interfaces are not equal. > * Here are the differences: > * > * - fec0 supports MII & RMII modes while fec1 only supports RMII > @@ -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; > -- > 2.2.1 The patch limits all dts to use MDIO/phy-handle property that means don't support legacy mode like V1 patch comments case1 & case3. Maybe it is the shortcut method. In additional, it is better to split the patch to two part with dts and driver part. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
