Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe
@ 2020-07-28 12:11 Vikas Singh
  2020-07-31 16:54 ` Andrew Lunn
  0 siblings, 1 reply; 7+ messages in thread
From: Vikas Singh @ 2020-07-28 12:11 UTC (permalink / raw)
  To: andrew, f.fainelli, hkallweit1, linux, netdev
  Cc: calvin.johnson, kuldip.dwivedi, madalin.bucur, vikas.singh, Vikas Singh

Auto-probe of c45 devices with extended scanning in xgmac_mdio works
well but fails to update device "fwnode" while registering PHYs on
MDIO bus.
This patch is based on https://www.spinics.net/lists/netdev/msg662173.html

This change will update the "fwnode" while PHYs get registered and allow
lookup for registered PHYs on MDIO bus from other drivers while probing.

Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com>
---
 drivers/net/phy/mdio_bus.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 46b3370..7275eff 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -447,8 +447,25 @@ static void of_mdiobus_link_mdiodev(struct mii_bus *bus,
 	struct device *dev = &mdiodev->dev;
 	struct device_node *child;
 
-	if (dev->of_node || !bus->dev.of_node)
+	if (dev->of_node || !bus->dev.of_node) {
+		/* Checking for acpi node, before returning */
+		struct fwnode_handle *fwnode;
+
+		/* Set the device firmware node to look for child nodes */
+		bus->dev.fwnode = bus->parent->fwnode;
+
+		device_for_each_child_node(&bus->dev, fwnode) {
+			int addr;
+
+			if (fwnode_property_read_u32(fwnode, "reg", &addr))
+				continue;
+			if (addr == mdiodev->addr) {
+				dev->fwnode = fwnode;
+				break;
+			}
+		}
 		return;
+	}
 
 	for_each_available_child_of_node(bus->dev.of_node, child) {
 		int addr;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe
  2020-07-28 12:11 [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe Vikas Singh
@ 2020-07-31 16:54 ` Andrew Lunn
       [not found]   ` <CADvVLtUAd0X7c39BX791CuyWBcmfBsp7Xvw9Jyp05w45agJY9g@mail.gmail.com>
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Lunn @ 2020-07-31 16:54 UTC (permalink / raw)
  To: Vikas Singh
  Cc: f.fainelli, hkallweit1, linux, netdev, calvin.johnson,
	kuldip.dwivedi, madalin.bucur, vikas.singh

On Tue, Jul 28, 2020 at 05:41:38PM +0530, Vikas Singh wrote:
> Auto-probe of c45 devices with extended scanning in xgmac_mdio works
> well but fails to update device "fwnode" while registering PHYs on
> MDIO bus.
> This patch is based on https://www.spinics.net/lists/netdev/msg662173.html
> 
> This change will update the "fwnode" while PHYs get registered and allow
> lookup for registered PHYs on MDIO bus from other drivers while probing.
> 
> Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com>
> ---
>  drivers/net/phy/mdio_bus.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 46b3370..7275eff 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -447,8 +447,25 @@ static void of_mdiobus_link_mdiodev(struct mii_bus *bus,

Why would a function called of_mdiobus_link_mdiodev() be poking
around trying to find ACPI properties?

       Andrew

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe
       [not found]   ` <CADvVLtUAd0X7c39BX791CuyWBcmfBsp7Xvw9Jyp05w45agJY9g@mail.gmail.com>
@ 2020-08-01  5:08     ` Florian Fainelli
  2020-08-01  9:38     ` Russell King - ARM Linux admin
  1 sibling, 0 replies; 7+ messages in thread
From: Florian Fainelli @ 2020-08-01  5:08 UTC (permalink / raw)
  To: Vikas Singh, Andrew Lunn
  Cc: hkallweit1, linux, netdev, Calvin Johnson (OSS),
	Kuldip Dwivedi, Madalin Bucur (OSS),
	Vikas Singh



On 7/31/2020 9:53 PM, Vikas Singh wrote:
> Hi Andrew,
>  
> As i have already mentioned that this patch is based on
> https://www.spinics.net/lists/netdev/msg662173.html,
> <https://www.spinics.net/lists/netdev/msg662173.html>
> 
> When MDIO bus gets registered itself along with devices on it , the
> function mdiobus_register() inside of_mdiobus_register(), brings
> up all the PHYs on the mdio bus and attach them to the bus with the help
> of_mdiobus_link_mdiodev() inside mdiobus_scan() .
> Additionally it has been discussed with the maintainers that the
> mdiobus_register() function should be capable of handling both ACPI &
> DTB stuff
> without any change to existing implementation.
> Now of_mdiobus_link_mdiodev() inside mdiobus_scan() see if the
> auto-probed phy has a corresponding child in the bus node, and set the
> "of_node" pointer in DT case.
> But lacks to set the "fwnode" pointer in ACPI case which is resulting in
> mdiobus_register() failure as an end result theoretically.
> 
> Now this patch set (changes) will attempt to fill this gap and
> generalise the mdiobus_register() implementation for both ACPI & DT with
> no duplicacy or redundancy.

