[U-Boot] [PATCH v7] net: ll_temac: Add LL TEMAC driver to u-boot

Andy Fleming
2011-12-29T17:57:09Z

On Thu, Dec 1, 2011 at 2:39 PM, Stephan Linz <linz@li-pro.net> wrote:
> +static int phywrite(struct eth_device *dev, u8 phy_addr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? u8 reg_addr, u16 phy_data)
> +{
> + ? ? ? struct temac_reg *regs = (struct temac_reg *)dev->iobase;
> +
> + ? ? ? out_be32(&regs->lsw, (phy_data & LSW_REGDAT_MASK));
> + ? ? ? out_be32(&regs->ctl, CTL_WEN | TEMAC_MIIMWD);
> +
> + ? ? ? out_be32(&regs->lsw,
> + ? ? ? ? ? ? ? ((phy_addr << LSW_PHYAD_POS) & LSW_PHYAD_MASK) |
> + ? ? ? ? ? ? ? (reg_addr & LSW_REGAD_MASK));
> + ? ? ? out_be32(&regs->ctl, CTL_WEN | TEMAC_MIIMAI);
> +
> + ? ? ? if (check_status(regs, RSE_MIIM_WR))
> + ? ? ? ? ? ? ? return 1;
> +
> + ? ? ? return 0;
> +}
> +
> +/*
> + * Indirect MII PHY read via ll_temac.
> + *
> + * http://www.xilinx.com/support/documentation/ip_documentation/xps_ll_temac.pdf
> + * page 67, Using the MII Management to Access PHY Registers
> + */
> +static int phyread(struct eth_device *dev, u8 phy_addr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? u8 reg_addr, u16 *value)

phywrite() and phyread() are very generic function names that nearly
override the PHY Lib's phy_read() and phy_write(). I also notice that
these functions don't appear to implement the PHY Lib read/write
interface.

> +
> +/* setting sub-controller and ll_temac to proper setting */
> +static int setup_ctrl(struct eth_device *dev)
> +{
> + ? ? ? struct ll_temac *ll_temac = dev->priv;
> +
> + ? ? ? if (ll_temac->ctrlreset) {
> +
> + ? ? ? ? ? ? ? if (ll_temac->ctrlreset(dev))
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? }

Minor nit - this could be:

if (ll_temac->ctrlreset && ll_temac->ctrlreset(dev))
return 0;

Certainly it needs neither the 2 empty lines, nor the braces.

> +
> + ? ? ? if (ll_temac->ctrlinit) {
> +
> + ? ? ? ? ? ? ? if (ll_temac->ctrlinit(dev))
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? }

Same here.

> +static int ll_temac_miiphy_read(const char *devname, unsigned char addr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned char reg, unsigned short *value)
> +{
> + ? ? ? int ret;
> + ? ? ? struct eth_device *dev = eth_get_dev();
> +
> + ? ? ? ret = phyread(dev, addr, reg, value);
> + ? ? ? debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, *value);
> + ? ? ? return ret;
> +}
> +
> +static int ll_temac_miiphy_write(const char *devname, unsigned char addr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned char reg, unsigned short value)
> +{
> + ? ? ? struct eth_device *dev = eth_get_dev();
> +
> + ? ? ? debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, value);
> + ? ? ? return phywrite(dev, addr, reg, value);
> +}

Please don't implement these functions. They are deprecated. This way
of doing things will cause problems if eth_get_dev() doesn't return
the ethernet interface you want, and makes it very difficult to do
MDIO via something that's not an ethernet interface (Like gpio pins).

