diff mbox series

[v1,6/7] net: phylink: Introduce phylink_fwnode_phy_connect()

Message ID 20200131153440.20870-7-calvin.johnson@nxp.com
State Deferred
Delegated to: David Miller
Headers show
Series ACPI support for xgmac_mdio and dpaa2-mac drivers. | expand

Commit Message

Calvin Johnson Jan. 31, 2020, 3:34 p.m. UTC
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(+)

Comments

kernel test robot Feb. 3, 2020, 6:21 p.m. UTC | #1
Hi Calvin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.5]
[cannot apply to driver-core/driver-core-testing net-next/master net/master linus/master sparc-next/master next-20200203]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Calvin-Johnson/ACPI-support-for-xgmac_mdio-and-dpaa2-mac-drivers/20200203-070754
base:    d5226fa6dbae0569ee43ecfc08bdcd6770fc4755
reproduce: make htmldocs

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   Warning: The Sphinx 'sphinx_rtd_theme' HTML theme was not found. Make sure you have the theme installed to produce pretty HTML output. Falling back to the default theme.
   WARNING: dot(1) not found, for better output quality install graphviz from http://www.graphviz.org
   WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
   include/linux/spi/spi.h:207: warning: Function parameter or member 'driver_override' not described in 'spi_device'
   include/linux/spi/spi.h:650: warning: Function parameter or member 'irq_flags' not described in 'spi_controller'
   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c:1: warning: no structured comments found
   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1: warning: no structured comments found
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h:254: warning: Function parameter or member 'hdcp_workqueue' not described in 'amdgpu_display_manager'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'quotactl' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'quota_on' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_free_mnt_opts' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_eat_lsm_opts' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_kern_mount' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_show_options' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'sb_add_mnt_opt' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'd_instantiate' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'getprocattr' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'setprocattr' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'locked_down' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_open' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_alloc' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_free' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_read' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 'perf_event_write' not described in 'security_list_options'
   drivers/usb/typec/bus.c:1: warning: 'typec_altmode_register_driver' not found
   drivers/usb/typec/bus.c:1: warning: 'typec_altmode_unregister_driver' not found
   drivers/usb/typec/class.c:1: warning: 'typec_altmode_register_notifier' not found
   drivers/usb/typec/class.c:1: warning: 'typec_altmode_unregister_notifier' not found
   include/linux/regulator/machine.h:196: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints'
   include/linux/regulator/driver.h:223: warning: Function parameter or member 'resume' not described in 'regulator_ops'
   sound/soc/soc-core.c:2522: warning: Function parameter or member 'legacy_dai_naming' not described in 'snd_soc_register_dai'
   include/linux/skbuff.h:888: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'list' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'vlan_present' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:888: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:232: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_portpair' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_cookie' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_listener' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common'
   include/net/sock.h:514: warning: Function parameter or member 'sk_rx_skb_cache' not described in 'sock'
   include/net/sock.h:514: warning: Function parameter or member 'sk_wq_raw' not described in 'sock'
   include/net/sock.h:514: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock'
   include/net/sock.h:514: warning: Function parameter or member 'sk_tx_skb_cache' not described in 'sock'
   include/net/sock.h:514: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock'
   include/net/sock.h:514: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock'
   include/net/sock.h:514: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock'
   include/net/sock.h:514: warning: Function parameter or member 'sk_bpf_storage' not described in 'sock'
   include/net/sock.h:2459: warning: Function parameter or member 'tcp_rx_skb_cache_key' not described in 'DECLARE_STATIC_KEY_FALSE'
   include/net/sock.h:2459: warning: Excess function parameter 'sk' description in 'DECLARE_STATIC_KEY_FALSE'
   include/net/sock.h:2459: warning: Excess function parameter 'skb' description in 'DECLARE_STATIC_KEY_FALSE'
   net/core/skbuff.c:5489: warning: Function parameter or member 'ethernet' not described in 'skb_mpls_push'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'gso_partial_features' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'name_assign_type' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'mpls_ptr' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'xdp_prog' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'qdisc_hash' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device'
   include/linux/netdevice.h:2082: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device'
   drivers/net/phy/mdio_bus.c:861: warning: Function parameter or member 'child' not described in 'fwnode_mdiobus_child_is_phy'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state'
   include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state'