Please reply in plain text and do not top-post, thank you.
-- 
Florian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe
       [not found]   ` <CADvVLtUAd0X7c39BX791CuyWBcmfBsp7Xvw9Jyp05w45agJY9g@mail.gmail.com>
  2020-08-01  5:08     ` Florian Fainelli
@ 2020-08-01  9:38     ` Russell King - ARM Linux admin
  1 sibling, 0 replies; 7+ messages in thread
From: Russell King - ARM Linux admin @ 2020-08-01  9:38 UTC (permalink / raw)
  To: Vikas Singh
  Cc: Andrew Lunn, f.fainelli, hkallweit1, netdev, Calvin Johnson (OSS),
	Kuldip Dwivedi, Madalin Bucur (OSS),
	Vikas Singh

On Sat, Aug 01, 2020 at 10:23:38AM +0530, Vikas Singh wrote:
> Hi Andrew,
> 
> As i have already mentioned that this patch is based on
> https://www.spinics.net/lists/netdev/msg662173.html,
> <https://www.spinics.net/lists/netdev/msg662173.html>
> 
> When MDIO bus gets registered itself along with devices on it , the
> function mdiobus_register() inside of_mdiobus_register(), brings
> up all the PHYs on the mdio bus and attach them to the bus with the help
> of_mdiobus_link_mdiodev() inside mdiobus_scan() .
> Additionally it has been discussed with the maintainers that the
> mdiobus_register() function should be capable of handling both ACPI & DTB
> stuff
> without any change to existing implementation.
> Now of_mdiobus_link_mdiodev() inside mdiobus_scan() see if the auto-probed
> phy has a corresponding child in the bus node, and set the "of_node"
> pointer in DT case.
> But lacks to set the "fwnode" pointer in ACPI case which is resulting in
> mdiobus_register() failure as an end result theoretically.
> 
> Now this patch set (changes) will attempt to fill this gap and generalise
> the mdiobus_register() implementation for both ACPI & DT with no duplicacy
> or redundancy.

Please do not top-post.

What Andrew is asking is why _should_ a DT specific function (which
starts with of_, meaning "open firmware") have anything to do with
ACPI.  The function you refer to (of_mdiobus_link_mdiodev()) is only
built when CONFIG_OF_MDIO is enabled, which is again, a DT specific
thing.

So, why should a DT specific function care about ACPI?

We're not asking about the fine details, we're asking about the high
level idea that DT functions should know about ACPI.

The assignment in of_mdiobus_link_mdiodev() to dev->fwnode is not
itself about ACPI, it's about enabling drivers that wish to access
DT properties through the fwnode property APIs can do so.

IMHO, the right way to go about this is to implement it as a non-DT
function.  Given that it is a static function, Andrew may find it
acceptable if you also renamed of_mdiobus_link_mdiodev() as
mdiobus_link_mdiodev() and moved it out of the #ifdef.

+               bus->dev.fwnode = bus->parent->fwnode;

That should be done elsewhere, not here.  of_mdiobus_register() already
ensures that this is appropriately set, and if it isn't, maybe there's
a bug elsewhere.

Lastly, note that you don't need two loops, one for ACPI and one for
DT (it's a shame there isn't a device_for_each_available_child_node()):

	int addr;

	if (dev->fwnode && !bus->dev.fwnode)
		return;

	device_for_each_child_node(&bus->dev, fwnode) {
		if (!fwnode_device_is_available(fwnode))
			continue;

		if (is_of_node(fwnode))
			addr = of_mdio_parse_addr(dev, to_of_node(fwnode));
		else if (fwnode_property_read_u32(fwnode, "reg", &addr))
			continue;

		if (addr == mdiodev->addr) {
			dev->of_node = to_of_node(fwnode);
			dev->fwnode = fwnode;
			return;
		}
	}

which, I think, will behave identically to the existing implementation
when called in a DT setup, but should also add what you want.

So, maybe with the above, moving it out from under the ifdef, and
renaming it _may_ be acceptable.  This is just a suggestion.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe
  2020-07-22 11:22 Vikas Singh
  2020-07-22 14:22 ` Andrew Lunn
