| Message ID | 20200709175722.5228-6-calvin.johnson@oss.nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,v4,1/6] Documentation: ACPI: DSD: Document MDIO PHY | expand |
On Thu, Jul 9, 2020 at 8:58 PM Calvin Johnson <calvin.johnson@oss.nxp.com> wrote: > > Define phylink_fwnode_phy_connect() to connect phy specified by > a fwnode to a phylink instance. ... > + if (is_of_node(fwnode)) { > + ret = phylink_of_phy_connect(pl, to_of_node(fwnode), flags); > + } else if (is_acpi_device_node(fwnode)) { > + phy_dev = phy_find_by_fwnode(fwnode); > + if (!phy_dev) > + return -ENODEV; > + ret = phylink_connect_phy(pl, phy_dev); > + } Looking at this more I really don't like how this if-else-if looks like. I would rather expect something like phy_dev = phy_find_by_fwnode(fwnode); if (!phy_dev) return -ENODEV; ret = phylink_connect_phy(pl, phy_dev); Where phy_find_by_fwnode() will take care about OF or any other possible fwnode cases.
On Thu, Jul 09, 2020 at 11:48:03PM +0300, Andy Shevchenko wrote: > On Thu, Jul 9, 2020 at 8:58 PM Calvin Johnson > <calvin.johnson@oss.nxp.com> wrote: > > > > Define phylink_fwnode_phy_connect() to connect phy specified by > > a fwnode to a phylink instance. > > ... > > > + if (is_of_node(fwnode)) { > > + ret = phylink_of_phy_connect(pl, to_of_node(fwnode), flags); > > + } else if (is_acpi_device_node(fwnode)) { > > + phy_dev = phy_find_by_fwnode(fwnode); > > + if (!phy_dev) > > + return -ENODEV; > > + ret = phylink_connect_phy(pl, phy_dev); > > + } > > Looking at this more I really don't like how this if-else-if looks like. > > I would rather expect something like > > phy_dev = phy_find_by_fwnode(fwnode); > if (!phy_dev) > return -ENODEV; > ret = phylink_connect_phy(pl, phy_dev); > > Where phy_find_by_fwnode() will take care about OF or any other > possible fwnode cases. phy_find_by_fwnode() has a different purpose from that of phylink_fwnode_phy_connect(). Current implementation looks good to me as it clearly takes different paths for DT and ACPI cases. Thanks Calvin
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index dae6c8b51d7f..35d4dfbf5567 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -5,6 +5,7 @@ * * Copyright (C) 2015 Russell King */ +#include <linux/acpi.h> #include <linux/ethtool.h> #include <linux/export.h> #include <linux/gpio/consumer.h> @@ -1017,6 +1018,38 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn, } EXPORT_SYMBOL_GPL(phylink_of_phy_connect); +/** + * phylink_fwnode_phy_connect() - connect the PHY specified in the fwnode. + * @pl: a pointer to a &struct phylink returned from phylink_create() + * @fwnode: a pointer to a &struct fwnode_handle. + * @flags: PHY-specific flags to communicate to the PHY device driver + * + * Connect the phy specified @fwnode to the phylink instance specified + * by @pl. Actions specified in phylink_connect_phy() will be + * performed. + * + * Returns 0 on success or a negative errno. + */ +int phylink_fwnode_phy_connect(struct phylink *pl, + struct fwnode_handle *fwnode, + u32 flags) +{ + struct phy_device *phy_dev; + int ret = 0; + + if (is_of_node(fwnode)) { + ret = phylink_of_phy_connect(pl, to_of_node(fwnode), flags); + } else if (is_acpi_device_node(fwnode)) { + phy_dev = phy_find_by_fwnode(fwnode); + if (!phy_dev) + return -ENODEV; + ret = phylink_connect_phy(pl, phy_dev); + } + + return ret; +} +EXPORT_SYMBOL_GPL(phylink_fwnode_phy_connect); + /** * phylink_disconnect_phy() - disconnect any PHY attached to the phylink * instance. diff --git a/include/linux/phylink.h b/include/linux/phylink.h index b32b8b45421b..b27eed5e49a9 100644 --- a/include/linux/phylink.h +++ b/include/linux/phylink.h @@ -368,6 +368,9 @@ void phylink_destroy(struct phylink *); int phylink_connect_phy(struct phylink *, struct phy_device *); int phylink_of_phy_connect(struct phylink *, struct device_node *, u32 flags); +int phylink_fwnode_phy_connect(struct phylink *pl, + struct fwnode_handle *fwnode, + u32 flags); void phylink_disconnect_phy(struct phylink *); void phylink_mac_change(struct phylink *, bool up);
Define phylink_fwnode_phy_connect() to connect phy specified by a fwnode to a phylink instance. Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com> --- Changes in v4: None Changes in v3: None Changes in v2: None drivers/net/phy/phylink.c | 33 +++++++++++++++++++++++++++++++++ include/linux/phylink.h | 3 +++ 2 files changed, 36 insertions(+)