2022-10-09 17:18:42

by Soha Jin

[permalink] [raw]
Subject: [PATCH] net: mdiobus: add fwnode_phy_is_fixed_link()

A helper function to check if PHY is fixed link with fwnode properties.
This is similar to of_phy_is_fixed_link.

Signed-off-by: Soha Jin <[email protected]>
---
drivers/net/mdio/fwnode_mdio.c | 30 +++++++++++++++++++++++++++++-
include/linux/fwnode_mdio.h | 2 ++
2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 689e728345ce..8e1773e4d304 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -6,8 +6,9 @@
* out of the fwnode and using it to populate an mii_bus.
*/

-#include <linux/acpi.h>
#include <linux/fwnode_mdio.h>
+#include <linux/property.h>
+#include <linux/acpi.h>
#include <linux/of.h>
#include <linux/phy.h>
#include <linux/pse-pd/pse.h>
@@ -183,3 +184,30 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
return rc;
}
EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
+
+bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *dn;
+ int err;
+ const char *managed;
+
+ /* New binding: 'fixed-link' is a sub-node of the Ethernet device. */
+ dn = fwnode_get_named_child_node(fwnode, "fixed-link");
+ if (dn) {
+ fwnode_handle_put(dn);
+ return true;
+ }
+
+ err = fwnode_property_read_string(fwnode, "managed", &managed);
+ if (err == 0 && strcmp(managed, "auto") != 0)
+ return true;
+
+ /* Old binding: 'fixed-link' was a property with 5 cells encoding
+ * various information about the fixed PHY.
+ */
+ if (fwnode_property_count_u32(fwnode, "fixed-link") == 5)
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(fwnode_phy_is_fixed_link);
diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
index faf603c48c86..f35e447e524a 100644
--- a/include/linux/fwnode_mdio.h
+++ b/include/linux/fwnode_mdio.h
@@ -32,4 +32,6 @@ static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
}
#endif

+bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode);
+
#endif /* __LINUX_FWNODE_MDIO_H */
--
2.30.2


2022-10-10 15:10:41

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] net: mdiobus: add fwnode_phy_is_fixed_link()

On Mon, Oct 10, 2022 at 12:20:06AM +0800, Soha Jin wrote:
> A helper function to check if PHY is fixed link with fwnode properties.
> This is similar to of_phy_is_fixed_link.

You need to include a user of this new function.

Also, not that ACPI only defines the 'new binding' for fixed-link. If
this is being called on a device which is ACPI underneath, it should
only return true for the 'new binding', not the old binding.

Andrew

2022-10-10 16:05:23

by Soha Jin

[permalink] [raw]
Subject: RE: [PATCH] net: mdiobus: add fwnode_phy_is_fixed_link()

Hello Andrew,

> From: Andrew Lunn <[email protected]>
> Sent: Monday, October 10, 2022 11:03 PM
>
> On Mon, Oct 10, 2022 at 12:20:06AM +0800, Soha Jin wrote:
> > A helper function to check if PHY is fixed link with fwnode properties.
> > This is similar to of_phy_is_fixed_link.
>
> You need to include a user of this new function.

Greg already notified me about this. I was thinking this patch and my
driver patches are on different trees, so I split the patches into
different parts. I will include this patch in my driver patch's v2 later
and ask you to review then. Sorry for the mistake.

> Also, not that ACPI only defines the 'new binding' for fixed-link. If this is
> being called on a device which is ACPI underneath, it should only return true
> for the 'new binding', not the old binding.

Thanks for the information, I will correct this.

Regards,
Soha

2022-10-10 16:58:45

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH] net: mdiobus: add fwnode_phy_is_fixed_link()

On Mon, Oct 10, 2022 at 05:03:27PM +0200, Andrew Lunn wrote:
> On Mon, Oct 10, 2022 at 12:20:06AM +0800, Soha Jin wrote:
> > A helper function to check if PHY is fixed link with fwnode properties.
> > This is similar to of_phy_is_fixed_link.
>
> You need to include a user of this new function.
>
> Also, not that ACPI only defines the 'new binding' for fixed-link. If
> this is being called on a device which is ACPI underneath, it should
> only return true for the 'new binding', not the old binding.

Do we want to support the "managed" property in the fwnode variant,
or persuade people to switch to phylink if they want that?

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

2022-10-10 17:12:41

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] net: mdiobus: add fwnode_phy_is_fixed_link()

On Mon, Oct 10, 2022 at 05:43:07PM +0100, Russell King (Oracle) wrote:
> On Mon, Oct 10, 2022 at 05:03:27PM +0200, Andrew Lunn wrote:
> > On Mon, Oct 10, 2022 at 12:20:06AM +0800, Soha Jin wrote:
> > > A helper function to check if PHY is fixed link with fwnode properties.
> > > This is similar to of_phy_is_fixed_link.
> >
> > You need to include a user of this new function.
> >
> > Also, not that ACPI only defines the 'new binding' for fixed-link. If
> > this is being called on a device which is ACPI underneath, it should
> > only return true for the 'new binding', not the old binding.
>
> Do we want to support the "managed" property in the fwnode variant,
> or persuade people to switch to phylink if they want that?

managed has been documented in
Documentation/firmware-guide/acpi/dsd/phy.rst so i think we need to
support it.

Andrew