Thread (24 messages) 24 messages, 5 authors, 4d ago

RE: [PATCH net-next 2/5] net: ethernet: oa_tc6: Allow custom mii_bus

From: Selvamani Rajagopal <hidden>
Date: 2026-05-03 17:35:08
Also in: linux-devicetree, linux-doc, lkml

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Saturday, May 2, 2026 8:51 PM
To: ciprian.regus@analog.com
Cc: Parthiban Veerasooran <parthiban.veerasooran@microchip.com>; Andrew Lunn
[off-list ref]; David S. Miller [off-list ref]; Eric Dumazet
[off-list ref]; Jakub Kicinski [off-list ref]; Paolo Abeni
[off-list ref]; Simon Horman [off-list ref]; Jonathan Corbet
[off-list ref]; Shuah Khan [off-list ref]; Heiner Kallweit
[off-list ref]; Russell King [off-list ref]; Rob Herring
[off-list ref]; Krzysztof Kozlowski [off-list ref]; Conor Dooley
[off-list ref]; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
doc@vger.kernel.org; devicetree@vger.kernel.org
Subject: Re: [PATCH net-next 2/5] net: ethernet: oa_tc6: Allow custom mii_bus


This Message Is From an External Sender
This message came from outside your organization.
quoted
@@ -538,32 +539,37 @@ static int oa_tc6_mdiobus_register(struct oa_tc6 *tc6)
{
int ret;

- tc6->mdiobus = mdiobus_alloc();
if (!tc6->mdiobus) {
- netdev_err(tc6->netdev, "MDIO bus alloc failed\n");
- return -ENOMEM;
+ tc6->mdiobus = mdiobus_alloc();
+ if (!tc6->mdiobus) {
+ netdev_err(tc6->netdev, "MDIO bus alloc failed\n");
+ return -ENOMEM;
+ }
+
+ tc6->mdiobus->read = oa_tc6_mdiobus_read;
+ tc6->mdiobus->write = oa_tc6_mdiobus_write;
+ /* OPEN Alliance 10BASE-T1x compliance MAC-PHYs will have both C22 and
+ * C45 registers space. If the PHY is discovered via C22 bus protocol it
+ * assumes it uses C22 protocol and always uses C22 registers indirect
+ * access to access C45 registers. This is because, we don't have a
+ * clean separation between C22/C45 register space and C22/C45 MDIO bus
+ * protocols. Resulting, PHY C45 registers direct access can't be used
+ * which can save multiple SPI bus access. To support this feature, PHY
+ * drivers can set .read_mmd/.write_mmd in the PHY driver to call
+ * .read_c45/.write_c45. Ex: drivers/net/phy/microchip_t1s.c
+ */
+ tc6->mdiobus->read_c45 = oa_tc6_mdiobus_read_c45;
+ tc6->mdiobus->write_c45 = oa_tc6_mdiobus_write_c45;
+
+ tc6->own_mdiobus = true;
}

tc6->mdiobus->priv = tc6;
- tc6->mdiobus->read = oa_tc6_mdiobus_read;
- tc6->mdiobus->write = oa_tc6_mdiobus_write;
- /* OPEN Alliance 10BASE-T1x compliance MAC-PHYs will have both C22 and
- * C45 registers space. If the PHY is discovered via C22 bus protocol it
- * assumes it uses C22 protocol and always uses C22 registers indirect
- * access to access C45 registers. This is because, we don't have a
- * clean separation between C22/C45 register space and C22/C45 MDIO bus
- * protocols. Resulting, PHY C45 registers direct access can't be used
- * which can save multiple SPI bus access. To support this feature, PHY
- * drivers can set .read_mmd/.write_mmd in the PHY driver to call
- * .read_c45/.write_c45. Ex: drivers/net/phy/microchip_t1s.c
- */
- tc6->mdiobus->read_c45 = oa_tc6_mdiobus_read_c45;
- tc6->mdiobus->write_c45 = oa_tc6_mdiobus_write_c45;
- tc6->mdiobus->name = "oa-tc6-mdiobus";
tc6->mdiobus->parent = tc6->dev;
+ tc6->mdiobus->name = "oa-tc6-mdiobus";

snprintf(tc6->mdiobus->id, ARRAY_SIZE(tc6->mdiobus->id), "%s",
- dev_name(&tc6->spi->dev));
+ dev_name(&tc6->spi->dev));

ret = mdiobus_register(tc6->mdiobus);
if (ret) {
@@ -577,19 +583,30 @@ static int oa_tc6_mdiobus_register(struct oa_tc6 *tc6)
static void oa_tc6_mdiobus_unregister(struct oa_tc6 *tc6)
{
+ if (!tc6->mdiobus)
+ return;
+
mdiobus_unregister(tc6->mdiobus);
- mdiobus_free(tc6->mdiobus);
+
+ if (tc6->own_mdiobus)
+ mdiobus_free(tc6->mdiobus);
}

static int oa_tc6_phy_init(struct oa_tc6 *tc6)
{
int ret;

- ret = oa_tc6_check_phy_reg_direct_access_capability(tc6);
- if (ret) {
- netdev_err(tc6->netdev,
- "Direct PHY register access is not supported by the MAC-PHY\n");
- return ret;
+ /* If the driver provided a mii_bus, it is also responsible for
+ * implementing the bus access methods, so we don't have to worry
+ * about checking the PHY access mode.
+ */
+ if (!tc6->mdiobus) {
+ ret = oa_tc6_check_phy_reg_direct_access_capability(tc6);
+ if (ret) {
+ netdev_err(tc6->netdev,
+ "Direct PHY register access is not supported by the MAC-PHY\n");
+ return ret;
+ }
This all seems pretty invasive and ugly. Please could you think what
happens if instead of passing in an mdiobus, you pass a phydev. Is the
change to the core simpler and cleaner?

Andrew
Kind of agree. Initially we were thinking about changing the existing code (Microchip's vendor code) to alloc mii_bus so that code would be same across multiple vendors. Either way, it would be invasive changes. So, we decide to go with minimal change to other vendor's code.

Trying to understand your suggestion. Are you suggesting to move entire mii_bus allocation/APIs implementation to vendor side and keep only phy dev usage in oa_tc6.c?

If my understanding is correct, I guess it would be cleaner. I can try that. Let me know.

Selva

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help