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

["Álvaro Fernández Rojas" <noltari@gmail.com>]

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.