@ 2020-07-22 20:06 ` David Miller
  1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2020-07-22 20:06 UTC (permalink / raw)
  To: vikas.singh
  Cc: netdev, andrew, f.fainelli, hkallweit1, linux, kuldip.dwivedi,
	calvin.johnson, madalin.bucur, vikas.singh

From: Vikas Singh <vikas.singh@puresoftware.com>
Date: Wed, 22 Jul 2020 16:52:14 +0530

> *Disclaimer* -The information transmitted is intended solely for the 
> individual
> or entity to which it is addressed and may contain confidential 
> and/or
> privileged material. Any review, re-transmission, dissemination or 
> other use of
> or taking action in reliance upon this information by persons 
> or entities other
> than the intended recipient is prohibited. If you have 
> received this email in
> error please contact the sender and delete the 
> material from any computer. In
> such instances you are further prohibited 
> from reproducing, disclosing,
> distributing or taking any action in reliance 
> on it.As a recipient of this email,
> you are responsible for screening its 
> contents and the contents of any
> attachments for the presence of viruses. 
> No liability is accepted for any
> damages caused by any virus transmitted by 
> this email.

Postings with such disclaimers will be ignored.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe
  2020-07-22 11:22 Vikas Singh
@ 2020-07-22 14:22 ` Andrew Lunn
  2020-07-22 20:06 ` David Miller
  1 sibling, 0 replies; 7+ messages in thread
From: Andrew Lunn @ 2020-07-22 14:22 UTC (permalink / raw)
  To: Vikas Singh
  Cc: netdev, f.fainelli, hkallweit1, linux, kuldip.dwivedi,
	calvin.johnson, madalin.bucur, vikas.singh

> *Disclaimer* -The information transmitted is intended solely for the
> individual or entity to which it is addressed and may contain
> confidential and/or privileged material. Any review,
> re-transmission, dissemination or other use of or taking action in
> reliance upon this information by persons or entities other than the
> intended recipient is prohibited. If you have received this email in
> error please contact the sender and delete the material from any
> computer.

I.m going to partially ignore this request to delete your email...

This patch is dependent on

Calvin Johnson  [net-next PATCH v7 0/6]  ACPI support for dpaa2 MAC driver.

which is stuck in limbo waiting for an ACPI maintainer to ACK/NACK it.
I've also had off list suggestions it should be NACKed. If it does get
NACKed, it makes this patch pointless.

	Andrew

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe
@ 2020-07-22 11:22 Vikas Singh
  2020-07-22 14:22 ` Andrew Lunn
  2020-07-22 20:06 ` David Miller
  0 siblings, 2 replies; 7+ messages in thread
From: Vikas Singh @ 2020-07-22 11:22 UTC (permalink / raw)
  To: netdev, andrew, f.fainelli, hkallweit1, linux
  Cc: kuldip.dwivedi, calvin.johnson, madalin.bucur, vikas.singh, Vikas Singh

Auto-probe of c45 devices with extended scanning in xgmac_mdio works
well but fails to update device "fwnode" while registering PHYs on
MDIO bus.
This patch is based on https://www.spinics.net/lists/netdev/msg662173.html

This change will update the "fwnode" while PHYs get registered and allow
lookup for registered PHYs on MDIO bus from other drivers while probing.

Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com>
---
 drivers/net/phy/mdio_bus.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 46b3370..7275eff 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -447,8 +447,25 @@ static void of_mdiobus_link_mdiodev(struct mii_bus *bus,
 	struct device *dev = &mdiodev->dev;
 	struct device_node *child;
 
-	if (dev->of_node || !bus->dev.of_node)
+	if (dev->of_node || !bus->dev.of_node) {
+		/* Checking for acpi node, before returning */
+		struct fwnode_handle *fwnode;
+
+		/* Set the device firmware node to look for child nodes */
+		bus->dev.fwnode = bus->parent->fwnode;
+
+		device_for_each_child_node(&bus->dev, fwnode) {
+			int addr;
+
+			if (fwnode_property_read_u32(fwnode, "reg", &addr))
+				continue;
+			if (addr == mdiodev->addr) {
+				dev->fwnode = fwnode;
+				break;
+			}
+		}
 		return;
+	}
 
 	for_each_available_child_of_node(bus->dev.of_node, child) {
 		int addr;
-- 
2.7.4


-- 




*Disclaimer* -The information transmitted is intended solely for the 
individual
or entity to which it is addressed and may contain confidential 
and/or
privileged material. Any review, re-transmission, dissemination or 
other use of
or taking action in reliance upon this information by persons 
or entities other
than the intended recipient is prohibited. If you have 
received this email in
error please contact the sender and delete the 
material from any computer. In
such instances you are further prohibited 
from reproducing, disclosing,
distributing or taking any action in reliance 
on it.As a recipient of this email,
you are responsible for screening its 
contents and the contents of any
attachments for the presence of viruses. 
No liability is accepted for any
damages caused by any virus transmitted by 
this email.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 12:11 [PATCH] net: Phy: Add PHY lookup support on MDIO bus in case of ACPI probe Vikas Singh
2020-07-31 16:54 ` Andrew Lunn
     [not found]   ` <CADvVLtUAd0X7c39BX791CuyWBcmfBsp7Xvw9Jyp05w45agJY9g@mail.gmail.com>
2020-08-01  5:08     ` Florian Fainelli
2020-08-01  9:38     ` Russell King - ARM Linux admin
  -- strict thread matches above, loose matches on Subject: below --
2020-07-22 11:22 Vikas Singh
2020-07-22 14:22 ` Andrew Lunn
2020-07-22 20:06 ` David Miller

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git