> +
> +/*
> + * bis: ? ? ? ? ? ? ? ?board information
> + * base_addr: ?LL TEMAC register bank
> + * ctrl_addr: ?LL TEMAC sub-controller register bank (FIFO or SDMA)
> + * mode: ? ? ? driver mode bit flags (see xilinx_ll_temac.h)
> + */
> +int xilinx_ll_temac_initialize(bd_t *bis, unsigned long base_addr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int mode, unsigned long ctrl_addr)
> +{
> + ? ? ? struct eth_device *dev;
> + ? ? ? struct ll_temac *ll_temac;
> +
> + ? ? ? dev = calloc(1, sizeof(*dev));
> + ? ? ? if (dev == NULL)
> + ? ? ? ? ? ? ? return -1;
> +
> + ? ? ? dev->priv = calloc(1, sizeof(struct ll_temac));
> + ? ? ? if (dev->priv == NULL) {
> + ? ? ? ? ? ? ? free(dev);
> + ? ? ? ? ? ? ? return -1;
> + ? ? ? }
> +
> + ? ? ? ll_temac = dev->priv;
> +
> + ? ? ? sprintf(dev->name, "Xlltem.%lx", base_addr);
> +
> + ? ? ? dev->iobase = base_addr;
> + ? ? ? ll_temac->ctrladdr = ctrl_addr;
> + ? ? ? ll_temac->rx_bp = rx_buffer;
> + ? ? ? ll_temac->tx_bp = tx_buffer;
> + ? ? ? ll_temac->rx_dp = &rx_descr;
> + ? ? ? ll_temac->tx_dp = &tx_descr;
> + ? ? ? ll_temac->phyaddr = CONFIG_PHY_ADDR;
> +
> + ? ? ? dev->init = ll_temac_init;
> + ? ? ? dev->halt = ll_temac_halt;
> + ? ? ? dev->write_hwaddr = ll_temac_addr_setup;
> +
> + ? ? ? if (mode & M_SDMA) {
> +#if defined(CONFIG_XILINX_440) || defined(CONFIG_XILINX_405)
> + ? ? ? ? ? ? ? if (mode & M_DCR) {
> + ? ? ? ? ? ? ? ? ? ? ? ll_temac_collect_xldcr_sdma_reg_addr(dev);
> + ? ? ? ? ? ? ? ? ? ? ? ll_temac->in32 = ll_temac_xldcr_in32;
> + ? ? ? ? ? ? ? ? ? ? ? ll_temac->out32 = ll_temac_xldcr_out32;
> + ? ? ? ? ? ? ? } else
> +#endif
> + ? ? ? ? ? ? ? {
> + ? ? ? ? ? ? ? ? ? ? ? ll_temac_collect_xlplb_sdma_reg_addr(dev);
> + ? ? ? ? ? ? ? ? ? ? ? ll_temac->in32 = ll_temac_xlplb_in32;
> + ? ? ? ? ? ? ? ? ? ? ? ll_temac->out32 = ll_temac_xlplb_out32;
> + ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? ll_temac->ctrlinit = ll_temac_init_sdma;
> + ? ? ? ? ? ? ? ll_temac->ctrlhalt = ll_temac_halt_sdma;
> + ? ? ? ? ? ? ? ll_temac->ctrlreset = ll_temac_reset_sdma;
> + ? ? ? ? ? ? ? dev->recv = ll_temac_recv_sdma;
> + ? ? ? ? ? ? ? dev->send = ll_temac_send_sdma;
> + ? ? ? } else {
> + ? ? ? ? ? ? ? ll_temac->in32 = NULL;
> + ? ? ? ? ? ? ? ll_temac->out32 = NULL;
> + ? ? ? ? ? ? ? ll_temac->ctrlinit = NULL;
> + ? ? ? ? ? ? ? ll_temac->ctrlhalt = NULL;
> + ? ? ? ? ? ? ? ll_temac->ctrlreset = ll_temac_reset_fifo;
> + ? ? ? ? ? ? ? dev->recv = ll_temac_recv_fifo;
> + ? ? ? ? ? ? ? dev->send = ll_temac_send_fifo;
> + ? ? ? }
> +
> + ? ? ? eth_register(dev);
> +
> + ? ? ? miiphy_register(dev->name, ll_temac_miiphy_read, ll_temac_miiphy_write);
> + ? ? ? ll_temac->bus = miiphy_get_dev_by_name(dev->name);
> + ? ? ? ll_temac->bus->reset = NULL;

This is fairly broken. miiphy_register() is a deprecated interface,
and to use it with PHYLIB is backwards. Use the mdio_register()
function, and implement the proper PHY read/write callbacks which are
passed a bus handle instead of an ethernet name.

Andy