>> drivers/net/phy/phylink.c:836: warning: Function parameter or member 'fwnode' not described in 'phylink_fwnode_phy_connect'
   drivers/net/phy/phylink.c:836: warning: Excess function parameter 'dn' description in 'phylink_fwnode_phy_connect'
   drivers/infiniband/core/umem_odp.c:167: warning: Function parameter or member 'ops' not described in 'ib_umem_odp_alloc_child'
   drivers/infiniband/core/umem_odp.c:217: warning: Function parameter or member 'ops' not described in 'ib_umem_odp_get'
   drivers/infiniband/ulp/iser/iscsi_iser.h:401: warning: Function parameter or member 'all_list' not described in 'iser_fr_desc'
   drivers/infiniband/ulp/iser/iscsi_iser.h:415: warning: Function parameter or member 'all_list' not described in 'iser_fr_pool'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd0' not described in 'opa_vesw_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd1' not described in 'opa_vesw_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd2' not described in 'opa_vesw_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd3' not described in 'opa_vesw_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd4' not described in 'opa_vesw_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd0' not described in 'opa_per_veswport_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd1' not described in 'opa_per_veswport_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd2' not described in 'opa_per_veswport_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd3' not described in 'opa_per_veswport_info'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:263: warning: Function parameter or member 'tbl_entries' not described in 'opa_veswport_mactable'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:342: warning: Function parameter or member 'reserved' not described in 'opa_veswport_summary_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd0' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd1' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd2' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd3' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd4' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd5' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd6' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd7' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd8' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd9' not described in 'opa_veswport_error_counters'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:460: warning: Function parameter or member 'reserved' not described in 'opa_vnic_vema_mad'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:485: warning: Function parameter or member 'reserved' not described in 'opa_vnic_notice_attr'
   drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:500: warning: Function parameter or member 'reserved' not described in 'opa_vnic_vema_mad_trap'
   include/linux/input/sparse-keymap.h:43: warning: Function parameter or member 'sw' not described in 'key_entry'
   include/drm/drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs'
   include/drm/drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
   include/net/cfg80211.h:1189: warning: Function parameter or member 'txpwr' not described in 'station_parameters'
   include/net/mac80211.h:4081: warning: Function parameter or member 'sta_set_txpwr' not described in 'ieee80211_ops'
   include/net/mac80211.h:2036: warning: Function parameter or member 'txpwr' not described in 'ieee80211_sta'
   include/linux/devfreq.h:187: warning: Function parameter or member 'last_status' not described in 'devfreq'
   drivers/devfreq/devfreq.c:1818: warning: bad line: - Resource-managed devfreq_register_notifier()
   drivers/devfreq/devfreq.c:1854: warning: bad line: - Resource-managed devfreq_unregister_notifier()
   drivers/devfreq/devfreq-event.c:355: warning: Function parameter or member 'edev' not described in 'devfreq_event_remove_edev'
   drivers/devfreq/devfreq-event.c:355: warning: Excess function parameter 'dev' description in 'devfreq_event_remove_edev'
   Documentation/admin-guide/hw-vuln/tsx_async_abort.rst:142: WARNING: duplicate label virt_mechanism, other instance in Documentation/admin-guide/hw-vuln/mds.rst
   Documentation/admin-guide/ras.rst:358: WARNING: Definition list ends without a blank line; unexpected unindent.
   Documentation/admin-guide/ras.rst:358: WARNING: Definition list ends without a blank line; unexpected unindent.
   Documentation/admin-guide/ras.rst:363: WARNING: Definition list ends without a blank line; unexpected unindent.
   Documentation/admin-guide/ras.rst:363: WARNING: Definition list ends without a blank line; unexpected unindent.
   Documentation/driver-api/driver-model/driver.rst:215: WARNING: Inline emphasis start-string without end-string.
   Documentation/driver-api/driver-model/driver.rst:215: WARNING: Inline emphasis start-string without end-string.
   include/uapi/linux/firewire-cdev.h:312: WARNING: Inline literal start-string without end-string.
   drivers/firewire/core-transaction.c:606: WARNING: Inline strong start-string without end-string.
   Documentation/x86/boot.rst:72: WARNING: Malformed table.
   Text in column margin in table line 57.

