On 05/24/2016 07:35 PM, Joe Hershberger wrote: > On Tue, May 24, 2016 at 10:22 AM, Marek Vasut <ma...@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 <ma...@denx.de> >> Cc: Daniel Schwierzeck <daniel.schwierz...@gmail.com> >> Cc: Joe Hershberger <joe.hershber...@ni.com> >> Cc: Wills Wang <wills.w...@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, ®); >> + 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, ®); >> + 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot