[U-Boot] [PATCH V2] net: Add ag7xxx driver for Atheros MIPS

[Marek Vasut <marex@denx.de>]

On 05/24/2016 07:35 PM, Joe Hershberger wrote:
> On Tue, May 24, 2016 at 10:22 AM, Marek Vasut <marex@denx.de> wrote:
>> Add ethernet driver for the AR933x and AR934x Atheros MIPS machines.
>> The driver could be easily extended to other WiSoCs.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>> Cc: Joe Hershberger <joe.hershberger@ni.com>
>> Cc: Wills Wang <wills.wang@live.com>
>> ---
>> V2: - Drop the printf() in case malloc fails, it's pointless to try
>>       and print something if we cannot allocate memory, since printf
>>       also allocates memory.
>> ---
>>  drivers/net/Kconfig  |   9 +
>>  drivers/net/Makefile |   1 +
>>  drivers/net/ag7xxx.c | 980 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 990 insertions(+)
>>  create mode 100644 drivers/net/ag7xxx.c
>>
> 
> <snip>
> 
>> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
>> new file mode 100644
>> index 0000000..7ab9049
>> --- /dev/null
>> +++ b/drivers/net/ag7xxx.c
> 
> <snip>
> 
>> +static int ag7xxx_switch_reg_read(struct mii_dev *bus, int reg, u32 *val)
>> +{
>> +       struct ar7xxx_eth_priv *priv = bus->priv;
>> +       u32 phy_addr;
>> +       u32 reg_addr;
>> +       u32 phy_temp;
>> +       u32 reg_temp;
>> +       u16 rv = 0;
>> +       int ret;
>> +
>> +       if (priv->model == AG7XXX_MODEL_AG933X) {
>> +               phy_addr = 0x1f;
>> +               reg_addr = 0x10;
>> +       } else if (priv->model == AG7XXX_MODEL_AG934X) {
>> +               phy_addr = 0x18;
>> +               reg_addr = 0x00;
>> +       } else
>> +               return -EINVAL;
>> +
>> +       ret = ag7xxx_switch_write(bus, phy_addr, reg_addr, reg >> 9);
>> +       if (ret)
>> +               return ret;
>> +
>> +       phy_temp = ((reg >> 6) & 0x7) | 0x10;
>> +       reg_temp = (reg >> 1) & 0x1e;
>> +       *val = 0;
> 
> Why all the magic numbers?

Because there is no decent documentation for this block/chip :-(

To summarize all of it, most of the PHY init is mysterious register
poking salvaged from the obscure u-boot ports for various boards. It
does work, which I verified and the comments try to clarify what it
is supposed to do, but the documentation for that sequence is just not
available.

I tried plucking out as many of the magic constants as possible, but
this is as good as it gets. I _hope_ that getting some support for
the ath79 mips into mainline will induce some more attention and
maybe push qca to be more open regarding these chips, but I wouldn't bet
on it.

>> +       ret = ag7xxx_switch_read(bus, phy_temp, reg_temp | 0, &rv);
>> +       if (ret < 0)
>> +               return ret;
>> +       *val |= rv;
>> +
>> +       ret = ag7xxx_switch_read(bus, phy_temp, reg_temp | 1, &rv);
>> +       if (ret < 0)
>> +               return ret;
>> +       *val |= (rv << 16);
>> +
>> +       return 0;
>> +}

[...]

>> +static int ag933x_phy_setup_common(struct udevice *dev)
>> +{
>> +       struct ar7xxx_eth_priv *priv = dev_get_priv(dev);
>> +       int i, ret, phymax;
>> +
>> +       if (priv->model == AG7XXX_MODEL_AG933X)
>> +               phymax = 4;
>> +       else if (priv->model == AG7XXX_MODEL_AG934X)
>> +               phymax = 5;
>> +       else
>> +               return -EINVAL;
>> +
>> +       if (priv->interface == PHY_INTERFACE_MODE_RMII) {
>> +               ret = ag933x_phy_setup_reset_set(dev, phymax);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               ret = ag933x_phy_setup_reset_fin(dev, phymax);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               /* Read out link status */
>> +               ret = ag7xxx_mdio_read(priv->bus, phymax, 0, 0x11);
> 
> What is this?  Is this intended to be MII_MIPSCR?

Fixed, I didn't find this register before, thanks.

>> +               if (ret < 0)
>> +                       return ret;
>> +
>> +               return 0;
>> +       }
>> +
>> +       /* Switch ports */
>> +       for (i = 0; i < phymax; i++) {
>> +               ret = ag933x_phy_setup_reset_set(dev, i);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       for (i = 0; i < phymax; i++) {
>> +               ret = ag933x_phy_setup_reset_fin(dev, i);
>> +               if (ret)
>> +                       return ret;
>> +       }
>> +
>> +       for (i = 0; i < phymax; i++) {
>> +               /* Read out link status */
>> +               ret = ag7xxx_mdio_read(priv->bus, i, 0, 0x11);
> 
> Same here.

Fixed

>> +               if (ret < 0)
>> +                       return ret;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int ag934x_phy_setup(struct udevice *dev)
>> +{
>> +       struct ar7xxx_eth_priv *priv = dev_get_priv(dev);
>> +       int i, ret;
>> +       u32 reg;
>> +
>> +       ret = ag7xxx_switch_reg_write(priv->bus, 0x624, 0x7f7f7f7f);
>> +       if (ret)
>> +               return ret;
>> +       ret = ag7xxx_switch_reg_write(priv->bus, 0x10, 0x40000000);
>> +       if (ret)
>> +               return ret;
>> +       ret = ag7xxx_switch_reg_write(priv->bus, 0x4, 0x07600000);
>> +       if (ret)
>> +               return ret;
>> +       ret = ag7xxx_switch_reg_write(priv->bus, 0xc, 0x01000000);
>> +       if (ret)
>> +               return ret;
>> +       ret = ag7xxx_switch_reg_write(priv->bus, 0x7c, 0x0000007e);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* AR8327/AR8328 v1.0 fixup */
>> +       ret = ag7xxx_switch_reg_read(priv->bus, 0, &reg);
>> +       if (ret)
>> +               return ret;
>> +       if ((reg & 0xffff) == 0x1201) {
>> +               for (i = 0; i < 5; i++) {
>> +                       ret = ag7xxx_mdio_write(priv->bus, i, 0, 0x1d, 0x0);
>> +                       if (ret)
>> +                               return ret;
>> +                       ret = ag7xxx_mdio_write(priv->bus, i, 0, 0x1e, 0x02ea);
>> +                       if (ret)
>> +                               return ret;
>> +                       ret = ag7xxx_mdio_write(priv->bus, i, 0, 0x1d, 0x3d);
>> +                       if (ret)
>> +                               return ret;
>> +                       ret = ag7xxx_mdio_write(priv->bus, i, 0, 0x1e, 0x68a0);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>> +       }
>> +
>> +       ret = ag7xxx_switch_reg_read(priv->bus, 0x66c, &reg);
>> +       if (ret)
>> +               return ret;
>> +       reg &= ~0x70000;
>> +       ret = ag7xxx_switch_reg_write(priv->bus, 0x66c, reg);
>> +       if (ret)
>> +               return ret;
>> +
>> +       return 0;
> 
> Many more magic numbers.

More undocumented goop salvaged from the interwebs, yes.

>> +}
>> +
>> +static int ag7xxx_mac_probe(struct udevice *dev)
>> +{
>> +       struct ar7xxx_eth_priv *priv = dev_get_priv(dev);
>> +       int ret;
>> +
>> +       ag7xxx_hw_setup(dev);
>> +       ret = ag7xxx_mii_setup(dev);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ag7xxx_eth_write_hwaddr(dev);
> 
> Any reason not to let the framework call this?

For reason unknown, the MAC needs to be loaded into the device before
you start setting up the PHY.

>> +
>> +       if (priv->model == AG7XXX_MODEL_AG933X) {
>> +               if (priv->interface == PHY_INTERFACE_MODE_RMII)
>> +                       ret = ag933x_phy_setup_wan(dev);
>> +               else
>> +                       ret = ag933x_phy_setup_lan(dev);
>> +       } else if (priv->model == AG7XXX_MODEL_AG934X) {
>> +               ret = ag934x_phy_setup(dev);
>> +       } else {
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (ret)
>> +               return ret;
>> +
>> +       return ag933x_phy_setup_common(dev);
>> +}
>> +
>> +static int ag7xxx_mdio_probe(struct udevice *dev)
>> +{
>> +       struct ar7xxx_eth_priv *priv = dev_get_priv(dev);
>> +       struct mii_dev *bus = mdio_alloc();
>> +
>> +       if (!bus)
>> +               return -ENOMEM;
>> +
>> +       bus->read = ag7xxx_mdio_read;
>> +       bus->write = ag7xxx_mdio_write;
>> +       snprintf(bus->name, sizeof(bus->name), dev->name);
>> +
>> +       bus->priv = (void *)priv;
>> +
>> +       return mdio_register(bus);
>> +}
>> +
>> +static int ag7xxx_get_phy_iface_offset(struct udevice *dev)
>> +{
>> +       int offset;
>> +
>> +       offset = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset, "phy");
>> +       if (offset <= 0) {
>> +               debug("%s: PHY OF node not found (ret=%i)\n", __func__, offset);
>> +               return -EINVAL;
>> +       }
>> +
>> +       offset = fdt_parent_offset(gd->fdt_blob, offset);
>> +       if (offset <= 0) {
>> +               debug("%s: PHY OF node parent MDIO bus not found (ret=%i)\n",
>> +                     __func__, offset);
>> +               return -EINVAL;
>> +       }
>> +
>> +       offset = fdt_parent_offset(gd->fdt_blob, offset);
>> +       if (offset <= 0) {
>> +               debug("%s: PHY MDIO OF node parent MAC not found (ret=%i)\n",
>> +                     __func__, offset);
>> +               return -EINVAL;
>> +       }
>> +
>> +       return offset;
>> +}
>> +
>> +static int ag7xxx_eth_probe(struct udevice *dev)
>> +{
>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> +       struct ar7xxx_eth_priv *priv = dev_get_priv(dev);
>> +       void __iomem *iobase, *phyiobase;
>> +       int ret, phyreg;
>> +
>> +       /* Decoding of convoluted PHY wiring on Atheros MIPS. */
>> +       ret = ag7xxx_get_phy_iface_offset(dev);
>> +       if (ret <= 0)
>> +               return ret;
>> +       phyreg = fdtdec_get_int(gd->fdt_blob, ret, "reg", -1);
>> +
>> +       iobase = map_physmem(pdata->iobase, 0x200, MAP_NOCACHE);
>> +       phyiobase = map_physmem(phyreg, 0x200, MAP_NOCACHE);
>> +
>> +       debug("%s, iobase=%p, phyiobase=%p, priv=%p\n",
>> +             __func__, iobase, phyiobase, priv);
>> +       priv->regs = iobase;
>> +       priv->phyregs = phyiobase;
>> +       priv->interface = pdata->phy_interface;
>> +       priv->model = dev_get_driver_data(dev);
>> +
>> +       ret = ag7xxx_mdio_probe(dev);
>> +       if (ret)
>> +               return ret;
>> +
>> +       priv->bus = miiphy_get_dev_by_name(dev->name);
>> +
>> +       ret = ag7xxx_mac_probe(dev);
>> +       debug("%s, ret=%d\n", __func__, ret);
>> +
>> +       return ret;
>> +}
>> +
>> +static int ag7xxx_eth_remove(struct udevice *dev)
>> +{
>> +       struct ar7xxx_eth_priv *priv = dev_get_priv(dev);
>> +
>> +       free(priv->phydev);
>> +       mdio_unregister(priv->bus);
>> +       mdio_free(priv->bus);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct eth_ops ag7xxx_eth_ops = {
>> +       .start                  = ag7xxx_eth_start,
>> +       .send                   = ag7xxx_eth_send,
>> +       .recv                   = ag7xxx_eth_recv,
>> +       .free_pkt               = ag7xxx_eth_free_pkt,
>> +       .stop                   = ag7xxx_eth_stop,
>> +       .write_hwaddr           = ag7xxx_eth_write_hwaddr,
>> +};
>> +
>> +static int ag7xxx_eth_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> +       const char *phy_mode;
>> +       int ret;
>> +
>> +       pdata->iobase = dev_get_addr(dev);
>> +       pdata->phy_interface = -1;
>> +
>> +       /* Decoding of convoluted PHY wiring on Atheros MIPS. */
>> +       ret = ag7xxx_get_phy_iface_offset(dev);
>> +       if (ret <= 0)
>> +               return ret;
>> +
>> +       phy_mode = fdt_getprop(gd->fdt_blob, ret, "phy-mode", NULL);
>> +       if (phy_mode)
>> +               pdata->phy_interface = phy_get_interface_by_name(phy_mode);
>> +       if (pdata->phy_interface == -1) {
>> +               debug("%s: Invalid PHY interface '%s'\n", __func__, phy_mode);
>> +               return -EINVAL;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct udevice_id ag7xxx_eth_ids[] = {
>> +       { .compatible = "qca,ag933x-mac", .data = AG7XXX_MODEL_AG933X },
>> +       { .compatible = "qca,ag934x-mac", .data = AG7XXX_MODEL_AG934X },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(eth_ag7xxx) = {
>> +       .name           = "eth_ag7xxx",
>> +       .id             = UCLASS_ETH,
>> +       .of_match       = ag7xxx_eth_ids,
>> +       .ofdata_to_platdata = ag7xxx_eth_ofdata_to_platdata,
>> +       .probe          = ag7xxx_eth_probe,
>> +       .remove         = ag7xxx_eth_remove,
>> +       .ops            = &ag7xxx_eth_ops,
>> +       .priv_auto_alloc_size = sizeof(struct ar7xxx_eth_priv),
>> +       .platdata_auto_alloc_size = sizeof(struct eth_pdata),
>> +       .flags          = DM_FLAG_ALLOC_PRIV_DMA,
>> +};
>> --
>> 2.7.0

Sorry I couldn't give you any better answer(s) to the questions about
the magic numbers.


-- 
Best regards,
Marek Vasut