vim +836 drivers/net/phy/phylink.c

   820	
   821	/**
   822	 * phylink_fwnode_phy_connect() - connect the PHY specified in the fwnode.
   823	 * @pl: a pointer to a &struct phylink returned from phylink_create()
   824	 * @dn: a pointer to a &struct device_node.
   825	 * @flags: PHY-specific flags to communicate to the PHY device driver
   826	 *
   827	 * Connect the phy specified in the device node @dn to the phylink instance
   828	 * specified by @pl. Actions specified in phylink_connect_phy() will be
   829	 * performed.
   830	 *
   831	 * Returns 0 on success or a negative errno.
   832	 */
   833	int phylink_fwnode_phy_connect(struct phylink *pl,
   834				       struct fwnode_handle *fwnode,
   835				       u32 flags)
 > 836	{
   837		struct fwnode_handle *phy_node;
   838		struct phy_device *phy_dev;
   839		int ret;
   840		int status;
   841		struct fwnode_reference_args args;
   842	
   843		/* Fixed links and 802.3z are handled without needing a PHY */
   844		if (pl->link_an_mode == MLO_AN_FIXED ||
   845		    (pl->link_an_mode == MLO_AN_INBAND &&
   846		     phy_interface_mode_is_8023z(pl->link_interface)))
   847			return 0;
   848	
   849		status = acpi_node_get_property_reference(fwnode, "phy-handle", 0,
   850							  &args);
   851		if (ACPI_FAILURE(status) || !is_acpi_device_node(args.fwnode))
   852			status = acpi_node_get_property_reference(fwnode, "phy", 0,
   853								  &args);
   854		if (ACPI_FAILURE(status) || !is_acpi_device_node(args.fwnode))
   855			status = acpi_node_get_property_reference(fwnode,
   856								  "phy-device", 0,
   857								  &args);
   858	
   859		if (ACPI_FAILURE(status) || !is_acpi_device_node(args.fwnode)) {
   860			if (pl->link_an_mode == MLO_AN_PHY)
   861				return -ENODEV;
   862			return 0;
   863		}
   864	
   865		phy_dev = fwnode_phy_find_device(args.fwnode);
   866		if (phy_dev)
   867			phy_attach_direct(pl->netdev, phy_dev, flags,
   868					  pl->link_interface);
   869	
   870		/* refcount is held by phy_attach_direct() on success */
   871		put_device(&phy_dev->mdio.dev);
   872	
   873		if (!phy_dev)
   874			return -ENODEV;
   875	
   876		ret = phylink_bringup_phy(pl, phy_dev);
   877		if (ret)
   878			phy_detach(phy_dev);
   879	
   880		return ret;
   881	}
   882	EXPORT_SYMBOL_GPL(phylink_fwnode_phy_connect);
   883	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
Russell King (Oracle) Feb. 3, 2020, 6:41 p.m. UTC | #2
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.
Russell King (Oracle) Feb. 3, 2020, 6:43 p.m. UTC | #3
On Mon, Feb 03, 2020 at 06:41:21PM +0000, Russell King - ARM Linux admin wrote:
> 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.

Also, I don't see any ACPI folk in the list of recipients to your
series.
Calvin Johnson Feb. 5, 2020, 11:33 a.m. UTC | #4
> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Tuesday, February 4, 2020 12:11 AM
> To: Calvin Johnson <calvin.johnson@nxp.com>

<snip>

> > 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.

Ok. I'll remove it.

> 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.

Did you mean phylink_of_phy_connect replaced with phylink_fwnode_phy_connect?
I can add DT handling also inside phylink_fwnode_phy_connect. Please let me know.

Thanks for pointing out about adding linux-acpi ML. I started added them in my responses.
I was assuming they would be added by get_maintainer.pl. 

Thanks
Calvin
diff mbox series

Patch

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);
+
+	if (ACPI_FAILURE(status) || !is_acpi_device_node(args.fwnode)) {
+		if (pl->link_an_mode == MLO_AN_PHY)
+			return -ENODEV;
+		return 0;
+	}
+
+	phy_dev = fwnode_phy_find_device(args.fwnode);
+	if (phy_dev)
+		phy_attach_direct(pl->netdev, phy_dev, flags,
+				  pl->link_interface);
+
+	/* refcount is held by phy_attach_direct() on success */
+	put_device(&phy_dev->mdio.dev);
+
+	if (!phy_dev)
+		return -ENODEV;
+
+	ret = phylink_bringup_phy(pl, phy_dev);
+	if (ret)
+		phy_detach(phy_dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phylink_fwnode_phy_connect);
+
 /**
  * phylink_of_phy_connect() - connect the PHY specified in the DT mode.
  * @pl: a pointer to a &struct phylink returned from phylink_create()
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index fed5488e3c75..cb07cf7a832e 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -240,6 +240,8 @@  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 *);
 int phylink_fixed_state_cb(struct phylink *,
 			   void (*cb)(struct net_device *dev,