| Message ID | 20201215164315.3666-3-calvin.johnson@oss.nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ACPI support for dpaa2 driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 625 this patch: 625 |
| netdev/kdoc | success | Errors and warnings before: 1 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 106 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 541 this patch: 541 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Tue, Dec 15, 2020 at 6:44 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > > 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. ... > +#include <linux/acpi.h> Not sure we need this. See below. ... > +/** > + * fwnode_phy_find_device - Find phy_device on the mdiobus for the provided > + * phy_fwnode. Can we keep a summary on one line? > + * @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; > + if (!phy_fwnode) > + return NULL; Why is this needed? Perhaps a comment to the function description explains a case when @phy_fwnode == NULL. > + 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; > +} ... > + * For ACPI, only "phy-handle" is supported. DT supports all the three > + * named references to the phy node. ... > + /* 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.
On Tue, Dec 15, 2020 at 07:23:26PM +0200, Andy Shevchenko wrote: > On Tue, Dec 15, 2020 at 6:44 PM Calvin Johnson > <calvin.johnson@oss.nxp.com> wrote: > > > > 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. > > ... > > > +#include <linux/acpi.h> > > Not sure we need this. See below. This is required to use is_acpi_node(). > > ... > > > +/** > > + * fwnode_phy_find_device - Find phy_device on the mdiobus for the provided > > + * phy_fwnode. > > Can we keep a summary on one line? Ok > > > + * @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; > > > + 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 > > > + 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; > > +} > > ... > > > + * For ACPI, only "phy-handle" is supported. DT supports all the three > > + * named references to the phy node. > > ... > > > + /* 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
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 80c2e646c093..c153273606c1 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -9,6 +9,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/acpi.h> #include <linux/bitmap.h> #include <linux/delay.h> #include <linux/errno.h> @@ -2829,6 +2830,69 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv) return phydrv->config_intr && phydrv->handle_interrupt; } +/** + * fwnode_phy_find_device - Find phy_device on the mdiobus for the provided + * phy_fwnode. + * @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; + + if (!phy_fwnode) + return NULL; + + 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; +} +EXPORT_SYMBOL(fwnode_phy_find_device); + +/** + * device_phy_find_device - For the given device, get the phy_device + * @dev: Pointer to the given device + * + * Refer return conditions of fwnode_phy_find_device(). + */ +struct phy_device *device_phy_find_device(struct device *dev) +{ + return fwnode_phy_find_device(dev_fwnode(dev)); +} +EXPORT_SYMBOL_GPL(device_phy_find_device); + +/** + * fwnode_get_phy_node - Get the phy_node using the named reference. + * @fwnode: Pointer to fwnode from which phy_node has to be obtained. + * + * Refer return conditions of fwnode_find_reference(). + * For ACPI, only "phy-handle" is supported. DT supports all the three + * named references to the phy node. + */ +struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode) +{ + struct fwnode_handle *phy_node; + + /* 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; + phy_node = fwnode_find_reference(fwnode, "phy", 0); + if (IS_ERR(phy_node)) + phy_node = fwnode_find_reference(fwnode, "phy-device", 0); + return phy_node; +} +EXPORT_SYMBOL_GPL(fwnode_get_phy_node); + /** * phy_probe - probe and init a PHY device * @dev: device to probe and init diff --git a/include/linux/phy.h b/include/linux/phy.h index 381a95732b6a..7790a9a56d0f 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1341,10 +1341,30 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id, bool is_c45, struct phy_c45_device_ids *c45_ids); #if IS_ENABLED(CONFIG_PHYLIB) +struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode); +struct phy_device *device_phy_find_device(struct device *dev); +struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode); struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45); int phy_device_register(struct phy_device *phy); void phy_device_free(struct phy_device *phydev); #else +static inline +struct phy_device *fwnode_phy_find_device(struct fwnode_handle *phy_fwnode) +{ + return NULL; +} + +static inline struct phy_device *device_phy_find_device(struct device *dev) +{ + return NULL; +} + +static inline +struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode) +{ + return NULL; +} + static inline struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45) {
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. Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> --- Changes in v2: - use reverse christmas tree ordering for local variables drivers/net/phy/phy_device.c | 64 ++++++++++++++++++++++++++++++++++++ include/linux/phy.h | 20 +++++++++++ 2 files changed, 84 insertions(+)