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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D39D163A-C6B3-4B66-B650-8FF0A06EF7A2@gmail.com>
Date:   Mon, 15 Mar 2021 15:02:37 +0100
From:   Álvaro Fernández Rojas <noltari@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Jonas Gorski <jonas.gorski@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] net: mdio: Add BCM6368 MDIO mux bus controller

Hi Andrew,

> El 8 mar 2021, a las 22:00, Andrew Lunn <andrew@...n.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.