[OpenWrt-Devel] [PATCH] [RFC] ath79: ag71xx: apply interface mode to MII0/1_CNTL on ar71xx/ar913x

Jonas Gorski jonas.gorski at gmail.com
Fri Aug 17 06:04:39 EDT 2018


Hi,

On 16 August 2018 at 05:05, Chuanhong Guo <gch981213 at gmail.com> wrote:
> Signed-off-by: Chuanhong Guo <gch981213 at 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 = &eth0;
     ethernet1 = &eth1;
};


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 @@
>  };
>
>  &eth0 {
> -       compatible = "qca,ar7100-eth";
> +       compatible = "qca,ar7100-eth0", "qca,ar7100-eth";
>         reg = <0x19000000 0x200
>                 0x18070000 0x4>;
>
> @@ -189,7 +189,7 @@
>  };
>
>  &eth1 {
> -       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 @@
>  };
>
>  &eth0 {
> -       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 at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel



More information about the openwrt-devel mailing list