| Message ID | 20210311062011.8054-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 | fail | Series longer than 15 patches |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| 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: 661 this patch: 661 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | CHECK: Please use a blank line after function/struct/union/enum declarations |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 578 this patch: 578 |
| netdev/header_inline | success | Link |
On Wed, Mar 10, 2021 at 10:21 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > > Define fwnode_mdio_find_device() to get a pointer to the > mdio_device from fwnode passed to the function. > > Refactor of_mdio_find_device() to use fwnode_mdio_find_device(). > > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> > --- > > Changes in v7: > - correct fwnode_mdio_find_device() description > > Changes in v6: > - fix warning for function parameter of fwnode_mdio_find_device() > > Changes in v5: None > Changes in v4: None > Changes in v3: None > Changes in v2: None > > drivers/net/mdio/of_mdio.c | 11 +---------- > drivers/net/phy/phy_device.c | 23 +++++++++++++++++++++++ > include/linux/phy.h | 6 ++++++ > 3 files changed, 30 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c > index ea9d5855fb52..d5e0970b2561 100644 > --- a/drivers/net/mdio/of_mdio.c > +++ b/drivers/net/mdio/of_mdio.c > @@ -347,16 +347,7 @@ EXPORT_SYMBOL(of_mdiobus_register); > */ > struct mdio_device *of_mdio_find_device(struct device_node *np) > { > - struct device *d; > - > - if (!np) > - return NULL; > - > - d = bus_find_device_by_of_node(&mdio_bus_type, np); > - if (!d) > - return NULL; > - > - return to_mdio_device(d); > + return fwnode_mdio_find_device(of_fwnode_handle(np)); > } > EXPORT_SYMBOL(of_mdio_find_device); > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index cc38e326405a..daabb17bba00 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -2819,6 +2819,29 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv) > return phydrv->config_intr && phydrv->handle_interrupt; > } > > +/** > + * fwnode_mdio_find_device - Given a fwnode, find the mdio_device > + * @fwnode: pointer to the mdio_device's fwnode > + * > + * If successful, returns a pointer to the mdio_device with the embedded > + * struct device refcount incremented by one, or NULL on failure. > + * The caller should call put_device() on the mdio_device after its use. > + */ > +struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode) > +{ > + struct device *d; > + > + if (!fwnode) > + return NULL; > + > + d = bus_find_device_by_fwnode(&mdio_bus_type, fwnode); Sorry about the late review, but can you look into using get_dev_from_fwnode()? As long as you aren't registering two devices for the same fwnode, it's an O(1) operation instead of having to loop through a list of devices in a bus. You can check the returned device's bus type if you aren't sure about not registering two devices with the same fw_node and then fall back to this looping. -Saravana > + if (!d) > + return NULL; > + > + return to_mdio_device(d); > +} > +EXPORT_SYMBOL(fwnode_mdio_find_device); > + > /** > * 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 1a12e4436b5b..f5eb1e3981a1 100644 > --- a/include/linux/phy.h > +++ b/include/linux/phy.h > @@ -1366,11 +1366,17 @@ 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 mdio_device *fwnode_mdio_find_device(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 mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode) > +{ > + return 0; > +} > +static inline > struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45) > { > return NULL; > -- > 2.17.1 >
On Wed, Mar 10, 2021 at 10:50:57PM -0800, Saravana Kannan wrote: > On Wed, Mar 10, 2021 at 10:21 PM Calvin Johnson > <calvin.johnson@oss.nxp.com> wrote: > > > > Define fwnode_mdio_find_device() to get a pointer to the > > mdio_device from fwnode passed to the function. > > > > Refactor of_mdio_find_device() to use fwnode_mdio_find_device(). > > > > Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> > > --- > > > > Changes in v7: > > - correct fwnode_mdio_find_device() description > > > > Changes in v6: > > - fix warning for function parameter of fwnode_mdio_find_device() > > > > Changes in v5: None > > Changes in v4: None > > Changes in v3: None > > Changes in v2: None > > > > drivers/net/mdio/of_mdio.c | 11 +---------- > > drivers/net/phy/phy_device.c | 23 +++++++++++++++++++++++ > > include/linux/phy.h | 6 ++++++ > > 3 files changed, 30 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c > > index ea9d5855fb52..d5e0970b2561 100644 > > --- a/drivers/net/mdio/of_mdio.c > > +++ b/drivers/net/mdio/of_mdio.c > > @@ -347,16 +347,7 @@ EXPORT_SYMBOL(of_mdiobus_register); > > */ > > struct mdio_device *of_mdio_find_device(struct device_node *np) > > { > > - struct device *d; > > - > > - if (!np) > > - return NULL; > > - > > - d = bus_find_device_by_of_node(&mdio_bus_type, np); > > - if (!d) > > - return NULL; > > - > > - return to_mdio_device(d); > > + return fwnode_mdio_find_device(of_fwnode_handle(np)); > > } > > EXPORT_SYMBOL(of_mdio_find_device); > > > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > > index cc38e326405a..daabb17bba00 100644 > > --- a/drivers/net/phy/phy_device.c > > +++ b/drivers/net/phy/phy_device.c > > @@ -2819,6 +2819,29 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv) > > return phydrv->config_intr && phydrv->handle_interrupt; > > } > > > > +/** > > + * fwnode_mdio_find_device - Given a fwnode, find the mdio_device > > + * @fwnode: pointer to the mdio_device's fwnode > > + * > > + * If successful, returns a pointer to the mdio_device with the embedded > > + * struct device refcount incremented by one, or NULL on failure. > > + * The caller should call put_device() on the mdio_device after its use. > > + */ > > +struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode) > > +{ > > + struct device *d; > > + > > + if (!fwnode) > > + return NULL; > > + > > + d = bus_find_device_by_fwnode(&mdio_bus_type, fwnode); > > Sorry about the late review, but can you look into using > get_dev_from_fwnode()? As long as you aren't registering two devices > for the same fwnode, it's an O(1) operation instead of having to loop > through a list of devices in a bus. You can check the returned > device's bus type if you aren't sure about not registering two devices > with the same fw_node and then fall back to this looping. I think it is better to keep it simple and clear with bus_find_device_by_fwnode() instead of the additional code that comes with get_dev_from_fwnode() until it has clear advantage over the former. regards Calvin
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c index ea9d5855fb52..d5e0970b2561 100644 --- a/drivers/net/mdio/of_mdio.c +++ b/drivers/net/mdio/of_mdio.c @@ -347,16 +347,7 @@ EXPORT_SYMBOL(of_mdiobus_register); */ struct mdio_device *of_mdio_find_device(struct device_node *np) { - struct device *d; - - if (!np) - return NULL; - - d = bus_find_device_by_of_node(&mdio_bus_type, np); - if (!d) - return NULL; - - return to_mdio_device(d); + return fwnode_mdio_find_device(of_fwnode_handle(np)); } EXPORT_SYMBOL(of_mdio_find_device); diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index cc38e326405a..daabb17bba00 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -2819,6 +2819,29 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv) return phydrv->config_intr && phydrv->handle_interrupt; } +/** + * fwnode_mdio_find_device - Given a fwnode, find the mdio_device + * @fwnode: pointer to the mdio_device's fwnode + * + * If successful, returns a pointer to the mdio_device with the embedded + * struct device refcount incremented by one, or NULL on failure. + * The caller should call put_device() on the mdio_device after its use. + */ +struct mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode) +{ + struct device *d; + + if (!fwnode) + return NULL; + + d = bus_find_device_by_fwnode(&mdio_bus_type, fwnode); + if (!d) + return NULL; + + return to_mdio_device(d); +} +EXPORT_SYMBOL(fwnode_mdio_find_device); + /** * 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 1a12e4436b5b..f5eb1e3981a1 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1366,11 +1366,17 @@ 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 mdio_device *fwnode_mdio_find_device(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 mdio_device *fwnode_mdio_find_device(struct fwnode_handle *fwnode) +{ + return 0; +} +static inline struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45) { return NULL;
Define fwnode_mdio_find_device() to get a pointer to the mdio_device from fwnode passed to the function. Refactor of_mdio_find_device() to use fwnode_mdio_find_device(). Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> --- Changes in v7: - correct fwnode_mdio_find_device() description Changes in v6: - fix warning for function parameter of fwnode_mdio_find_device() Changes in v5: None Changes in v4: None Changes in v3: None Changes in v2: None drivers/net/mdio/of_mdio.c | 11 +---------- drivers/net/phy/phy_device.c | 23 +++++++++++++++++++++++ include/linux/phy.h | 6 ++++++ 3 files changed, 30 insertions(+), 10 deletions(-)