| Message ID | 20200413170107.246509-1-robert.marko@sartura.hr |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [1/3] net: phy: mdio: add IPQ40xx MDIO driver | expand |
On 13.04.2020 19:01, Robert Marko wrote: > This patch adds the driver for the MDIO interface > inside of Qualcomm IPQ40xx series SoC-s. > > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > Signed-off-by: Robert Marko <robert.marko@sartura.hr> > Cc: Luka Perkov <luka.perkov@sartura.hr> > --- > drivers/net/phy/Kconfig | 7 ++ > drivers/net/phy/Makefile | 1 + > drivers/net/phy/mdio-ipq40xx.c | 180 +++++++++++++++++++++++++++++++++ > 3 files changed, 188 insertions(+) > create mode 100644 drivers/net/phy/mdio-ipq40xx.c > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 9dabe03a668c..614d08635012 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -157,6 +157,13 @@ config MDIO_I2C > > This is library mode. > > +config MDIO_IPQ40XX > + tristate "Qualcomm IPQ40xx MDIO interface" > + depends on HAS_IOMEM && OF > + help > + This driver supports the MDIO interface found in Qualcomm > + IPQ40xx series Soc-s. > + > config MDIO_MOXART > tristate "MOXA ART MDIO interface support" > depends on ARCH_MOXART || COMPILE_TEST > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index fe5badf13b65..c89fc187fd74 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -36,6 +36,7 @@ obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o > obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o > obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o > obj-$(CONFIG_MDIO_I2C) += mdio-i2c.o > +obj-$(CONFIG_MDIO_IPQ40XX) += mdio-ipq40xx.o > obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o > obj-$(CONFIG_MDIO_MSCC_MIIM) += mdio-mscc-miim.o > obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o > diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c > new file mode 100644 > index 000000000000..8068f1e6a077 > --- /dev/null > +++ b/drivers/net/phy/mdio-ipq40xx.c > @@ -0,0 +1,180 @@ > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause > +/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */ > + > +#include <linux/delay.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/io.h> > +#include <linux/of_address.h> > +#include <linux/of_mdio.h> > +#include <linux/phy.h> > +#include <linux/platform_device.h> > + > +#define MDIO_CTRL_0_REG 0x40 > +#define MDIO_CTRL_1_REG 0x44 > +#define MDIO_CTRL_2_REG 0x48 > +#define MDIO_CTRL_3_REG 0x4c > +#define MDIO_CTRL_4_REG 0x50 > +#define MDIO_CTRL_4_ACCESS_BUSY BIT(16) > +#define MDIO_CTRL_4_ACCESS_START BIT(8) > +#define MDIO_CTRL_4_ACCESS_CODE_READ 0 > +#define MDIO_CTRL_4_ACCESS_CODE_WRITE 1 > +#define CTRL_0_REG_DEFAULT_VALUE 0x150FF > + > +#define IPQ40XX_MDIO_RETRY 1000 > +#define IPQ40XX_MDIO_DELAY 10 > + > +struct ipq40xx_mdio_data { > + struct mii_bus *mii_bus; > + void __iomem *membase; > + struct device *dev; > +}; > + > +static int ipq40xx_mdio_wait_busy(struct ipq40xx_mdio_data *am) > +{ > + int i; > + > + for (i = 0; i < IPQ40XX_MDIO_RETRY; i++) { > + unsigned int busy; > + > + busy = readl(am->membase + MDIO_CTRL_4_REG) & > + MDIO_CTRL_4_ACCESS_BUSY; > + if (!busy) > + return 0; > + > + /* BUSY might take to be cleard by 15~20 times of loop */ > + udelay(IPQ40XX_MDIO_DELAY); > + } > + > + dev_err(am->dev, "%s: MDIO operation timed out\n", am->mii_bus->name); > + > + return -ETIMEDOUT; > +} > + > +static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum) > +{ > + struct ipq40xx_mdio_data *am = bus->priv; > + int value = 0; > + unsigned int cmd = 0; > + > + lockdep_assert_held(&bus->mdio_lock); > + > + if (ipq40xx_mdio_wait_busy(am)) > + return -ETIMEDOUT; > + > + /* issue the phy address and reg */ > + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); > + > + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_READ; > + > + /* issue read command */ > + writel(cmd, am->membase + MDIO_CTRL_4_REG); > + > + /* Wait read complete */ > + if (ipq40xx_mdio_wait_busy(am)) > + return -ETIMEDOUT; > + > + /* Read data */ > + value = readl(am->membase + MDIO_CTRL_3_REG); > + > + return value; > +} > + > +static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum, > + u16 value) > +{ > + struct ipq40xx_mdio_data *am = bus->priv; > + unsigned int cmd = 0; > + > + lockdep_assert_held(&bus->mdio_lock); > + > + if (ipq40xx_mdio_wait_busy(am)) > + return -ETIMEDOUT; > + > + /* issue the phy address and reg */ > + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); > + > + /* issue write data */ > + writel(value, am->membase + MDIO_CTRL_2_REG); > + > + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_WRITE; > + /* issue write command */ > + writel(cmd, am->membase + MDIO_CTRL_4_REG); > + > + /* Wait write complete */ > + if (ipq40xx_mdio_wait_busy(am)) > + return -ETIMEDOUT; > + > + return 0; > +} > + > +static int ipq40xx_mdio_probe(struct platform_device *pdev) > +{ > + struct ipq40xx_mdio_data *am; > + struct resource *res; > + > + am = devm_kzalloc(&pdev->dev, sizeof(*am), GFP_KERNEL); > + if (!am) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "no iomem resource found\n"); > + return -ENXIO; > + } > + > + am->membase = devm_ioremap_resource(&pdev->dev, res); You can use devm_platform_ioremap_resource() here. > + if (IS_ERR(am->membase)) { > + dev_err(&pdev->dev, "unable to ioremap registers\n"); > + return PTR_ERR(am->membase); > + } > + > + am->mii_bus = devm_mdiobus_alloc(&pdev->dev); > + if (!am->mii_bus) > + return -ENOMEM; > + You could use devm_mdiobus_alloc_size() and omit allocating am separately. > + writel(CTRL_0_REG_DEFAULT_VALUE, am->membase + MDIO_CTRL_0_REG); > + > + am->mii_bus->name = "ipq40xx_mdio"; > + am->mii_bus->read = ipq40xx_mdio_read; > + am->mii_bus->write = ipq40xx_mdio_write; > + am->mii_bus->priv = am; > + am->mii_bus->parent = &pdev->dev; > + snprintf(am->mii_bus->id, MII_BUS_ID_SIZE, "%s", dev_name(&pdev->dev)); > + > + am->dev = &pdev->dev; > + platform_set_drvdata(pdev, am); > + > + return of_mdiobus_register(am->mii_bus, pdev->dev.of_node); > +} > + > +static int ipq40xx_mdio_remove(struct platform_device *pdev) > +{ > + struct ipq40xx_mdio_data *am = platform_get_drvdata(pdev); > + > + mdiobus_unregister(am->mii_bus); > + > + return 0; > +} > + > +static const struct of_device_id ipq40xx_mdio_dt_ids[] = { > + { .compatible = "qcom,ipq40xx-mdio" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ipq40xx_mdio_dt_ids); > + > +static struct platform_driver ipq40xx_mdio_driver = { > + .probe = ipq40xx_mdio_probe, > + .remove = ipq40xx_mdio_remove, > + .driver = { > + .name = "ipq40xx-mdio", > + .of_match_table = ipq40xx_mdio_dt_ids, > + }, > +}; > + > +module_platform_driver(ipq40xx_mdio_driver); > + > +MODULE_DESCRIPTION("IPQ40XX MDIO interface driver"); > +MODULE_AUTHOR("Qualcomm Atheros"); > +MODULE_LICENSE("Dual BSD/GPL"); >
> --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -36,6 +36,7 @@ obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o > obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o > obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o > obj-$(CONFIG_MDIO_I2C) += mdio-i2c.o > +obj-$(CONFIG_MDIO_IPQ40XX) += mdio-ipq40xx.o > obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o > obj-$(CONFIG_MDIO_MSCC_MIIM) += mdio-mscc-miim.o Hi Robert That looks odd. What happened to the obj-$(CONFIG_MDIO_IPQ8064) += mdio-ipq8064.o > obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o > diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c > new file mode 100644 > index 000000000000..8068f1e6a077 > --- /dev/null > +++ b/drivers/net/phy/mdio-ipq40xx.c > @@ -0,0 +1,180 @@ > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause > +/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */ > + > +#include <linux/delay.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/io.h> > +#include <linux/of_address.h> > +#include <linux/of_mdio.h> > +#include <linux/phy.h> > +#include <linux/platform_device.h> > + > +#define MDIO_CTRL_0_REG 0x40 > +#define MDIO_CTRL_1_REG 0x44 > +#define MDIO_CTRL_2_REG 0x48 > +#define MDIO_CTRL_3_REG 0x4c > +#define MDIO_CTRL_4_REG 0x50 Can we have better names than as. It seems like 3 is read data, 2 is write data, etc. > +#define MDIO_CTRL_4_ACCESS_BUSY BIT(16) > +#define MDIO_CTRL_4_ACCESS_START BIT(8) > +#define MDIO_CTRL_4_ACCESS_CODE_READ 0 > +#define MDIO_CTRL_4_ACCESS_CODE_WRITE 1 > +#define CTRL_0_REG_DEFAULT_VALUE 0x150FF No magic numbers please. Try to explain what each of these bits do. I'm guessing they are clock speed, preamble enable, maybe C22/C45? > + > +#define IPQ40XX_MDIO_RETRY 1000 > +#define IPQ40XX_MDIO_DELAY 10 > + > +struct ipq40xx_mdio_data { > + struct mii_bus *mii_bus; > + void __iomem *membase; > + struct device *dev; > +}; > + > +static int ipq40xx_mdio_wait_busy(struct ipq40xx_mdio_data *am) > +{ > + int i; > + > + for (i = 0; i < IPQ40XX_MDIO_RETRY; i++) { > + unsigned int busy; > + > + busy = readl(am->membase + MDIO_CTRL_4_REG) & > + MDIO_CTRL_4_ACCESS_BUSY; > + if (!busy) > + return 0; > + > + /* BUSY might take to be cleard by 15~20 times of loop */ > + udelay(IPQ40XX_MDIO_DELAY); > + } > + > + dev_err(am->dev, "%s: MDIO operation timed out\n", am->mii_bus->name); dev_err() should give you enough to identify the device. No need to print am->mii_bus->name as well. > + > + return -ETIMEDOUT; > +} > + > +static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum) > +{ > + struct ipq40xx_mdio_data *am = bus->priv; > + int value = 0; > + unsigned int cmd = 0; > + > + lockdep_assert_held(&bus->mdio_lock); Do you think the core is broken? Please check if the request is for a C45 read, and return -EOPNOTSUPP if so. > + > + if (ipq40xx_mdio_wait_busy(am)) > + return -ETIMEDOUT; > + > + /* issue the phy address and reg */ > + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); > + > + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_READ; > + > + /* issue read command */ > + writel(cmd, am->membase + MDIO_CTRL_4_REG); > + > + /* Wait read complete */ > + if (ipq40xx_mdio_wait_busy(am)) > + return -ETIMEDOUT; > + > + /* Read data */ > + value = readl(am->membase + MDIO_CTRL_3_REG); > + > + return value; > +} > + > +static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum, > + u16 value) > +{ > + struct ipq40xx_mdio_data *am = bus->priv; > + unsigned int cmd = 0; > + > + lockdep_assert_held(&bus->mdio_lock); > + > + if (ipq40xx_mdio_wait_busy(am)) > + return -ETIMEDOUT; > + > + /* issue the phy address and reg */ > + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); > + > + /* issue write data */ > + writel(value, am->membase + MDIO_CTRL_2_REG); > + > + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_WRITE; > + /* issue write command */ > + writel(cmd, am->membase + MDIO_CTRL_4_REG); > + > + /* Wait write complete */ > + if (ipq40xx_mdio_wait_busy(am)) > + return -ETIMEDOUT; > + > + return 0; > +} > + > +static int ipq40xx_mdio_probe(struct platform_device *pdev) > +{ > + struct ipq40xx_mdio_data *am; Why the name am? Generally priv is used. I could also understand bus, or even data, but am? Andrew
On Mon, Apr 13, 2020 at 8:42 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > --- a/drivers/net/phy/Makefile > > +++ b/drivers/net/phy/Makefile > > @@ -36,6 +36,7 @@ obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o > > obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o > > obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o > > obj-$(CONFIG_MDIO_I2C) += mdio-i2c.o > > +obj-$(CONFIG_MDIO_IPQ40XX) += mdio-ipq40xx.o > > obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o > > obj-$(CONFIG_MDIO_MSCC_MIIM) += mdio-mscc-miim.o > > Hi Robert > > That looks odd. What happened to the > > obj-$(CONFIG_MDIO_IPQ8064) += mdio-ipq8064.o > Sorry, this was based off kernel 5.6 instead of net-next. > > obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o > > diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c > > new file mode 100644 > > index 000000000000..8068f1e6a077 > > --- /dev/null > > +++ b/drivers/net/phy/mdio-ipq40xx.c > > @@ -0,0 +1,180 @@ > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause > > +/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */ > > + > > +#include <linux/delay.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/io.h> > > +#include <linux/of_address.h> > > +#include <linux/of_mdio.h> > > +#include <linux/phy.h> > > +#include <linux/platform_device.h> > > + > > +#define MDIO_CTRL_0_REG 0x40 > > +#define MDIO_CTRL_1_REG 0x44 > > +#define MDIO_CTRL_2_REG 0x48 > > +#define MDIO_CTRL_3_REG 0x4c > > +#define MDIO_CTRL_4_REG 0x50 > > Can we have better names than as. It seems like 3 is read data, 2 is > write data, etc. I agree these ones don't really tell whats the function. Unfortunately, I don't have access to documentation and this is all based on GPL code from Qualcomm's SDK. So I don't really know whats their purpose. It has been floating around for years, so I thought that it would be good to clean it up and upstream. > > > +#define MDIO_CTRL_4_ACCESS_BUSY BIT(16) > > +#define MDIO_CTRL_4_ACCESS_START BIT(8) > > +#define MDIO_CTRL_4_ACCESS_CODE_READ 0 > > +#define MDIO_CTRL_4_ACCESS_CODE_WRITE 1 > > +#define CTRL_0_REG_DEFAULT_VALUE 0x150FF > > No magic numbers please. Try to explain what each of these bits > do. I'm guessing they are clock speed, preamble enable, maybe C22/C45? Fortunately, it seems that this is a leftover from early preproduction HW as my test boards work without it. Something similar was the case in other Qualcomm SDK drivers. So, this will be dropped in v2. > > > + > > +#define IPQ40XX_MDIO_RETRY 1000 > > +#define IPQ40XX_MDIO_DELAY 10 > > + > > +struct ipq40xx_mdio_data { > > + struct mii_bus *mii_bus; > > + void __iomem *membase; > > + struct device *dev; > > +}; > > + > > +static int ipq40xx_mdio_wait_busy(struct ipq40xx_mdio_data *am) > > +{ > > + int i; > > + > > + for (i = 0; i < IPQ40XX_MDIO_RETRY; i++) { > > + unsigned int busy; > > + > > + busy = readl(am->membase + MDIO_CTRL_4_REG) & > > + MDIO_CTRL_4_ACCESS_BUSY; > > + if (!busy) > > + return 0; > > + > > + /* BUSY might take to be cleard by 15~20 times of loop */ > > + udelay(IPQ40XX_MDIO_DELAY); > > + } > > + > > + dev_err(am->dev, "%s: MDIO operation timed out\n", am->mii_bus->name); > > dev_err() should give you enough to identify the device. No need to > print am->mii_bus->name as well. It will be fixed in v2, thanks. > > > + > > + return -ETIMEDOUT; > > +} > > + > > +static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum) > > +{ > > + struct ipq40xx_mdio_data *am = bus->priv; > > + int value = 0; > > + unsigned int cmd = 0; > > + > > + lockdep_assert_held(&bus->mdio_lock); > > Do you think the core is broken? No, this is also an old relic from the SDK driver. It works without this perfectly fine, so I will drop it from v2. > > Please check if the request is for a C45 read, and return -EOPNOTSUPP > if so. It will be added to v2, thanks. > > > > + > > + if (ipq40xx_mdio_wait_busy(am)) > > + return -ETIMEDOUT; > > + > > + /* issue the phy address and reg */ > > + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); > > + > > + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_READ; > > + > > + /* issue read command */ > > + writel(cmd, am->membase + MDIO_CTRL_4_REG); > > + > > + /* Wait read complete */ > > + if (ipq40xx_mdio_wait_busy(am)) > > + return -ETIMEDOUT; > > + > > + /* Read data */ > > + value = readl(am->membase + MDIO_CTRL_3_REG); > > + > > + return value; > > +} > > + > > +static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum, > > + u16 value) > > +{ > > + struct ipq40xx_mdio_data *am = bus->priv; > > + unsigned int cmd = 0; > > + > > + lockdep_assert_held(&bus->mdio_lock); > > + > > + if (ipq40xx_mdio_wait_busy(am)) > > + return -ETIMEDOUT; > > + > > + /* issue the phy address and reg */ > > + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); > > + > > + /* issue write data */ > > + writel(value, am->membase + MDIO_CTRL_2_REG); > > + > > + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_WRITE; > > + /* issue write command */ > > + writel(cmd, am->membase + MDIO_CTRL_4_REG); > > + > > + /* Wait write complete */ > > + if (ipq40xx_mdio_wait_busy(am)) > > + return -ETIMEDOUT; > > + > > + return 0; > > +} > > + > > +static int ipq40xx_mdio_probe(struct platform_device *pdev) > > +{ > > + struct ipq40xx_mdio_data *am; > > Why the name am? Generally priv is used. I could also understand bus, > or even data, but am? Like most stuff in this driver, its a leftover from the SDK driver. I have changed it to priv in v2, also I switched the driver to devm_mdiobus_alloc_size() to try and simplify/modernize the driver also. Thanks for the suggestions. I will send a v2 soon. Best regards, Andrew > > Andrew
On Mon, Apr 13, 2020 at 7:18 PM Heiner Kallweit <hkallweit1@gmail.com> wrote: > > On 13.04.2020 19:01, Robert Marko wrote: > > This patch adds the driver for the MDIO interface > > inside of Qualcomm IPQ40xx series SoC-s. > > > > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > > Signed-off-by: Robert Marko <robert.marko@sartura.hr> > > Cc: Luka Perkov <luka.perkov@sartura.hr> > > --- > > drivers/net/phy/Kconfig | 7 ++ > > drivers/net/phy/Makefile | 1 + > > drivers/net/phy/mdio-ipq40xx.c | 180 +++++++++++++++++++++++++++++++++ > > 3 files changed, 188 insertions(+) > > create mode 100644 drivers/net/phy/mdio-ipq40xx.c > > > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > > index 9dabe03a668c..614d08635012 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -157,6 +157,13 @@ config MDIO_I2C > > > > This is library mode. > > > > +config MDIO_IPQ40XX > > + tristate "Qualcomm IPQ40xx MDIO interface" > > + depends on HAS_IOMEM && OF > > + help > > + This driver supports the MDIO interface found in Qualcomm > > + IPQ40xx series Soc-s. > > + > > config MDIO_MOXART > > tristate "MOXA ART MDIO interface support" > > depends on ARCH_MOXART || COMPILE_TEST > > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > > index fe5badf13b65..c89fc187fd74 100644 > > --- a/drivers/net/phy/Makefile > > +++ b/drivers/net/phy/Makefile > > @@ -36,6 +36,7 @@ obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o > > obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o > > obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o > > obj-$(CONFIG_MDIO_I2C) += mdio-i2c.o > > +obj-$(CONFIG_MDIO_IPQ40XX) += mdio-ipq40xx.o > > obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o > > obj-$(CONFIG_MDIO_MSCC_MIIM) += mdio-mscc-miim.o > > obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o > > diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c > > new file mode 100644 > > index 000000000000..8068f1e6a077 > > --- /dev/null > > +++ b/drivers/net/phy/mdio-ipq40xx.c > > @@ -0,0 +1,180 @@ > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause > > +/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */ > > + > > +#include <linux/delay.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/mutex.h> > > +#include <linux/io.h> > > +#include <linux/of_address.h> > > +#include <linux/of_mdio.h> > > +#include <linux/phy.h> > > +#include <linux/platform_device.h> > > + > > +#define MDIO_CTRL_0_REG 0x40 > > +#define MDIO_CTRL_1_REG 0x44 > > +#define MDIO_CTRL_2_REG 0x48 > > +#define MDIO_CTRL_3_REG 0x4c > > +#define MDIO_CTRL_4_REG 0x50 > > +#define MDIO_CTRL_4_ACCESS_BUSY BIT(16) > > +#define MDIO_CTRL_4_ACCESS_START BIT(8) > > +#define MDIO_CTRL_4_ACCESS_CODE_READ 0 > > +#define MDIO_CTRL_4_ACCESS_CODE_WRITE 1 > > +#define CTRL_0_REG_DEFAULT_VALUE 0x150FF > > + > > +#define IPQ40XX_MDIO_RETRY 1000 > > +#define IPQ40XX_MDIO_DELAY 10 > > + > > +struct ipq40xx_mdio_data { > > + struct mii_bus *mii_bus; > > + void __iomem *membase; > > + struct device *dev; > > +}; > > + > > +static int ipq40xx_mdio_wait_busy(struct ipq40xx_mdio_data *am) > > +{ > > + int i; > > + > > + for (i = 0; i < IPQ40XX_MDIO_RETRY; i++) { > > + unsigned int busy; > > + > > + busy = readl(am->membase + MDIO_CTRL_4_REG) & > > + MDIO_CTRL_4_ACCESS_BUSY; > > + if (!busy) > > + return 0; > > + > > + /* BUSY might take to be cleard by 15~20 times of loop */ > > + udelay(IPQ40XX_MDIO_DELAY); > > + } > > + > > + dev_err(am->dev, "%s: MDIO operation timed out\n", am->mii_bus->name); > > + > > + return -ETIMEDOUT; > > +} > > + > > +static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum) > > +{ > > + struct ipq40xx_mdio_data *am = bus->priv; > > + int value = 0; > > + unsigned int cmd = 0; > > + > > + lockdep_assert_held(&bus->mdio_lock); > > + > > + if (ipq40xx_mdio_wait_busy(am)) > > + return -ETIMEDOUT; > > + > > + /* issue the phy address and reg */ > > + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); > > + > > + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_READ; > > + > > + /* issue read command */ > > + writel(cmd, am->membase + MDIO_CTRL_4_REG); > > + > > + /* Wait read complete */ > > + if (ipq40xx_mdio_wait_busy(am)) > > + return -ETIMEDOUT; > > + > > + /* Read data */ > > + value = readl(am->membase + MDIO_CTRL_3_REG); > > + > > + return value; > > +} > > + > > +static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum, > > + u16 value) > > +{ > > + struct ipq40xx_mdio_data *am = bus->priv; > > + unsigned int cmd = 0; > > + > > + lockdep_assert_held(&bus->mdio_lock); > > + > > + if (ipq40xx_mdio_wait_busy(am)) > > + return -ETIMEDOUT; > > + > > + /* issue the phy address and reg */ > > + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); > > + > > + /* issue write data */ > > + writel(value, am->membase + MDIO_CTRL_2_REG); > > + > > + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_WRITE; > > + /* issue write command */ > > + writel(cmd, am->membase + MDIO_CTRL_4_REG); > > + > > + /* Wait write complete */ > > + if (ipq40xx_mdio_wait_busy(am)) > > + return -ETIMEDOUT; > > + > > + return 0; > > +} > > + > > +static int ipq40xx_mdio_probe(struct platform_device *pdev) > > +{ > > + struct ipq40xx_mdio_data *am; > > + struct resource *res; > > + > > + am = devm_kzalloc(&pdev->dev, sizeof(*am), GFP_KERNEL); > > + if (!am) > > + return -ENOMEM; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) { > > + dev_err(&pdev->dev, "no iomem resource found\n"); > > + return -ENXIO; > > + } > > + > > + am->membase = devm_ioremap_resource(&pdev->dev, res); > > You can use devm_platform_ioremap_resource() here. Thanks, its now used in v2. > > > + if (IS_ERR(am->membase)) { > > + dev_err(&pdev->dev, "unable to ioremap registers\n"); > > + return PTR_ERR(am->membase); > > + } > > + > > + am->mii_bus = devm_mdiobus_alloc(&pdev->dev); > > + if (!am->mii_bus) > > + return -ENOMEM; > > + > > You could use devm_mdiobus_alloc_size() and omit allocating am > separately. Thanks, I switched to it in v2 along some other improvements. > > > + writel(CTRL_0_REG_DEFAULT_VALUE, am->membase + MDIO_CTRL_0_REG); > > + > > + am->mii_bus->name = "ipq40xx_mdio"; > > + am->mii_bus->read = ipq40xx_mdio_read; > > + am->mii_bus->write = ipq40xx_mdio_write; > > + am->mii_bus->priv = am; > > + am->mii_bus->parent = &pdev->dev; > > + snprintf(am->mii_bus->id, MII_BUS_ID_SIZE, "%s", dev_name(&pdev->dev)); > > + > > + am->dev = &pdev->dev; > > + platform_set_drvdata(pdev, am); > > + > > + return of_mdiobus_register(am->mii_bus, pdev->dev.of_node); > > +} > > + > > +static int ipq40xx_mdio_remove(struct platform_device *pdev) > > +{ > > + struct ipq40xx_mdio_data *am = platform_get_drvdata(pdev); > > + > > + mdiobus_unregister(am->mii_bus); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id ipq40xx_mdio_dt_ids[] = { > > + { .compatible = "qcom,ipq40xx-mdio" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, ipq40xx_mdio_dt_ids); > > + > > +static struct platform_driver ipq40xx_mdio_driver = { > > + .probe = ipq40xx_mdio_probe, > > + .remove = ipq40xx_mdio_remove, > > + .driver = { > > + .name = "ipq40xx-mdio", > > + .of_match_table = ipq40xx_mdio_dt_ids, > > + }, > > +}; > > + > > +module_platform_driver(ipq40xx_mdio_driver); > > + > > +MODULE_DESCRIPTION("IPQ40XX MDIO interface driver"); > > +MODULE_AUTHOR("Qualcomm Atheros"); > > +MODULE_LICENSE("Dual BSD/GPL"); > > >
> Unfortunately, I don't have access to documentation and this is all based on > GPL code from Qualcomm's SDK. > So I don't really know whats their purpose. Then please try to reverse engineer it. Just looking at the code, it seems like one register is read, and the other is write. So use that in the name. > No, this is also an old relic from the SDK driver. > It works without this perfectly fine, so I will drop it from v2. Part of cleaning up 'Vendor Crap' is throwing out all the stuff like this. What you end up with should be something you would of been happy to write yourself. > > Why the name am? Generally priv is used. I could also understand bus, > > or even data, but am? > Like most stuff in this driver, its a leftover from the SDK driver. I guessed as much. But this is the sort of thing you need to fix when cleaning up 'vendor crap'. Andrew
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 9dabe03a668c..614d08635012 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -157,6 +157,13 @@ config MDIO_I2C This is library mode. +config MDIO_IPQ40XX + tristate "Qualcomm IPQ40xx MDIO interface" + depends on HAS_IOMEM && OF + help + This driver supports the MDIO interface found in Qualcomm + IPQ40xx series Soc-s. + config MDIO_MOXART tristate "MOXA ART MDIO interface support" depends on ARCH_MOXART || COMPILE_TEST diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index fe5badf13b65..c89fc187fd74 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -36,6 +36,7 @@ obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o obj-$(CONFIG_MDIO_I2C) += mdio-i2c.o +obj-$(CONFIG_MDIO_IPQ40XX) += mdio-ipq40xx.o obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o obj-$(CONFIG_MDIO_MSCC_MIIM) += mdio-mscc-miim.o obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c new file mode 100644 index 000000000000..8068f1e6a077 --- /dev/null +++ b/drivers/net/phy/mdio-ipq40xx.c @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause +/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */ + +#include <linux/delay.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/io.h> +#include <linux/of_address.h> +#include <linux/of_mdio.h> +#include <linux/phy.h> +#include <linux/platform_device.h> + +#define MDIO_CTRL_0_REG 0x40 +#define MDIO_CTRL_1_REG 0x44 +#define MDIO_CTRL_2_REG 0x48 +#define MDIO_CTRL_3_REG 0x4c +#define MDIO_CTRL_4_REG 0x50 +#define MDIO_CTRL_4_ACCESS_BUSY BIT(16) +#define MDIO_CTRL_4_ACCESS_START BIT(8) +#define MDIO_CTRL_4_ACCESS_CODE_READ 0 +#define MDIO_CTRL_4_ACCESS_CODE_WRITE 1 +#define CTRL_0_REG_DEFAULT_VALUE 0x150FF + +#define IPQ40XX_MDIO_RETRY 1000 +#define IPQ40XX_MDIO_DELAY 10 + +struct ipq40xx_mdio_data { + struct mii_bus *mii_bus; + void __iomem *membase; + struct device *dev; +}; + +static int ipq40xx_mdio_wait_busy(struct ipq40xx_mdio_data *am) +{ + int i; + + for (i = 0; i < IPQ40XX_MDIO_RETRY; i++) { + unsigned int busy; + + busy = readl(am->membase + MDIO_CTRL_4_REG) & + MDIO_CTRL_4_ACCESS_BUSY; + if (!busy) + return 0; + + /* BUSY might take to be cleard by 15~20 times of loop */ + udelay(IPQ40XX_MDIO_DELAY); + } + + dev_err(am->dev, "%s: MDIO operation timed out\n", am->mii_bus->name); + + return -ETIMEDOUT; +} + +static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum) +{ + struct ipq40xx_mdio_data *am = bus->priv; + int value = 0; + unsigned int cmd = 0; + + lockdep_assert_held(&bus->mdio_lock); + + if (ipq40xx_mdio_wait_busy(am)) + return -ETIMEDOUT; + + /* issue the phy address and reg */ + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); + + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_READ; + + /* issue read command */ + writel(cmd, am->membase + MDIO_CTRL_4_REG); + + /* Wait read complete */ + if (ipq40xx_mdio_wait_busy(am)) + return -ETIMEDOUT; + + /* Read data */ + value = readl(am->membase + MDIO_CTRL_3_REG); + + return value; +} + +static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum, + u16 value) +{ + struct ipq40xx_mdio_data *am = bus->priv; + unsigned int cmd = 0; + + lockdep_assert_held(&bus->mdio_lock); + + if (ipq40xx_mdio_wait_busy(am)) + return -ETIMEDOUT; + + /* issue the phy address and reg */ + writel((mii_id << 8) | regnum, am->membase + MDIO_CTRL_1_REG); + + /* issue write data */ + writel(value, am->membase + MDIO_CTRL_2_REG); + + cmd = MDIO_CTRL_4_ACCESS_START | MDIO_CTRL_4_ACCESS_CODE_WRITE; + /* issue write command */ + writel(cmd, am->membase + MDIO_CTRL_4_REG); + + /* Wait write complete */ + if (ipq40xx_mdio_wait_busy(am)) + return -ETIMEDOUT; + + return 0; +} + +static int ipq40xx_mdio_probe(struct platform_device *pdev) +{ + struct ipq40xx_mdio_data *am; + struct resource *res; + + am = devm_kzalloc(&pdev->dev, sizeof(*am), GFP_KERNEL); + if (!am) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) { + dev_err(&pdev->dev, "no iomem resource found\n"); + return -ENXIO; + } + + am->membase = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(am->membase)) { + dev_err(&pdev->dev, "unable to ioremap registers\n"); + return PTR_ERR(am->membase); + } + + am->mii_bus = devm_mdiobus_alloc(&pdev->dev); + if (!am->mii_bus) + return -ENOMEM; + + writel(CTRL_0_REG_DEFAULT_VALUE, am->membase + MDIO_CTRL_0_REG); + + am->mii_bus->name = "ipq40xx_mdio"; + am->mii_bus->read = ipq40xx_mdio_read; + am->mii_bus->write = ipq40xx_mdio_write; + am->mii_bus->priv = am; + am->mii_bus->parent = &pdev->dev; + snprintf(am->mii_bus->id, MII_BUS_ID_SIZE, "%s", dev_name(&pdev->dev)); + + am->dev = &pdev->dev; + platform_set_drvdata(pdev, am); + + return of_mdiobus_register(am->mii_bus, pdev->dev.of_node); +} + +static int ipq40xx_mdio_remove(struct platform_device *pdev) +{ + struct ipq40xx_mdio_data *am = platform_get_drvdata(pdev); + + mdiobus_unregister(am->mii_bus); + + return 0; +} + +static const struct of_device_id ipq40xx_mdio_dt_ids[] = { + { .compatible = "qcom,ipq40xx-mdio" }, + { } +}; +MODULE_DEVICE_TABLE(of, ipq40xx_mdio_dt_ids); + +static struct platform_driver ipq40xx_mdio_driver = { + .probe = ipq40xx_mdio_probe, + .remove = ipq40xx_mdio_remove, + .driver = { + .name = "ipq40xx-mdio", + .of_match_table = ipq40xx_mdio_dt_ids, + }, +}; + +module_platform_driver(ipq40xx_mdio_driver); + +MODULE_DESCRIPTION("IPQ40XX MDIO interface driver"); +MODULE_AUTHOR("Qualcomm Atheros"); +MODULE_LICENSE("Dual BSD/GPL");