Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 3 Feb 2020 18:41:21 +0000
From:   Russell King - ARM Linux admin <linux@armlinux.org.uk>
To:     Calvin Johnson <calvin.johnson@nxp.com>
Cc:     linux.cj@gmail.com, Jon Nettleton <jon@solid-run.com>,
        Makarand Pawagi <makarand.pawagi@nxp.com>,
        cristian.sovaiala@nxp.com, laurentiu.tudor@nxp.com,
        ioana.ciornei@nxp.com, V.Sethi@nxp.com, pankaj.bansal@nxp.com,
        "Rajesh V . Bikkina" <rajesh.bikkina@nxp.com>,
        Calvin Johnson <calvin.johnson@oss.nxp.com>,
        Andrew Lunn <andrew@lunn.ch>,
        "David S. Miller" <davem@davemloft.net>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v1 6/7] net: phylink: Introduce
 phylink_fwnode_phy_connect()
Message-ID: <20200203184121.GR25745@shell.armlinux.org.uk>
References: <20200131153440.20870-1-calvin.johnson@nxp.com>
 <20200131153440.20870-7-calvin.johnson@nxp.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200131153440.20870-7-calvin.johnson@nxp.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jan 31, 2020 at 09:04:39PM +0530, Calvin Johnson wrote:
> From: Calvin Johnson <calvin.johnson@oss.nxp.com>
> 
> Introduce phylink_fwnode_phy_connect API to connect the PHY using
> fwnode.
> 
> Signed-off-by: Calvin Johnson <calvin.johnson@oss.nxp.com>
> ---
> 
>  drivers/net/phy/phylink.c | 64 +++++++++++++++++++++++++++++++++++++++
>  include/linux/phylink.h   |  2 ++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index ee7a718662c6..f211f62283b5 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -18,6 +18,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/timer.h>
>  #include <linux/workqueue.h>
> +#include <linux/acpi.h>
>  
>  #include "sfp.h"
>  #include "swphy.h"
> @@ -817,6 +818,69 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
>  }
>  EXPORT_SYMBOL_GPL(phylink_connect_phy);
>  
> +/**
> + * phylink_fwnode_phy_connect() - connect the PHY specified in the fwnode.
> + * @pl: a pointer to a &struct phylink returned from phylink_create()
> + * @dn: a pointer to a &struct device_node.
> + * @flags: PHY-specific flags to communicate to the PHY device driver
> + *
> + * Connect the phy specified in the device node @dn 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 fwnode_handle *phy_node;
> +	struct phy_device *phy_dev;
> +	int ret;
> +	int status;
> +	struct fwnode_reference_args args;
> +
> +	/* Fixed links and 802.3z are handled without needing a PHY */
> +	if (pl->link_an_mode == MLO_AN_FIXED ||
> +	    (pl->link_an_mode == MLO_AN_INBAND &&
> +	     phy_interface_mode_is_8023z(pl->link_interface)))
> +		return 0;
> +
> +	status = acpi_node_get_property_reference(fwnode, "phy-handle", 0,
> +						  &args);
> +	if (ACPI_FAILURE(status) || !is_acpi_device_node(args.fwnode))
> +		status = acpi_node_get_property_reference(fwnode, "phy", 0,
> +							  &args);
> +	if (ACPI_FAILURE(status) || !is_acpi_device_node(args.fwnode))
> +		status = acpi_node_get_property_reference(fwnode,
> +							  "phy-device", 0,
> +							  &args);

This is a copy-and-paste of phylink_of_phy_connect() without much
thought.

There is no need to duplicate the legacy DT functionality of
phy/phy-device/phy-handle in ACPI - there is no legacy to support,
so it's pointless trying to find one of three properties here.

I'd prefer both the DT and ACPI variants to be more integrated, so
we don't have two almost identical functions except for the firmware
specific detail.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up