linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yinbo Zhu <zhuyinbo@loongson.cn>
To: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Nick Desaulniers <ndesaulniers@google.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kbuild@vger.kernel.org
Cc: zhuyinbo@loongson.cn
Subject: [PATCH v1 2/2] net: mdio: fixup ethernet phy module auto-load function
Date: Tue, 23 Nov 2021 13:32:23 +0800	[thread overview]
Message-ID: <1637645543-24618-1-git-send-email-zhuyinbo@loongson.cn> (raw)

the phy_id is only phy identifier, that phy module auto-load function
should according the phy_id event rather than other information, this
patch is remove other unnecessary information and add phy_id event in
mdio_uevent function and ethernet phy module auto-load function will
work well.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
Hi Russell King,

I don't see that mail from you, but I have a look about your advice for my patch on netdev patchwork

> The MDIO bus contains more than just PHYs. This completely breaks
> anything that isn't a PHY device - likely by performing an
> out-of-bounds access.
> 
> This change also _totally_ breaks any MDIO devices that rely on
> matching via the "of:" mechanism using the compatible specified in
> DT. An example of that is the B53 DSA switch.
>
> Sorry, but we've already learnt this lesson from a similar case with
> SPI. Once one particular way of dealing with MODALIAS has been
> established for auto-loading modules for a subsystem, it is very
> difficult to change it without causing regressions.

> We need a very clear description of the problem that these patches are
> attempting to address, and then we need to see that effort has been
> put in to verify that changing the auto-loading mechanism is safe to
> do - such as auditing every single driver that use the MDIO subsystem.

if mdio_uevent doesn't include my patch, you will see that mdio uevent is like 
"MODALIAS= of:NphyTethernet-phyCmarvell,88E1512", "marvell,88E1512" is only a
phy dts compatible, and that name can use any a string that in the same driver, 
if phy driver not use dts, and this MODALIAS is NULL, it is not unique, and even
thoug use that modalias, that do_mdio_entry doesn't get that compatibe 
information, event though it can get compatible and it looks ugly, but that phy id 
is unique if phy chip following 802.3 spec,
whatever whether use dts, use phy id it will always okay that phy dev to match phy
 driver, because phy it is getted by mdiobus_register
and mdio device driver will call mdiobus_register whatever whether use dts.

>  struct bus_type mdio_bus_type = {
> -	.name		= "mdio_bus",
> +	.name		= "mdio",

> This looks like an unrelated user-interface breaking change. This
> changes the path of all MDIO devices and drivers in /sys/bus/mdio_bus/*


I think mdio_bus is ugly, you can other bus, eg. usb,pci.  in addition, mdio bus name 
should be Consistent with mdio alias configure, eg. MDIO_MODULE_PREFIX.

BRs,
Yinbo Zhu. 

 drivers/net/phy/mdio_bus.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 6865d93..999f0d4 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -962,12 +962,12 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv)
 
 static int mdio_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
-	int rc;
+	struct phy_device *pdev;
 
-	/* Some devices have extra OF data and an OF-style MODALIAS */
-	rc = of_device_uevent_modalias(dev, env);
-	if (rc != -ENODEV)
-		return rc;
+	pdev = to_phy_device(dev);
+
+	if (add_uevent_var(env, "MODALIAS=mdio:p%08X", pdev->phy_id))
+		return -ENOMEM;
 
 	return 0;
 }
@@ -991,7 +991,7 @@ static int mdio_uevent(struct device *dev, struct kobj_uevent_env *env)
 };
 
 struct bus_type mdio_bus_type = {
-	.name		= "mdio_bus",
+	.name		= "mdio",
 	.dev_groups	= mdio_bus_dev_groups,
 	.match		= mdio_bus_match,
 	.uevent		= mdio_uevent,
-- 
1.8.3.1


             reply	other threads:[~2021-11-23  5:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-23  5:32 Yinbo Zhu [this message]
  -- strict thread matches above, loose matches on Subject: below --
2021-11-22 12:14 [PATCH v1 1/2] modpost: file2alias: fixup mdio alias garbled code in modules.alias Yinbo Zhu
2021-11-22 12:14 ` [PATCH v1 2/2] net: mdio: fixup ethernet phy module auto-load function Yinbo Zhu
2021-11-22 14:54   ` Russell King (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1637645543-24618-1-git-send-email-zhuyinbo@loongson.cn \
    --to=zhuyinbo@loongson.cn \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=masahiroy@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=ndesaulniers@google.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).