[U-Boot] [PATCH v5 2/5] net: fec_mxc: Convert into driver model

Jagan Teki jagannadh.teki at gmail.com
Wed Oct 12 18:12:18 CEST 2016


On Wed, Oct 12, 2016 at 2:15 AM, Joe Hershberger
<joe.hershberger at gmail.com> wrote:
> On Thu, Oct 6, 2016 at 12:55 PM, Jagan Teki <jteki at openedev.com> wrote:
>> From: Jagan Teki <jagan at amarulasolutions.com>
>>
>> This patch add driver model support for fec_mxc driver.
>>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Joe Hershberger <joe.hershberger at ni.com>
>> Cc: Peng Fan <peng.fan at nxp.com>
>> Cc: Stefano Babic <sbabic at denx.de>
>> Cc: Michael Trimarchi <michael at amarulasolutions.com>
>> Signed-off-by: Jagan Teki <jagan at amarulasolutions.com>
>> ---
>>  drivers/net/fec_mxc.c | 314 ++++++++++++++++++++++++++++++++++++++++++--------
>>  drivers/net/fec_mxc.h |   4 +
>>  2 files changed, 271 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
>> index 0838d58..c0ec976 100644
>> --- a/drivers/net/fec_mxc.c
>> +++ b/drivers/net/fec_mxc.c
>> @@ -9,6 +9,7 @@
>>   */
>>
>>  #include <common.h>
>> +#include <dm.h>
>>  #include <malloc.h>
>>  #include <memalign.h>
>>  #include <net.h>
>> @@ -368,11 +369,8 @@ static int fec_get_hwaddr(int dev_id, unsigned char *mac)
>>         return !is_valid_ethaddr(mac);
>>  }
>>
>> -static int fec_set_hwaddr(struct eth_device *dev)
>> +static int _fec_set_hwaddr(struct fec_priv *fec, uchar *mac)
>>  {
>> -       uchar *mac = dev->enetaddr;
>> -       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>> -
>>         writel(0, &fec->eth->iaddr1);
>>         writel(0, &fec->eth->iaddr2);
>>         writel(0, &fec->eth->gaddr1);
>> @@ -426,9 +424,8 @@ static void fec_reg_setup(struct fec_priv *fec)
>>   * Start the FEC engine
>>   * @param[in] dev Our device to handle
>>   */
>> -static int fec_open(struct eth_device *edev)
>> +static int fec_open(struct fec_priv *fec)
>>  {
>> -       struct fec_priv *fec = (struct fec_priv *)edev->priv;
>>         int speed;
>>         uint32_t addr, size;
>>         int i;
>> @@ -534,14 +531,13 @@ static int fec_open(struct eth_device *edev)
>>         return 0;
>>  }
>>
>> -static int fec_init(struct eth_device *dev, bd_t* bd)
>> +static int _fec_init(struct fec_priv *fec, uchar *mac)
>>  {
>> -       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>>         uint32_t mib_ptr = (uint32_t)&fec->eth->rmon_t_drop;
>>         int i;
>>
>>         /* Initialize MAC address */
>> -       fec_set_hwaddr(dev);
>> +       _fec_set_hwaddr(fec, mac);
>>
>>         /*
>>          * Setup transmit descriptors, there are two in total.
>> @@ -587,7 +583,7 @@ static int fec_init(struct eth_device *dev, bd_t* bd)
>>         if (fec->xcv_type != SEVENWIRE)
>>                 miiphy_restart_aneg(dev);
>>  #endif
>> -       fec_open(dev);
>> +       fec_open(fec);
>>         return 0;
>>  }
>>
>> @@ -595,9 +591,8 @@ static int fec_init(struct eth_device *dev, bd_t* bd)
>>   * Halt the FEC engine
>>   * @param[in] dev Our device to handle
>>   */
>> -static void fec_halt(struct eth_device *dev)
>> +static void _fec_halt(struct fec_priv *fec)
>>  {
>> -       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>>         int counter = 0xffff;
>>
>>         /*
>> @@ -637,7 +632,7 @@ static void fec_halt(struct eth_device *dev)
>>   * @param[in] length Data count in bytes
>>   * @return 0 on success
>>   */
>> -static int fec_send(struct eth_device *dev, void *packet, int length)
>> +static int _fec_send(struct fec_priv *fec, void *packet, int length)
>>  {
>>         unsigned int status;
>>         uint32_t size, end;
>> @@ -649,8 +644,6 @@ static int fec_send(struct eth_device *dev, void *packet, int length)
>>          * This routine transmits one frame.  This routine only accepts
>>          * 6-byte Ethernet addresses.
>>          */
>> -       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>> -
>>         /*
>>          * Check for valid length of data.
>>          */
>> @@ -777,14 +770,14 @@ out:
>>         return ret;
>>  }
>>
>> +
>>  /**
>>   * Pull one frame from the card
>>   * @param[in] dev Our ethernet device to handle
>>   * @return Length of packet read
>>   */
>> -static int fec_recv(struct eth_device *dev)
>> +static int _fec_recv(struct fec_priv *fec, uchar *mac)
>>  {
>> -       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>>         struct fec_bd *rbd = &fec->rbd_base[fec->rbd_index];
>>         unsigned long ievent;
>>         int frame_length, len = 0;
>> @@ -800,8 +793,8 @@ static int fec_recv(struct eth_device *dev)
>>         writel(ievent, &fec->eth->ievent);
>>         debug("fec_recv: ievent 0x%lx\n", ievent);
>>         if (ievent & FEC_IEVENT_BABR) {
>> -               fec_halt(dev);
>> -               fec_init(dev, fec->bd);
>> +               _fec_halt(fec);
>> +               _fec_init(fec, mac);
>>                 printf("some error: 0x%08lx\n", ievent);
>>                 return 0;
>>         }
>> @@ -813,10 +806,10 @@ static int fec_recv(struct eth_device *dev)
>>         if (ievent & FEC_IEVENT_GRA) {
>>                 /* Graceful stop complete */
>>                 if (readl(&fec->eth->x_cntrl) & 0x00000001) {
>> -                       fec_halt(dev);
>> +                       _fec_halt(fec);
>>                         writel(~0x00000001 & readl(&fec->eth->x_cntrl),
>>                                         &fec->eth->x_cntrl);
>> -                       fec_init(dev, fec->bd);
>> +                       _fec_init(fec, mac);
>>                 }
>>         }
>>
>> @@ -970,6 +963,71 @@ static void fec_free_descs(struct fec_priv *fec)
>>         free(fec->tbd_base);
>>  }
>>
>> +struct mii_dev *fec_get_miibus(uint32_t base_addr, int dev_id)
>> +{
>> +       struct ethernet_regs *eth = (struct ethernet_regs *)base_addr;
>> +       struct mii_dev *bus;
>> +       int ret;
>> +
>> +       bus = mdio_alloc();
>> +       if (!bus) {
>> +               printf("mdio_alloc failed\n");
>> +               return NULL;
>> +       }
>> +       bus->read = fec_phy_read;
>> +       bus->write = fec_phy_write;
>> +       bus->priv = eth;
>> +       fec_set_dev_name(bus->name, dev_id);
>> +
>> +       ret = mdio_register(bus);
>> +       if (ret) {
>> +               printf("mdio_register failed\n");
>> +               free(bus);
>> +               return NULL;
>> +       }
>> +       fec_mii_setspeed(eth);
>> +       return bus;
>> +}
>> +
>> +#ifndef CONFIG_DM_ETH
>
> Use positive logic where possible.
> #ifdef CONFIG_DM_ETH
> ...
> #else /* !CONFIG_DM_ETH */
> ...
> #endif

It look easy to find the driver has dm support when we add dm code at
the end and once all set we can anyway remove non-dm code.

>
>> +static int fec_recv(struct eth_device *dev)
>> +{
>> +       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>> +       uchar *mac = dev->enetaddr;
>> +
>> +       return _fec_recv(fec, mac);
>> +}
>> +
>> +static int fec_send(struct eth_device *dev, void *packet, int length)
>> +{
>> +       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>> +
>> +       return _fec_send(fec, packet, length);
>> +}
>> +
>> +static void fec_halt(struct eth_device *dev)
>> +{
>> +       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>> +
>> +       _fec_halt(fec);
>> +}
>> +
>> +static int fec_set_hwaddr(struct eth_device *dev)
>> +{
>> +       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>> +       uchar *mac = dev->enetaddr;
>> +
>> +       return _fec_set_hwaddr(fec, mac);
>> +}
>> +
>> +static int fec_init(struct eth_device *dev, bd_t *bd)
>> +{
>> +       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>> +       uchar *mac = dev->enetaddr;
>> +
>> +       return _fec_init(fec, mac);
>> +}
>> +
>>  #ifdef CONFIG_PHYLIB
>>  int fec_probe(bd_t *bd, int dev_id, uint32_t base_addr,
>>                 struct mii_dev *bus, struct phy_device *phydev)
>> @@ -1061,32 +1119,6 @@ err1:
>>         return ret;
>>  }
>>
>> -struct mii_dev *fec_get_miibus(uint32_t base_addr, int dev_id)
>> -{
>> -       struct ethernet_regs *eth = (struct ethernet_regs *)base_addr;
>> -       struct mii_dev *bus;
>> -       int ret;
>> -
>> -       bus = mdio_alloc();
>> -       if (!bus) {
>> -               printf("mdio_alloc failed\n");
>> -               return NULL;
>> -       }
>> -       bus->read = fec_phy_read;
>> -       bus->write = fec_phy_write;
>> -       bus->priv = eth;
>> -       fec_set_dev_name(bus->name, dev_id);
>> -
>> -       ret = mdio_register(bus);
>> -       if (ret) {
>> -               printf("mdio_register failed\n");
>> -               free(bus);
>> -               return NULL;
>> -       }
>> -       fec_mii_setspeed(eth);
>> -       return bus;
>> -}
>> -
>>  int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
>>  {
>>         uint32_t base_mii;
>> @@ -1146,3 +1178,191 @@ int fecmxc_register_mii_postcall(struct eth_device *dev, int (*cb)(int))
>>         return 0;
>>  }
>>  #endif
>> +
>> +#else
>> +
>> +static int fec_set_hwaddr(struct udevice *dev)
>> +{
>> +       struct fec_priv *fec = dev_get_priv(dev);
>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> +       uchar *mac = pdata->enetaddr;
>> +
>> +       return _fec_set_hwaddr(fec, mac);
>> +}
>> +
>> +static void fec_halt(struct udevice *dev)
>> +{
>> +       struct fec_priv *fec = dev_get_priv(dev);
>> +
>> +       _fec_halt(fec);
>> +}
>> +
>> +static int fec_recv(struct udevice *dev, int flags, uchar **packetp)
>> +{
>> +       struct fec_priv *fec = dev_get_priv(dev);
>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> +       uchar *mac = pdata->enetaddr;
>> +
>> +       return _fec_recv(fec, mac);
>> +}
>> +
>> +static int fec_send(struct udevice *dev, void *packet, int length)
>> +{
>> +       struct fec_priv *fec = dev_get_priv(dev);
>> +
>> +       return _fec_send(fec, packet, length);
>> +}
>> +
>> +static int fec_init(struct udevice *dev)
>> +{
>> +       struct fec_priv *fec = dev_get_priv(dev);
>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> +       uchar *mac = pdata->enetaddr;
>> +
>> +       return _fec_init(fec, mac);
>> +}
>> +
>> +static const struct eth_ops fecmxc_ops = {
>> +       .start                  = fec_init,
>> +       .send                   = fec_send,
>> +       .recv                   = fec_recv,
>> +       .stop                   = fec_halt,
>> +       .write_hwaddr           = fec_set_hwaddr,
>> +};
>> +
>> +static int fec_phy_init(struct fec_priv *priv, struct udevice *dev)
>> +{
>> +       struct phy_device *phydev;
>> +       int mask = 0xffffffff;
>> +
>> +#ifdef CONFIG_PHYLIB
>> +       mask = 1 << CONFIG_FEC_MXC_PHYADDR;
>> +#endif
>> +
>> +       phydev = phy_find_by_mask(priv->bus, mask, priv->interface);
>> +       if (!phydev)
>> +               return -ENODEV;
>> +
>> +       phy_connect_dev(phydev, dev);
>> +
>> +       priv->phydev = phydev;
>> +       phy_config(phydev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int fecmxc_probe(struct udevice *dev)
>> +{
>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> +       struct fec_priv *priv = dev_get_priv(dev);
>> +       struct mii_dev *bus = NULL;
>> +       int dev_id = -1;
>> +       unsigned char ethaddr[6];
>> +       uint32_t start;
>> +       int ret;
>> +
>> +       ret = fec_alloc_descs(priv);
>> +       if (ret)
>> +               return ret;
>> +
>> +       bus = fec_get_miibus((uint32_t)priv->eth, dev_id);
>> +       if (!bus)
>> +               goto err_mii;
>> +
>> +       priv->bus = bus;
>> +       priv->xcv_type = CONFIG_FEC_XCV_TYPE;
>> +       priv->interface = pdata->phy_interface;
>> +       ret = fec_phy_init(priv, dev);
>> +       if (ret)
>> +               goto err_phy;
>> +
>> +       /* Reset chip. */
>> +       writel(readl(&priv->eth->ecntrl) | FEC_ECNTRL_RESET, &priv->eth->ecntrl);
>> +       start = get_timer(0);
>> +       while (readl(&priv->eth->ecntrl) & FEC_ECNTRL_RESET) {
>> +               if (get_timer(start) > (CONFIG_SYS_HZ * 5)) {
>> +                       printf("FEC MXC: Timeout reseting chip\n");
>> +                       goto err_timeout;
>> +               }
>> +               udelay(10);
>> +       }
>> +
>> +       fec_reg_setup(priv);
>> +       fec_set_dev_name((char *)dev->name, dev_id);
>> +       priv->dev_id = (dev_id == -1) ? 0 : dev_id;
>> +
>> +       ret = fec_get_hwaddr(dev_id, ethaddr);
>
> Is there a reason not to have this registered as ops->read_rom_hwaddr?

post-poned to add it later has get_hwaddr already in non-dm area.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.


More information about the U-Boot mailing list