Hi, On 16 August 2018 at 05:05, Chuanhong Guo <gch981...@gmail.com> wrote: > Signed-off-by: Chuanhong Guo <gch981...@gmail.com> > --- > RFC: > Previous discussion about this patch can be found on GitHub PR#1271. > This patch applies correct interface mode to MII0/1_CNTL register at > 0x18070000/ > 0x18070004. But there is a small difference in values for these two registers: > | GMAC0 | GMAC1 | > |----------|---------| > | 0 GMII | 0 RGMII | > | 1 MII | 1 RMII | > | 2 RGMII | | > | 3 RMII | | > I currently have 4 ways of dealing with this: > 1. Use a bool value in dts indicating whether this is the second GMAC. This > one > is pretty dirty and I dropped it. > 2. Split MII_CNTL into separated dt node and use different compatible for them > like we did for ETH_CFG (gmac node) on ar933x and later SoCs. After some > discussion > on GitHub it turns out to be unreasonable to treat those in separated > nodes. > 3. Use ar7100-eth0/ar7100-eth1 as compatible string. This is what I've done in > this patch I sent here. But I think my way of using compatible string here > is ugly :( > A possible cleaner implementation would be introducing > ar7100-eth0/ar7100-eth1/ > ar9130-eth0/ar9130-eth1 to replace ar7100-eth/ar9130-eth. But I doubt > whether > introducing 4 new compatible strings for such a slight difference is > worthy. > 4. Somehow write all the possible values for each interface mode in device > tree. > e.g. qca,if-modes = "gmii","mii","rgmii","rmii"; for eth0 and > qca,if-modes = "rgmii","rmii"; for eth1. > > Which one is better? Is there any other possible solutions for this problem?
I'd prefer 4, or a modified 3: do something like aliases { .... ethernet0 = ð0; ethernet1 = ð1; }; and then you can find out if you are eth0 or eth1 by calling id = of_alias_get_id(node, "ethernet"); and maybe warn (and don't configure mii mode) if it returns neither 0 or 1. Regards Jonas > > target/linux/ath79/dts/ar7100.dtsi | 4 +- > target/linux/ath79/dts/ar9132.dtsi | 2 +- > .../net/ethernet/atheros/ag71xx/ag71xx_main.c | 61 +++++++++++++++++++ > 3 files changed, 64 insertions(+), 3 deletions(-) > > diff --git a/target/linux/ath79/dts/ar7100.dtsi > b/target/linux/ath79/dts/ar7100.dtsi > index 8994a7d688..89c17bcede 100644 > --- a/target/linux/ath79/dts/ar7100.dtsi > +++ b/target/linux/ath79/dts/ar7100.dtsi > @@ -171,7 +171,7 @@ > }; > > ð0 { > - compatible = "qca,ar7100-eth"; > + compatible = "qca,ar7100-eth0", "qca,ar7100-eth"; > reg = <0x19000000 0x200 > 0x18070000 0x4>; > > @@ -189,7 +189,7 @@ > }; > > ð1 { > - compatible = "qca,ar7100-eth"; > + compatible = "qca,ar7100-eth1", "qca,ar7100-eth"; > reg = <0x1a000000 0x200 > 0x18070004 0x4>; > > diff --git a/target/linux/ath79/dts/ar9132.dtsi > b/target/linux/ath79/dts/ar9132.dtsi > index 9d8ddcf9ba..a136b06e69 100644 > --- a/target/linux/ath79/dts/ar9132.dtsi > +++ b/target/linux/ath79/dts/ar9132.dtsi > @@ -185,7 +185,7 @@ > }; > > ð0 { > - compatible = "qca,ar9130-eth", "syscon"; > + compatible = "qca,ar9130-eth", "qca,ar7100-eth0", "syscon"; > reg = <0x19000000 0x200 > 0x18070000 0x4>; > pll-data = <0x1a000000 0x13000a44 0x00441099>; > diff --git > a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c > b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c > index 1e0bb6937f..5ea9ef40d2 100644 > --- > a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c > +++ > b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c > @@ -529,6 +529,60 @@ static void ath79_set_pll(struct ag71xx *ag) > udelay(100); > } > > +static void ath79_mii_ctrl_set_if(struct ag71xx *ag, unsigned int mii_if) > +{ > + u32 t; > + > + t = __raw_readl(ag->mii_base); > + t &= ~(AR71XX_MII_CTRL_IF_MASK); > + t |= (mii_if & AR71XX_MII_CTRL_IF_MASK); > + __raw_writel(t, ag->mii_base); > +} > + > +static void ath79_mii0_ctrl_set_if(struct ag71xx *ag) > +{ > + unsigned int mii_if; > + > + switch (ag->phy_if_mode) { > + case PHY_INTERFACE_MODE_MII: > + mii_if = AR71XX_MII0_CTRL_IF_MII; > + break; > + case PHY_INTERFACE_MODE_GMII: > + mii_if = AR71XX_MII0_CTRL_IF_GMII; > + break; > + case PHY_INTERFACE_MODE_RGMII: > + mii_if = AR71XX_MII0_CTRL_IF_RGMII; > + break; > + case PHY_INTERFACE_MODE_RMII: > + mii_if = AR71XX_MII0_CTRL_IF_RMII; > + break; > + default: > + WARN(1, "Impossible PHY mode defined.\n"); > + return; > + } > + > + ath79_mii_ctrl_set_if(ag, mii_if); > +} > + > +static void ath79_mii1_ctrl_set_if(struct ag71xx *ag) > +{ > + unsigned int mii_if; > + > + switch (ag->phy_if_mode) { > + case PHY_INTERFACE_MODE_RMII: > + mii_if = AR71XX_MII1_CTRL_IF_RMII; > + break; > + case PHY_INTERFACE_MODE_RGMII: > + mii_if = AR71XX_MII1_CTRL_IF_RGMII; > + break; > + default: > + WARN(1, "Impossible PHY mode defined.\n"); > + return; > + } > + > + ath79_mii_ctrl_set_if(ag, mii_if); > +} > + > static void ath79_mii_ctrl_set_speed(struct ag71xx *ag) > { > unsigned int mii_speed; > @@ -1427,6 +1481,13 @@ static int ag71xx_probe(struct platform_device *pdev) > goto err_free; > } > > + if (ag->mii_base) { > + if (of_device_is_compatible(np, "qca,ar7100-eth0")) > + ath79_mii0_ctrl_set_if(ag); > + else if (of_device_is_compatible(np, "qca,ar7100-eth1")) > + ath79_mii1_ctrl_set_if(ag); > + } > + > netif_napi_add(dev, &ag->napi, ag71xx_poll, AG71XX_NAPI_WEIGHT); > > ag71xx_wr(ag, AG71XX_REG_MAC_CFG1, 0); > -- > 2.17.1 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel