linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver
@ 2020-04-15 15:02 Robert Marko
  2020-04-15 15:02 ` [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings Robert Marko
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Robert Marko @ 2020-04-15 15:02 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, linux, linux-kernel, netdev,
	agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree
  Cc: Robert Marko, Christian Lamparter, Luka Perkov

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>
---
Changes from v2 to v3:
* Rename registers
* Remove unnecessary variable initialisations
* Switch to readl_poll_timeout() instead of custom solution
* Drop unused header

Changes from v1 to v2:
* Remove magic default value
* Remove lockdep_assert_held
* Add C45 check
* Simplify the driver
* Drop device and mii_bus structs from private struct
* Use devm_mdiobus_alloc_size()

 drivers/net/phy/Kconfig        |   7 ++
 drivers/net/phy/Makefile       |   1 +
 drivers/net/phy/mdio-ipq40xx.c | 160 +++++++++++++++++++++++++++++++++
 3 files changed, 168 insertions(+)
 create mode 100644 drivers/net/phy/mdio-ipq40xx.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3fa33d27eeba..23bb5db033e3 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_MDIO
+	help
+	  This driver supports the MDIO interface found in Qualcomm
+	  IPQ40xx series Soc-s.
+
 config MDIO_IPQ8064
 	tristate "Qualcomm IPQ8064 MDIO interface support"
 	depends on HAS_IOMEM && OF_MDIO
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 2f5c7093a65b..36aafc6128c4 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -37,6 +37,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_IPQ8064)	+= mdio-ipq8064.o
 obj-$(CONFIG_MDIO_MOXART)	+= mdio-moxart.o
 obj-$(CONFIG_MDIO_MSCC_MIIM)	+= mdio-mscc-miim.o
diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c
new file mode 100644
index 000000000000..acf1230341bd
--- /dev/null
+++ b/drivers/net/phy/mdio-ipq40xx.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2020 Sartura Ltd. */
+
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+
+#define MDIO_ADDR_REG				0x44
+#define MDIO_DATA_WRITE_REG			0x48
+#define MDIO_DATA_READ_REG			0x4c
+#define MDIO_CMD_REG				0x50
+#define MDIO_CMD_ACCESS_BUSY		BIT(16)
+#define MDIO_CMD_ACCESS_START		BIT(8)
+#define MDIO_CMD_ACCESS_CODE_READ	0
+#define MDIO_CMD_ACCESS_CODE_WRITE	1
+
+#define IPQ40XX_MDIO_TIMEOUT	10000
+#define IPQ40XX_MDIO_SLEEP		10
+
+struct ipq40xx_mdio_data {
+	void __iomem	*membase;
+};
+
+static int ipq40xx_mdio_wait_busy(struct mii_bus *bus)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	unsigned int busy;
+
+	return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy,
+				  (busy & MDIO_CMD_ACCESS_BUSY) == 0, 
+				  IPQ40XX_MDIO_SLEEP, IPQ40XX_MDIO_TIMEOUT);
+}
+
+static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	unsigned int cmd;
+
+	/* Reject clause 45 */
+	if (regnum & MII_ADDR_C45)
+		return -EOPNOTSUPP;
+
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* issue the phy address and reg */
+	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
+
+	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_READ;
+
+	/* issue read command */
+	writel(cmd, priv->membase + MDIO_CMD_REG);
+
+	/* Wait read complete */
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* Read and return data */
+	return readl(priv->membase + MDIO_DATA_READ_REG);
+}
+
+static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
+							 u16 value)
+{
+	struct ipq40xx_mdio_data *priv = bus->priv;
+	unsigned int cmd;
+
+	/* Reject clause 45 */
+	if (regnum & MII_ADDR_C45)
+		return -EOPNOTSUPP;
+
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	/* issue the phy address and reg */
+	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
+
+	/* issue write data */
+	writel(value, priv->membase + MDIO_DATA_WRITE_REG);
+
+	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_WRITE;
+	/* issue write command */
+	writel(cmd, priv->membase + MDIO_CMD_REG);
+
+	/* Wait write complete */
+	if (ipq40xx_mdio_wait_busy(bus))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int ipq40xx_mdio_probe(struct platform_device *pdev)
+{
+	struct ipq40xx_mdio_data *priv;
+	struct mii_bus *bus;
+	int ret;
+
+	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
+	if (!bus)
+		return -ENOMEM;
+
+	priv = bus->priv;
+
+	priv->membase = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->membase))
+		return PTR_ERR(priv->membase);
+
+	bus->name = "ipq40xx_mdio";
+	bus->read = ipq40xx_mdio_read;
+	bus->write = ipq40xx_mdio_write;
+	bus->parent = &pdev->dev;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
+
+	ret = of_mdiobus_register(bus, pdev->dev.of_node);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, bus);
+
+	return 0;
+}
+
+static int ipq40xx_mdio_remove(struct platform_device *pdev)
+{
+	struct mii_bus *bus = platform_get_drvdata(pdev);
+
+	mdiobus_unregister(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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings
  2020-04-15 15:02 [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Robert Marko
@ 2020-04-15 15:02 ` Robert Marko
  2020-04-15 23:49   ` Andrew Lunn
                     ` (2 more replies)
  2020-04-15 15:02 ` [PATCH v3 3/3] ARM: dts: qcom: ipq4019: add MDIO node Robert Marko
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 14+ messages in thread
From: Robert Marko @ 2020-04-15 15:02 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, linux, linux-kernel, netdev,
	agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree
  Cc: Robert Marko, Luka Perkov

This patch adds the binding document for the IPQ40xx MDIO driver.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
Changes from v2 to v3:
* Remove status from example

 .../bindings/net/qcom,ipq40xx-mdio.yaml       | 61 +++++++++++++++++++
 1 file changed, 61 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..8d4542ccd38c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml
@@ -0,0 +1,61 @@
+# 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 <robert.marko@sartura.hr>
+
+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>;
+
+      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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/3] ARM: dts: qcom: ipq4019: add MDIO node
  2020-04-15 15:02 [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Robert Marko
  2020-04-15 15:02 ` [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings Robert Marko
@ 2020-04-15 15:02 ` Robert Marko
  2020-04-15 23:51   ` Andrew Lunn
  2020-04-15 23:56   ` Florian Fainelli
  2020-04-15 23:48 ` [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Andrew Lunn
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 14+ messages in thread
From: Robert Marko @ 2020-04-15 15:02 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, linux, linux-kernel, netdev,
	agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree
  Cc: Robert Marko, Christian Lamparter, Luka Perkov

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 <chunkeey@gmail.com>
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
Changes from v2 to v3:
* Correct commit title

 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver
  2020-04-15 15:02 [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Robert Marko
  2020-04-15 15:02 ` [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings Robert Marko
  2020-04-15 15:02 ` [PATCH v3 3/3] ARM: dts: qcom: ipq4019: add MDIO node Robert Marko
@ 2020-04-15 23:48 ` Andrew Lunn
  2020-04-16  8:43   ` Robert Marko
  2020-04-15 23:57 ` Florian Fainelli
  2020-04-16  7:28 ` Leon Romanovsky
  4 siblings, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2020-04-15 23:48 UTC (permalink / raw)
  To: Robert Marko
  Cc: f.fainelli, hkallweit1, linux, linux-kernel, netdev, agross,
	bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree, Christian Lamparter, Luka Perkov

Hi Robert

I should of said this earlier. With a patch set, you should include a
cover note, patch 0 of X, explaining the big picture of what the
patches do.

Also, for network patches, the subject line should indicate which tree
these patches are for. So

[PATCH net-next v3 0/3] 

On Wed, Apr 15, 2020 at 05:02:43PM +0200, 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>
> ---
> Changes from v2 to v3:
> * Rename registers
> * Remove unnecessary variable initialisations
> * Switch to readl_poll_timeout() instead of custom solution
> * Drop unused header
> 
> Changes from v1 to v2:
> * Remove magic default value
> * Remove lockdep_assert_held
> * Add C45 check
> * Simplify the driver
> * Drop device and mii_bus structs from private struct
> * Use devm_mdiobus_alloc_size()
> 
>  drivers/net/phy/Kconfig        |   7 ++
>  drivers/net/phy/Makefile       |   1 +
>  drivers/net/phy/mdio-ipq40xx.c | 160 +++++++++++++++++++++++++++++++++
>  3 files changed, 168 insertions(+)
>  create mode 100644 drivers/net/phy/mdio-ipq40xx.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 3fa33d27eeba..23bb5db033e3 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_MDIO
> +	help
> +	  This driver supports the MDIO interface found in Qualcomm
> +	  IPQ40xx series Soc-s.
> +
>  config MDIO_IPQ8064
>  	tristate "Qualcomm IPQ8064 MDIO interface support"
>  	depends on HAS_IOMEM && OF_MDIO
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 2f5c7093a65b..36aafc6128c4 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -37,6 +37,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_IPQ8064)	+= mdio-ipq8064.o
>  obj-$(CONFIG_MDIO_MOXART)	+= mdio-moxart.o
>  obj-$(CONFIG_MDIO_MSCC_MIIM)	+= mdio-mscc-miim.o
> diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c
> new file mode 100644
> index 000000000000..acf1230341bd
> --- /dev/null
> +++ b/drivers/net/phy/mdio-ipq40xx.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */
> +/* Copyright (c) 2020 Sartura Ltd. */
> +
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/of_address.h>
> +#include <linux/of_mdio.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define MDIO_ADDR_REG				0x44
> +#define MDIO_DATA_WRITE_REG			0x48
> +#define MDIO_DATA_READ_REG			0x4c
> +#define MDIO_CMD_REG				0x50
> +#define MDIO_CMD_ACCESS_BUSY		BIT(16)
> +#define MDIO_CMD_ACCESS_START		BIT(8)
> +#define MDIO_CMD_ACCESS_CODE_READ	0
> +#define MDIO_CMD_ACCESS_CODE_WRITE	1
> +
> +#define IPQ40XX_MDIO_TIMEOUT	10000
> +#define IPQ40XX_MDIO_SLEEP		10
> +
> +struct ipq40xx_mdio_data {
> +	void __iomem	*membase;
> +};
> +
> +static int ipq40xx_mdio_wait_busy(struct mii_bus *bus)
> +{
> +	struct ipq40xx_mdio_data *priv = bus->priv;
> +	unsigned int busy;
> +
> +	return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy,
> +				  (busy & MDIO_CMD_ACCESS_BUSY) == 0, 
> +				  IPQ40XX_MDIO_SLEEP, IPQ40XX_MDIO_TIMEOUT);

Do you have any documentation about _START and _BUSY? You are making
the assumption that the next read after writing the START bit will
have the BUSY bit set. That the hardware reacts that fast. It is not
an unreasonable assumption, but i've seen more designed where the
START bit is also the BUSY bit, so the write implicitly sets the busy
bit, and the hardware needs to clear it when it is done.

As i said, this is not unreasonable, so:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings
  2020-04-15 15:02 ` [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings Robert Marko
@ 2020-04-15 23:49   ` Andrew Lunn
  2020-04-15 23:55   ` Florian Fainelli
  2020-04-28 15:09   ` Rob Herring
  2 siblings, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2020-04-15 23:49 UTC (permalink / raw)
  To: Robert Marko
  Cc: f.fainelli, hkallweit1, linux, linux-kernel, netdev, agross,
	bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree, Luka Perkov

On Wed, Apr 15, 2020 at 05:02:46PM +0200, Robert Marko wrote:
> This patch adds the binding document for the IPQ40xx MDIO driver.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/3] ARM: dts: qcom: ipq4019: add MDIO node
  2020-04-15 15:02 ` [PATCH v3 3/3] ARM: dts: qcom: ipq4019: add MDIO node Robert Marko
@ 2020-04-15 23:51   ` Andrew Lunn
  2020-04-15 23:56   ` Florian Fainelli
  1 sibling, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2020-04-15 23:51 UTC (permalink / raw)
  To: Robert Marko
  Cc: f.fainelli, hkallweit1, linux, linux-kernel, netdev, agross,
	bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree, Christian Lamparter, Luka Perkov

On Wed, Apr 15, 2020 at 05:02:48PM +0200, Robert Marko wrote:
> 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 <chunkeey@gmail.com>
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings
  2020-04-15 15:02 ` [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings Robert Marko
  2020-04-15 23:49   ` Andrew Lunn
@ 2020-04-15 23:55   ` Florian Fainelli
  2020-04-16  8:43     ` Robert Marko
  2020-04-28 15:09   ` Rob Herring
  2 siblings, 1 reply; 14+ messages in thread
From: Florian Fainelli @ 2020-04-15 23:55 UTC (permalink / raw)
  To: Robert Marko, andrew, hkallweit1, linux, linux-kernel, netdev,
	agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree
  Cc: Luka Perkov



On 4/15/2020 8:02 AM, Robert Marko wrote:
> This patch adds the binding document for the IPQ40xx MDIO driver.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Please do not drop tags when they were given to you in an earlier version.
-- 
Florian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/3] ARM: dts: qcom: ipq4019: add MDIO node
  2020-04-15 15:02 ` [PATCH v3 3/3] ARM: dts: qcom: ipq4019: add MDIO node Robert Marko
  2020-04-15 23:51   ` Andrew Lunn
@ 2020-04-15 23:56   ` Florian Fainelli
  1 sibling, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2020-04-15 23:56 UTC (permalink / raw)
  To: Robert Marko, andrew, hkallweit1, linux, linux-kernel, netdev,
	agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree
  Cc: Christian Lamparter, Luka Perkov



On 4/15/2020 8:02 AM, Robert Marko wrote:
> 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 <chunkeey@gmail.com>
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver
  2020-04-15 15:02 [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Robert Marko
                   ` (2 preceding siblings ...)
  2020-04-15 23:48 ` [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Andrew Lunn
@ 2020-04-15 23:57 ` Florian Fainelli
  2020-04-16  7:28 ` Leon Romanovsky
  4 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2020-04-15 23:57 UTC (permalink / raw)
  To: Robert Marko, andrew, hkallweit1, linux, linux-kernel, netdev,
	agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree
  Cc: Christian Lamparter, Luka Perkov



On 4/15/2020 8:02 AM, 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>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver
  2020-04-15 15:02 [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Robert Marko
                   ` (3 preceding siblings ...)
  2020-04-15 23:57 ` Florian Fainelli
@ 2020-04-16  7:28 ` Leon Romanovsky
  4 siblings, 0 replies; 14+ messages in thread
From: Leon Romanovsky @ 2020-04-16  7:28 UTC (permalink / raw)
  To: Robert Marko
  Cc: andrew, f.fainelli, hkallweit1, linux, linux-kernel, netdev,
	agross, bjorn.andersson, robh+dt, mark.rutland, linux-arm-msm,
	devicetree, Christian Lamparter, Luka Perkov

On Wed, Apr 15, 2020 at 05:02:43PM +0200, 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>
> ---
> Changes from v2 to v3:
> * Rename registers
> * Remove unnecessary variable initialisations
> * Switch to readl_poll_timeout() instead of custom solution
> * Drop unused header
>
> Changes from v1 to v2:
> * Remove magic default value
> * Remove lockdep_assert_held
> * Add C45 check
> * Simplify the driver
> * Drop device and mii_bus structs from private struct
> * Use devm_mdiobus_alloc_size()
>
>  drivers/net/phy/Kconfig        |   7 ++
>  drivers/net/phy/Makefile       |   1 +
>  drivers/net/phy/mdio-ipq40xx.c | 160 +++++++++++++++++++++++++++++++++
>  3 files changed, 168 insertions(+)
>  create mode 100644 drivers/net/phy/mdio-ipq40xx.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 3fa33d27eeba..23bb5db033e3 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_MDIO
> +	help
> +	  This driver supports the MDIO interface found in Qualcomm
> +	  IPQ40xx series Soc-s.
> +
>  config MDIO_IPQ8064
>  	tristate "Qualcomm IPQ8064 MDIO interface support"
>  	depends on HAS_IOMEM && OF_MDIO
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 2f5c7093a65b..36aafc6128c4 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -37,6 +37,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_IPQ8064)	+= mdio-ipq8064.o
>  obj-$(CONFIG_MDIO_MOXART)	+= mdio-moxart.o
>  obj-$(CONFIG_MDIO_MSCC_MIIM)	+= mdio-mscc-miim.o
> diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c
> new file mode 100644
> index 000000000000..acf1230341bd
> --- /dev/null
> +++ b/drivers/net/phy/mdio-ipq40xx.c
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */
> +/* Copyright (c) 2020 Sartura Ltd. */
> +
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/of_address.h>
> +#include <linux/of_mdio.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +
> +#define MDIO_ADDR_REG				0x44
> +#define MDIO_DATA_WRITE_REG			0x48
> +#define MDIO_DATA_READ_REG			0x4c
> +#define MDIO_CMD_REG				0x50
> +#define MDIO_CMD_ACCESS_BUSY		BIT(16)
> +#define MDIO_CMD_ACCESS_START		BIT(8)
> +#define MDIO_CMD_ACCESS_CODE_READ	0
> +#define MDIO_CMD_ACCESS_CODE_WRITE	1
> +
> +#define IPQ40XX_MDIO_TIMEOUT	10000
> +#define IPQ40XX_MDIO_SLEEP		10
> +
> +struct ipq40xx_mdio_data {
> +	void __iomem	*membase;
> +};
> +
> +static int ipq40xx_mdio_wait_busy(struct mii_bus *bus)
> +{
> +	struct ipq40xx_mdio_data *priv = bus->priv;
> +	unsigned int busy;
> +
> +	return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy,
> +				  (busy & MDIO_CMD_ACCESS_BUSY) == 0,
> +				  IPQ40XX_MDIO_SLEEP, IPQ40XX_MDIO_TIMEOUT);
> +}
> +
> +static int ipq40xx_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> +{
> +	struct ipq40xx_mdio_data *priv = bus->priv;
> +	unsigned int cmd;
> +
> +	/* Reject clause 45 */
> +	if (regnum & MII_ADDR_C45)
> +		return -EOPNOTSUPP;
> +
> +	if (ipq40xx_mdio_wait_busy(bus))
> +		return -ETIMEDOUT;
> +
> +	/* issue the phy address and reg */
> +	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
> +
> +	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_READ;
> +
> +	/* issue read command */
> +	writel(cmd, priv->membase + MDIO_CMD_REG);
> +
> +	/* Wait read complete */
> +	if (ipq40xx_mdio_wait_busy(bus))
> +		return -ETIMEDOUT;
> +
> +	/* Read and return data */
> +	return readl(priv->membase + MDIO_DATA_READ_REG);
> +}
> +
> +static int ipq40xx_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> +							 u16 value)
> +{
> +	struct ipq40xx_mdio_data *priv = bus->priv;
> +	unsigned int cmd;
> +
> +	/* Reject clause 45 */
> +	if (regnum & MII_ADDR_C45)
> +		return -EOPNOTSUPP;
> +
> +	if (ipq40xx_mdio_wait_busy(bus))
> +		return -ETIMEDOUT;
> +
> +	/* issue the phy address and reg */
> +	writel((mii_id << 8) | regnum, priv->membase + MDIO_ADDR_REG);
> +
> +	/* issue write data */
> +	writel(value, priv->membase + MDIO_DATA_WRITE_REG);
> +
> +	cmd = MDIO_CMD_ACCESS_START | MDIO_CMD_ACCESS_CODE_WRITE;
> +	/* issue write command */
> +	writel(cmd, priv->membase + MDIO_CMD_REG);
> +
> +	/* Wait write complete */
> +	if (ipq40xx_mdio_wait_busy(bus))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int ipq40xx_mdio_probe(struct platform_device *pdev)
> +{
> +	struct ipq40xx_mdio_data *priv;
> +	struct mii_bus *bus;
> +	int ret;
> +
> +	bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	priv = bus->priv;
> +
> +	priv->membase = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->membase))
> +		return PTR_ERR(priv->membase);
> +
> +	bus->name = "ipq40xx_mdio";
> +	bus->read = ipq40xx_mdio_read;
> +	bus->write = ipq40xx_mdio_write;
> +	bus->parent = &pdev->dev;
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
> +
> +	ret = of_mdiobus_register(bus, pdev->dev.of_node);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, bus);
> +
> +	return 0;
> +}
> +
> +static int ipq40xx_mdio_remove(struct platform_device *pdev)
> +{
> +	struct mii_bus *bus = platform_get_drvdata(pdev);
> +
> +	mdiobus_unregister(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");

Strictly saying, but author can't be company.

> +MODULE_LICENSE("Dual BSD/GPL");
> --
> 2.26.0
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver
  2020-04-15 23:48 ` [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Andrew Lunn
@ 2020-04-16  8:43   ` Robert Marko
  0 siblings, 0 replies; 14+ messages in thread
From: Robert Marko @ 2020-04-16  8:43 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Heiner Kallweit,
	Russell King - ARM Linux admin, linux-kernel, netdev, Andy Gross,
	Bjorn Andersson, robh+dt, Mark Rutland, linux-arm-msm,
	devicetree, Christian Lamparter, Luka Perkov

On Thu, Apr 16, 2020 at 1:48 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> Hi Robert
>
> I should of said this earlier. With a patch set, you should include a
> cover note, patch 0 of X, explaining the big picture of what the
> patches do.

Sorry, I thought that was a relatively small patch set and cover
letter was not needed.
>
> Also, for network patches, the subject line should indicate which tree
> these patches are for. So
>
> [PATCH net-next v3 0/3]
OK, this is my first contribution to networking and I did not check first.
I am used to sending patches without it.
Now I know for the future,
Thanks
>
> On Wed, Apr 15, 2020 at 05:02:43PM +0200, 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>
> > ---
> > Changes from v2 to v3:
> > * Rename registers
> > * Remove unnecessary variable initialisations
> > * Switch to readl_poll_timeout() instead of custom solution
> > * Drop unused header
> >
> > Changes from v1 to v2:
> > * Remove magic default value
> > * Remove lockdep_assert_held
> > * Add C45 check
> > * Simplify the driver
> > * Drop device and mii_bus structs from private struct
> > * Use devm_mdiobus_alloc_size()
> >
> >  drivers/net/phy/Kconfig        |   7 ++
> >  drivers/net/phy/Makefile       |   1 +
> >  drivers/net/phy/mdio-ipq40xx.c | 160 +++++++++++++++++++++++++++++++++
> >  3 files changed, 168 insertions(+)
> >  create mode 100644 drivers/net/phy/mdio-ipq40xx.c
> >
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 3fa33d27eeba..23bb5db033e3 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_MDIO
> > +     help
> > +       This driver supports the MDIO interface found in Qualcomm
> > +       IPQ40xx series Soc-s.
> > +
> >  config MDIO_IPQ8064
> >       tristate "Qualcomm IPQ8064 MDIO interface support"
> >       depends on HAS_IOMEM && OF_MDIO
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index 2f5c7093a65b..36aafc6128c4 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -37,6 +37,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_IPQ8064)   += mdio-ipq8064.o
> >  obj-$(CONFIG_MDIO_MOXART)    += mdio-moxart.o
> >  obj-$(CONFIG_MDIO_MSCC_MIIM) += mdio-mscc-miim.o
> > diff --git a/drivers/net/phy/mdio-ipq40xx.c b/drivers/net/phy/mdio-ipq40xx.c
> > new file mode 100644
> > index 000000000000..acf1230341bd
> > --- /dev/null
> > +++ b/drivers/net/phy/mdio-ipq40xx.c
> > @@ -0,0 +1,160 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> > +/* Copyright (c) 2015, The Linux Foundation. All rights reserved. */
> > +/* Copyright (c) 2020 Sartura Ltd. */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_mdio.h>
> > +#include <linux/phy.h>
> > +#include <linux/platform_device.h>
> > +
> > +#define MDIO_ADDR_REG                                0x44
> > +#define MDIO_DATA_WRITE_REG                  0x48
> > +#define MDIO_DATA_READ_REG                   0x4c
> > +#define MDIO_CMD_REG                         0x50
> > +#define MDIO_CMD_ACCESS_BUSY         BIT(16)
> > +#define MDIO_CMD_ACCESS_START                BIT(8)
> > +#define MDIO_CMD_ACCESS_CODE_READ    0
> > +#define MDIO_CMD_ACCESS_CODE_WRITE   1
> > +
> > +#define IPQ40XX_MDIO_TIMEOUT 10000
> > +#define IPQ40XX_MDIO_SLEEP           10
> > +
> > +struct ipq40xx_mdio_data {
> > +     void __iomem    *membase;
> > +};
> > +
> > +static int ipq40xx_mdio_wait_busy(struct mii_bus *bus)
> > +{
> > +     struct ipq40xx_mdio_data *priv = bus->priv;
> > +     unsigned int busy;
> > +
> > +     return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy,
> > +                               (busy & MDIO_CMD_ACCESS_BUSY) == 0,
> > +                               IPQ40XX_MDIO_SLEEP, IPQ40XX_MDIO_TIMEOUT);
>
> Do you have any documentation about _START and _BUSY? You are making
> the assumption that the next read after writing the START bit will
> have the BUSY bit set. That the hardware reacts that fast. It is not
> an unreasonable assumption, but i've seen more designed where the
> START bit is also the BUSY bit, so the write implicitly sets the busy
> bit, and the hardware needs to clear it when it is done.
No, I don't have any docs at all for the SoC.
It's all based on GPL drivers from SDK and QCA comments in
the drivers as well as U-boot drivers.
>
> As i said, this is not unreasonable, so:
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
>     Andrew
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings
  2020-04-15 23:55   ` Florian Fainelli
@ 2020-04-16  8:43     ` Robert Marko
  0 siblings, 0 replies; 14+ messages in thread
From: Robert Marko @ 2020-04-16  8:43 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, Heiner Kallweit, Russell King - ARM Linux admin,
	linux-kernel, netdev, Andy Gross, Bjorn Andersson, robh+dt,
	Mark Rutland, linux-arm-msm, devicetree, Luka Perkov

On Thu, Apr 16, 2020 at 1:55 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 4/15/2020 8:02 AM, Robert Marko wrote:
> > This patch adds the binding document for the IPQ40xx MDIO driver.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > Cc: Luka Perkov <luka.perkov@sartura.hr>
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Please do not drop tags when they were given to you in an earlier version.
Sorry, I did not know that.
Wont do it in future,
thanks
> --
> Florian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings
  2020-04-15 15:02 ` [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings Robert Marko
  2020-04-15 23:49   ` Andrew Lunn
  2020-04-15 23:55   ` Florian Fainelli
@ 2020-04-28 15:09   ` Rob Herring
  2020-04-28 16:40     ` Robert Marko
  2 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2020-04-28 15:09 UTC (permalink / raw)
  To: Robert Marko
  Cc: andrew, f.fainelli, hkallweit1, linux, linux-kernel, netdev,
	agross, bjorn.andersson, mark.rutland, linux-arm-msm, devicetree,
	Luka Perkov

On Wed, Apr 15, 2020 at 05:02:46PM +0200, Robert Marko wrote:
> This patch adds the binding document for the IPQ40xx MDIO driver.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> ---
> Changes from v2 to v3:
> * Remove status from example
> 
>  .../bindings/net/qcom,ipq40xx-mdio.yaml       | 61 +++++++++++++++++++
>  1 file changed, 61 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..8d4542ccd38c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%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 <robert.marko@sartura.hr>
> +
> +allOf:
> +  - $ref: "mdio.yaml#"
> +
> +properties:
> +  compatible:
> +    const: qcom,ipq40xx-mdio

Don't use wildcards in compatible names. Should be SoC specific. If 'all 
the same', then use a fallback to the 1st implementation.

> +
> +  "#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>;
> +
> +      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
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings
  2020-04-28 15:09   ` Rob Herring
@ 2020-04-28 16:40     ` Robert Marko
  0 siblings, 0 replies; 14+ messages in thread
From: Robert Marko @ 2020-04-28 16:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit,
	Russell King - ARM Linux admin, linux-kernel, netdev, Andy Gross,
	Bjorn Andersson, Mark Rutland, linux-arm-msm, devicetree,
	Luka Perkov

On Tue, Apr 28, 2020 at 5:09 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Apr 15, 2020 at 05:02:46PM +0200, Robert Marko wrote:
> > This patch adds the binding document for the IPQ40xx MDIO driver.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > ---
> > Changes from v2 to v3:
> > * Remove status from example
> >
> >  .../bindings/net/qcom,ipq40xx-mdio.yaml       | 61 +++++++++++++++++++
> >  1 file changed, 61 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..8d4542ccd38c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/qcom,ipq40xx-mdio.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: GPL-2.0
>
> Dual license new bindings please:
>
> (GPL-2.0-only OR BSD-2-Clause)
Ok, will change it in next revision.
>
> > +%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 <robert.marko@sartura.hr>
> > +
> > +allOf:
> > +  - $ref: "mdio.yaml#"
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,ipq40xx-mdio
>
> Don't use wildcards in compatible names. Should be SoC specific. If 'all
> the same', then use a fallback to the 1st implementation.
Ok, I will change it to qcom,ipq4019 then.
The whole IPQ40xx series uses the same controller.
>
> > +
> > +  "#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>;
> > +
> > +      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
> >

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-04-28 16:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-15 15:02 [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Robert Marko
2020-04-15 15:02 ` [PATCH v3 2/3] dt-bindings: add Qualcomm IPQ4019 MDIO bindings Robert Marko
2020-04-15 23:49   ` Andrew Lunn
2020-04-15 23:55   ` Florian Fainelli
2020-04-16  8:43     ` Robert Marko
2020-04-28 15:09   ` Rob Herring
2020-04-28 16:40     ` Robert Marko
2020-04-15 15:02 ` [PATCH v3 3/3] ARM: dts: qcom: ipq4019: add MDIO node Robert Marko
2020-04-15 23:51   ` Andrew Lunn
2020-04-15 23:56   ` Florian Fainelli
2020-04-15 23:48 ` [PATCH v3 1/3] net: phy: mdio: add IPQ40xx MDIO driver Andrew Lunn
2020-04-16  8:43   ` Robert Marko
2020-04-15 23:57 ` Florian Fainelli
2020-04-16  7:28 ` Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).