| Message ID | 1443692893-19905-4-git-send-email-sjoerd.simons@collabora.co.uk |
|---|---|
| State | New |
| Headers | show |
Hi Sjoerd, On Thu, Oct 1, 2015 at 5:48 PM, Sjoerd Simons <sjoerd.simons@collabora.co.uk> wrote: > Add support for the snps,reset-gpio, snps,reset-active-low (optional) and > snps,reset-delays-us device-tree bindings. The combination of these > three define how the PHY should be reset to ensure it's in a sane state. > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > --- > > drivers/net/designware.c | 74 ++++++++++++++++++++++++++++++++++++++++++++---- > drivers/net/designware.h | 11 +++++++ > 2 files changed, 79 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/designware.c b/drivers/net/designware.c > index 6433896..f9ceec2 100644 > --- a/drivers/net/designware.c > +++ b/drivers/net/designware.c > @@ -28,7 +28,12 @@ DECLARE_GLOBAL_DATA_PTR; > > static int dw_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) > { > +#ifdef CONFIG_DM_ETH > + struct dw_eth_dev *priv = dev_get_priv((struct udevice *)bus->priv); > + struct eth_mac_regs *mac_p = priv->mac_regs_p; > +#else > struct eth_mac_regs *mac_p = bus->priv; > +#endif > ulong start; > u16 miiaddr; > int timeout = CONFIG_MDIO_TIMEOUT; > @@ -51,7 +56,12 @@ static int dw_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) > static int dw_mdio_write(struct mii_dev *bus, int addr, int devad, int reg, > u16 val) > { > +#ifdef CONFIG_DM_ETH > + struct dw_eth_dev *priv = dev_get_priv((struct udevice *)bus->priv); > + struct eth_mac_regs *mac_p = priv->mac_regs_p; > +#else > struct eth_mac_regs *mac_p = bus->priv; > +#endif > ulong start; > u16 miiaddr; > int ret = -ETIMEDOUT, timeout = CONFIG_MDIO_TIMEOUT; > @@ -74,7 +84,40 @@ static int dw_mdio_write(struct mii_dev *bus, int addr, int devad, int reg, > return ret; > } > > -static int dw_mdio_init(const char *name, struct eth_mac_regs *mac_regs_p) > +#if CONFIG_DM_ETH > +static int dw_mdio_reset(struct mii_dev *bus) > +{ > + struct udevice *dev = bus->priv; > + struct dw_eth_pdata *pdata = dev_get_platdata(dev); > + int ret; > + > + if (pdata->reset_gpio.dev == NULL) > + return 0; > + > + /* reset the phy */ > + ret = dm_gpio_set_value(&pdata->reset_gpio, 0); > + if (ret) > + return ret; > + Is this PHY reset a must every time we start the MAC? If this is just one-time reset, can it be moved to board-specific codes? > + udelay(pdata->reset_delays[0]); > + > + ret = dm_gpio_set_value(&pdata->reset_gpio, 1); > + if (ret) > + return ret; > + > + udelay(pdata->reset_delays[1]); > + > + ret = dm_gpio_set_value(&pdata->reset_gpio, 0); > + if (ret) > + return ret; > + > + udelay(pdata->reset_delays[2]); > + > + return 0; > +} > +#endif > + > +static int dw_mdio_init(const char *name, void *priv) > { > struct mii_dev *bus = mdio_alloc(); > > @@ -85,9 +128,12 @@ static int dw_mdio_init(const char *name, struct eth_mac_regs *mac_regs_p) > > bus->read = dw_mdio_read; > bus->write = dw_mdio_write; > +#ifdef CONFIG_DM_ETH > + bus->reset = dw_mdio_reset; > +#endif > snprintf(bus->name, sizeof(bus->name), name); > > - bus->priv = (void *)mac_regs_p; > + bus->priv = priv; > > return mdio_register(bus); > } > @@ -604,7 +650,7 @@ static int designware_eth_probe(struct udevice *dev) > priv->dma_regs_p = (struct eth_dma_regs *)(iobase + DW_DMA_BASE_OFFSET); > priv->interface = pdata->phy_interface; > > - dw_mdio_init(dev->name, priv->mac_regs_p); > + dw_mdio_init(dev->name, dev); > priv->bus = miiphy_get_dev_by_name(dev->name); > > ret = dw_phy_init(priv, dev); > @@ -624,8 +670,11 @@ static const struct eth_ops designware_eth_ops = { > > static int designware_eth_ofdata_to_platdata(struct udevice *dev) > { > - struct eth_pdata *pdata = dev_get_platdata(dev); > + struct dw_eth_pdata *dw_pdata = dev_get_platdata(dev); > + struct eth_pdata *pdata = &dw_pdata->eth_pdata; > const char *phy_mode; > + int reset_flags = GPIOD_IS_OUT; > + int ret = 0; > > pdata->iobase = dev_get_addr(dev); > pdata->phy_interface = -1; > @@ -637,7 +686,20 @@ static int designware_eth_ofdata_to_platdata(struct udevice *dev) > return -EINVAL; > } > > - return 0; > + if (fdtdec_get_bool(gd->fdt_blob, dev->of_offset, > + "snps,reset-active-low")) > + reset_flags |= GPIOD_ACTIVE_LOW; > + > + ret = gpio_request_by_name(dev, "snps,reset-gpio", 0, > + &dw_pdata->reset_gpio, reset_flags); > + if (ret == 0) { > + ret = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset, > + "snps,reset-delays-us", dw_pdata->reset_delays, 3); > + } else if (ret == -ENOENT) { > + ret = 0; > + } > + > + return ret; > } > > static const struct udevice_id designware_eth_ids[] = { > @@ -655,7 +717,7 @@ U_BOOT_DRIVER(eth_designware) = { > .probe = designware_eth_probe, > .ops = &designware_eth_ops, > .priv_auto_alloc_size = sizeof(struct dw_eth_dev), > - .platdata_auto_alloc_size = sizeof(struct eth_pdata), > + .platdata_auto_alloc_size = sizeof(struct dw_eth_pdata), > .flags = DM_FLAG_ALLOC_PRIV_DMA, > }; > > diff --git a/drivers/net/designware.h b/drivers/net/designware.h > index 4b9ec39..f34263b 100644 > --- a/drivers/net/designware.h > +++ b/drivers/net/designware.h > @@ -8,6 +8,9 @@ > #ifndef _DW_ETH_H > #define _DW_ETH_H > > +#include <common.h> <common.h> should not be in a header file. > +#include <asm/gpio.h> > + > #define CONFIG_TX_DESCR_NUM 16 > #define CONFIG_RX_DESCR_NUM 16 > #define CONFIG_ETH_BUFSIZE 2048 > @@ -235,4 +238,12 @@ struct dw_eth_dev { > struct mii_dev *bus; > }; > > +#ifdef CONFIG_DM_ETH > +struct dw_eth_pdata { > + struct eth_pdata eth_pdata; > + struct gpio_desc reset_gpio; > + u32 reset_delays[3]; > +}; > +#endif > + > #endif > -- Regards, Bin
On Thu, 2015-10-01 at 19:02 +0800, Bin Meng wrote: > Hi Sjoerd, > > On Thu, Oct 1, 2015 at 5:48 PM, Sjoerd Simons > <sjoerd.simons@collabora.co.uk> wrote: > > -static int dw_mdio_init(const char *name, struct eth_mac_regs > > *mac_regs_p) > > +#if CONFIG_DM_ETH > > +static int dw_mdio_reset(struct mii_dev *bus) > > +{ > > + struct udevice *dev = bus->priv; > > + struct dw_eth_pdata *pdata = dev_get_platdata(dev); > > + int ret; > > + > > + if (pdata->reset_gpio.dev == NULL) > > + return 0; > > + > > + /* reset the phy */ > > + ret = dm_gpio_set_value(&pdata->reset_gpio, 0); > > + if (ret) > > + return ret; > > + > > Is this PHY reset a must every time we start the MAC? If this is just > one-time reset, can it be moved to board-specific codes? The MDIO bus reset is only triggered from dw_phy_init, which is called by the probe function in a DM build. So that should already only run once, instead of at every MAC start. In any case, the whole point of implementing it here is that it's a common device-tree binding for all designware macs so it's much nice to implement it in a central place rather then duplicating it for all boards in board-specific code :)
Hi Sjoerd, On 1 October 2015 at 10:48, Sjoerd Simons <sjoerd.simons@collabora.co.uk> wrote: > Add support for the snps,reset-gpio, snps,reset-active-low (optional) and > snps,reset-delays-us device-tree bindings. The combination of these > three define how the PHY should be reset to ensure it's in a sane state. > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > --- > > drivers/net/designware.c | 74 ++++++++++++++++++++++++++++++++++++++++++++---- > drivers/net/designware.h | 11 +++++++ > 2 files changed, 79 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/designware.c b/drivers/net/designware.c > index 6433896..f9ceec2 100644 > --- a/drivers/net/designware.c > +++ b/drivers/net/designware.c > @@ -28,7 +28,12 @@ DECLARE_GLOBAL_DATA_PTR; > > static int dw_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) > { > +#ifdef CONFIG_DM_ETH > + struct dw_eth_dev *priv = dev_get_priv((struct udevice *)bus->priv); > + struct eth_mac_regs *mac_p = priv->mac_regs_p; > +#else > struct eth_mac_regs *mac_p = bus->priv; > +#endif > ulong start; > u16 miiaddr; > int timeout = CONFIG_MDIO_TIMEOUT; > @@ -51,7 +56,12 @@ static int dw_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) > static int dw_mdio_write(struct mii_dev *bus, int addr, int devad, int reg, > u16 val) > { > +#ifdef CONFIG_DM_ETH > + struct dw_eth_dev *priv = dev_get_priv((struct udevice *)bus->priv); > + struct eth_mac_regs *mac_p = priv->mac_regs_p; > +#else > struct eth_mac_regs *mac_p = bus->priv; > +#endif > ulong start; > u16 miiaddr; > int ret = -ETIMEDOUT, timeout = CONFIG_MDIO_TIMEOUT; > @@ -74,7 +84,40 @@ static int dw_mdio_write(struct mii_dev *bus, int addr, int devad, int reg, > return ret; > } > > -static int dw_mdio_init(const char *name, struct eth_mac_regs *mac_regs_p) > +#if CONFIG_DM_ETH > +static int dw_mdio_reset(struct mii_dev *bus) > +{ > + struct udevice *dev = bus->priv; > + struct dw_eth_pdata *pdata = dev_get_platdata(dev); > + int ret; > + > + if (pdata->reset_gpio.dev == NULL) !dm_gpio_is_valid(&pdata->reset_gpio) > + return 0; > + > + /* reset the phy */ > + ret = dm_gpio_set_value(&pdata->reset_gpio, 0); > + if (ret) > + return ret; > + > + udelay(pdata->reset_delays[0]); > + > + ret = dm_gpio_set_value(&pdata->reset_gpio, 1); > + if (ret) > + return ret; > + > + udelay(pdata->reset_delays[1]); > + > + ret = dm_gpio_set_value(&pdata->reset_gpio, 0); > + if (ret) > + return ret; > + > + udelay(pdata->reset_delays[2]); > + > + return 0; > +} > +#endif > + > +static int dw_mdio_init(const char *name, void *priv) > { > struct mii_dev *bus = mdio_alloc(); > > @@ -85,9 +128,12 @@ static int dw_mdio_init(const char *name, struct eth_mac_regs *mac_regs_p) > > bus->read = dw_mdio_read; > bus->write = dw_mdio_write; > +#ifdef CONFIG_DM_ETH > + bus->reset = dw_mdio_reset; > +#endif > snprintf(bus->name, sizeof(bus->name), name); > > - bus->priv = (void *)mac_regs_p; > + bus->priv = priv; > > return mdio_register(bus); > } > @@ -604,7 +650,7 @@ static int designware_eth_probe(struct udevice *dev) > priv->dma_regs_p = (struct eth_dma_regs *)(iobase + DW_DMA_BASE_OFFSET); > priv->interface = pdata->phy_interface; > > - dw_mdio_init(dev->name, priv->mac_regs_p); > + dw_mdio_init(dev->name, dev); > priv->bus = miiphy_get_dev_by_name(dev->name); > > ret = dw_phy_init(priv, dev); > @@ -624,8 +670,11 @@ static const struct eth_ops designware_eth_ops = { > > static int designware_eth_ofdata_to_platdata(struct udevice *dev) > { > - struct eth_pdata *pdata = dev_get_platdata(dev); > + struct dw_eth_pdata *dw_pdata = dev_get_platdata(dev); > + struct eth_pdata *pdata = &dw_pdata->eth_pdata; > const char *phy_mode; > + int reset_flags = GPIOD_IS_OUT; > + int ret = 0; > > pdata->iobase = dev_get_addr(dev); > pdata->phy_interface = -1; > @@ -637,7 +686,20 @@ static int designware_eth_ofdata_to_platdata(struct udevice *dev) > return -EINVAL; > } > > - return 0; > + if (fdtdec_get_bool(gd->fdt_blob, dev->of_offset, > + "snps,reset-active-low")) > + reset_flags |= GPIOD_ACTIVE_LOW; > + > + ret = gpio_request_by_name(dev, "snps,reset-gpio", 0, > + &dw_pdata->reset_gpio, reset_flags); It's odd to put a probed gpio into platdata. Should you use private data instead? (dev_get_priv()) > + if (ret == 0) { > + ret = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset, > + "snps,reset-delays-us", dw_pdata->reset_delays, 3); > + } else if (ret == -ENOENT) { > + ret = 0; > + } > + > + return ret; > } > > static const struct udevice_id designware_eth_ids[] = { > @@ -655,7 +717,7 @@ U_BOOT_DRIVER(eth_designware) = { > .probe = designware_eth_probe, > .ops = &designware_eth_ops, > .priv_auto_alloc_size = sizeof(struct dw_eth_dev), > - .platdata_auto_alloc_size = sizeof(struct eth_pdata), > + .platdata_auto_alloc_size = sizeof(struct dw_eth_pdata), > .flags = DM_FLAG_ALLOC_PRIV_DMA, > }; > > diff --git a/drivers/net/designware.h b/drivers/net/designware.h > index 4b9ec39..f34263b 100644 > --- a/drivers/net/designware.h > +++ b/drivers/net/designware.h > @@ -8,6 +8,9 @@ > #ifndef _DW_ETH_H > #define _DW_ETH_H > > +#include <common.h> Drop that one... > +#include <asm/gpio.h> > + > #define CONFIG_TX_DESCR_NUM 16 > #define CONFIG_RX_DESCR_NUM 16 > #define CONFIG_ETH_BUFSIZE 2048 > @@ -235,4 +238,12 @@ struct dw_eth_dev { > struct mii_dev *bus; > }; > > +#ifdef CONFIG_DM_ETH Do you need this #ifdef > +struct dw_eth_pdata { > + struct eth_pdata eth_pdata; > + struct gpio_desc reset_gpio; > + u32 reset_delays[3]; > +}; > +#endif > + > #endif > -- > 2.5.3 > Regards, Simon
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 6433896..f9ceec2 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -28,7 +28,12 @@ DECLARE_GLOBAL_DATA_PTR; static int dw_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) { +#ifdef CONFIG_DM_ETH + struct dw_eth_dev *priv = dev_get_priv((struct udevice *)bus->priv); + struct eth_mac_regs *mac_p = priv->mac_regs_p; +#else struct eth_mac_regs *mac_p = bus->priv; +#endif ulong start; u16 miiaddr; int timeout = CONFIG_MDIO_TIMEOUT; @@ -51,7 +56,12 @@ static int dw_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) static int dw_mdio_write(struct mii_dev *bus, int addr, int devad, int reg, u16 val) { +#ifdef CONFIG_DM_ETH + struct dw_eth_dev *priv = dev_get_priv((struct udevice *)bus->priv); + struct eth_mac_regs *mac_p = priv->mac_regs_p; +#else struct eth_mac_regs *mac_p = bus->priv; +#endif ulong start; u16 miiaddr; int ret = -ETIMEDOUT, timeout = CONFIG_MDIO_TIMEOUT; @@ -74,7 +84,40 @@ static int dw_mdio_write(struct mii_dev *bus, int addr, int devad, int reg, return ret; } -static int dw_mdio_init(const char *name, struct eth_mac_regs *mac_regs_p) +#if CONFIG_DM_ETH +static int dw_mdio_reset(struct mii_dev *bus) +{ + struct udevice *dev = bus->priv; + struct dw_eth_pdata *pdata = dev_get_platdata(dev); + int ret; + + if (pdata->reset_gpio.dev == NULL) + return 0; + + /* reset the phy */ + ret = dm_gpio_set_value(&pdata->reset_gpio, 0); + if (ret) + return ret; + + udelay(pdata->reset_delays[0]); + + ret = dm_gpio_set_value(&pdata->reset_gpio, 1); + if (ret) + return ret; + + udelay(pdata->reset_delays[1]); + + ret = dm_gpio_set_value(&pdata->reset_gpio, 0); + if (ret) + return ret; + + udelay(pdata->reset_delays[2]); + + return 0; +} +#endif + +static int dw_mdio_init(const char *name, void *priv) { struct mii_dev *bus = mdio_alloc(); @@ -85,9 +128,12 @@ static int dw_mdio_init(const char *name, struct eth_mac_regs *mac_regs_p) bus->read = dw_mdio_read; bus->write = dw_mdio_write; +#ifdef CONFIG_DM_ETH + bus->reset = dw_mdio_reset; +#endif snprintf(bus->name, sizeof(bus->name), name); - bus->priv = (void *)mac_regs_p; + bus->priv = priv; return mdio_register(bus); } @@ -604,7 +650,7 @@ static int designware_eth_probe(struct udevice *dev) priv->dma_regs_p = (struct eth_dma_regs *)(iobase + DW_DMA_BASE_OFFSET); priv->interface = pdata->phy_interface; - dw_mdio_init(dev->name, priv->mac_regs_p); + dw_mdio_init(dev->name, dev); priv->bus = miiphy_get_dev_by_name(dev->name); ret = dw_phy_init(priv, dev); @@ -624,8 +670,11 @@ static const struct eth_ops designware_eth_ops = { static int designware_eth_ofdata_to_platdata(struct udevice *dev) { - struct eth_pdata *pdata = dev_get_platdata(dev); + struct dw_eth_pdata *dw_pdata = dev_get_platdata(dev); + struct eth_pdata *pdata = &dw_pdata->eth_pdata; const char *phy_mode; + int reset_flags = GPIOD_IS_OUT; + int ret = 0; pdata->iobase = dev_get_addr(dev); pdata->phy_interface = -1; @@ -637,7 +686,20 @@ static int designware_eth_ofdata_to_platdata(struct udevice *dev) return -EINVAL; } - return 0; + if (fdtdec_get_bool(gd->fdt_blob, dev->of_offset, + "snps,reset-active-low")) + reset_flags |= GPIOD_ACTIVE_LOW; + + ret = gpio_request_by_name(dev, "snps,reset-gpio", 0, + &dw_pdata->reset_gpio, reset_flags); + if (ret == 0) { + ret = fdtdec_get_int_array(gd->fdt_blob, dev->of_offset, + "snps,reset-delays-us", dw_pdata->reset_delays, 3); + } else if (ret == -ENOENT) { + ret = 0; + } + + return ret; } static const struct udevice_id designware_eth_ids[] = { @@ -655,7 +717,7 @@ U_BOOT_DRIVER(eth_designware) = { .probe = designware_eth_probe, .ops = &designware_eth_ops, .priv_auto_alloc_size = sizeof(struct dw_eth_dev), - .platdata_auto_alloc_size = sizeof(struct eth_pdata), + .platdata_auto_alloc_size = sizeof(struct dw_eth_pdata), .flags = DM_FLAG_ALLOC_PRIV_DMA, }; diff --git a/drivers/net/designware.h b/drivers/net/designware.h index 4b9ec39..f34263b 100644 --- a/drivers/net/designware.h +++ b/drivers/net/designware.h @@ -8,6 +8,9 @@ #ifndef _DW_ETH_H #define _DW_ETH_H +#include <common.h> +#include <asm/gpio.h> + #define CONFIG_TX_DESCR_NUM 16 #define CONFIG_RX_DESCR_NUM 16 #define CONFIG_ETH_BUFSIZE 2048 @@ -235,4 +238,12 @@ struct dw_eth_dev { struct mii_dev *bus; }; +#ifdef CONFIG_DM_ETH +struct dw_eth_pdata { + struct eth_pdata eth_pdata; + struct gpio_desc reset_gpio; + u32 reset_delays[3]; +}; +#endif + #endif
Add support for the snps,reset-gpio, snps,reset-active-low (optional) and snps,reset-delays-us device-tree bindings. The combination of these three define how the PHY should be reset to ensure it's in a sane state. Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> --- drivers/net/designware.c | 74 ++++++++++++++++++++++++++++++++++++++++++++---- drivers/net/designware.h | 11 +++++++ 2 files changed, 79 insertions(+), 6 deletions(-)