mbox series

[0/2] net: mdio: Add BCM6368 MDIO mux bus controller

Message ID 20210308184102.3921-1-noltari@gmail.com
Headers show
Series net: mdio: Add BCM6368 MDIO mux bus controller | expand

Message

Álvaro Fernández Rojas March 8, 2021, 6:41 p.m. UTC
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

Comments

Andrew Lunn March 8, 2021, 9 p.m. UTC | #1
> +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
Álvaro Fernández Rojas March 15, 2021, 2:02 p.m. UTC | #2
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.