lkml.org 
[lkml]   [2020]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [net-next PATCH v2 1/3] device property: Introduce phy related fwnode functions
On Mon, Apr 27, 2020 at 06:54:07PM +0530, Calvin Johnson 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.
>
> Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
> ---
>
> Changes in v2:
> move phy code from base/property.c to net/phy/phy_device.c
> replace acpi & of code to get phy-handle with fwnode_find_reference
>
> drivers/net/phy/phy_device.c | 55 ++++++++++++++++++++++++++++++++++++
> include/linux/phy.h | 3 ++
> 2 files changed, 58 insertions(+)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 7e1ddd5745d2..a2f3dbba8a3c 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -31,6 +31,7 @@
> #include <linux/mdio.h>
> #include <linux/io.h>
> #include <linux/uaccess.h>
> +#include <linux/property.h>
>
> MODULE_DESCRIPTION("PHY library");
> MODULE_AUTHOR("Andy Fleming");
> @@ -2436,6 +2437,60 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
> return phydrv->config_intr && phydrv->ack_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 device *d;
> + struct mdio_device *mdiodev;
> +
> + 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
> + *
> + * 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 *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.
> + *
> + * Returns pointer to the phy fwnode, or ERR_PTR. Caller is responsible to
> + * call fwnode_handle_put() on the returned phy fwnode pointer.
> + */
> +struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode)
> +{
> + return fwnode_find_reference(fwnode, "phy-handle", 0);
> +}
> +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 e2bfb9240587..f0450ef2dc9b 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1328,6 +1328,9 @@ void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx);
> bool phy_validate_pause(struct phy_device *phydev,
> struct ethtool_pauseparam *pp);
> void phy_get_pause(struct phy_device *phydev, bool *tx_pause, bool *rx_pause);
> +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);
> void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv,
> bool *tx_pause, bool *rx_pause);
>

I think you can find a better location in this file to places these
function prototypes - putting them in the middle of a set of prototypes
for pause-related functions looks like you printed the file out, and
played "pin the tail on the donkey" to select somewhere to put them!

I think putting them near the get_phy_device() prototype would be
sensible. Also, consider the case where CONFIG_PHYLIB is not set.
Do we need stub implementations of these functions to avoid ifdefs
in C code?

Thanks.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up

\
 
 \ /
  Last update: 2020-04-27 15:37    [W:0.102 / U:3.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site