This patch adds the driver for the MDIO interface
inside of Qualcomm IPQ40xx series SoC-s.
Signed-off-by: Christian Lamparter <[email protected]>
Signed-off-by: Robert Marko <[email protected]>
Cc: Luka Perkov <[email protected]>
---
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);
+ 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");
--
2.26.0
This patch adds the necessary MDIO interface node
to the Qualcomm IPQ4019 DTSI.
Built-in QCA8337N switch is managed using it,
and since we have a driver for it lets add it.
Signed-off-by: Christian Lamparter <[email protected]>
Signed-off-by: Robert Marko <[email protected]>
Cc: Luka Perkov <[email protected]>
---
arch/arm/boot/dts/qcom-ipq4019.dtsi | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi
index b4803a428340..80d0a69e9fed 100644
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -578,6 +578,34 @@ wifi1: wifi@a800000 {
status = "disabled";
};
+ mdio: mdio@90000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "qcom,ipq40xx-mdio";
+ reg = <0x90000 0x64>;
+ status = "disabled";
+
+ ethphy0: ethernet-phy@0 {
+ reg = <0>;
+ };
+
+ ethphy1: ethernet-phy@1 {
+ reg = <1>;
+ };
+
+ ethphy2: ethernet-phy@2 {
+ reg = <2>;
+ };
+
+ ethphy3: ethernet-phy@3 {
+ reg = <3>;
+ };
+
+ ethphy4: ethernet-phy@4 {
+ reg = <4>;
+ };
+ };
+
usb3_ss_phy: ssphy@9a000 {
compatible = "qcom,usb-ss-ipq4019-phy";
#phy-cells = <0>;
--
2.26.0
This patch adds the binding document for the IPQ40xx MDIO driver.
Signed-off-by: Robert Marko <[email protected]>
Cc: Luka Perkov <[email protected]>
---
.../bindings/net/qcom,ipq40xx-mdio.yaml | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml
diff --git a/Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml
new file mode 100644
index 000000000000..3e2ccf417fb6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/qcom,ipq40xx-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings
+
+maintainers:
+ - Robert Marko <[email protected]>
+
+allOf:
+ - $ref: "mdio.yaml#"
+
+properties:
+ compatible:
+ const: qcom,ipq40xx-mdio
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+
+examples:
+ - |
+ mdio@90000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "qcom,ipq40xx-mdio";
+ reg = <0x90000 0x64>;
+ status = "disabled";
+
+ ethphy0: ethernet-phy@0 {
+ reg = <0>;
+ };
+
+ ethphy1: ethernet-phy@1 {
+ reg = <1>;
+ };
+
+ ethphy2: ethernet-phy@2 {
+ reg = <2>;
+ };
+
+ ethphy3: ethernet-phy@3 {
+ reg = <3>;
+ };
+
+ ethphy4: ethernet-phy@4 {
+ reg = <4>;
+ };
+ };
--
2.26.0
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 <[email protected]>
> Signed-off-by: Robert Marko <[email protected]>
> Cc: Luka Perkov <[email protected]>
> ---
> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > Signed-off-by: Robert Marko <[email protected]>
> > Cc: Luka Perkov <[email protected]>
> > ---
> > 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