On Tue, Dec 15, 2020 at 07:23:26PM +0200, Andy Shevchenko wrote:

On Tue, Dec 15, 2020 at 6:44 PM Calvin Johnson
[off-list ref] wrote:

quoted

Define fwnode_phy_find_device() to iterate an mdiobus and find the
phy device of the provided phy fwnode. Additionally define
device_phy_find_device() to find phy device of provided device.

Define fwnode_get_phy_node() to get phy_node using named reference.

...

quoted

+#include <linux/acpi.h>

Not sure we need this. See below.

This is required to use is_acpi_node().

...

quoted

+/**
+ * fwnode_phy_find_device - Find phy_device on the mdiobus for the provided
+ * phy_fwnode.

Can we keep a summary on one line?

Ok

quoted

+ * @phy_fwnode: Pointer to the phy's fwnode.
+ *
+ * If successful, returns a pointer to the phy_device with the embedded
+ * struct device refcount incremented by one, or NULL on failure.
+ */
+struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode)
+{
+       struct mdio_device *mdiodev;
+       struct device *d;

quoted

+       if (!phy_fwnode)
+               return NULL;

Why is this needed?
Perhaps a comment to the function description explains a case when
@phy_fwnode == NULL.

I think this function should be modified to follow of_phy_find_device() which
has NULL check. I'll add fwnode_mdio_find_device() also.
Here is a case where of_phy_find_device() is called without checking phy_np.
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/qualcomm/emac/emac-phy.c#L145

quoted

+       d = bus_find_device_by_fwnode(&mdio_bus_type, phy_fwnode);
+       if (d) {
+               mdiodev = to_mdio_device(d);
+               if (mdiodev->flags & MDIO_DEVICE_FLAG_PHY)
+                       return to_phy_device(d);
+               put_device(d);
+       }
+
+       return NULL;
+}

...

quoted

+ * For ACPI, only "phy-handle" is supported. DT supports all the three
+ * named references to the phy node.

...

quoted

+       /* Only phy-handle is used for ACPI */
+       phy_node = fwnode_find_reference(fwnode, "phy-handle", 0);
+       if (is_acpi_node(fwnode) || !IS_ERR(phy_node))
+               return phy_node;

So, what is the problem with going through the rest on ACPI?
Usually we describe the restrictions in the documentation.

Others are legacy DT properties which are not intended to be supported
in ACPI. I can add this info in the document.

Thanks for the review, Andy!

Regards
Calvin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel