linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] net: mdio: Add BCM6368 MDIO mux bus controller
@ 2021-03-08 18:41 Álvaro Fernández Rojas
  2021-03-08 18:41 ` [PATCH 1/2] dt-bindings: net: Add bcm6368-mdio-mux bindings Álvaro Fernández Rojas
  2021-03-08 18:41 ` [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller Álvaro Fernández Rojas
  0 siblings, 2 replies; 11+ messages in thread
From: Álvaro Fernández Rojas @ 2021-03-08 18:41 UTC (permalink / raw)
  To: jonas.gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Andrew Lunn, Heiner Kallweit, Russell King, Fernández Rojas,
	netdev, devicetree, linux-kernel

This controller is present on BCM6318, BCM6328, BCM6362, BCM6368 and BCM63268
SoCs.

Álvaro Fernández Rojas (2):
  dt-bindings: net: Add bcm6368-mdio-mux bindings
  net: mdio: Add BCM6368 MDIO mux bus controller

 .../bindings/net/brcm,bcm6368-mdio-mux.yaml   |  79 ++++++++
 drivers/net/mdio/Kconfig                      |  11 ++
 drivers/net/mdio/Makefile                     |   1 +
 drivers/net/mdio/mdio-mux-bcm6368.c           | 179 ++++++++++++++++++
 4 files changed, 270 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
 create mode 100644 drivers/net/mdio/mdio-mux-bcm6368.c

-- 
2.20.1


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

* [PATCH 1/2] dt-bindings: net: Add bcm6368-mdio-mux bindings
  2021-03-08 18:41 [PATCH 0/2] net: mdio: Add BCM6368 MDIO mux bus controller Álvaro Fernández Rojas
@ 2021-03-08 18:41 ` Álvaro Fernández Rojas
  2021-03-08 20:54   ` Andrew Lunn
  2021-03-08 18:41 ` [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller Álvaro Fernández Rojas
  1 sibling, 1 reply; 11+ messages in thread
From: Álvaro Fernández Rojas @ 2021-03-08 18:41 UTC (permalink / raw)
  To: jonas.gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Andrew Lunn, Heiner Kallweit, Russell King, Fernández Rojas,
	netdev, devicetree, linux-kernel

Add documentations for bcm6368 mdio mux driver.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 .../bindings/net/brcm,bcm6368-mdio-mux.yaml   | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml

diff --git a/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml b/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
new file mode 100644
index 000000000000..d3481d1c59ae
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/brcm,bcm6368-mdio-mux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM6368 MDIO bus multiplexer
+
+maintainers:
+  - Álvaro Fernández Rojas <noltari@gmail.com>
+
+description:
+  This MDIO bus multiplexer defines buses that could be internal as well as
+  external to SoCs. When child bus is selected, one needs to select these two
+  properties as well to generate desired MDIO transaction on appropriate bus.
+
+allOf:
+  - $ref: "mdio.yaml#"
+
+properties:
+  compatible:
+    const: brcm,bcm6368-mdio-mux
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  '^mdio@[0-1]$':
+    type: object
+    properties:
+      reg:
+        maxItems: 1
+
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+    required:
+      - reg
+      - "#address-cells"
+      - "#size-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mdio0: mdio@10e000b0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      compatible = "brcm,bcm6368-mdio-mux";
+      reg = <0x10e000b0 0x6>;
+
+      mdio_int: mdio@0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <0>;
+      };
+
+      mdio_ext: mdio@1 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <1>;
+      };
+    };
-- 
2.20.1


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

* [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller
  2021-03-08 18:41 [PATCH 0/2] net: mdio: Add BCM6368 MDIO mux bus controller Álvaro Fernández Rojas
  2021-03-08 18:41 ` [PATCH 1/2] dt-bindings: net: Add bcm6368-mdio-mux bindings Álvaro Fernández Rojas
@ 2021-03-08 18:41 ` Álvaro Fernández Rojas
  2021-03-08 19:57   ` Jakub Kicinski
  2021-03-08 21:00   ` Andrew Lunn
  1 sibling, 2 replies; 11+ messages in thread
From: Álvaro Fernández Rojas @ 2021-03-08 18:41 UTC (permalink / raw)
  To: jonas.gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Andrew Lunn, Heiner Kallweit, Russell King, Fernández Rojas,
	netdev, devicetree, linux-kernel

This controller is present on BCM6318, BCM6328, BCM6362, BCM6368 and BCM63268
SoCs.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 drivers/net/mdio/Kconfig            |  11 ++
 drivers/net/mdio/Makefile           |   1 +
 drivers/net/mdio/mdio-mux-bcm6368.c | 179 ++++++++++++++++++++++++++++
 3 files changed, 191 insertions(+)
 create mode 100644 drivers/net/mdio/mdio-mux-bcm6368.c

diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig
index a10cc460d7cf..d06e06f5e31a 100644
--- a/drivers/net/mdio/Kconfig
+++ b/drivers/net/mdio/Kconfig
@@ -200,6 +200,17 @@ config MDIO_BUS_MUX_MESON_G12A
 	  the amlogic g12a SoC. The multiplexers connects either the external
 	  or the internal MDIO bus to the parent bus.
 
+config MDIO_BUS_MUX_BCM6368
+	tristate "Broadcom BCM6368 MDIO bus multiplexers"
+	depends on OF && OF_MDIO && (BMIPS_GENERIC || COMPILE_TEST)
+	select MDIO_BUS_MUX
+	default BMIPS_GENERIC
+	help
+	  This module provides a driver for MDIO bus multiplexers found in
+	  BCM6368 based Broadcom SoCs. This multiplexer connects one of several
+	  child MDIO bus to a parent bus. Buses could be internal as well as
+	  external and selection logic lies inside the same multiplexer.
+
 config MDIO_BUS_MUX_BCM_IPROC
 	tristate "Broadcom iProc based MDIO bus multiplexers"
 	depends on OF && OF_MDIO && (ARCH_BCM_IPROC || COMPILE_TEST)
diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile
index 5c498dde463f..c3ec0ef989df 100644
--- a/drivers/net/mdio/Makefile
+++ b/drivers/net/mdio/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_MDIO_THUNDER)		+= mdio-thunder.o
 obj-$(CONFIG_MDIO_XGENE)		+= mdio-xgene.o
 
 obj-$(CONFIG_MDIO_BUS_MUX)		+= mdio-mux.o
+obj-$(CONFIG_MDIO_BUS_MUX_BCM6368)	+= mdio-mux-bcm6368.o
 obj-$(CONFIG_MDIO_BUS_MUX_BCM_IPROC)	+= mdio-mux-bcm-iproc.o
 obj-$(CONFIG_MDIO_BUS_MUX_GPIO)		+= mdio-mux-gpio.o
 obj-$(CONFIG_MDIO_BUS_MUX_MESON_G12A)	+= mdio-mux-meson-g12a.o
diff --git a/drivers/net/mdio/mdio-mux-bcm6368.c b/drivers/net/mdio/mdio-mux-bcm6368.c
new file mode 100644
index 000000000000..79ab2e2af5b9
--- /dev/null
+++ b/drivers/net/mdio/mdio-mux-bcm6368.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Broadcom BCM6368 mdiomux bus controller driver
+ *
+ * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mdio-mux.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+
+#define MDIOC_REG		0x0
+#define MDIOC_EXT_MASK		BIT(16)
+#define MDIOC_REG_SHIFT		20
+#define MDIOC_PHYID_SHIFT	25
+#define MDIOC_RD_MASK		BIT(30)
+#define MDIOC_WR_MASK		BIT(31)
+
+#define MDIOD_REG		0x4
+
+struct bcm6368_mdiomux_desc {
+	void *mux_handle;
+	void __iomem *base;
+	struct device *dev;
+	struct mii_bus *mii_bus;
+	int ext_phy;
+};
+
+static int bcm6368_mdiomux_read(struct mii_bus *bus, int phy_id, int loc)
+{
+	struct bcm6368_mdiomux_desc *md = bus->priv;
+	uint32_t reg;
+	int ret;
+
+	__raw_writel(0, md->base + MDIOC_REG);
+
+	reg = MDIOC_RD_MASK |
+	      (phy_id << MDIOC_PHYID_SHIFT) |
+	      (loc << MDIOC_REG_SHIFT);
+	if (md->ext_phy)
+		reg |= MDIOC_EXT_MASK;
+
+	__raw_writel(reg, md->base + MDIOC_REG);
+	udelay(50);
+	ret = __raw_readw(md->base + MDIOD_REG);
+
+	return ret;
+}
+
+static int bcm6368_mdiomux_write(struct mii_bus *bus, int phy_id, int loc,
+				 uint16_t val)
+{
+	struct bcm6368_mdiomux_desc *md = bus->priv;
+	uint32_t reg;
+
+	__raw_writel(0, md->base + MDIOC_REG);
+
+	reg = MDIOC_WR_MASK |
+	      (phy_id << MDIOC_PHYID_SHIFT) |
+	      (loc << MDIOC_REG_SHIFT);
+	if (md->ext_phy)
+		reg |= MDIOC_EXT_MASK;
+	reg |= val;
+
+	__raw_writel(reg, md->base + MDIOC_REG);
+	udelay(50);
+
+	return 0;
+}
+
+static int bcm6368_mdiomux_switch_fn(int current_child, int desired_child,
+				     void *data)
+{
+	struct bcm6368_mdiomux_desc *md = data;
+
+	md->ext_phy = desired_child;
+
+	return 0;
+}
+
+static int bcm6368_mdiomux_probe(struct platform_device *pdev)
+{
+	struct bcm6368_mdiomux_desc *md;
+	struct mii_bus *bus;
+	struct resource *res;
+	int rc;
+
+	md = devm_kzalloc(&pdev->dev, sizeof(*md), GFP_KERNEL);
+	if (!md)
+		return -ENOMEM;
+	md->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	/* Just ioremap, as this MDIO block is usually integrated into an
+	 * Ethernet MAC controller register range
+	 */
+	md->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	if (!md->base) {
+		dev_err(&pdev->dev, "failed to ioremap register\n");
+		return -ENOMEM;
+	}
+
+	md->mii_bus = devm_mdiobus_alloc(&pdev->dev);
+	if (!md->mii_bus) {
+		dev_err(&pdev->dev, "mdiomux bus alloc failed\n");
+		return ENOMEM;
+	}
+
+	bus = md->mii_bus;
+	bus->priv = md;
+	bus->name = "BCM6368 MDIO mux bus";
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d", pdev->name, pdev->id);
+	bus->parent = &pdev->dev;
+	bus->read = bcm6368_mdiomux_read;
+	bus->write = bcm6368_mdiomux_write;
+	bus->phy_mask = 0x3f;
+	bus->dev.of_node = pdev->dev.of_node;
+
+	rc = mdiobus_register(bus);
+	if (rc) {
+		dev_err(&pdev->dev, "mdiomux registration failed\n");
+		return rc;
+	}
+
+	platform_set_drvdata(pdev, md);
+
+	rc = mdio_mux_init(md->dev, md->dev->of_node,
+			   bcm6368_mdiomux_switch_fn, &md->mux_handle, md,
+			   md->mii_bus);
+	if (rc) {
+		dev_info(md->dev, "mdiomux initialization failed\n");
+		goto out_register;
+	}
+
+	dev_info(&pdev->dev, "Broadcom BCM6368 MDIO mux bus\n");
+
+	return 0;
+
+out_register:
+	mdiobus_unregister(bus);
+	return rc;
+}
+
+static int bcm6368_mdiomux_remove(struct platform_device *pdev)
+{
+	struct bcm6368_mdiomux_desc *md = platform_get_drvdata(pdev);
+
+	mdio_mux_uninit(md->mux_handle);
+	mdiobus_unregister(md->mii_bus);
+
+	return 0;
+}
+
+static const struct of_device_id bcm6368_mdiomux_ids[] = {
+	{ .compatible = "brcm,bcm6368-mdio-mux", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, bcm6368_mdiomux_ids);
+
+static struct platform_driver bcm6368_mdiomux_driver = {
+	.driver = {
+		.name = "bcm6368-mdio-mux",
+		.of_match_table = bcm6368_mdiomux_ids,
+	},
+	.probe	= bcm6368_mdiomux_probe,
+	.remove	= bcm6368_mdiomux_remove,
+};
+module_platform_driver(bcm6368_mdiomux_driver);
-- 
2.20.1


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

* Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller
  2021-03-08 18:41 ` [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller Álvaro Fernández Rojas
@ 2021-03-08 19:57   ` Jakub Kicinski
  2021-03-15 13:53     ` Álvaro Fernández Rojas
  2021-03-08 21:00   ` Andrew Lunn
  1 sibling, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2021-03-08 19:57 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: jonas.gorski, David S. Miller, Rob Herring, Andrew Lunn,
	Heiner Kallweit, Russell King, netdev, devicetree, linux-kernel

On Mon,  8 Mar 2021 19:41:02 +0100 Álvaro Fernández Rojas wrote:
> This controller is present on BCM6318, BCM6328, BCM6362, BCM6368 and BCM63268
> SoCs.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

make[2]: *** Deleting file 'Module.symvers'
ERROR: modpost: missing MODULE_LICENSE() in drivers/net/mdio/mdio-mux-bcm6368.o
make[2]: *** [Module.symvers] Error 1
make[1]: *** [modules] Error 2
make: *** [__sub-make] Error 2
make[2]: *** Deleting file 'Module.symvers'

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

* Re: [PATCH 1/2] dt-bindings: net: Add bcm6368-mdio-mux bindings
  2021-03-08 18:41 ` [PATCH 1/2] dt-bindings: net: Add bcm6368-mdio-mux bindings Álvaro Fernández Rojas
@ 2021-03-08 20:54   ` Andrew Lunn
  2021-03-15 13:51     ` Álvaro Fernández Rojas
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-03-08 20:54 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: jonas.gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Heiner Kallweit, Russell King, netdev, devicetree, linux-kernel

On Mon, Mar 08, 2021 at 07:41:01PM +0100, Álvaro Fernández Rojas wrote:
> +  clocks:
> +    maxItems: 1

Hi Álvaro

The driver does not make use of this clocks property. Is it really
needed?

	Andrew

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

* Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller
  2021-03-08 18:41 ` [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller Álvaro Fernández Rojas
  2021-03-08 19:57   ` Jakub Kicinski
@ 2021-03-08 21:00   ` Andrew Lunn
  2021-03-15 14:02     ` Álvaro Fernández Rojas
  1 sibling, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-03-08 21:00 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: jonas.gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Heiner Kallweit, Russell King, netdev, devicetree, linux-kernel

> +static int bcm6368_mdiomux_probe(struct platform_device *pdev)
> +{
> +	struct bcm6368_mdiomux_desc *md;
> +	struct mii_bus *bus;
> +	struct resource *res;
> +	int rc;
> +
> +	md = devm_kzalloc(&pdev->dev, sizeof(*md), GFP_KERNEL);
> +	if (!md)
> +		return -ENOMEM;
> +	md->dev = &pdev->dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -EINVAL;
> +
> +	/* Just ioremap, as this MDIO block is usually integrated into an
> +	 * Ethernet MAC controller register range
> +	 */
> +	md->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +	if (!md->base) {
> +		dev_err(&pdev->dev, "failed to ioremap register\n");
> +		return -ENOMEM;
> +	}
> +
> +	md->mii_bus = devm_mdiobus_alloc(&pdev->dev);
> +	if (!md->mii_bus) {
> +		dev_err(&pdev->dev, "mdiomux bus alloc failed\n");
> +		return ENOMEM;
> +	}
> +
> +	bus = md->mii_bus;
> +	bus->priv = md;
> +	bus->name = "BCM6368 MDIO mux bus";
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d", pdev->name, pdev->id);
> +	bus->parent = &pdev->dev;
> +	bus->read = bcm6368_mdiomux_read;
> +	bus->write = bcm6368_mdiomux_write;
> +	bus->phy_mask = 0x3f;
> +	bus->dev.of_node = pdev->dev.of_node;
> +
> +	rc = mdiobus_register(bus);
> +	if (rc) {
> +		dev_err(&pdev->dev, "mdiomux registration failed\n");
> +		return rc;
> +	}

So this is different to all the other mux drivers. Normally there is
an MDIO driver. And there is a mux driver. Two separate drivers. The
mux driver uses a phandle to reference the MDIO driver. Here we have
both in one driver.

Does this MDIO bus device exist as a standalone device? Without the
mux? If silicon does exist like that, having two separate drivers
would be better.

     Andrew

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

* Re: [PATCH 1/2] dt-bindings: net: Add bcm6368-mdio-mux bindings
  2021-03-08 20:54   ` Andrew Lunn
@ 2021-03-15 13:51     ` Álvaro Fernández Rojas
  0 siblings, 0 replies; 11+ messages in thread
From: Álvaro Fernández Rojas @ 2021-03-15 13:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jonas Gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Heiner Kallweit, Russell King, netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel

Hi Andrew,

> El 8 mar 2021, a las 21:54, Andrew Lunn <andrew@lunn.ch> escribió:
> 
> On Mon, Mar 08, 2021 at 07:41:01PM +0100, Álvaro Fernández Rojas wrote:
>> +  clocks:
>> +    maxItems: 1
> 
> Hi Álvaro
> 
> The driver does not make use of this clocks property. Is it really
> needed?

Nice catch, this was copy & pasted from other driver.
I will remove it on v2.

> 
> 	Andrew

Best regards,
Álvaro.

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

* Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller
  2021-03-08 19:57   ` Jakub Kicinski
@ 2021-03-15 13:53     ` Álvaro Fernández Rojas
  0 siblings, 0 replies; 11+ messages in thread
From: Álvaro Fernández Rojas @ 2021-03-15 13:53 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jonas Gorski, David S. Miller, Rob Herring, Andrew Lunn,
	Heiner Kallweit, Russell King, netdev, devicetree, linux-kernel

Hi Jakub,

> El 8 mar 2021, a las 20:57, Jakub Kicinski <kuba@kernel.org> escribió:
> 
> On Mon,  8 Mar 2021 19:41:02 +0100 Álvaro Fernández Rojas wrote:
>> This controller is present on BCM6318, BCM6328, BCM6362, BCM6368 and BCM63268
>> SoCs.
>> 
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> 
> make[2]: *** Deleting file 'Module.symvers'
> ERROR: modpost: missing MODULE_LICENSE() in drivers/net/mdio/mdio-mux-bcm6368.o
> make[2]: *** [Module.symvers] Error 1
> make[1]: *** [modules] Error 2
> make: *** [__sub-make] Error 2
> make[2]: *** Deleting file 'Module.symvers’

Ok, I’ll add the following in v2:
MODULE_AUTHOR("Álvaro Fernández Rojas <noltari@gmail.com>");
MODULE_DESCRIPTION("BCM6368 mdiomux bus controller driver");
MODULE_LICENSE("GPL v2");


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

* Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller
  2021-03-08 21:00   ` Andrew Lunn
@ 2021-03-15 14:02     ` Álvaro Fernández Rojas
  2021-03-17 17:49       ` Andrew Lunn
  0 siblings, 1 reply; 11+ messages in thread
From: Álvaro Fernández Rojas @ 2021-03-15 14:02 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jonas Gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Heiner Kallweit, Russell King, netdev, devicetree, linux-kernel

Hi Andrew,

> El 8 mar 2021, a las 22:00, Andrew Lunn <andrew@lunn.ch> escribió:
> 
>> +static int bcm6368_mdiomux_probe(struct platform_device *pdev)
>> +{
>> +	struct bcm6368_mdiomux_desc *md;
>> +	struct mii_bus *bus;
>> +	struct resource *res;
>> +	int rc;
>> +
>> +	md = devm_kzalloc(&pdev->dev, sizeof(*md), GFP_KERNEL);
>> +	if (!md)
>> +		return -ENOMEM;
>> +	md->dev = &pdev->dev;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res)
>> +		return -EINVAL;
>> +
>> +	/* Just ioremap, as this MDIO block is usually integrated into an
>> +	 * Ethernet MAC controller register range
>> +	 */
>> +	md->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>> +	if (!md->base) {
>> +		dev_err(&pdev->dev, "failed to ioremap register\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	md->mii_bus = devm_mdiobus_alloc(&pdev->dev);
>> +	if (!md->mii_bus) {
>> +		dev_err(&pdev->dev, "mdiomux bus alloc failed\n");
>> +		return ENOMEM;
>> +	}
>> +
>> +	bus = md->mii_bus;
>> +	bus->priv = md;
>> +	bus->name = "BCM6368 MDIO mux bus";
>> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-%d", pdev->name, pdev->id);
>> +	bus->parent = &pdev->dev;
>> +	bus->read = bcm6368_mdiomux_read;
>> +	bus->write = bcm6368_mdiomux_write;
>> +	bus->phy_mask = 0x3f;
>> +	bus->dev.of_node = pdev->dev.of_node;
>> +
>> +	rc = mdiobus_register(bus);
>> +	if (rc) {
>> +		dev_err(&pdev->dev, "mdiomux registration failed\n");
>> +		return rc;
>> +	}
> 
> So this is different to all the other mux drivers. Normally there is
> an MDIO driver. And there is a mux driver. Two separate drivers. The
> mux driver uses a phandle to reference the MDIO driver. Here we have
> both in one driver.
> 
> Does this MDIO bus device exist as a standalone device? Without the
> mux? If silicon does exist like that, having two separate drivers
> would be better.

BCM6368 (and newer) SoCs have an integrated ethernet switch controller with dedicated internal phys, but it also supports connecting to external phys not integrated in the internal switch.
Ports 0-3 are internal, ports 4-7 are external and can be connected to external switches or phys and port 8 is the CPU.
This MDIO bus device is integrated in the BCM63xx switch registers, which corresponds to the same registers present in drivers/net/dsa/b53/b53_regs.h.
As you can see in the source code, registers are the same for the internal and external bus. The only difference is that if MDIOC_EXT_MASK (bit 16) is set, the MDIO bus accessed will be the external, and on the contrary, if bit 16 isn’t set, the MDIO bus accessed will be the internal one.

I don’t know if this answers your question, but I think that adding it as mdiomux is the way to go.

> 
>     Andrew

Best regards,
Álvaro.

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

* Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller
  2021-03-15 14:02     ` Álvaro Fernández Rojas
@ 2021-03-17 17:49       ` Andrew Lunn
  2021-03-17 21:42         ` Florian Fainelli
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-03-17 17:49 UTC (permalink / raw)
  To: Álvaro Fernández Rojas
  Cc: Jonas Gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Heiner Kallweit, Russell King, netdev, devicetree, linux-kernel

> BCM6368 (and newer) SoCs have an integrated ethernet switch controller with dedicated internal phys, but it also supports connecting to external phys not integrated in the internal switch.
> Ports 0-3 are internal, ports 4-7 are external and can be connected to external switches or phys and port 8 is the CPU.
> This MDIO bus device is integrated in the BCM63xx switch registers, which corresponds to the same registers present in drivers/net/dsa/b53/b53_regs.h.
> As you can see in the source code, registers are the same for the internal and external bus. The only difference is that if MDIOC_EXT_MASK (bit 16) is set, the MDIO bus accessed will be the external, and on the contrary, if bit 16 isn’t set, the MDIO bus accessed will be the internal one.
> 
> I don’t know if this answers your question, but I think that adding it as mdiomux is the way to go.

Hi Álvaro

The Marvell mv88e6390 family of switches has a very similar setup. An
internal and an external MDIO bus, one bit difference in a
register. When i wrote the code for that, i decided it was not a mux
as such, but two MDIO busses. So i register two MDIO busses, and rely
on a higher level switch register mutex to prevent parallel operations
on the two busses.

The reason i decided it was not a mux, is that all the other mux
drivers are separate drivers which rely on another MDIO bus
driver. The mux driver gets a handle to the underlying MDIO bus
driver, and and builds on it. Here you have it all combined in one, so
it does not follow the pattern.

So if you want to use a max, please break this up into an MDIO driver,
and a mux driver. Or have one driver which registers two mdio busses,
no mux.

   Andrew

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

* Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller
  2021-03-17 17:49       ` Andrew Lunn
@ 2021-03-17 21:42         ` Florian Fainelli
  0 siblings, 0 replies; 11+ messages in thread
From: Florian Fainelli @ 2021-03-17 21:42 UTC (permalink / raw)
  To: Andrew Lunn, Álvaro Fernández Rojas
  Cc: Jonas Gorski, David S. Miller, Jakub Kicinski, Rob Herring,
	Heiner Kallweit, Russell King, netdev, devicetree, linux-kernel



On 3/17/2021 10:49 AM, Andrew Lunn wrote:
>> BCM6368 (and newer) SoCs have an integrated ethernet switch controller with dedicated internal phys, but it also supports connecting to external phys not integrated in the internal switch.
>> Ports 0-3 are internal, ports 4-7 are external and can be connected to external switches or phys and port 8 is the CPU.
>> This MDIO bus device is integrated in the BCM63xx switch registers, which corresponds to the same registers present in drivers/net/dsa/b53/b53_regs.h.
>> As you can see in the source code, registers are the same for the internal and external bus. The only difference is that if MDIOC_EXT_MASK (bit 16) is set, the MDIO bus accessed will be the external, and on the contrary, if bit 16 isn’t set, the MDIO bus accessed will be the internal one.
>>
>> I don’t know if this answers your question, but I think that adding it as mdiomux is the way to go.
> 
> Hi Álvaro
> 
> The Marvell mv88e6390 family of switches has a very similar setup. An
> internal and an external MDIO bus, one bit difference in a
> register. When i wrote the code for that, i decided it was not a mux
> as such, but two MDIO busses. So i register two MDIO busses, and rely
> on a higher level switch register mutex to prevent parallel operations
> on the two busses.
> 
> The reason i decided it was not a mux, is that all the other mux
> drivers are separate drivers which rely on another MDIO bus
> driver. The mux driver gets a handle to the underlying MDIO bus
> driver, and and builds on it. Here you have it all combined in one, so
> it does not follow the pattern.
> 
> So if you want to use a max, please break this up into an MDIO driver,
> and a mux driver. Or have one driver which registers two mdio busses,
> no mux.

Also, if you were to support some dynamic power management in the
future, it may be desirable to have the switch drivers and the MDIO mux
controller be within the same driver, so you are in complete control of
when the resources are needed and you can fully clock gate everything.
-- 
Florian

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

end of thread, other threads:[~2021-03-17 21:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 18:41 [PATCH 0/2] net: mdio: Add BCM6368 MDIO mux bus controller Álvaro Fernández Rojas
2021-03-08 18:41 ` [PATCH 1/2] dt-bindings: net: Add bcm6368-mdio-mux bindings Álvaro Fernández Rojas
2021-03-08 20:54   ` Andrew Lunn
2021-03-15 13:51     ` Álvaro Fernández Rojas
2021-03-08 18:41 ` [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller Álvaro Fernández Rojas
2021-03-08 19:57   ` Jakub Kicinski
2021-03-15 13:53     ` Álvaro Fernández Rojas
2021-03-08 21:00   ` Andrew Lunn
2021-03-15 14:02     ` Álvaro Fernández Rojas
2021-03-17 17:49       ` Andrew Lunn
2021-03-17 21:42         ` Florian Fainelli

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).