| Message ID | 20181026033816.7544-6-grygorii.strashko@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | driver: net: ti: clean up and code optimization | expand |
On Thu, Oct 25, 2018 at 10:38:14PM -0500, Grygorii Strashko wrote: > All existing TI SoCs network HW have similar MDIO implementation, so > introduce common mdio support library which can be reused by TI networking > drivers. > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> Reviewed-by: Tom Rini <trini@konsulko.com> -- Tom
On 10/26/18 5:38 AM, Grygorii Strashko wrote: > All existing TI SoCs network HW have similar MDIO implementation, so > introduce common mdio support library which can be reused by TI networking > drivers. > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > --- > drivers/net/ti/Makefile | 2 +- > drivers/net/ti/cpsw_mdio.c | 205 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/net/ti/cpsw_mdio.h | 18 ++++ > 3 files changed, 224 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/ti/cpsw_mdio.c > create mode 100644 drivers/net/ti/cpsw_mdio.h > Hi, why we don't combine this with: http://patchwork.ozlabs.org/patch/939726/ ? looks much more elegant to me. cheers, Hannes
On Mon, Oct 29, 2018 at 8:04 AM Hannes Schmelzer <hannes@schmelzer.or.at> wrote: > > > On 10/26/18 5:38 AM, Grygorii Strashko wrote: > > All existing TI SoCs network HW have similar MDIO implementation, so > > introduce common mdio support library which can be reused by TI networking > > drivers. > > > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > > --- > > drivers/net/ti/Makefile | 2 +- > > drivers/net/ti/cpsw_mdio.c | 205 +++++++++++++++++++++++++++++++++++++++++++++ > > drivers/net/ti/cpsw_mdio.h | 18 ++++ > > 3 files changed, 224 insertions(+), 1 deletion(-) > > create mode 100644 drivers/net/ti/cpsw_mdio.c > > create mode 100644 drivers/net/ti/cpsw_mdio.h > > > Hi, > why we don't combine this with: > http://patchwork.ozlabs.org/patch/939726/ ? looks much more elegant to me. I agree we should be moving that direction, but consolidating TI MDIO implementation is fairly orthogonal. Also haven't heard back from Ken on feedback to that patch. > cheers, > Hannes > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Thu, Oct 25, 2018 at 10:39 PM Grygorii Strashko <grygorii.strashko@ti.com> wrote: > > All existing TI SoCs network HW have similar MDIO implementation, so > introduce common mdio support library which can be reused by TI networking > drivers. > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > --- > drivers/net/ti/Makefile | 2 +- > drivers/net/ti/cpsw_mdio.c | 205 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/net/ti/cpsw_mdio.h | 18 ++++ > 3 files changed, 224 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/ti/cpsw_mdio.c > create mode 100644 drivers/net/ti/cpsw_mdio.h > > diff --git a/drivers/net/ti/Makefile b/drivers/net/ti/Makefile > index 4ab4a27..d2b6f20 100644 > --- a/drivers/net/ti/Makefile > +++ b/drivers/net/ti/Makefile > @@ -2,6 +2,6 @@ > # > # Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > > -obj-$(CONFIG_DRIVER_TI_CPSW) += cpsw.o cpsw-common.o > +obj-$(CONFIG_DRIVER_TI_CPSW) += cpsw.o cpsw-common.o cpsw_mdio.o > obj-$(CONFIG_DRIVER_TI_EMAC) += davinci_emac.o > obj-$(CONFIG_DRIVER_TI_KEYSTONE_NET) += keystone_net.o > diff --git a/drivers/net/ti/cpsw_mdio.c b/drivers/net/ti/cpsw_mdio.c > new file mode 100644 > index 0000000..ed9dd27 > --- /dev/null > +++ b/drivers/net/ti/cpsw_mdio.c > @@ -0,0 +1,205 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * CPSW MDIO generic driver for TI AMxx/K2x/EMAC devices. > + * > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + */ > + > +#include <common.h> > +#include <asm/io.h> > +#include <miiphy.h> > + > +struct cpsw_mdio_regs { > + u32 version; > + u32 control; > +#define CONTROL_IDLE BIT(31) > +#define CONTROL_ENABLE BIT(30) > +#define CONTROL_FAULT BIT(19) > +#define CONTROL_FAULT_ENABLE BIT(18) > +#define CONTROL_DIV_MASK GENMASK(15, 0) > + > + u32 alive; > + u32 link; > + u32 linkintraw; > + u32 linkintmasked; > + u32 __reserved_0[2]; > + u32 userintraw; > + u32 userintmasked; > + u32 userintmaskset; > + u32 userintmaskclr; > + u32 __reserved_1[20]; > + > + struct { > + u32 access; > + u32 physel; > +#define USERACCESS_GO BIT(31) > +#define USERACCESS_WRITE BIT(30) > +#define USERACCESS_ACK BIT(29) > +#define USERACCESS_READ (0) > +#define USERACCESS_PHY_REG_SHIFT (21) > +#define USERACCESS_PHY_ADDR_SHIFT (16) > +#define USERACCESS_DATA GENMASK(15, 0) > + } user[0]; > +}; > + > +#define CPSW_MDIO_DIV_DEF 0xff > +#define PHY_REG_MASK 0x1f > +#define PHY_ID_MASK 0x1f > + > +/* > + * This timeout definition is a worst-case ultra defensive measure against > + * unexpected controller lock ups. Ideally, we should never ever hit this > + * scenario in practice. > + */ > +#define CPSW_MDIO_TIMEOUT 100 /* msecs */ > + > +struct cpsw_mdio { > + struct cpsw_mdio_regs *regs; > + struct mii_dev *bus; > + int div; > +}; > + > +/* wait until hardware is ready for another user access */ > +static inline u32 cpsw_mdio_wait_for_user_access(struct cpsw_mdio *mdio) > +{ > + u32 reg = 0; > + int timeout = CPSW_MDIO_TIMEOUT; > + > + while (timeout-- && ((reg = readl(&mdio->regs->user[0].access)) > + & USERACCESS_GO)) > + udelay(10); Seems we should be using wait_for_bit instead of this function. > + > + if (timeout == -1) { > + printf("wait_for_user_access Timeout\n"); > + return -ETIMEDOUT; > + } > + > + return reg; > +} > + > +static int cpsw_mdio_read(struct mii_dev *bus, int phy_id, > + int dev_addr, int phy_reg) > +{ > + struct cpsw_mdio *mdio = bus->priv; > + int data; > + u32 reg; > + > + if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK) > + return -EINVAL; > + > + cpsw_mdio_wait_for_user_access(mdio); > + reg = (USERACCESS_GO | USERACCESS_READ | > + (phy_reg << USERACCESS_PHY_REG_SHIFT) | > + (phy_id << USERACCESS_PHY_ADDR_SHIFT)); > + writel(reg, &mdio->regs->user[0].access); > + reg = cpsw_mdio_wait_for_user_access(mdio); > + > + data = (reg & USERACCESS_ACK) ? (reg & USERACCESS_DATA) : -1; > + return data; > +} > + > +static int cpsw_mdio_write(struct mii_dev *bus, int phy_id, int dev_addr, > + int phy_reg, u16 data) > +{ > + struct cpsw_mdio *mdio = bus->priv; > + u32 reg; > + > + if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK) > + return -EINVAL; > + > + cpsw_mdio_wait_for_user_access(mdio); > + reg = (USERACCESS_GO | USERACCESS_WRITE | > + (phy_reg << USERACCESS_PHY_REG_SHIFT) | > + (phy_id << USERACCESS_PHY_ADDR_SHIFT) | > + (data & USERACCESS_DATA)); > + writel(reg, &mdio->regs->user[0].access); > + cpsw_mdio_wait_for_user_access(mdio); > + > + return 0; > +} > + > +u32 cpsw_mdio_get_alive(struct mii_dev *bus) > +{ > + struct cpsw_mdio *mdio = bus->priv; > + u32 val; > + > + val = readl(&mdio->regs->control); > + return val & GENMASK(15, 0); > +} > + > +struct mii_dev *cpsw_mdio_init(const char *name, u32 mdio_base, > + u32 bus_freq, int fck_freq) > +{ > + struct cpsw_mdio *cpsw_mdio; > + int ret; > + > + cpsw_mdio = calloc(1, sizeof(*cpsw_mdio)); > + if (!cpsw_mdio) { > + debug("failed to alloc cpsw_mdio\n"); > + return NULL; > + } > + > + cpsw_mdio->bus = mdio_alloc(); > + if (!cpsw_mdio->bus) { > + debug("failed to alloc mii bus\n"); > + free(cpsw_mdio); > + return NULL; > + } > + > + cpsw_mdio->regs = (struct cpsw_mdio_regs *)mdio_base; > + > + if (!bus_freq || !fck_freq) > + cpsw_mdio->div = CPSW_MDIO_DIV_DEF; > + else > + cpsw_mdio->div = (fck_freq / bus_freq) - 1; > + cpsw_mdio->div &= CONTROL_DIV_MASK; > + > + /* set enable and clock divider */ > + writel(cpsw_mdio->div | CONTROL_ENABLE | CONTROL_FAULT | > + CONTROL_FAULT_ENABLE, &cpsw_mdio->regs->control); > + while (readl(&cpsw_mdio->regs->control) & CONTROL_IDLE) > + ; > + > + /* > + * wait for scan logic to settle: > + * the scan time consists of (a) a large fixed component, and (b) a > + * small component that varies with the mii bus frequency. These > + * were estimated using measurements at 1.1 and 2.2 MHz on tnetv107x > + * silicon. Since the effect of (b) was found to be largely > + * negligible, we keep things simple here. > + */ > + udelay(1000); mdelay(1); > + > + cpsw_mdio->bus->read = cpsw_mdio_read; > + cpsw_mdio->bus->write = cpsw_mdio_write; > + cpsw_mdio->bus->priv = cpsw_mdio; > + strcpy(cpsw_mdio->bus->name, name); snprintf(cpsw_mdio->bus->name, sizeof(cpsw_mdio->bus->name), name); > + > + ret = mdio_register(cpsw_mdio->bus); > + if (ret < 0) { > + debug("failed to register mii bus\n"); > + goto free_bus; > + } > + > + return cpsw_mdio->bus; > + > +free_bus: > + free(cpsw_mdio->bus); mdio_free(cpsw_mdio->bus); > + free(cpsw_mdio); > + return NULL; > +} > + > +void cpsw_mdio_free(struct mii_dev *bus) > +{ > + struct cpsw_mdio *mdio = bus->priv; > + u32 reg; > + > + /* disable mdio */ > + reg = readl(&mdio->regs->control); > + reg &= ~CONTROL_ENABLE; > + writel(reg, &mdio->regs->control); > + > + mdio_unregister(bus); > + mdio_free(bus); > + free(mdio); > +} > diff --git a/drivers/net/ti/cpsw_mdio.h b/drivers/net/ti/cpsw_mdio.h > new file mode 100644 > index 0000000..4a76d4e > --- /dev/null > +++ b/drivers/net/ti/cpsw_mdio.h > @@ -0,0 +1,18 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * CPSW MDIO generic driver API for TI AMxx/K2x/EMAC devices. > + * > + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ > + */ > + > +#ifndef CPSW_MDIO_H_ > +#define CPSW_MDIO_H_ > + > +struct cpsw_mdio; > + > +struct mii_dev *cpsw_mdio_init(const char *name, u32 mdio_base, > + u32 bus_freq, int fck_freq); > +void cpsw_mdio_free(struct mii_dev *bus); > +u32 cpsw_mdio_get_alive(struct mii_dev *bus); > + > +#endif /* CPSW_MDIO_H_ */ > -- > 2.10.5 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On 10/29/18 2:40 PM, Joe Hershberger wrote: > On Mon, Oct 29, 2018 at 8:04 AM Hannes Schmelzer <hannes@schmelzer.or.at> wrote: >> >> >> On 10/26/18 5:38 AM, Grygorii Strashko wrote: >>> All existing TI SoCs network HW have similar MDIO implementation, so >>> introduce common mdio support library which can be reused by TI networking >>> drivers. >>> >>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >>> --- >>> drivers/net/ti/Makefile | 2 +- >>> drivers/net/ti/cpsw_mdio.c | 205 +++++++++++++++++++++++++++++++++++++++++++++ >>> drivers/net/ti/cpsw_mdio.h | 18 ++++ >>> 3 files changed, 224 insertions(+), 1 deletion(-) >>> create mode 100644 drivers/net/ti/cpsw_mdio.c >>> create mode 100644 drivers/net/ti/cpsw_mdio.h >>> >> Hi, >> why we don't combine this with: >> http://patchwork.ozlabs.org/patch/939726/ ? looks much more elegant to me. > > I agree we should be moving that direction, but consolidating TI MDIO > implementation is fairly orthogonal. Also haven't heard back from Ken > on feedback to that patch. The mdio here is not modeled as device/driver and I, honestly, see no reasons for this at least now.
diff --git a/drivers/net/ti/Makefile b/drivers/net/ti/Makefile index 4ab4a27..d2b6f20 100644 --- a/drivers/net/ti/Makefile +++ b/drivers/net/ti/Makefile @@ -2,6 +2,6 @@ # # Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ -obj-$(CONFIG_DRIVER_TI_CPSW) += cpsw.o cpsw-common.o +obj-$(CONFIG_DRIVER_TI_CPSW) += cpsw.o cpsw-common.o cpsw_mdio.o obj-$(CONFIG_DRIVER_TI_EMAC) += davinci_emac.o obj-$(CONFIG_DRIVER_TI_KEYSTONE_NET) += keystone_net.o diff --git a/drivers/net/ti/cpsw_mdio.c b/drivers/net/ti/cpsw_mdio.c new file mode 100644 index 0000000..ed9dd27 --- /dev/null +++ b/drivers/net/ti/cpsw_mdio.c @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * CPSW MDIO generic driver for TI AMxx/K2x/EMAC devices. + * + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + */ + +#include <common.h> +#include <asm/io.h> +#include <miiphy.h> + +struct cpsw_mdio_regs { + u32 version; + u32 control; +#define CONTROL_IDLE BIT(31) +#define CONTROL_ENABLE BIT(30) +#define CONTROL_FAULT BIT(19) +#define CONTROL_FAULT_ENABLE BIT(18) +#define CONTROL_DIV_MASK GENMASK(15, 0) + + u32 alive; + u32 link; + u32 linkintraw; + u32 linkintmasked; + u32 __reserved_0[2]; + u32 userintraw; + u32 userintmasked; + u32 userintmaskset; + u32 userintmaskclr; + u32 __reserved_1[20]; + + struct { + u32 access; + u32 physel; +#define USERACCESS_GO BIT(31) +#define USERACCESS_WRITE BIT(30) +#define USERACCESS_ACK BIT(29) +#define USERACCESS_READ (0) +#define USERACCESS_PHY_REG_SHIFT (21) +#define USERACCESS_PHY_ADDR_SHIFT (16) +#define USERACCESS_DATA GENMASK(15, 0) + } user[0]; +}; + +#define CPSW_MDIO_DIV_DEF 0xff +#define PHY_REG_MASK 0x1f +#define PHY_ID_MASK 0x1f + +/* + * This timeout definition is a worst-case ultra defensive measure against + * unexpected controller lock ups. Ideally, we should never ever hit this + * scenario in practice. + */ +#define CPSW_MDIO_TIMEOUT 100 /* msecs */ + +struct cpsw_mdio { + struct cpsw_mdio_regs *regs; + struct mii_dev *bus; + int div; +}; + +/* wait until hardware is ready for another user access */ +static inline u32 cpsw_mdio_wait_for_user_access(struct cpsw_mdio *mdio) +{ + u32 reg = 0; + int timeout = CPSW_MDIO_TIMEOUT; + + while (timeout-- && ((reg = readl(&mdio->regs->user[0].access)) + & USERACCESS_GO)) + udelay(10); + + if (timeout == -1) { + printf("wait_for_user_access Timeout\n"); + return -ETIMEDOUT; + } + + return reg; +} + +static int cpsw_mdio_read(struct mii_dev *bus, int phy_id, + int dev_addr, int phy_reg) +{ + struct cpsw_mdio *mdio = bus->priv; + int data; + u32 reg; + + if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK) + return -EINVAL; + + cpsw_mdio_wait_for_user_access(mdio); + reg = (USERACCESS_GO | USERACCESS_READ | + (phy_reg << USERACCESS_PHY_REG_SHIFT) | + (phy_id << USERACCESS_PHY_ADDR_SHIFT)); + writel(reg, &mdio->regs->user[0].access); + reg = cpsw_mdio_wait_for_user_access(mdio); + + data = (reg & USERACCESS_ACK) ? (reg & USERACCESS_DATA) : -1; + return data; +} + +static int cpsw_mdio_write(struct mii_dev *bus, int phy_id, int dev_addr, + int phy_reg, u16 data) +{ + struct cpsw_mdio *mdio = bus->priv; + u32 reg; + + if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK) + return -EINVAL; + + cpsw_mdio_wait_for_user_access(mdio); + reg = (USERACCESS_GO | USERACCESS_WRITE | + (phy_reg << USERACCESS_PHY_REG_SHIFT) | + (phy_id << USERACCESS_PHY_ADDR_SHIFT) | + (data & USERACCESS_DATA)); + writel(reg, &mdio->regs->user[0].access); + cpsw_mdio_wait_for_user_access(mdio); + + return 0; +} + +u32 cpsw_mdio_get_alive(struct mii_dev *bus) +{ + struct cpsw_mdio *mdio = bus->priv; + u32 val; + + val = readl(&mdio->regs->control); + return val & GENMASK(15, 0); +} + +struct mii_dev *cpsw_mdio_init(const char *name, u32 mdio_base, + u32 bus_freq, int fck_freq) +{ + struct cpsw_mdio *cpsw_mdio; + int ret; + + cpsw_mdio = calloc(1, sizeof(*cpsw_mdio)); + if (!cpsw_mdio) { + debug("failed to alloc cpsw_mdio\n"); + return NULL; + } + + cpsw_mdio->bus = mdio_alloc(); + if (!cpsw_mdio->bus) { + debug("failed to alloc mii bus\n"); + free(cpsw_mdio); + return NULL; + } + + cpsw_mdio->regs = (struct cpsw_mdio_regs *)mdio_base; + + if (!bus_freq || !fck_freq) + cpsw_mdio->div = CPSW_MDIO_DIV_DEF; + else + cpsw_mdio->div = (fck_freq / bus_freq) - 1; + cpsw_mdio->div &= CONTROL_DIV_MASK; + + /* set enable and clock divider */ + writel(cpsw_mdio->div | CONTROL_ENABLE | CONTROL_FAULT | + CONTROL_FAULT_ENABLE, &cpsw_mdio->regs->control); + while (readl(&cpsw_mdio->regs->control) & CONTROL_IDLE) + ; + + /* + * wait for scan logic to settle: + * the scan time consists of (a) a large fixed component, and (b) a + * small component that varies with the mii bus frequency. These + * were estimated using measurements at 1.1 and 2.2 MHz on tnetv107x + * silicon. Since the effect of (b) was found to be largely + * negligible, we keep things simple here. + */ + udelay(1000); + + cpsw_mdio->bus->read = cpsw_mdio_read; + cpsw_mdio->bus->write = cpsw_mdio_write; + cpsw_mdio->bus->priv = cpsw_mdio; + strcpy(cpsw_mdio->bus->name, name); + + ret = mdio_register(cpsw_mdio->bus); + if (ret < 0) { + debug("failed to register mii bus\n"); + goto free_bus; + } + + return cpsw_mdio->bus; + +free_bus: + free(cpsw_mdio->bus); + free(cpsw_mdio); + return NULL; +} + +void cpsw_mdio_free(struct mii_dev *bus) +{ + struct cpsw_mdio *mdio = bus->priv; + u32 reg; + + /* disable mdio */ + reg = readl(&mdio->regs->control); + reg &= ~CONTROL_ENABLE; + writel(reg, &mdio->regs->control); + + mdio_unregister(bus); + mdio_free(bus); + free(mdio); +} diff --git a/drivers/net/ti/cpsw_mdio.h b/drivers/net/ti/cpsw_mdio.h new file mode 100644 index 0000000..4a76d4e --- /dev/null +++ b/drivers/net/ti/cpsw_mdio.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * CPSW MDIO generic driver API for TI AMxx/K2x/EMAC devices. + * + * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + */ + +#ifndef CPSW_MDIO_H_ +#define CPSW_MDIO_H_ + +struct cpsw_mdio; + +struct mii_dev *cpsw_mdio_init(const char *name, u32 mdio_base, + u32 bus_freq, int fck_freq); +void cpsw_mdio_free(struct mii_dev *bus); +u32 cpsw_mdio_get_alive(struct mii_dev *bus); + +#endif /* CPSW_MDIO_H_ */
All existing TI SoCs network HW have similar MDIO implementation, so introduce common mdio support library which can be reused by TI networking drivers. Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> --- drivers/net/ti/Makefile | 2 +- drivers/net/ti/cpsw_mdio.c | 205 +++++++++++++++++++++++++++++++++++++++++++++ drivers/net/ti/cpsw_mdio.h | 18 ++++ 3 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 drivers/net/ti/cpsw_mdio.c create mode 100644 drivers/net/ti/cpsw_mdio.h