Hi Marek,
On Wed, 2020-03-25 at 16:36 +0100, Marek Vasut wrote:
> Introduce a private data structure for this driver with embedded
> struct eth_device and pass it around. This prepares the driver to
> work with both DM and non-DM systems.
>
> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
> Cc: Joe Hershberger <joe.hershber...@ni.com>
> Cc: Masahiro Yamada <yamada.masah...@socionext.com>
> ---
> V2: No change
> V3: No change
> ---
> drivers/net/smc911x.c | 230 ++++++++++++++++++++++--------------------
> 1 file changed, 123 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index 364f8c5da8..786ab220b2 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -20,6 +20,13 @@ struct chip_id {
> char *name;
> };
>
> +struct smc911x_priv {
> + struct eth_device dev;
> + phys_addr_t iobase;
> + const struct chip_id *chipid;
> + unsigned char enetaddr[6];
> +};
> +
> static const struct chip_id chip_ids[] = {
> { CHIP_89218, "LAN89218" },
> { CHIP_9115, "LAN9115" },
> @@ -45,57 +52,57 @@ static const struct chip_id chip_ids[] = {
> #endif
>
> #if defined (CONFIG_SMC911X_32_BIT)
> -static u32 smc911x_reg_read(struct eth_device *dev, u32 offset)
> +static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> {
> - return readl(dev->iobase + offset);
> + return readl(priv->iobase + offset);
> }
>
> -static void smc911x_reg_write(struct eth_device *dev, u32 offset, u32 val)
> +static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> {
> - writel(val, dev->iobase + offset);
> + writel(val, priv->iobase + offset);
> }
> #elif defined (CONFIG_SMC911X_16_BIT)
> -static u32 smc911x_reg_read(struct eth_device *dev, u32 offset)
> +static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> {
> - return (readw(dev->iobase + offset) & 0xffff) |
> - (readw(dev->iobase + offset + 2) << 16);
> + return (readw(priv->iobase + offset) & 0xffff) |
> + (readw(priv->iobase + offset + 2) << 16);
> }
> -static void smc911x_reg_write(struct eth_device *dev, u32 offset, u32 val)
> +static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> {
> - writew(val & 0xffff, dev->iobase + offset);
> - writew(val >> 16, dev->iobase + offset + 2);
> + writew(val & 0xffff, priv->iobase + offset);
> + writew(val >> 16, priv->iobase + offset + 2);
> }
> #else
> #error "SMC911X: undefined bus width"
> #endif /* CONFIG_SMC911X_16_BIT */
>
> -static u32 smc911x_get_mac_csr(struct eth_device *dev, u8 reg)
> +static u32 smc911x_get_mac_csr(struct smc911x_priv *priv, u8 reg)
> {
> - while (smc911x_reg_read(dev, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> + while (smc911x_reg_read(priv, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> ;
> - smc911x_reg_write(dev, MAC_CSR_CMD,
> + smc911x_reg_write(priv, MAC_CSR_CMD,
> MAC_CSR_CMD_CSR_BUSY | MAC_CSR_CMD_R_NOT_W | reg);
> - while (smc911x_reg_read(dev, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> + while (smc911x_reg_read(priv, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> ;
>
> - return smc911x_reg_read(dev, MAC_CSR_DATA);
> + return smc911x_reg_read(priv, MAC_CSR_DATA);
> }
>
> -static void smc911x_set_mac_csr(struct eth_device *dev, u8 reg, u32 data)
> +static void smc911x_set_mac_csr(struct smc911x_priv *priv, u8 reg, u32 data)
> {
> - while (smc911x_reg_read(dev, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> + while (smc911x_reg_read(priv, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> ;
> - smc911x_reg_write(dev, MAC_CSR_DATA, data);
> - smc911x_reg_write(dev, MAC_CSR_CMD, MAC_CSR_CMD_CSR_BUSY | reg);
> - while (smc911x_reg_read(dev, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> + smc911x_reg_write(priv, MAC_CSR_DATA, data);
> + smc911x_reg_write(priv, MAC_CSR_CMD, MAC_CSR_CMD_CSR_BUSY | reg);
> + while (smc911x_reg_read(priv, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> ;
> }
>
> -static int smc911x_detect_chip(struct eth_device *dev)
> +static int smc911x_detect_chip(struct smc911x_priv *priv)
> {
> unsigned long val, i;
>
> - val = smc911x_reg_read(dev, BYTE_TEST);
> + val = smc911x_reg_read(priv, BYTE_TEST);
> if (val == 0xffffffff) {
> /* Special case -- no chip present */
> return -1;
> @@ -104,7 +111,7 @@ static int smc911x_detect_chip(struct eth_device *dev)
> return -1;
> }
>
> - val = smc911x_reg_read(dev, ID_REV) >> 16;
> + val = smc911x_reg_read(priv, ID_REV) >> 16;
> for (i = 0; chip_ids[i].id != 0; i++) {
> if (chip_ids[i].id == val) break;
> }
> @@ -113,12 +120,12 @@ static int smc911x_detect_chip(struct eth_device *dev)
> return -1;
> }
>
> - dev->priv = (void *)&chip_ids[i];
> + priv->chipid = &chip_ids[i];
>
> return 0;
> }
>
> -static void smc911x_reset(struct eth_device *dev)
> +static void smc911x_reset(struct smc911x_priv *priv)
> {
> int timeout;
>
> @@ -126,14 +133,14 @@ static void smc911x_reset(struct eth_device *dev)
> * Take out of PM setting first
> * Device is already wake up if PMT_CTRL_READY bit is set
> */
> - if ((smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY) == 0) {
> + if ((smc911x_reg_read(priv, PMT_CTRL) & PMT_CTRL_READY) == 0) {
> /* Write to the bytetest will take out of powerdown */
> - smc911x_reg_write(dev, BYTE_TEST, 0x0);
> + smc911x_reg_write(priv, BYTE_TEST, 0x0);
>
> timeout = 10;
>
> while (timeout-- &&
> - !(smc911x_reg_read(dev, PMT_CTRL) & PMT_CTRL_READY))
> + !(smc911x_reg_read(priv, PMT_CTRL) & PMT_CTRL_READY))
> udelay(10);
> if (timeout < 0) {
> printf(DRIVERNAME
> @@ -143,12 +150,12 @@ static void smc911x_reset(struct eth_device *dev)
> }
>
> /* Disable interrupts */
> - smc911x_reg_write(dev, INT_EN, 0);
> + smc911x_reg_write(priv, INT_EN, 0);
>
> - smc911x_reg_write(dev, HW_CFG, HW_CFG_SRST);
> + smc911x_reg_write(priv, HW_CFG, HW_CFG_SRST);
>
> timeout = 1000;
> - while (timeout-- && smc911x_reg_read(dev, E2P_CMD) & E2P_CMD_EPC_BUSY)
> + while (timeout-- && smc911x_reg_read(priv, E2P_CMD) & E2P_CMD_EPC_BUSY)
> udelay(10);
>
> if (timeout < 0) {
> @@ -157,83 +164,83 @@ static void smc911x_reset(struct eth_device *dev)
> }
>
> /* Reset the FIFO level and flow control settings */
> - smc911x_set_mac_csr(dev, FLOW, FLOW_FCPT | FLOW_FCEN);
> - smc911x_reg_write(dev, AFC_CFG, 0x0050287F);
> + smc911x_set_mac_csr(priv, FLOW, FLOW_FCPT | FLOW_FCEN);
> + smc911x_reg_write(priv, AFC_CFG, 0x0050287F);
>
> /* Set to LED outputs */
> - smc911x_reg_write(dev, GPIO_CFG, 0x70070000);
> + smc911x_reg_write(priv, GPIO_CFG, 0x70070000);
> }
>
> -static void smc911x_handle_mac_address(struct eth_device *dev)
> +static void smc911x_handle_mac_address(struct smc911x_priv *priv)
> {
> unsigned long addrh, addrl;
> - uchar *m = dev->enetaddr;
> + unsigned char *m = priv->enetaddr;
>
> addrl = m[0] | (m[1] << 8) | (m[2] << 16) | (m[3] << 24);
> addrh = m[4] | (m[5] << 8);
> - smc911x_set_mac_csr(dev, ADDRL, addrl);
> - smc911x_set_mac_csr(dev, ADDRH, addrh);
> + smc911x_set_mac_csr(priv, ADDRL, addrl);
> + smc911x_set_mac_csr(priv, ADDRH, addrh);
>
> printf(DRIVERNAME ": MAC %pM\n", m);
> }
>
> -static int smc911x_eth_phy_read(struct eth_device *dev,
> +static int smc911x_eth_phy_read(struct smc911x_priv *priv,
> u8 phy, u8 reg, u16 *val)
> {
> - while (smc911x_get_mac_csr(dev, MII_ACC) & MII_ACC_MII_BUSY)
> + while (smc911x_get_mac_csr(priv, MII_ACC) & MII_ACC_MII_BUSY)
> ;
>
> - smc911x_set_mac_csr(dev, MII_ACC, phy << 11 | reg << 6 |
> + smc911x_set_mac_csr(priv, MII_ACC, phy << 11 | reg << 6 |
> MII_ACC_MII_BUSY);
>
> - while (smc911x_get_mac_csr(dev, MII_ACC) & MII_ACC_MII_BUSY)
> + while (smc911x_get_mac_csr(priv, MII_ACC) & MII_ACC_MII_BUSY)
> ;
>
> - *val = smc911x_get_mac_csr(dev, MII_DATA);
> + *val = smc911x_get_mac_csr(priv, MII_DATA);
>
> return 0;
> }
>
> -static int smc911x_eth_phy_write(struct eth_device *dev,
> +static int smc911x_eth_phy_write(struct smc911x_priv *priv,
> u8 phy, u8 reg, u16 val)
> {
> - while (smc911x_get_mac_csr(dev, MII_ACC) & MII_ACC_MII_BUSY)
> + while (smc911x_get_mac_csr(priv, MII_ACC) & MII_ACC_MII_BUSY)
> ;
>
> - smc911x_set_mac_csr(dev, MII_DATA, val);
> - smc911x_set_mac_csr(dev, MII_ACC,
> + smc911x_set_mac_csr(priv, MII_DATA, val);
> + smc911x_set_mac_csr(priv, MII_ACC,
> phy << 11 | reg << 6 | MII_ACC_MII_BUSY | MII_ACC_MII_WRITE);
>
> - while (smc911x_get_mac_csr(dev, MII_ACC) & MII_ACC_MII_BUSY)
> + while (smc911x_get_mac_csr(priv, MII_ACC) & MII_ACC_MII_BUSY)
> ;
> return 0;
> }
>
> -static int smc911x_phy_reset(struct eth_device *dev)
> +static int smc911x_phy_reset(struct smc911x_priv *priv)
> {
> u32 reg;
>
> - reg = smc911x_reg_read(dev, PMT_CTRL);
> + reg = smc911x_reg_read(priv, PMT_CTRL);
> reg &= ~0xfffff030;
> reg |= PMT_CTRL_PHY_RST;
> - smc911x_reg_write(dev, PMT_CTRL, reg);
> + smc911x_reg_write(priv, PMT_CTRL, reg);
>
> mdelay(100);
>
> return 0;
> }
>
> -static void smc911x_phy_configure(struct eth_device *dev)
> +static void smc911x_phy_configure(struct smc911x_priv *priv)
> {
> int timeout;
> u16 status;
>
> - smc911x_phy_reset(dev);
> + smc911x_phy_reset(priv);
>
> - smc911x_eth_phy_write(dev, 1, MII_BMCR, BMCR_RESET);
> + smc911x_eth_phy_write(priv, 1, MII_BMCR, BMCR_RESET);
> mdelay(1);
> - smc911x_eth_phy_write(dev, 1, MII_ADVERTISE, 0x01e1);
> - smc911x_eth_phy_write(dev, 1, MII_BMCR, BMCR_ANENABLE |
> + smc911x_eth_phy_write(priv, 1, MII_ADVERTISE, 0x01e1);
> + smc911x_eth_phy_write(priv, 1, MII_BMCR, BMCR_ANENABLE |
> BMCR_ANRESTART);
>
> timeout = 5000;
> @@ -242,7 +249,7 @@ static void smc911x_phy_configure(struct eth_device *dev)
> if ((timeout--) == 0)
> goto err_out;
>
> - if (smc911x_eth_phy_read(dev, 1, MII_BMSR, &status) != 0)
> + if (smc911x_eth_phy_read(priv, 1, MII_BMSR, &status) != 0)
> goto err_out;
> } while (!(status & BMSR_LSTATUS));
>
> @@ -254,64 +261,66 @@ err_out:
> printf(DRIVERNAME ": autonegotiation timed out\n");
> }
>
> -static void smc911x_enable(struct eth_device *dev)
> +static void smc911x_enable(struct smc911x_priv *priv)
> {
> /* Enable TX */
> - smc911x_reg_write(dev, HW_CFG, 8 << 16 | HW_CFG_SF);
> + smc911x_reg_write(priv, HW_CFG, 8 << 16 | HW_CFG_SF);
>
> - smc911x_reg_write(dev, GPT_CFG, GPT_CFG_TIMER_EN | 10000);
> + smc911x_reg_write(priv, GPT_CFG, GPT_CFG_TIMER_EN | 10000);
>
> - smc911x_reg_write(dev, TX_CFG, TX_CFG_TX_ON);
> + smc911x_reg_write(priv, TX_CFG, TX_CFG_TX_ON);
>
> /* no padding to start of packets */
> - smc911x_reg_write(dev, RX_CFG, 0);
> + smc911x_reg_write(priv, RX_CFG, 0);
>
> - smc911x_set_mac_csr(dev, MAC_CR, MAC_CR_TXEN | MAC_CR_RXEN |
> + smc911x_set_mac_csr(priv, MAC_CR, MAC_CR_TXEN | MAC_CR_RXEN |
> MAC_CR_HBDIS);
> }
>
> static int smc911x_init(struct eth_device *dev, bd_t * bd)
> {
> - struct chip_id *id = dev->priv;
> + struct smc911x_priv *priv = container_of(dev, struct smc911x_priv, dev);
> + const struct chip_id *id = priv->chipid;
>
> printf(DRIVERNAME ": detected %s controller\n", id->name);
>
> - smc911x_reset(dev);
> + smc911x_reset(priv);
>
> /* Configure the PHY, initialize the link state */
> - smc911x_phy_configure(dev);
> + smc911x_phy_configure(priv);
>
> - smc911x_handle_mac_address(dev);
> + smc911x_handle_mac_address(priv);
>
> /* Turn on Tx + Rx */
> - smc911x_enable(dev);
> + smc911x_enable(priv);
>
> return 0;
> }
>
> static int smc911x_send(struct eth_device *dev, void *packet, int length)
> {
> + struct smc911x_priv *priv = container_of(dev, struct smc911x_priv, dev);
> u32 *data = (u32*)packet;
> u32 tmplen;
> u32 status;
>
> - smc911x_reg_write(dev, TX_DATA_FIFO, TX_CMD_A_INT_FIRST_SEG |
> + smc911x_reg_write(priv, TX_DATA_FIFO, TX_CMD_A_INT_FIRST_SEG |
> TX_CMD_A_INT_LAST_SEG | length);
> - smc911x_reg_write(dev, TX_DATA_FIFO, length);
> + smc911x_reg_write(priv, TX_DATA_FIFO, length);
>
> tmplen = (length + 3) / 4;
>
> while (tmplen--)
> - smc911x_reg_write(dev, TX_DATA_FIFO, *data++);
> + smc911x_reg_write(priv, TX_DATA_FIFO, *data++);
>
> /* wait for transmission */
> - while (!((smc911x_reg_read(dev, TX_FIFO_INF) &
> + while (!((smc911x_reg_read(priv, TX_FIFO_INF) &
> TX_FIFO_INF_TSUSED) >> 16));
>
> /* get status. Ignore 'no carrier' error, it has no meaning for
> * full duplex operation
> */
> - status = smc911x_reg_read(dev, TX_STATUS_FIFO) &
> + status = smc911x_reg_read(priv, TX_STATUS_FIFO) &
> (TX_STS_LOC | TX_STS_LATE_COLL | TX_STS_MANY_COLL |
> TX_STS_MANY_DEFER | TX_STS_UNDERRUN);
>
> @@ -330,25 +339,28 @@ static int smc911x_send(struct eth_device *dev, void
> *packet, int length)
>
> static void smc911x_halt(struct eth_device *dev)
> {
> - smc911x_reset(dev);
> - smc911x_handle_mac_address(dev);
> + struct smc911x_priv *priv = container_of(dev, struct smc911x_priv, dev);
> +
> + smc911x_reset(priv);
> + smc911x_handle_mac_address(priv);
> }
>
> static int smc911x_recv(struct eth_device *dev)
> {
> + struct smc911x_priv *priv = container_of(dev, struct smc911x_priv, dev);
> u32 *data = (u32 *)net_rx_packets[0];
> u32 pktlen, tmplen;
> u32 status;
>
> - if ((smc911x_reg_read(dev, RX_FIFO_INF) & RX_FIFO_INF_RXSUSED) >> 16) {
> - status = smc911x_reg_read(dev, RX_STATUS_FIFO);
> + if ((smc911x_reg_read(priv, RX_FIFO_INF) & RX_FIFO_INF_RXSUSED) >> 16) {
> + status = smc911x_reg_read(priv, RX_STATUS_FIFO);
> pktlen = (status & RX_STS_PKT_LEN) >> 16;
>
> - smc911x_reg_write(dev, RX_CFG, 0);
> + smc911x_reg_write(priv, RX_CFG, 0);
>
> tmplen = (pktlen + 3) / 4;
> while (tmplen--)
> - *data++ = smc911x_reg_read(dev, RX_DATA_FIFO);
> + *data++ = smc911x_reg_read(priv, RX_DATA_FIFO);
>
> if (status & RX_STS_ES)
> printf(DRIVERNAME
> @@ -367,31 +379,34 @@ static int smc911x_miiphy_read(struct mii_dev *bus, int
> phy, int devad,
> int reg)
> {
> struct eth_device *dev = eth_get_dev_by_name(bus->name);
> + struct smc911x_priv *priv = container_of(dev, struct smc911x_priv, dev);
> u16 val = 0;
> int ret;
>
> - if (!dev)
> + if (!dev || !priv)
> return -ENODEV;
>
> - ret = smc911x_eth_phy_read(dev, phy, reg, &val);
> + ret = smc911x_eth_phy_read(priv, phy, reg, &val);
> if (ret < 0)
> return ret;
>
> return val;
> }
> +
> /* wrapper for smc911x_eth_phy_write */
> static int smc911x_miiphy_write(struct mii_dev *bus, int phy, int devad,
> int reg, u16 val)
> {
> struct eth_device *dev = eth_get_dev_by_name(bus->name);
> + struct smc911x_priv *priv = container_of(dev, struct smc911x_priv, dev);
>
> - if (!dev)
> + if (!dev || !priv)
> return -ENODEV;
>
> - return smc911x_eth_phy_write(dev, phy, reg, val);
> + return smc911x_eth_phy_write(priv, phy, reg, val);
> }
>
> -static int smc911x_initialize_mii(struct eth_device *dev)
> +static int smc911x_initialize_mii(struct smc911x_priv *priv)
> {
> struct mii_dev *mdiodev = mdio_alloc();
> int ret;
> @@ -399,7 +414,7 @@ static int smc911x_initialize_mii(struct eth_device *dev)
> if (!mdiodev)
> return -ENOMEM;
>
> - strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
> + strncpy(mdiodev->name, priv->dev.name, MDIO_NAME_LEN);
> mdiodev->read = smc911x_miiphy_read;
> mdiodev->write = smc911x_miiphy_write;
>
> @@ -412,7 +427,7 @@ static int smc911x_initialize_mii(struct eth_device *dev)
> return 0;
> }
> #else
> -static int smc911x_initialize_mii(struct eth_device *dev)
> +static int smc911x_initialize_mii(struct smc911x_priv *priv)
> {
> return 0;
> }
> @@ -421,51 +436,52 @@ static int smc911x_initialize_mii(struct eth_device
> *dev)
> int smc911x_initialize(u8 dev_num, int base_addr)
> {
> unsigned long addrl, addrh;
> - struct eth_device *dev;
> + struct smc911x_priv *priv;
> int ret;
>
> - dev = calloc(1, sizeof(*dev));
> + dev = calloc(1, sizeof(*priv));
> if (!dev)
> return -ENOMEM;
Shouldn't this be
priv = calloc(1, sizeof(*priv));
if (!priv)
return -ENOMEM;
?
> - dev->iobase = base_addr;
> + priv->iobase = base_addr;
> + priv->dev.iobase = base_addr;
>
> /* Try to detect chip. Will fail if not present. */
> - ret = smc911x_detect_chip(dev);
> + ret = smc911x_detect_chip(priv);
> if (ret) {
> ret = 0; /* Card not detected is not an error */
> goto err_detect;
> }
>
> - addrh = smc911x_get_mac_csr(dev, ADDRH);
> - addrl = smc911x_get_mac_csr(dev, ADDRL);
> + addrh = smc911x_get_mac_csr(priv, ADDRH);
> + addrl = smc911x_get_mac_csr(priv, ADDRL);
> if (!(addrl == 0xffffffff && addrh == 0x0000ffff)) {
> /* address is obtained from optional eeprom */
> - dev->enetaddr[0] = addrl;
> - dev->enetaddr[1] = addrl >> 8;
> - dev->enetaddr[2] = addrl >> 16;
> - dev->enetaddr[3] = addrl >> 24;
> - dev->enetaddr[4] = addrh;
> - dev->enetaddr[5] = addrh >> 8;
> + priv->enetaddr[0] = addrl;
> + priv->enetaddr[1] = addrl >> 8;
> + priv->enetaddr[2] = addrl >> 16;
> + priv->enetaddr[3] = addrl >> 24;
> + priv->enetaddr[4] = addrh;
> + priv->enetaddr[5] = addrh >> 8;
> }
>
> - dev->init = smc911x_init;
> - dev->halt = smc911x_halt;
> - dev->send = smc911x_send;
> - dev->recv = smc911x_recv;
> - sprintf(dev->name, "%s-%hu", DRIVERNAME, dev_num);
> + priv->dev.init = smc911x_init;
> + priv->dev.halt = smc911x_halt;
> + priv->dev.send = smc911x_send;
> + priv->dev.recv = smc911x_recv;
> + sprintf(priv->dev.name, "%s-%hu", DRIVERNAME, dev_num);
>
> - eth_register(dev);
> + eth_register(&priv->dev);
>
> - ret = smc911x_initialize_mii(dev);
> + ret = smc911x_initialize_mii(priv);
> if (ret)
> goto err_mii;
>
> return 1;
>
> err_mii:
> - eth_unregister(dev);
> + eth_unregister(&priv->dev);
> err_detect:
> - free(dev);
> + free(priv);
> return ret;
> }
--
Harald
