All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device
@ 2019-05-28 17:38 Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 01/11] net: phy: Add phy_sysfs_create_links helper function Ioana Ciornei
                   ` (11 more replies)
  0 siblings, 12 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

Following two separate discussion threads in:
  https://www.spinics.net/lists/netdev/msg569087.html
and:
  https://www.spinics.net/lists/netdev/msg570450.html

Previous RFC patch set: https://www.spinics.net/lists/netdev/msg571995.html

PHYLINK was reworked in order to accept multiple operation types,
PHYLINK_NETDEV and PHYLINK_DEV, passed through a phylink_config
structure alongside the corresponding struct device.

One of the main concerns expressed in the RFC was that using notifiers
to signal the corresponding phylink_mac_ops would break PHYLINK's API
unity and that it would become harder to grep for its users.
Using the current approach, we maintain a common API for all users.
Also, printing useful information in PHYLINK, when decoupled from a
net_device, is achieved using dev_err&co on the struct device received
(in DSA's case is the device corresponding to the dsa_switch).

PHYLIB (which PHYLINK uses) was reworked to the extent that it does not
crash when connecting to a PHY and the net_device pointer is NULL.

Lastly, DSA has been reworked in its way that it handles PHYs for ports
that lack a net_device (CPU and DSA ports).  For these, it was
previously using PHYLIB and is now using the PHYLINK_DEV operation type.
Previously, a driver that wanted to support PHY operations on CPU/DSA
ports has to implement .adjust_link(). This patch set not only gives
drivers the options to use PHYLINK uniformly but also urges them to
convert to it. For compatibility, the old code is kept but it will be
removed once all drivers switch over.

The patchset was tested on the NXP LS1021A-TSN board having the
following Ethernet layout:
  https://lkml.org/lkml/2019/5/5/279
The CPU port was moved from the internal RGMII fixed-link (enet2 ->
switch port 4) to an external loopback Cat5 cable between the enet1 port
and the front-facing swp2 SJA1105 port. In this mode, both the master
and the CPU port have an attached PHY which detects link change events:

[   49.105426] fsl-gianfar soc:ethernet@2d50000 eth1: Link is Down
[   50.305486] sja1105 spi0.1: Link is Down
[   53.265596] fsl-gianfar soc:ethernet@2d50000 eth1: Link is Up - 1Gbps/Full - flow control off
[   54.466304] sja1105 spi0.1: Link is Up - 1Gbps/Full - flow control off

Changes in v2:
  - fixed sparse warnings
  - updated 'Documentation/ABI/testing/sysfs-class-net-phydev'

Ioana Ciornei (9):
  net: phy: Guard against the presence of a netdev
  net: phy: Check against net_device being NULL
  net: phy: Add phy_standalone sysfs entry
  net: phylink: Add phylink_mac_link_{up,down} wrapper functions
  net: phylink: Add struct phylink_config to PHYLINK API
  net: phylink: Add PHYLINK_DEV operation type
  net: phylink: Add phylink_{printk,err,warn,info,dbg} macros
  net: dsa: Move the phylink driver calls into port.c
  net: dsa: Use PHYLINK for the CPU/DSA ports

Vladimir Oltean (2):
  net: phy: Add phy_sysfs_create_links helper function
  net: dsa: sja1105: Fix broken fixed-link interfaces on user ports

 Documentation/ABI/testing/sysfs-class-net-phydev |   8 +
 Documentation/networking/sfp-phylink.rst         |   5 +-
 drivers/net/dsa/sja1105/sja1105_main.c           |  11 +-
 drivers/net/ethernet/marvell/mvneta.c            |  36 ++--
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h       |   1 +
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c  |  43 +++--
 drivers/net/phy/phy_device.c                     | 100 ++++++++---
 drivers/net/phy/phylink.c                        | 220 ++++++++++++++---------
 include/linux/phylink.h                          |  57 +++---
 include/net/dsa.h                                |   2 +
 net/dsa/dsa_priv.h                               |  17 ++
 net/dsa/port.c                                   | 157 ++++++++++++++++
 net/dsa/slave.c                                  |  99 +---------
 13 files changed, 499 insertions(+), 257 deletions(-)

-- 
1.9.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 01/11] net: phy: Add phy_sysfs_create_links helper function
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 02/11] net: phy: Guard against the presence of a netdev Ioana Ciornei
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

From: Vladimir Oltean <olteanv@gmail.com>

This is a cosmetic patch that wraps the operation of creating sysfs
links between the netdev->phydev and the phydev->attached_dev.

This is needed to keep the indentation level in check in a follow-up
patch where this function will be guarded against the existence of a
phydev->attached_dev.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - none

 drivers/net/phy/phy_device.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 5d288da9a3b0..8fd1bf37718b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1133,6 +1133,31 @@ void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
 }
 EXPORT_SYMBOL(phy_attached_print);
 
+static void phy_sysfs_create_links(struct phy_device *phydev)
+{
+	struct net_device *dev = phydev->attached_dev;
+	int err;
+
+	err = sysfs_create_link(&phydev->mdio.dev.kobj, &dev->dev.kobj,
+				"attached_dev");
+	if (err)
+		return;
+
+	err = sysfs_create_link_nowarn(&dev->dev.kobj,
+				       &phydev->mdio.dev.kobj,
+				       "phydev");
+	if (err) {
+		dev_err(&dev->dev, "could not add device link to %s err %d\n",
+			kobject_name(&phydev->mdio.dev.kobj),
+			err);
+		/* non-fatal - some net drivers can use one netdevice
+		 * with more then one phy
+		 */
+	}
+
+	phydev->sysfs_links = true;
+}
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1216,23 +1241,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	 */
 	phydev->sysfs_links = false;
 
-	err = sysfs_create_link(&phydev->mdio.dev.kobj, &dev->dev.kobj,
-				"attached_dev");
-	if (!err) {
-		err = sysfs_create_link_nowarn(&dev->dev.kobj,
-					       &phydev->mdio.dev.kobj,
-					       "phydev");
-		if (err) {
-			dev_err(&dev->dev, "could not add device link to %s err %d\n",
-				kobject_name(&phydev->mdio.dev.kobj),
-				err);
-			/* non-fatal - some net drivers can use one netdevice
-			 * with more then one phy
-			 */
-		}
-
-		phydev->sysfs_links = true;
-	}
+	phy_sysfs_create_links(phydev);
 
 	phydev->dev_flags = flags;
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 02/11] net: phy: Guard against the presence of a netdev
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 01/11] net: phy: Add phy_sysfs_create_links helper function Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 03/11] net: phy: Check against net_device being NULL Ioana Ciornei
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

A prerequisite for PHYLIB to work in the absence of a struct net_device
is to not access pointers to it.

Changes are needed in the following areas:

 - Printing: In some places netdev_err was replaced with phydev_err.

 - Incrementing reference count to the parent MDIO bus driver: If there
   is no net device, then the reference count should definitely be
   incremented since there is no chance that it was an Ethernet driver
   who registered the MDIO bus.

 - Sysfs links are not created in case there is no attached_dev.

 - No netif_carrier_off is done if there is no attached_dev.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - none

 drivers/net/phy/phy_device.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8fd1bf37718b..da3bf3f70d63 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1138,6 +1138,9 @@ static void phy_sysfs_create_links(struct phy_device *phydev)
 	struct net_device *dev = phydev->attached_dev;
 	int err;
 
+	if (!dev)
+		return;
+
 	err = sysfs_create_link(&phydev->mdio.dev.kobj, &dev->dev.kobj,
 				"attached_dev");
 	if (err)
@@ -1176,9 +1179,9 @@ static void phy_sysfs_create_links(struct phy_device *phydev)
 int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 		      u32 flags, phy_interface_t interface)
 {
-	struct module *ndev_owner = dev->dev.parent->driver->owner;
 	struct mii_bus *bus = phydev->mdio.bus;
 	struct device *d = &phydev->mdio.dev;
+	struct module *ndev_owner = NULL;
 	bool using_genphy = false;
 	int err;
 
@@ -1187,8 +1190,10 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	 * our own module->refcnt here, otherwise we would not be able to
 	 * unload later on.
 	 */
+	if (dev)
+		ndev_owner = dev->dev.parent->driver->owner;
 	if (ndev_owner != bus->owner && !try_module_get(bus->owner)) {
-		dev_err(&dev->dev, "failed to get the bus module\n");
+		phydev_err(phydev, "failed to get the bus module\n");
 		return -EIO;
 	}
 
@@ -1207,7 +1212,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	}
 
 	if (!try_module_get(d->driver->owner)) {
-		dev_err(&dev->dev, "failed to get the device driver module\n");
+		phydev_err(phydev, "failed to get the device driver module\n");
 		err = -EIO;
 		goto error_put_device;
 	}
@@ -1228,8 +1233,10 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	}
 
 	phydev->phy_link_change = phy_link_change;
-	phydev->attached_dev = dev;
-	dev->phydev = phydev;
+	if (dev) {
+		phydev->attached_dev = dev;
+		dev->phydev = phydev;
+	}
 
 	/* Some Ethernet drivers try to connect to a PHY device before
 	 * calling register_netdevice() -> netdev_register_kobject() and
@@ -1252,7 +1259,8 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	/* Initial carrier state is off as the phy is about to be
 	 * (re)initialized.
 	 */
-	netif_carrier_off(phydev->attached_dev);
+	if (dev)
+		netif_carrier_off(phydev->attached_dev);
 
 	/* Do initial configuration here, now that
 	 * we have certain key parameters
@@ -1358,16 +1366,19 @@ bool phy_driver_is_genphy_10g(struct phy_device *phydev)
 void phy_detach(struct phy_device *phydev)
 {
 	struct net_device *dev = phydev->attached_dev;
-	struct module *ndev_owner = dev->dev.parent->driver->owner;
+	struct module *ndev_owner = NULL;
 	struct mii_bus *bus;
 
 	if (phydev->sysfs_links) {
-		sysfs_remove_link(&dev->dev.kobj, "phydev");
+		if (dev)
+			sysfs_remove_link(&dev->dev.kobj, "phydev");
 		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
 	}
 	phy_suspend(phydev);
-	phydev->attached_dev->phydev = NULL;
-	phydev->attached_dev = NULL;
+	if (dev) {
+		phydev->attached_dev->phydev = NULL;
+		phydev->attached_dev = NULL;
+	}
 	phydev->phylink = NULL;
 
 	phy_led_triggers_unregister(phydev);
@@ -1390,6 +1401,8 @@ void phy_detach(struct phy_device *phydev)
 	bus = phydev->mdio.bus;
 
 	put_device(&phydev->mdio.dev);
+	if (dev)
+		ndev_owner = dev->dev.parent->driver->owner;
 	if (ndev_owner != bus->owner)
 		module_put(bus->owner);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 03/11] net: phy: Check against net_device being NULL
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 01/11] net: phy: Add phy_sysfs_create_links helper function Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 02/11] net: phy: Guard against the presence of a netdev Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:57   ` Florian Fainelli
  2019-05-28 17:38 ` [PATCH v2 net-next 04/11] net: phy: Add phy_standalone sysfs entry Ioana Ciornei
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

In general, we don't want MAC drivers calling phy_attach_direct with the
net_device being NULL. Add checks against this in all the functions
calling it: phy_attach() and phy_connect_direct().

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
Changes in v2:
 - return directly -EINVAL instead of ERR_PTR(-EINVAL) in phy_connect_direct

 drivers/net/phy/phy_device.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index da3bf3f70d63..1b540ed9b326 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -948,6 +948,9 @@ int phy_connect_direct(struct net_device *dev, struct phy_device *phydev,
 {
 	int rc;
 
+	if (!dev)
+		return -EINVAL;
+
 	rc = phy_attach_direct(dev, phydev, phydev->dev_flags, interface);
 	if (rc)
 		return rc;
@@ -1307,6 +1310,9 @@ struct phy_device *phy_attach(struct net_device *dev, const char *bus_id,
 	struct device *d;
 	int rc;
 
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
 	/* Search the list of PHY devices on the mdio bus for the
 	 * PHY with the requested name
 	 */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 04/11] net: phy: Add phy_standalone sysfs entry
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (2 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 03/11] net: phy: Check against net_device being NULL Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 05/11] net: phylink: Add phylink_mac_link_{up,down} wrapper functions Ioana Ciornei
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

Export a phy_standalone device attribute that is meant to give the
indication that this PHY lacks an attached_dev and its corresponding
sysfs link. The attribute will be created only when the
phy_attach_direct() function will be called with a NULL net_device.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - add new entry in Documentation/ABI/testing/sysfs-class-net-phydev

 Documentation/ABI/testing/sysfs-class-net-phydev |  8 ++++++++
 drivers/net/phy/phy_device.c                     | 22 ++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-net-phydev b/Documentation/ABI/testing/sysfs-class-net-phydev
index 6ebabfb27912..b7c46c2f3fd0 100644
--- a/Documentation/ABI/testing/sysfs-class-net-phydev
+++ b/Documentation/ABI/testing/sysfs-class-net-phydev
@@ -34,3 +34,11 @@ Description:
 		xgmii, moca, qsgmii, trgmii, 1000base-x, 2500base-x, rxaui,
 		xaui, 10gbase-kr, unknown
 
+What:		/sys/class/mdio_bus/<bus>/<device>/phy_standalone
+Date:		May 2019
+KernelVersion:	5.3
+Contact:	netdev@vger.kernel.org
+Description:
+		Boolean value indicating whether the PHY device is used in
+		standalone mode, without a net_device associated, by PHYLINK.
+		Attribute created only when this is the case.
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 1b540ed9b326..8b4fc3b4f269 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1164,6 +1164,16 @@ static void phy_sysfs_create_links(struct phy_device *phydev)
 	phydev->sysfs_links = true;
 }
 
+static ssize_t
+phy_standalone_show(struct device *dev, struct device_attribute *attr,
+		    char *buf)
+{
+	struct phy_device *phydev = to_phy_device(dev);
+
+	return sprintf(buf, "%d\n", !phydev->attached_dev);
+}
+static DEVICE_ATTR_RO(phy_standalone);
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1253,6 +1263,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 
 	phy_sysfs_create_links(phydev);
 
+	if (!phydev->attached_dev) {
+		err = sysfs_create_file(&phydev->mdio.dev.kobj,
+					&dev_attr_phy_standalone.attr);
+		if (err)
+			phydev_err(phydev, "error creating 'phy_standalone' sysfs entry\n");
+	}
+
 	phydev->dev_flags = flags;
 
 	phydev->interface = interface;
@@ -1380,6 +1397,11 @@ void phy_detach(struct phy_device *phydev)
 			sysfs_remove_link(&dev->dev.kobj, "phydev");
 		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
 	}
+
+	if (!phydev->attached_dev)
+		sysfs_remove_file(&phydev->mdio.dev.kobj,
+				  &dev_attr_phy_standalone.attr);
+
 	phy_suspend(phydev);
 	if (dev) {
 		phydev->attached_dev->phydev = NULL;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 05/11] net: phylink: Add phylink_mac_link_{up,down} wrapper functions
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (3 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 04/11] net: phy: Add phy_standalone sysfs entry Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 06/11] net: phylink: Add struct phylink_config to PHYLINK API Ioana Ciornei
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

This is a cosmetic patch that reduces the clutter in phylink_resolve
around calling the .mac_link_up/.mac_link_down driver callbacks.  In a
further patch this logic will be extended to emit notifications in case
a net device does not exist.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - none

 drivers/net/phy/phylink.c | 50 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 74983593834b..83ab83c3edba 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -395,6 +395,34 @@ static const char *phylink_pause_to_str(int pause)
 	}
 }
 
+static void phylink_mac_link_up(struct phylink *pl,
+				struct phylink_link_state link_state)
+{
+	struct net_device *ndev = pl->netdev;
+
+	pl->ops->mac_link_up(ndev, pl->link_an_mode,
+			     pl->phy_state.interface,
+			     pl->phydev);
+
+	netif_carrier_on(ndev);
+
+	netdev_info(ndev,
+		    "Link is Up - %s/%s - flow control %s\n",
+		    phy_speed_to_str(link_state.speed),
+		    phy_duplex_to_str(link_state.duplex),
+		    phylink_pause_to_str(link_state.pause));
+}
+
+static void phylink_mac_link_down(struct phylink *pl)
+{
+	struct net_device *ndev = pl->netdev;
+
+	netif_carrier_off(ndev);
+	pl->ops->mac_link_down(ndev, pl->link_an_mode,
+			       pl->phy_state.interface);
+	netdev_info(ndev, "Link is Down\n");
+}
+
 static void phylink_resolve(struct work_struct *w)
 {
 	struct phylink *pl = container_of(w, struct phylink, resolve);
@@ -443,24 +471,10 @@ static void phylink_resolve(struct work_struct *w)
 	}
 
 	if (link_state.link != netif_carrier_ok(ndev)) {
-		if (!link_state.link) {
-			netif_carrier_off(ndev);
-			pl->ops->mac_link_down(ndev, pl->link_an_mode,
-					       pl->phy_state.interface);
-			netdev_info(ndev, "Link is Down\n");
-		} else {
-			pl->ops->mac_link_up(ndev, pl->link_an_mode,
-					     pl->phy_state.interface,
-					     pl->phydev);
-
-			netif_carrier_on(ndev);
-
-			netdev_info(ndev,
-				    "Link is Up - %s/%s - flow control %s\n",
-				    phy_speed_to_str(link_state.speed),
-				    phy_duplex_to_str(link_state.duplex),
-				    phylink_pause_to_str(link_state.pause));
-		}
+		if (!link_state.link)
+			phylink_mac_link_down(pl);
+		else
+			phylink_mac_link_up(pl, link_state);
 	}
 	if (!link_state.link && pl->mac_link_dropped) {
 		pl->mac_link_dropped = false;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 06/11] net: phylink: Add struct phylink_config to PHYLINK API
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (4 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 05/11] net: phylink: Add phylink_mac_link_{up,down} wrapper functions Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-29  7:28   ` Maxime Chevallier
  2019-05-28 17:38 ` [PATCH v2 net-next 07/11] net: phylink: Add PHYLINK_DEV operation type Ioana Ciornei
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

The phylink_config structure will encapsulate a pointer to a struct
device and the operation type requested for this instance of PHYLINK.
This patch does not make any functional changes, it just transitions the
PHYLINK internals and all its users to the new API.

A pointer to a phylink_config structure will be passed to
phylink_create() instead of the net_device directly. Also, the same
phylink_config pointer will be passed back to all phylink_mac_ops
callbacks instead of the net_device. Using this mechanism, a PHYLINK
user can get the original net_device using a structure such as
'to_net_dev(config->dev)' or directly the structure containing the
phylink_config using a container_of call.

At the moment, only the PHYLINK_NETDEV is defined as a valid operation
type for PHYLINK. In this mode, a valid reference to a struct device
linked to the original net_device should be passed to PHYLINK through
the phylink_config structure.

This API changes is mainly driven by the necessity of adding a new
operation type in PHYLINK that disconnects the phy_device from the
net_device and also works when the net_device is lacking.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - none

 Documentation/networking/sfp-phylink.rst        |  5 ++-
 drivers/net/ethernet/marvell/mvneta.c           | 36 ++++++++++------
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h      |  1 +
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 43 +++++++++++--------
 drivers/net/phy/phylink.c                       | 26 ++++++++----
 include/linux/phylink.h                         | 56 ++++++++++++++++---------
 include/net/dsa.h                               |  2 +
 net/dsa/slave.c                                 | 31 ++++++++------
 8 files changed, 128 insertions(+), 72 deletions(-)

diff --git a/Documentation/networking/sfp-phylink.rst b/Documentation/networking/sfp-phylink.rst
index 5bd26cb07244..91446b431b70 100644
--- a/Documentation/networking/sfp-phylink.rst
+++ b/Documentation/networking/sfp-phylink.rst
@@ -98,6 +98,7 @@ this documentation.
 4. Add::
 
 	struct phylink *phylink;
+	struct phylink_config phylink_config;
 
    to the driver's private data structure.  We shall refer to the
    driver's private data pointer as ``priv`` below, and the driver's
@@ -223,8 +224,10 @@ this documentation.
    .. code-block:: c
 
 	struct phylink *phylink;
+	priv->phylink_config.dev = &dev.dev;
+	priv->phylink_config.type = PHYLINK_NETDEV;
 
-	phylink = phylink_create(dev, node, phy_mode, &phylink_ops);
+	phylink = phylink_create(&priv->phylink_config, node, phy_mode, &phylink_ops);
 	if (IS_ERR(phylink)) {
 		err = PTR_ERR(phylink);
 		fail probe;
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index e758650b2c26..adbbcdde73e6 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -437,6 +437,7 @@ struct mvneta_port {
 	struct device_node *dn;
 	unsigned int tx_csum_limit;
 	struct phylink *phylink;
+	struct phylink_config phylink_config;
 	struct phy *comphy;
 
 	struct mvneta_bm *bm_priv;
@@ -3356,9 +3357,11 @@ static int mvneta_set_mac_addr(struct net_device *dev, void *addr)
 	return 0;
 }
 
-static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
+static void mvneta_validate(struct phylink_config *config,
+			    unsigned long *supported,
 			    struct phylink_link_state *state)
 {
+	struct net_device *ndev = to_net_dev(config->dev);
 	struct mvneta_port *pp = netdev_priv(ndev);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
@@ -3408,9 +3411,10 @@ static void mvneta_validate(struct net_device *ndev, unsigned long *supported,
 	phylink_helper_basex_speed(state);
 }
 
-static int mvneta_mac_link_state(struct net_device *ndev,
+static int mvneta_mac_link_state(struct phylink_config *config,
 				 struct phylink_link_state *state)
 {
+	struct net_device *ndev = to_net_dev(config->dev);
 	struct mvneta_port *pp = netdev_priv(ndev);
 	u32 gmac_stat;
 
@@ -3438,8 +3442,9 @@ static int mvneta_mac_link_state(struct net_device *ndev,
 	return 1;
 }
 
-static void mvneta_mac_an_restart(struct net_device *ndev)
+static void mvneta_mac_an_restart(struct phylink_config *config)
 {
+	struct net_device *ndev = to_net_dev(config->dev);
 	struct mvneta_port *pp = netdev_priv(ndev);
 	u32 gmac_an = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG);
 
@@ -3449,9 +3454,10 @@ static void mvneta_mac_an_restart(struct net_device *ndev)
 		    gmac_an & ~MVNETA_GMAC_INBAND_RESTART_AN);
 }
 
-static void mvneta_mac_config(struct net_device *ndev, unsigned int mode,
-	const struct phylink_link_state *state)
+static void mvneta_mac_config(struct phylink_config *config, unsigned int mode,
+			      const struct phylink_link_state *state)
 {
+	struct net_device *ndev = to_net_dev(config->dev);
 	struct mvneta_port *pp = netdev_priv(ndev);
 	u32 new_ctrl0, gmac_ctrl0 = mvreg_read(pp, MVNETA_GMAC_CTRL_0);
 	u32 new_ctrl2, gmac_ctrl2 = mvreg_read(pp, MVNETA_GMAC_CTRL_2);
@@ -3581,9 +3587,10 @@ static void mvneta_set_eee(struct mvneta_port *pp, bool enable)
 	mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi_ctl1);
 }
 
-static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode,
-				 phy_interface_t interface)
+static void mvneta_mac_link_down(struct phylink_config *config,
+				 unsigned int mode, phy_interface_t interface)
 {
+	struct net_device *ndev = to_net_dev(config->dev);
 	struct mvneta_port *pp = netdev_priv(ndev);
 	u32 val;
 
@@ -3600,10 +3607,11 @@ static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode,
 	mvneta_set_eee(pp, false);
 }
 
-static void mvneta_mac_link_up(struct net_device *ndev, unsigned int mode,
+static void mvneta_mac_link_up(struct phylink_config *config, unsigned int mode,
 			       phy_interface_t interface,
 			       struct phy_device *phy)
 {
+	struct net_device *ndev = to_net_dev(config->dev);
 	struct mvneta_port *pp = netdev_priv(ndev);
 	u32 val;
 
@@ -4500,8 +4508,14 @@ static int mvneta_probe(struct platform_device *pdev)
 		comphy = NULL;
 	}
 
-	phylink = phylink_create(dev, pdev->dev.fwnode, phy_mode,
-				 &mvneta_phylink_ops);
+	pp = netdev_priv(dev);
+	spin_lock_init(&pp->lock);
+
+	pp->phylink_config.dev = &dev->dev;
+	pp->phylink_config.type = PHYLINK_NETDEV;
+
+	phylink = phylink_create(&pp->phylink_config, pdev->dev.fwnode,
+				 phy_mode, &mvneta_phylink_ops);
 	if (IS_ERR(phylink)) {
 		err = PTR_ERR(phylink);
 		goto err_free_irq;
@@ -4513,8 +4527,6 @@ static int mvneta_probe(struct platform_device *pdev)
 
 	dev->ethtool_ops = &mvneta_eth_tool_ops;
 
-	pp = netdev_priv(dev);
-	spin_lock_init(&pp->lock);
 	pp->phylink = phylink;
 	pp->comphy = comphy;
 	pp->phy_interface = phy_mode;
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 18ae8d06b692..d67c970f02e5 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -915,6 +915,7 @@ struct mvpp2_port {
 
 	phy_interface_t phy_interface;
 	struct phylink *phylink;
+	struct phylink_config phylink_config;
 	struct phy *comphy;
 
 	struct mvpp2_bm_pool *pool_long;
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 3ed713b8dea5..757f8e31645e 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -56,9 +56,9 @@ enum mvpp2_bm_pool_log_num {
 /* The prototype is added here to be used in start_dev when using ACPI. This
  * will be removed once phylink is used for all modes (dt+ACPI).
  */
-static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
+static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
 			     const struct phylink_link_state *state);
-static void mvpp2_mac_link_up(struct net_device *dev, unsigned int mode,
+static void mvpp2_mac_link_up(struct phylink_config *config, unsigned int mode,
 			      phy_interface_t interface, struct phy_device *phy);
 
 /* Queue modes */
@@ -3239,9 +3239,9 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
 		struct phylink_link_state state = {
 			.interface = port->phy_interface,
 		};
-		mvpp2_mac_config(port->dev, MLO_AN_INBAND, &state);
-		mvpp2_mac_link_up(port->dev, MLO_AN_INBAND, port->phy_interface,
-				  NULL);
+		mvpp2_mac_config(&port->phylink_config, MLO_AN_INBAND, &state);
+		mvpp2_mac_link_up(&port->phylink_config, MLO_AN_INBAND,
+				  port->phy_interface, NULL);
 	}
 
 	netif_tx_start_all_queues(port->dev);
@@ -4463,11 +4463,12 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
 	eth_hw_addr_random(dev);
 }
 
-static void mvpp2_phylink_validate(struct net_device *dev,
+static void mvpp2_phylink_validate(struct phylink_config *config,
 				   unsigned long *supported,
 				   struct phylink_link_state *state)
 {
-	struct mvpp2_port *port = netdev_priv(dev);
+	struct mvpp2_port *port = container_of(config, struct mvpp2_port,
+					       phylink_config);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
 	/* Invalid combinations */
@@ -4591,10 +4592,11 @@ static void mvpp2_gmac_link_state(struct mvpp2_port *port,
 		state->pause |= MLO_PAUSE_TX;
 }
 
-static int mvpp2_phylink_mac_link_state(struct net_device *dev,
+static int mvpp2_phylink_mac_link_state(struct phylink_config *config,
 					struct phylink_link_state *state)
 {
-	struct mvpp2_port *port = netdev_priv(dev);
+	struct mvpp2_port *port = container_of(config, struct mvpp2_port,
+					       phylink_config);
 
 	if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
 		u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
@@ -4610,9 +4612,10 @@ static int mvpp2_phylink_mac_link_state(struct net_device *dev,
 	return 1;
 }
 
-static void mvpp2_mac_an_restart(struct net_device *dev)
+static void mvpp2_mac_an_restart(struct phylink_config *config)
 {
-	struct mvpp2_port *port = netdev_priv(dev);
+	struct mvpp2_port *port = container_of(config, struct mvpp2_port,
+					       phylink_config);
 	u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
 	writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
@@ -4797,9 +4800,10 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
 	}
 }
 
-static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
+static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
 			     const struct phylink_link_state *state)
 {
+	struct net_device *dev = to_net_dev(config->dev);
 	struct mvpp2_port *port = netdev_priv(dev);
 	bool change_interface = port->phy_interface != state->interface;
 
@@ -4839,9 +4843,10 @@ static void mvpp2_mac_config(struct net_device *dev, unsigned int mode,
 	mvpp2_port_enable(port);
 }
 
-static void mvpp2_mac_link_up(struct net_device *dev, unsigned int mode,
+static void mvpp2_mac_link_up(struct phylink_config *config, unsigned int mode,
 			      phy_interface_t interface, struct phy_device *phy)
 {
+	struct net_device *dev = to_net_dev(config->dev);
 	struct mvpp2_port *port = netdev_priv(dev);
 	u32 val;
 
@@ -4866,9 +4871,10 @@ static void mvpp2_mac_link_up(struct net_device *dev, unsigned int mode,
 	netif_tx_wake_all_queues(dev);
 }
 
-static void mvpp2_mac_link_down(struct net_device *dev, unsigned int mode,
-				phy_interface_t interface)
+static void mvpp2_mac_link_down(struct phylink_config *config,
+				unsigned int mode, phy_interface_t interface)
 {
+	struct net_device *dev = to_net_dev(config->dev);
 	struct mvpp2_port *port = netdev_priv(dev);
 	u32 val;
 
@@ -5125,8 +5131,11 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 
 	/* Phylink isn't used w/ ACPI as of now */
 	if (port_node) {
-		phylink = phylink_create(dev, port_fwnode, phy_mode,
-					 &mvpp2_phylink_ops);
+		port->phylink_config.dev = &dev->dev;
+		port->phylink_config.type = PHYLINK_NETDEV;
+
+		phylink = phylink_create(&port->phylink_config, port_fwnode,
+					 phy_mode, &mvpp2_phylink_ops);
 		if (IS_ERR(phylink)) {
 			err = PTR_ERR(phylink);
 			goto err_free_port_pcpu;
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 83ab83c3edba..5a283bf9d402 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -41,6 +41,7 @@ struct phylink {
 	/* private: */
 	struct net_device *netdev;
 	const struct phylink_mac_ops *ops;
+	struct phylink_config *config;
 
 	unsigned long phylink_disable_state; /* bitmask of disables */
 	struct phy_device *phydev;
@@ -111,7 +112,7 @@ static const char *phylink_an_mode_str(unsigned int mode)
 static int phylink_validate(struct phylink *pl, unsigned long *supported,
 			    struct phylink_link_state *state)
 {
-	pl->ops->validate(pl->netdev, supported, state);
+	pl->ops->validate(pl->config, supported, state);
 
 	return phylink_is_empty_linkmode(supported) ? -EINVAL : 0;
 }
@@ -299,7 +300,7 @@ static void phylink_mac_config(struct phylink *pl,
 		   __ETHTOOL_LINK_MODE_MASK_NBITS, state->advertising,
 		   state->pause, state->link, state->an_enabled);
 
-	pl->ops->mac_config(pl->netdev, pl->link_an_mode, state);
+	pl->ops->mac_config(pl->config, pl->link_an_mode, state);
 }
 
 static void phylink_mac_config_up(struct phylink *pl,
@@ -313,12 +314,11 @@ static void phylink_mac_an_restart(struct phylink *pl)
 {
 	if (pl->link_config.an_enabled &&
 	    phy_interface_mode_is_8023z(pl->link_config.interface))
-		pl->ops->mac_an_restart(pl->netdev);
+		pl->ops->mac_an_restart(pl->config);
 }
 
 static int phylink_get_mac_state(struct phylink *pl, struct phylink_link_state *state)
 {
-	struct net_device *ndev = pl->netdev;
 
 	linkmode_copy(state->advertising, pl->link_config.advertising);
 	linkmode_zero(state->lp_advertising);
@@ -330,7 +330,7 @@ static int phylink_get_mac_state(struct phylink *pl, struct phylink_link_state *
 	state->an_complete = 0;
 	state->link = 1;
 
-	return pl->ops->mac_link_state(ndev, state);
+	return pl->ops->mac_link_state(pl->config, state);
 }
 
 /* The fixed state is... fixed except for the link state,
@@ -400,7 +400,7 @@ static void phylink_mac_link_up(struct phylink *pl,
 {
 	struct net_device *ndev = pl->netdev;
 
-	pl->ops->mac_link_up(ndev, pl->link_an_mode,
+	pl->ops->mac_link_up(pl->config, pl->link_an_mode,
 			     pl->phy_state.interface,
 			     pl->phydev);
 
@@ -418,7 +418,7 @@ static void phylink_mac_link_down(struct phylink *pl)
 	struct net_device *ndev = pl->netdev;
 
 	netif_carrier_off(ndev);
-	pl->ops->mac_link_down(ndev, pl->link_an_mode,
+	pl->ops->mac_link_down(pl->config, pl->link_an_mode,
 			       pl->phy_state.interface);
 	netdev_info(ndev, "Link is Down\n");
 }
@@ -553,7 +553,7 @@ static int phylink_register_sfp(struct phylink *pl,
  * Returns a pointer to a &struct phylink, or an error-pointer value. Users
  * must use IS_ERR() to check for errors from this function.
  */
-struct phylink *phylink_create(struct net_device *ndev,
+struct phylink *phylink_create(struct phylink_config *config,
 			       struct fwnode_handle *fwnode,
 			       phy_interface_t iface,
 			       const struct phylink_mac_ops *ops)
@@ -567,7 +567,15 @@ struct phylink *phylink_create(struct net_device *ndev,
 
 	mutex_init(&pl->state_mutex);
 	INIT_WORK(&pl->resolve, phylink_resolve);
-	pl->netdev = ndev;
+
+	pl->config = config;
+	if (config->type == PHYLINK_NETDEV) {
+		pl->netdev = to_net_dev(config->dev);
+	} else {
+		kfree(pl);
+		return ERR_PTR(-EINVAL);
+	}
+
 	pl->phy_state.interface = iface;
 	pl->link_interface = iface;
 	if (iface == PHY_INTERFACE_MODE_MOCA)
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 6411c624f63a..67f35f07ac4b 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -54,6 +54,20 @@ struct phylink_link_state {
 	unsigned int an_complete:1;
 };
 
+enum phylink_op_type {
+	PHYLINK_NETDEV = 0,
+};
+
+/**
+ * struct phylink_config - PHYLINK configuration structure
+ * @dev: a pointer to a struct device associated with the MAC
+ * @type: operation type of PHYLINK instance
+ */
+struct phylink_config {
+	struct device *dev;
+	enum phylink_op_type type;
+};
+
 /**
  * struct phylink_mac_ops - MAC operations structure.
  * @validate: Validate and update the link configuration.
@@ -66,16 +80,17 @@ struct phylink_link_state {
  * The individual methods are described more fully below.
  */
 struct phylink_mac_ops {
-	void (*validate)(struct net_device *ndev, unsigned long *supported,
+	void (*validate)(struct phylink_config *config,
+			 unsigned long *supported,
 			 struct phylink_link_state *state);
-	int (*mac_link_state)(struct net_device *ndev,
+	int (*mac_link_state)(struct phylink_config *config,
 			      struct phylink_link_state *state);
-	void (*mac_config)(struct net_device *ndev, unsigned int mode,
+	void (*mac_config)(struct phylink_config *config, unsigned int mode,
 			   const struct phylink_link_state *state);
-	void (*mac_an_restart)(struct net_device *ndev);
-	void (*mac_link_down)(struct net_device *ndev, unsigned int mode,
+	void (*mac_an_restart)(struct phylink_config *config);
+	void (*mac_link_down)(struct phylink_config *config, unsigned int mode,
 			      phy_interface_t interface);
-	void (*mac_link_up)(struct net_device *ndev, unsigned int mode,
+	void (*mac_link_up)(struct phylink_config *config, unsigned int mode,
 			    phy_interface_t interface,
 			    struct phy_device *phy);
 };
@@ -83,7 +98,7 @@ struct phylink_mac_ops {
 #if 0 /* For kernel-doc purposes only. */
 /**
  * validate - Validate and update the link configuration
- * @ndev: a pointer to a &struct net_device for the MAC.
+ * @config: a pointer to a &struct phylink_config.
  * @supported: ethtool bitmask for supported link modes.
  * @state: a pointer to a &struct phylink_link_state.
  *
@@ -100,12 +115,12 @@ struct phylink_mac_ops {
  * based on @state->advertising and/or @state->speed and update
  * @state->interface accordingly.
  */
-void validate(struct net_device *ndev, unsigned long *supported,
+void validate(struct phylink_config *config, unsigned long *supported,
 	      struct phylink_link_state *state);
 
 /**
  * mac_link_state() - Read the current link state from the hardware
- * @ndev: a pointer to a &struct net_device for the MAC.
+ * @config: a pointer to a &struct phylink_config.
  * @state: a pointer to a &struct phylink_link_state.
  *
  * Read the current link state from the MAC, reporting the current
@@ -114,12 +129,12 @@ void validate(struct net_device *ndev, unsigned long *supported,
  * negotiation completion state in @state->an_complete, and link
  * up state in @state->link.
  */
-int mac_link_state(struct net_device *ndev,
+int mac_link_state(struct phylink_config *config,
 		   struct phylink_link_state *state);
 
 /**
  * mac_config() - configure the MAC for the selected mode and state
- * @ndev: a pointer to a &struct net_device for the MAC.
+ * @config: a pointer to a &struct phylink_config.
  * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
  * @state: a pointer to a &struct phylink_link_state.
  *
@@ -157,18 +172,18 @@ int mac_link_state(struct net_device *ndev,
  * down.  This "update" behaviour is critical to avoid bouncing the
  * link up status.
  */
-void mac_config(struct net_device *ndev, unsigned int mode,
+void mac_config(struct phylink_config *config, unsigned int mode,
 		const struct phylink_link_state *state);
 
 /**
  * mac_an_restart() - restart 802.3z BaseX autonegotiation
- * @ndev: a pointer to a &struct net_device for the MAC.
+ * @config: a pointer to a &struct phylink_config.
  */
-void mac_an_restart(struct net_device *ndev);
+void mac_an_restart(struct phylink_config *config);
 
 /**
  * mac_link_down() - take the link down
- * @ndev: a pointer to a &struct net_device for the MAC.
+ * @config: a pointer to a &struct phylink_config.
  * @mode: link autonegotiation mode
  * @interface: link &typedef phy_interface_t mode
  *
@@ -177,12 +192,12 @@ void mac_config(struct net_device *ndev, unsigned int mode,
  * Energy Efficient Ethernet MAC configuration. Interface type
  * selection must be done in mac_config().
  */
-void mac_link_down(struct net_device *ndev, unsigned int mode,
+void mac_link_down(struct phylink_config *config, unsigned int mode,
 		   phy_interface_t interface);
 
 /**
  * mac_link_up() - allow the link to come up
- * @ndev: a pointer to a &struct net_device for the MAC.
+ * @config: a pointer to a &struct phylink_config.
  * @mode: link autonegotiation mode
  * @interface: link &typedef phy_interface_t mode
  * @phy: any attached phy
@@ -193,13 +208,14 @@ void mac_link_down(struct net_device *ndev, unsigned int mode,
  * phy_init_eee() and perform appropriate MAC configuration for EEE.
  * Interface type selection must be done in mac_config().
  */
-void mac_link_up(struct net_device *ndev, unsigned int mode,
+void mac_link_up(struct phylink_config *config, unsigned int mode,
 		 phy_interface_t interface,
 		 struct phy_device *phy);
 #endif
 
-struct phylink *phylink_create(struct net_device *, struct fwnode_handle *,
-	phy_interface_t iface, const struct phylink_mac_ops *ops);
+struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
+			       phy_interface_t iface,
+			       const struct phylink_mac_ops *ops);
 void phylink_destroy(struct phylink *);
 
 int phylink_connect_phy(struct phylink *, struct phy_device *);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 685294817712..a7f36219904f 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -22,6 +22,7 @@
 #include <linux/net_tstamp.h>
 #include <linux/phy.h>
 #include <linux/platform_data/dsa.h>
+#include <linux/phylink.h>
 #include <net/devlink.h>
 #include <net/switchdev.h>
 
@@ -193,6 +194,7 @@ struct dsa_port {
 	struct net_device	*bridge_dev;
 	struct devlink_port	devlink_port;
 	struct phylink		*pl;
+	struct phylink_config	pl_config;
 
 	struct work_struct	xmit_work;
 	struct sk_buff_head	xmit_queue;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9892ca1f6859..48e017637d4f 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1164,11 +1164,11 @@ static struct devlink_port *dsa_slave_get_devlink_port(struct net_device *dev)
 	.name	= "dsa",
 };
 
-static void dsa_slave_phylink_validate(struct net_device *dev,
+static void dsa_slave_phylink_validate(struct phylink_config *config,
 				       unsigned long *supported,
 				       struct phylink_link_state *state)
 {
-	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
 	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->phylink_validate)
@@ -1177,10 +1177,10 @@ static void dsa_slave_phylink_validate(struct net_device *dev,
 	ds->ops->phylink_validate(ds, dp->index, supported, state);
 }
 
-static int dsa_slave_phylink_mac_link_state(struct net_device *dev,
+static int dsa_slave_phylink_mac_link_state(struct phylink_config *config,
 					    struct phylink_link_state *state)
 {
-	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
 	struct dsa_switch *ds = dp->ds;
 
 	/* Only called for SGMII and 802.3z */
@@ -1190,11 +1190,11 @@ static int dsa_slave_phylink_mac_link_state(struct net_device *dev,
 	return ds->ops->phylink_mac_link_state(ds, dp->index, state);
 }
 
-static void dsa_slave_phylink_mac_config(struct net_device *dev,
+static void dsa_slave_phylink_mac_config(struct phylink_config *config,
 					 unsigned int mode,
 					 const struct phylink_link_state *state)
 {
-	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
 	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->phylink_mac_config)
@@ -1203,9 +1203,9 @@ static void dsa_slave_phylink_mac_config(struct net_device *dev,
 	ds->ops->phylink_mac_config(ds, dp->index, mode, state);
 }
 
-static void dsa_slave_phylink_mac_an_restart(struct net_device *dev)
+static void dsa_slave_phylink_mac_an_restart(struct phylink_config *config)
 {
-	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
 	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->phylink_mac_an_restart)
@@ -1214,11 +1214,12 @@ static void dsa_slave_phylink_mac_an_restart(struct net_device *dev)
 	ds->ops->phylink_mac_an_restart(ds, dp->index);
 }
 
-static void dsa_slave_phylink_mac_link_down(struct net_device *dev,
+static void dsa_slave_phylink_mac_link_down(struct phylink_config *config,
 					    unsigned int mode,
 					    phy_interface_t interface)
 {
-	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+	struct net_device *dev = dp->slave;
 	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->phylink_mac_link_down) {
@@ -1230,12 +1231,13 @@ static void dsa_slave_phylink_mac_link_down(struct net_device *dev,
 	ds->ops->phylink_mac_link_down(ds, dp->index, mode, interface);
 }
 
-static void dsa_slave_phylink_mac_link_up(struct net_device *dev,
+static void dsa_slave_phylink_mac_link_up(struct phylink_config *config,
 					  unsigned int mode,
 					  phy_interface_t interface,
 					  struct phy_device *phydev)
 {
-	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+	struct net_device *dev = dp->slave;
 	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->phylink_mac_link_up) {
@@ -1303,7 +1305,10 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
 	if (mode < 0)
 		mode = PHY_INTERFACE_MODE_NA;
 
-	dp->pl = phylink_create(slave_dev, of_fwnode_handle(port_dn), mode,
+	dp->pl_config.dev = &slave_dev->dev;
+	dp->pl_config.type = PHYLINK_NETDEV;
+
+	dp->pl = phylink_create(&dp->pl_config, of_fwnode_handle(port_dn), mode,
 				&dsa_slave_phylink_mac_ops);
 	if (IS_ERR(dp->pl)) {
 		netdev_err(slave_dev,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 07/11] net: phylink: Add PHYLINK_DEV operation type
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (5 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 06/11] net: phylink: Add struct phylink_config to PHYLINK API Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 08/11] net: phylink: Add phylink_{printk,err,warn,info,dbg} macros Ioana Ciornei
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

In the PHYLINK_DEV operation type, the PHYLINK infrastructure can work
without an attached net_device. For printing usecases, instead, a struct
device * should be passed to PHYLINK using the phylink_config structure.

Also, netif_carrier_* calls ar guarded by the presence of a valid
net_device. When using the PHYLINK_DEV operation type, we cannot check
link status using the netif_carrier_ok() API so instead, keep an
internal state of the MAC and call mac_link_{down,up} only when the link
changed.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - none

 drivers/net/phy/phylink.c | 25 ++++++++++++++++++++-----
 include/linux/phylink.h   |  1 +
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 5a283bf9d402..5f6120f3fa3f 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -42,6 +42,8 @@ struct phylink {
 	struct net_device *netdev;
 	const struct phylink_mac_ops *ops;
 	struct phylink_config *config;
+	struct device *dev;
+	unsigned int old_link_state:1;
 
 	unsigned long phylink_disable_state; /* bitmask of disables */
 	struct phy_device *phydev;
@@ -404,7 +406,8 @@ static void phylink_mac_link_up(struct phylink *pl,
 			     pl->phy_state.interface,
 			     pl->phydev);
 
-	netif_carrier_on(ndev);
+	if (ndev)
+		netif_carrier_on(ndev);
 
 	netdev_info(ndev,
 		    "Link is Up - %s/%s - flow control %s\n",
@@ -417,7 +420,8 @@ static void phylink_mac_link_down(struct phylink *pl)
 {
 	struct net_device *ndev = pl->netdev;
 
-	netif_carrier_off(ndev);
+	if (ndev)
+		netif_carrier_off(ndev);
 	pl->ops->mac_link_down(pl->config, pl->link_an_mode,
 			       pl->phy_state.interface);
 	netdev_info(ndev, "Link is Down\n");
@@ -428,6 +432,7 @@ static void phylink_resolve(struct work_struct *w)
 	struct phylink *pl = container_of(w, struct phylink, resolve);
 	struct phylink_link_state link_state;
 	struct net_device *ndev = pl->netdev;
+	int link_changed;
 
 	mutex_lock(&pl->state_mutex);
 	if (pl->phylink_disable_state) {
@@ -470,7 +475,13 @@ static void phylink_resolve(struct work_struct *w)
 		}
 	}
 
-	if (link_state.link != netif_carrier_ok(ndev)) {
+	if (pl->netdev)
+		link_changed = (link_state.link != netif_carrier_ok(ndev));
+	else
+		link_changed = (link_state.link != pl->old_link_state);
+
+	if (link_changed) {
+		pl->old_link_state = link_state.link;
 		if (!link_state.link)
 			phylink_mac_link_down(pl);
 		else
@@ -571,6 +582,8 @@ struct phylink *phylink_create(struct phylink_config *config,
 	pl->config = config;
 	if (config->type == PHYLINK_NETDEV) {
 		pl->netdev = to_net_dev(config->dev);
+	} else if (config->type == PHYLINK_DEV) {
+		pl->dev = config->dev;
 	} else {
 		kfree(pl);
 		return ERR_PTR(-EINVAL);
@@ -910,7 +923,8 @@ void phylink_start(struct phylink *pl)
 		    phy_modes(pl->link_config.interface));
 
 	/* Always set the carrier off */
-	netif_carrier_off(pl->netdev);
+	if (pl->netdev)
+		netif_carrier_off(pl->netdev);
 
 	/* Apply the link configuration to the MAC when starting. This allows
 	 * a fixed-link to start with the correct parameters, and also
@@ -1255,7 +1269,8 @@ int phylink_ethtool_set_pauseparam(struct phylink *pl,
 		switch (pl->link_an_mode) {
 		case MLO_AN_PHY:
 			/* Silently mark the carrier down, and then trigger a resolve */
-			netif_carrier_off(pl->netdev);
+			if (pl->netdev)
+				netif_carrier_off(pl->netdev);
 			phylink_run_resolve(pl);
 			break;
 
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 67f35f07ac4b..0f6f65bb9d44 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -56,6 +56,7 @@ struct phylink_link_state {
 
 enum phylink_op_type {
 	PHYLINK_NETDEV = 0,
+	PHYLINK_DEV,
 };
 
 /**
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 08/11] net: phylink: Add phylink_{printk,err,warn,info,dbg} macros
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (6 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 07/11] net: phylink: Add PHYLINK_DEV operation type Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 09/11] net: dsa: Move the phylink driver calls into port.c Ioana Ciornei
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

With the latest addition to the PHYLINK infrastructure, we are faced
with a decision on when to print necessary info using the struct
net_device and when with the struct device.

Add a series of macros that encapsulate this decision and replace all
uses of netdev_err&co with phylink_err.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - none

 drivers/net/phy/phylink.c | 137 ++++++++++++++++++++++++++--------------------
 1 file changed, 77 insertions(+), 60 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 5f6120f3fa3f..68d0a89c52be 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -68,6 +68,23 @@ struct phylink {
 	struct sfp_bus *sfp_bus;
 };
 
+#define phylink_printk(level, pl, fmt, ...) \
+	do { \
+		if ((pl)->config->type == PHYLINK_NETDEV) \
+			netdev_printk(level, (pl)->netdev, fmt, ##__VA_ARGS__); \
+		else if ((pl)->config->type == PHYLINK_DEV) \
+			dev_printk(level, (pl)->dev, fmt, ##__VA_ARGS__); \
+	} while (0)
+
+#define phylink_err(pl, fmt, ...) \
+	phylink_printk(KERN_ERR, pl, fmt, ##__VA_ARGS__)
+#define phylink_warn(pl, fmt, ...) \
+	phylink_printk(KERN_WARNING, pl, fmt, ##__VA_ARGS__)
+#define phylink_info(pl, fmt, ...) \
+	phylink_printk(KERN_INFO, pl, fmt, ##__VA_ARGS__)
+#define phylink_dbg(pl, fmt, ...) \
+	phylink_printk(KERN_DEBUG, pl, fmt, ##__VA_ARGS__)
+
 /**
  * phylink_set_port_modes() - set the port type modes in the ethtool mask
  * @mask: ethtool link mode mask
@@ -164,7 +181,7 @@ static int phylink_parse_fixedlink(struct phylink *pl,
 		ret = fwnode_property_read_u32_array(fwnode, "fixed-link",
 						     NULL, 0);
 		if (ret != ARRAY_SIZE(prop)) {
-			netdev_err(pl->netdev, "broken fixed-link?\n");
+			phylink_err(pl, "broken fixed-link?\n");
 			return -EINVAL;
 		}
 
@@ -183,8 +200,8 @@ static int phylink_parse_fixedlink(struct phylink *pl,
 
 	if (pl->link_config.speed > SPEED_1000 &&
 	    pl->link_config.duplex != DUPLEX_FULL)
-		netdev_warn(pl->netdev, "fixed link specifies half duplex for %dMbps link?\n",
-			    pl->link_config.speed);
+		phylink_warn(pl, "fixed link specifies half duplex for %dMbps link?\n",
+			     pl->link_config.speed);
 
 	bitmap_fill(pl->supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
 	linkmode_copy(pl->link_config.advertising, pl->supported);
@@ -197,9 +214,9 @@ static int phylink_parse_fixedlink(struct phylink *pl,
 	if (s) {
 		__set_bit(s->bit, pl->supported);
 	} else {
-		netdev_warn(pl->netdev, "fixed link %s duplex %dMbps not recognised\n",
-			    pl->link_config.duplex == DUPLEX_FULL ? "full" : "half",
-			    pl->link_config.speed);
+		phylink_warn(pl, "fixed link %s duplex %dMbps not recognised\n",
+			     pl->link_config.duplex == DUPLEX_FULL ? "full" : "half",
+			     pl->link_config.speed);
 	}
 
 	linkmode_and(pl->link_config.advertising, pl->link_config.advertising,
@@ -224,8 +241,8 @@ static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
 	if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
 	    strcmp(managed, "in-band-status") == 0) {
 		if (pl->link_an_mode == MLO_AN_FIXED) {
-			netdev_err(pl->netdev,
-				   "can't use both fixed-link and in-band-status\n");
+			phylink_err(pl,
+				    "can't use both fixed-link and in-band-status\n");
 			return -EINVAL;
 		}
 
@@ -272,17 +289,17 @@ static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
 			break;
 
 		default:
-			netdev_err(pl->netdev,
-				   "incorrect link mode %s for in-band status\n",
-				   phy_modes(pl->link_config.interface));
+			phylink_err(pl,
+				    "incorrect link mode %s for in-band status\n",
+				    phy_modes(pl->link_config.interface));
 			return -EINVAL;
 		}
 
 		linkmode_copy(pl->link_config.advertising, pl->supported);
 
 		if (phylink_validate(pl, pl->supported, &pl->link_config)) {
-			netdev_err(pl->netdev,
-				   "failed to validate link configuration for in-band status\n");
+			phylink_err(pl,
+				    "failed to validate link configuration for in-band status\n");
 			return -EINVAL;
 		}
 	}
@@ -293,14 +310,14 @@ static int phylink_parse_mode(struct phylink *pl, struct fwnode_handle *fwnode)
 static void phylink_mac_config(struct phylink *pl,
 			       const struct phylink_link_state *state)
 {
-	netdev_dbg(pl->netdev,
-		   "%s: mode=%s/%s/%s/%s adv=%*pb pause=%02x link=%u an=%u\n",
-		   __func__, phylink_an_mode_str(pl->link_an_mode),
-		   phy_modes(state->interface),
-		   phy_speed_to_str(state->speed),
-		   phy_duplex_to_str(state->duplex),
-		   __ETHTOOL_LINK_MODE_MASK_NBITS, state->advertising,
-		   state->pause, state->link, state->an_enabled);
+	phylink_dbg(pl,
+		    "%s: mode=%s/%s/%s/%s adv=%*pb pause=%02x link=%u an=%u\n",
+		    __func__, phylink_an_mode_str(pl->link_an_mode),
+		    phy_modes(state->interface),
+		    phy_speed_to_str(state->speed),
+		    phy_duplex_to_str(state->duplex),
+		    __ETHTOOL_LINK_MODE_MASK_NBITS, state->advertising,
+		    state->pause, state->link, state->an_enabled);
 
 	pl->ops->mac_config(pl->config, pl->link_an_mode, state);
 }
@@ -409,11 +426,11 @@ static void phylink_mac_link_up(struct phylink *pl,
 	if (ndev)
 		netif_carrier_on(ndev);
 
-	netdev_info(ndev,
-		    "Link is Up - %s/%s - flow control %s\n",
-		    phy_speed_to_str(link_state.speed),
-		    phy_duplex_to_str(link_state.duplex),
-		    phylink_pause_to_str(link_state.pause));
+	phylink_info(pl,
+		     "Link is Up - %s/%s - flow control %s\n",
+		     phy_speed_to_str(link_state.speed),
+		     phy_duplex_to_str(link_state.duplex),
+		     phylink_pause_to_str(link_state.pause));
 }
 
 static void phylink_mac_link_down(struct phylink *pl)
@@ -424,7 +441,7 @@ static void phylink_mac_link_down(struct phylink *pl)
 		netif_carrier_off(ndev);
 	pl->ops->mac_link_down(pl->config, pl->link_an_mode,
 			       pl->phy_state.interface);
-	netdev_info(ndev, "Link is Down\n");
+	phylink_info(pl, "Link is Down\n");
 }
 
 static void phylink_resolve(struct work_struct *w)
@@ -537,8 +554,8 @@ static int phylink_register_sfp(struct phylink *pl,
 		if (ret == -ENOENT)
 			return 0;
 
-		netdev_err(pl->netdev, "unable to parse \"sfp\" node: %d\n",
-			   ret);
+		phylink_err(pl, "unable to parse \"sfp\" node: %d\n",
+			    ret);
 		return ret;
 	}
 
@@ -670,10 +687,10 @@ static void phylink_phy_change(struct phy_device *phydev, bool up,
 
 	phylink_run_resolve(pl);
 
-	netdev_dbg(pl->netdev, "phy link %s %s/%s/%s\n", up ? "up" : "down",
-		   phy_modes(phydev->interface),
-		   phy_speed_to_str(phydev->speed),
-		   phy_duplex_to_str(phydev->duplex));
+	phylink_dbg(pl, "phy link %s %s/%s/%s\n", up ? "up" : "down",
+		    phy_modes(phydev->interface),
+		    phy_speed_to_str(phydev->speed),
+		    phy_duplex_to_str(phydev->duplex));
 }
 
 static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy)
@@ -706,9 +723,9 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy)
 	phy->phylink = pl;
 	phy->phy_link_change = phylink_phy_change;
 
-	netdev_info(pl->netdev,
-		    "PHY [%s] driver [%s]\n", dev_name(&phy->mdio.dev),
-		    phy->drv->name);
+	phylink_info(pl,
+		     "PHY [%s] driver [%s]\n", dev_name(&phy->mdio.dev),
+		     phy->drv->name);
 
 	mutex_lock(&phy->lock);
 	mutex_lock(&pl->state_mutex);
@@ -721,10 +738,10 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy)
 	mutex_unlock(&pl->state_mutex);
 	mutex_unlock(&phy->lock);
 
-	netdev_dbg(pl->netdev,
-		   "phy: setting supported %*pb advertising %*pb\n",
-		   __ETHTOOL_LINK_MODE_MASK_NBITS, pl->supported,
-		   __ETHTOOL_LINK_MODE_MASK_NBITS, phy->advertising);
+	phylink_dbg(pl,
+		    "phy: setting supported %*pb advertising %*pb\n",
+		    __ETHTOOL_LINK_MODE_MASK_NBITS, pl->supported,
+		    __ETHTOOL_LINK_MODE_MASK_NBITS, phy->advertising);
 
 	if (phy_interrupt_is_valid(phy))
 		phy_request_interrupt(phy);
@@ -902,7 +919,7 @@ void phylink_mac_change(struct phylink *pl, bool up)
 	if (!up)
 		pl->mac_link_dropped = true;
 	phylink_run_resolve(pl);
-	netdev_dbg(pl->netdev, "mac link %s\n", up ? "up" : "down");
+	phylink_dbg(pl, "mac link %s\n", up ? "up" : "down");
 }
 EXPORT_SYMBOL_GPL(phylink_mac_change);
 
@@ -918,9 +935,9 @@ void phylink_start(struct phylink *pl)
 {
 	ASSERT_RTNL();
 
-	netdev_info(pl->netdev, "configuring for %s/%s link mode\n",
-		    phylink_an_mode_str(pl->link_an_mode),
-		    phy_modes(pl->link_config.interface));
+	phylink_info(pl, "configuring for %s/%s link mode\n",
+		     phylink_an_mode_str(pl->link_an_mode),
+		     phy_modes(pl->link_config.interface));
 
 	/* Always set the carrier off */
 	if (pl->netdev)
@@ -1631,33 +1648,33 @@ static int phylink_sfp_module_insert(void *upstream,
 	/* Ignore errors if we're expecting a PHY to attach later */
 	ret = phylink_validate(pl, support, &config);
 	if (ret) {
-		netdev_err(pl->netdev, "validation with support %*pb failed: %d\n",
-			   __ETHTOOL_LINK_MODE_MASK_NBITS, support, ret);
+		phylink_err(pl, "validation with support %*pb failed: %d\n",
+			    __ETHTOOL_LINK_MODE_MASK_NBITS, support, ret);
 		return ret;
 	}
 
 	iface = sfp_select_interface(pl->sfp_bus, id, config.advertising);
 	if (iface == PHY_INTERFACE_MODE_NA) {
-		netdev_err(pl->netdev,
-			   "selection of interface failed, advertisement %*pb\n",
-			   __ETHTOOL_LINK_MODE_MASK_NBITS, config.advertising);
+		phylink_err(pl,
+			    "selection of interface failed, advertisement %*pb\n",
+			    __ETHTOOL_LINK_MODE_MASK_NBITS, config.advertising);
 		return -EINVAL;
 	}
 
 	config.interface = iface;
 	ret = phylink_validate(pl, support, &config);
 	if (ret) {
-		netdev_err(pl->netdev, "validation of %s/%s with support %*pb failed: %d\n",
-			   phylink_an_mode_str(MLO_AN_INBAND),
-			   phy_modes(config.interface),
-			   __ETHTOOL_LINK_MODE_MASK_NBITS, support, ret);
+		phylink_err(pl, "validation of %s/%s with support %*pb failed: %d\n",
+			    phylink_an_mode_str(MLO_AN_INBAND),
+			    phy_modes(config.interface),
+			    __ETHTOOL_LINK_MODE_MASK_NBITS, support, ret);
 		return ret;
 	}
 
-	netdev_dbg(pl->netdev, "requesting link mode %s/%s with support %*pb\n",
-		   phylink_an_mode_str(MLO_AN_INBAND),
-		   phy_modes(config.interface),
-		   __ETHTOOL_LINK_MODE_MASK_NBITS, support);
+	phylink_dbg(pl, "requesting link mode %s/%s with support %*pb\n",
+		    phylink_an_mode_str(MLO_AN_INBAND),
+		    phy_modes(config.interface),
+		    __ETHTOOL_LINK_MODE_MASK_NBITS, support);
 
 	if (phy_interface_mode_is_8023z(iface) && pl->phydev)
 		return -EINVAL;
@@ -1676,9 +1693,9 @@ static int phylink_sfp_module_insert(void *upstream,
 
 		changed = true;
 
-		netdev_info(pl->netdev, "switched to %s/%s link mode\n",
-			    phylink_an_mode_str(MLO_AN_INBAND),
-			    phy_modes(config.interface));
+		phylink_info(pl, "switched to %s/%s link mode\n",
+			     phylink_an_mode_str(MLO_AN_INBAND),
+			     phy_modes(config.interface));
 	}
 
 	pl->link_port = port;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 09/11] net: dsa: Move the phylink driver calls into port.c
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (7 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 08/11] net: phylink: Add phylink_{printk,err,warn,info,dbg} macros Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 10/11] net: dsa: Use PHYLINK for the CPU/DSA ports Ioana Ciornei
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

In order to have a common handling of PHYLINK for the slave and non-user
ports, the DSA core glue logic (between PHYLINK and the driver) must use
an API that does not rely on a struct net_device.

These will also be called by the CPU-port-handling code in a further
patch.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - none

 net/dsa/dsa_priv.h |  17 +++++++++
 net/dsa/port.c     | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/dsa/slave.c    |  96 +-------------------------------------------------
 3 files changed, 118 insertions(+), 95 deletions(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 8f1222324646..418490bda3a4 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -163,6 +163,23 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 int dsa_port_vid_del(struct dsa_port *dp, u16 vid);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
+void dsa_port_phylink_validate(struct phylink_config *config,
+			       unsigned long *supported,
+			       struct phylink_link_state *state);
+int dsa_port_phylink_mac_link_state(struct phylink_config *config,
+				    struct phylink_link_state *state);
+void dsa_port_phylink_mac_config(struct phylink_config *config,
+				 unsigned int mode,
+				 const struct phylink_link_state *state);
+void dsa_port_phylink_mac_an_restart(struct phylink_config *config);
+void dsa_port_phylink_mac_link_down(struct phylink_config *config,
+				    unsigned int mode,
+				    phy_interface_t interface);
+void dsa_port_phylink_mac_link_up(struct phylink_config *config,
+				  unsigned int mode,
+				  phy_interface_t interface,
+				  struct phy_device *phydev);
+extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
 
 /* slave.c */
 extern const struct dsa_device_ops notag_netdev_ops;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index ed8ba9daa3ba..0051f5006248 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -422,6 +422,106 @@ static struct phy_device *dsa_port_get_phy_device(struct dsa_port *dp)
 	return phydev;
 }
 
+void dsa_port_phylink_validate(struct phylink_config *config,
+			       unsigned long *supported,
+			       struct phylink_link_state *state)
+{
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->phylink_validate)
+		return;
+
+	ds->ops->phylink_validate(ds, dp->index, supported, state);
+}
+EXPORT_SYMBOL_GPL(dsa_port_phylink_validate);
+
+int dsa_port_phylink_mac_link_state(struct phylink_config *config,
+				    struct phylink_link_state *state)
+{
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+	struct dsa_switch *ds = dp->ds;
+
+	/* Only called for SGMII and 802.3z */
+	if (!ds->ops->phylink_mac_link_state)
+		return -EOPNOTSUPP;
+
+	return ds->ops->phylink_mac_link_state(ds, dp->index, state);
+}
+EXPORT_SYMBOL_GPL(dsa_port_phylink_mac_link_state);
+
+void dsa_port_phylink_mac_config(struct phylink_config *config,
+				 unsigned int mode,
+				 const struct phylink_link_state *state)
+{
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->phylink_mac_config)
+		return;
+
+	ds->ops->phylink_mac_config(ds, dp->index, mode, state);
+}
+EXPORT_SYMBOL_GPL(dsa_port_phylink_mac_config);
+
+void dsa_port_phylink_mac_an_restart(struct phylink_config *config)
+{
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->phylink_mac_an_restart)
+		return;
+
+	ds->ops->phylink_mac_an_restart(ds, dp->index);
+}
+EXPORT_SYMBOL_GPL(dsa_port_phylink_mac_an_restart);
+
+void dsa_port_phylink_mac_link_down(struct phylink_config *config,
+				    unsigned int mode,
+				    phy_interface_t interface)
+{
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+	struct net_device *dev = dp->slave;
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->phylink_mac_link_down) {
+		if (ds->ops->adjust_link && dev->phydev)
+			ds->ops->adjust_link(ds, dp->index, dev->phydev);
+		return;
+	}
+
+	ds->ops->phylink_mac_link_down(ds, dp->index, mode, interface);
+}
+EXPORT_SYMBOL_GPL(dsa_port_phylink_mac_link_down);
+
+void dsa_port_phylink_mac_link_up(struct phylink_config *config,
+				  unsigned int mode,
+				  phy_interface_t interface,
+				  struct phy_device *phydev)
+{
+	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
+	struct net_device *dev = dp->slave;
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->phylink_mac_link_up) {
+		if (ds->ops->adjust_link && dev->phydev)
+			ds->ops->adjust_link(ds, dp->index, dev->phydev);
+		return;
+	}
+
+	ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev);
+}
+EXPORT_SYMBOL_GPL(dsa_port_phylink_mac_link_up);
+
+const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
+	.validate = dsa_port_phylink_validate,
+	.mac_link_state = dsa_port_phylink_mac_link_state,
+	.mac_config = dsa_port_phylink_mac_config,
+	.mac_an_restart = dsa_port_phylink_mac_an_restart,
+	.mac_link_down = dsa_port_phylink_mac_link_down,
+	.mac_link_up = dsa_port_phylink_mac_link_up,
+};
+
 static int dsa_port_setup_phy_of(struct dsa_port *dp, bool enable)
 {
 	struct dsa_switch *ds = dp->ds;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 48e017637d4f..1e2ae9d59b88 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1164,100 +1164,6 @@ static struct devlink_port *dsa_slave_get_devlink_port(struct net_device *dev)
 	.name	= "dsa",
 };
 
-static void dsa_slave_phylink_validate(struct phylink_config *config,
-				       unsigned long *supported,
-				       struct phylink_link_state *state)
-{
-	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct dsa_switch *ds = dp->ds;
-
-	if (!ds->ops->phylink_validate)
-		return;
-
-	ds->ops->phylink_validate(ds, dp->index, supported, state);
-}
-
-static int dsa_slave_phylink_mac_link_state(struct phylink_config *config,
-					    struct phylink_link_state *state)
-{
-	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct dsa_switch *ds = dp->ds;
-
-	/* Only called for SGMII and 802.3z */
-	if (!ds->ops->phylink_mac_link_state)
-		return -EOPNOTSUPP;
-
-	return ds->ops->phylink_mac_link_state(ds, dp->index, state);
-}
-
-static void dsa_slave_phylink_mac_config(struct phylink_config *config,
-					 unsigned int mode,
-					 const struct phylink_link_state *state)
-{
-	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct dsa_switch *ds = dp->ds;
-
-	if (!ds->ops->phylink_mac_config)
-		return;
-
-	ds->ops->phylink_mac_config(ds, dp->index, mode, state);
-}
-
-static void dsa_slave_phylink_mac_an_restart(struct phylink_config *config)
-{
-	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct dsa_switch *ds = dp->ds;
-
-	if (!ds->ops->phylink_mac_an_restart)
-		return;
-
-	ds->ops->phylink_mac_an_restart(ds, dp->index);
-}
-
-static void dsa_slave_phylink_mac_link_down(struct phylink_config *config,
-					    unsigned int mode,
-					    phy_interface_t interface)
-{
-	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct net_device *dev = dp->slave;
-	struct dsa_switch *ds = dp->ds;
-
-	if (!ds->ops->phylink_mac_link_down) {
-		if (ds->ops->adjust_link && dev->phydev)
-			ds->ops->adjust_link(ds, dp->index, dev->phydev);
-		return;
-	}
-
-	ds->ops->phylink_mac_link_down(ds, dp->index, mode, interface);
-}
-
-static void dsa_slave_phylink_mac_link_up(struct phylink_config *config,
-					  unsigned int mode,
-					  phy_interface_t interface,
-					  struct phy_device *phydev)
-{
-	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct net_device *dev = dp->slave;
-	struct dsa_switch *ds = dp->ds;
-
-	if (!ds->ops->phylink_mac_link_up) {
-		if (ds->ops->adjust_link && dev->phydev)
-			ds->ops->adjust_link(ds, dp->index, dev->phydev);
-		return;
-	}
-
-	ds->ops->phylink_mac_link_up(ds, dp->index, mode, interface, phydev);
-}
-
-static const struct phylink_mac_ops dsa_slave_phylink_mac_ops = {
-	.validate = dsa_slave_phylink_validate,
-	.mac_link_state = dsa_slave_phylink_mac_link_state,
-	.mac_config = dsa_slave_phylink_mac_config,
-	.mac_an_restart = dsa_slave_phylink_mac_an_restart,
-	.mac_link_down = dsa_slave_phylink_mac_link_down,
-	.mac_link_up = dsa_slave_phylink_mac_link_up,
-};
-
 void dsa_port_phylink_mac_change(struct dsa_switch *ds, int port, bool up)
 {
 	const struct dsa_port *dp = dsa_to_port(ds, port);
@@ -1309,7 +1215,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
 	dp->pl_config.type = PHYLINK_NETDEV;
 
 	dp->pl = phylink_create(&dp->pl_config, of_fwnode_handle(port_dn), mode,
-				&dsa_slave_phylink_mac_ops);
+				&dsa_port_phylink_mac_ops);
 	if (IS_ERR(dp->pl)) {
 		netdev_err(slave_dev,
 			   "error creating PHYLINK: %ld\n", PTR_ERR(dp->pl));
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 10/11] net: dsa: Use PHYLINK for the CPU/DSA ports
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (8 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 09/11] net: dsa: Move the phylink driver calls into port.c Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-28 17:38 ` [PATCH v2 net-next 11/11] net: dsa: sja1105: Fix broken fixed-link interfaces on user ports Ioana Ciornei
  2019-05-30  4:49 ` [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device David Miller
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

For DSA switches that do not have an .adjust_link callback, aka those
who transitioned totally to the PHYLINK-compliant API, use PHYLINK to
drive the CPU/DSA ports.

The PHYLIB usage and .adjust_link are kept but deprecated, and users are
asked to transition from it.  The reason why we can't do anything for
them is because PHYLINK does not wrap the fixed-link state behind a
phydev object, so we cannot wrap .phylink_mac_config into .adjust_link
unless we fabricate a phy_device structure.

For these ports, the newly introduced PHYLINK_DEV operation type is
used and the dsa_switch device structure is passed to PHYLINK for
printing purposes.  The handling of the PHYLINK_NETDEV and PHYLINK_DEV
PHYLINK instances is common from the perspective of the driver.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - make dsa_port_phylink_register() function static

 net/dsa/port.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index 0051f5006248..d74bc9df1359 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -481,12 +481,15 @@ void dsa_port_phylink_mac_link_down(struct phylink_config *config,
 				    phy_interface_t interface)
 {
 	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct net_device *dev = dp->slave;
+	struct phy_device *phydev = NULL;
 	struct dsa_switch *ds = dp->ds;
 
+	if (dsa_is_user_port(ds, dp->index))
+		phydev = dp->slave->phydev;
+
 	if (!ds->ops->phylink_mac_link_down) {
-		if (ds->ops->adjust_link && dev->phydev)
-			ds->ops->adjust_link(ds, dp->index, dev->phydev);
+		if (ds->ops->adjust_link && phydev)
+			ds->ops->adjust_link(ds, dp->index, phydev);
 		return;
 	}
 
@@ -500,12 +503,11 @@ void dsa_port_phylink_mac_link_up(struct phylink_config *config,
 				  struct phy_device *phydev)
 {
 	struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
-	struct net_device *dev = dp->slave;
 	struct dsa_switch *ds = dp->ds;
 
 	if (!ds->ops->phylink_mac_link_up) {
-		if (ds->ops->adjust_link && dev->phydev)
-			ds->ops->adjust_link(ds, dp->index, dev->phydev);
+		if (ds->ops->adjust_link && phydev)
+			ds->ops->adjust_link(ds, dp->index, phydev);
 		return;
 	}
 
@@ -599,8 +601,53 @@ static int dsa_port_fixed_link_register_of(struct dsa_port *dp)
 	return 0;
 }
 
+static int dsa_port_phylink_register(struct dsa_port *dp)
+{
+	struct dsa_switch *ds = dp->ds;
+	struct device_node *port_dn = dp->dn;
+	int mode, err;
+
+	mode = of_get_phy_mode(port_dn);
+	if (mode < 0)
+		mode = PHY_INTERFACE_MODE_NA;
+
+	dp->pl_config.dev = ds->dev;
+	dp->pl_config.type = PHYLINK_DEV;
+
+	dp->pl = phylink_create(&dp->pl_config, of_fwnode_handle(port_dn),
+				mode, &dsa_port_phylink_mac_ops);
+	if (IS_ERR(dp->pl)) {
+		pr_err("error creating PHYLINK: %ld\n", PTR_ERR(dp->pl));
+		return PTR_ERR(dp->pl);
+	}
+
+	err = phylink_of_phy_connect(dp->pl, port_dn, 0);
+	if (err) {
+		pr_err("could not attach to PHY: %d\n", err);
+		goto err_phy_connect;
+	}
+
+	rtnl_lock();
+	phylink_start(dp->pl);
+	rtnl_unlock();
+
+	return 0;
+
+err_phy_connect:
+	phylink_destroy(dp->pl);
+	return err;
+}
+
 int dsa_port_link_register_of(struct dsa_port *dp)
 {
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->adjust_link)
+		return dsa_port_phylink_register(dp);
+
+	dev_warn(ds->dev,
+		 "Using legacy PHYLIB callbacks. Please migrate to PHYLINK!\n");
+
 	if (of_phy_is_fixed_link(dp->dn))
 		return dsa_port_fixed_link_register_of(dp);
 	else
@@ -609,6 +656,16 @@ int dsa_port_link_register_of(struct dsa_port *dp)
 
 void dsa_port_link_unregister_of(struct dsa_port *dp)
 {
+	struct dsa_switch *ds = dp->ds;
+
+	if (!ds->ops->adjust_link) {
+		rtnl_lock();
+		phylink_disconnect_phy(dp->pl);
+		rtnl_unlock();
+		phylink_destroy(dp->pl);
+		return;
+	}
+
 	if (of_phy_is_fixed_link(dp->dn))
 		of_phy_deregister_fixed_link(dp->dn);
 	else
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 net-next 11/11] net: dsa: sja1105: Fix broken fixed-link interfaces on user ports
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (9 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 10/11] net: dsa: Use PHYLINK for the CPU/DSA ports Ioana Ciornei
@ 2019-05-28 17:38 ` Ioana Ciornei
  2019-05-30  4:49 ` [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device David Miller
  11 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-28 17:38 UTC (permalink / raw)
  To: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev, Ioana Ciornei

From: Vladimir Oltean <olteanv@gmail.com>

PHYLIB and PHYLINK handle fixed-link interfaces differently. PHYLIB
wraps them in a software PHY ("pseudo fixed link") phydev construct such
that .adjust_link driver callbacks see an unified API. Whereas PHYLINK
simply creates a phylink_link_state structure and passes it to
.mac_config.

At the time the driver was introduced, DSA was using PHYLIB for the
CPU/cascade ports (the ones with no net devices) and PHYLINK for
everything else.

As explained below:

commit aab9c4067d2389d0adfc9c53806437df7b0fe3d5
Author: Florian Fainelli <f.fainelli@gmail.com>
Date:   Thu May 10 13:17:36 2018 -0700

  net: dsa: Plug in PHYLINK support

  Drivers that utilize fixed links for user-facing ports (e.g: bcm_sf2)
  will need to implement phylink_mac_ops from now on to preserve
  functionality, since PHYLINK *does not* create a phy_device instance
  for fixed links.

In the above patch, DSA guards the .phylink_mac_config callback against
a NULL phydev pointer.  Therefore, .adjust_link is not called in case of
a fixed-link user port.

This patch fixes the situation by converting the driver from using
.adjust_link to .phylink_mac_config.  This can be done now in a unified
fashion for both slave and CPU/cascade ports because DSA now uses
PHYLINK for all ports.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
 - none

 drivers/net/dsa/sja1105/sja1105_main.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 0663b78a2f6c..cfdefd9f1905 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -734,15 +734,16 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
 	return sja1105_clocking_setup_port(priv, port);
 }
 
-static void sja1105_adjust_link(struct dsa_switch *ds, int port,
-				struct phy_device *phydev)
+static void sja1105_mac_config(struct dsa_switch *ds, int port,
+			       unsigned int link_an_mode,
+			       const struct phylink_link_state *state)
 {
 	struct sja1105_private *priv = ds->priv;
 
-	if (!phydev->link)
+	if (!state->link)
 		sja1105_adjust_port_config(priv, port, 0, false);
 	else
-		sja1105_adjust_port_config(priv, port, phydev->speed, true);
+		sja1105_adjust_port_config(priv, port, state->speed, true);
 }
 
 static void sja1105_phylink_validate(struct dsa_switch *ds, int port,
@@ -1515,9 +1516,9 @@ static int sja1105_set_ageing_time(struct dsa_switch *ds,
 static const struct dsa_switch_ops sja1105_switch_ops = {
 	.get_tag_protocol	= sja1105_get_tag_protocol,
 	.setup			= sja1105_setup,
-	.adjust_link		= sja1105_adjust_link,
 	.set_ageing_time	= sja1105_set_ageing_time,
 	.phylink_validate	= sja1105_phylink_validate,
+	.phylink_mac_config	= sja1105_mac_config,
 	.get_strings		= sja1105_get_strings,
 	.get_ethtool_stats	= sja1105_get_ethtool_stats,
 	.get_sset_count		= sja1105_get_sset_count,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 net-next 03/11] net: phy: Check against net_device being NULL
  2019-05-28 17:38 ` [PATCH v2 net-next 03/11] net: phy: Check against net_device being NULL Ioana Ciornei
@ 2019-05-28 17:57   ` Florian Fainelli
  0 siblings, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2019-05-28 17:57 UTC (permalink / raw)
  To: ioana.ciornei, linux, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, davem, vivien.didelot
  Cc: netdev

On 5/28/19 10:38 AM, Ioana Ciornei wrote:
> In general, we don't want MAC drivers calling phy_attach_direct with the
> net_device being NULL. Add checks against this in all the functions
> calling it: phy_attach() and phy_connect_direct().
> 
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 net-next 06/11] net: phylink: Add struct phylink_config to PHYLINK API
  2019-05-28 17:38 ` [PATCH v2 net-next 06/11] net: phylink: Add struct phylink_config to PHYLINK API Ioana Ciornei
@ 2019-05-29  7:28   ` Maxime Chevallier
  2019-05-29 16:18     ` Ioana Ciornei
  0 siblings, 1 reply; 16+ messages in thread
From: Maxime Chevallier @ 2019-05-29  7:28 UTC (permalink / raw)
  To: Ioana Ciornei
  Cc: linux, f.fainelli, andrew, hkallweit1, olteanv, thomas.petazzoni,
	davem, vivien.didelot, netdev

Hello Ioana,

On Tue, 28 May 2019 20:38:12 +0300
Ioana Ciornei <ioana.ciornei@nxp.com> wrote:

>The phylink_config structure will encapsulate a pointer to a struct
>device and the operation type requested for this instance of PHYLINK.
>This patch does not make any functional changes, it just transitions the
>PHYLINK internals and all its users to the new API.
>
>A pointer to a phylink_config structure will be passed to
>phylink_create() instead of the net_device directly. Also, the same
>phylink_config pointer will be passed back to all phylink_mac_ops
>callbacks instead of the net_device. Using this mechanism, a PHYLINK
>user can get the original net_device using a structure such as
>'to_net_dev(config->dev)' or directly the structure containing the
>phylink_config using a container_of call.

I see that you mixed both to_net_dev and container_of uses in mvpp2, is
there a reason for that ?

Other than that, for the mvpp2 part,

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Thanks,

Maxime

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v2 net-next 06/11] net: phylink: Add struct phylink_config to PHYLINK API
  2019-05-29  7:28   ` Maxime Chevallier
@ 2019-05-29 16:18     ` Ioana Ciornei
  0 siblings, 0 replies; 16+ messages in thread
From: Ioana Ciornei @ 2019-05-29 16:18 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: linux, f.fainelli, andrew, hkallweit1, olteanv, thomas.petazzoni,
	davem, vivien.didelot, netdev


> Subject: Re: [PATCH v2 net-next 06/11] net: phylink: Add struct phylink_config to
> PHYLINK API
> 
> Hello Ioana,
> 
> On Tue, 28 May 2019 20:38:12 +0300
> Ioana Ciornei <ioana.ciornei@nxp.com> wrote:
> 
> >The phylink_config structure will encapsulate a pointer to a struct
> >device and the operation type requested for this instance of PHYLINK.
> >This patch does not make any functional changes, it just transitions
> >the PHYLINK internals and all its users to the new API.
> >
> >A pointer to a phylink_config structure will be passed to
> >phylink_create() instead of the net_device directly. Also, the same
> >phylink_config pointer will be passed back to all phylink_mac_ops
> >callbacks instead of the net_device. Using this mechanism, a PHYLINK
> >user can get the original net_device using a structure such as
> >'to_net_dev(config->dev)' or directly the structure containing the
> >phylink_config using a container_of call.
> 
> I see that you mixed both to_net_dev and container_of uses in mvpp2, is there a
> reason for that ?

When only the mvpp2_port was needed I chose to use a container_of directly rather than in 2 steps: to_net_dev and then netdev_priv.
On the other hand, when both the netdev and the mvpp2_port was used, adding just a to_net_dev was the least disruptive.

> 
> Other than that, for the mvpp2 part,
> 
> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> 

Thanks a lot for testing.

--
Ioana

> Thanks,
> 
> Maxime

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device
  2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
                   ` (10 preceding siblings ...)
  2019-05-28 17:38 ` [PATCH v2 net-next 11/11] net: dsa: sja1105: Fix broken fixed-link interfaces on user ports Ioana Ciornei
@ 2019-05-30  4:49 ` David Miller
  11 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2019-05-30  4:49 UTC (permalink / raw)
  To: ioana.ciornei
  Cc: linux, f.fainelli, andrew, hkallweit1, maxime.chevallier,
	olteanv, thomas.petazzoni, vivien.didelot, netdev

From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: Tue, 28 May 2019 20:38:06 +0300

> Following two separate discussion threads in:
>   https://www.spinics.net/lists/netdev/msg569087.html
> and:
>   https://www.spinics.net/lists/netdev/msg570450.html
> 
> Previous RFC patch set: https://www.spinics.net/lists/netdev/msg571995.html
> 
> PHYLINK was reworked in order to accept multiple operation types,
> PHYLINK_NETDEV and PHYLINK_DEV, passed through a phylink_config
> structure alongside the corresponding struct device.
> 
> One of the main concerns expressed in the RFC was that using notifiers
> to signal the corresponding phylink_mac_ops would break PHYLINK's API
> unity and that it would become harder to grep for its users.
> Using the current approach, we maintain a common API for all users.
> Also, printing useful information in PHYLINK, when decoupled from a
> net_device, is achieved using dev_err&co on the struct device received
> (in DSA's case is the device corresponding to the dsa_switch).
> 
> PHYLIB (which PHYLINK uses) was reworked to the extent that it does not
> crash when connecting to a PHY and the net_device pointer is NULL.
> 
> Lastly, DSA has been reworked in its way that it handles PHYs for ports
> that lack a net_device (CPU and DSA ports).  For these, it was
> previously using PHYLIB and is now using the PHYLINK_DEV operation type.
> Previously, a driver that wanted to support PHY operations on CPU/DSA
> ports has to implement .adjust_link(). This patch set not only gives
> drivers the options to use PHYLINK uniformly but also urges them to
> convert to it. For compatibility, the old code is kept but it will be
> removed once all drivers switch over.
 ...

Series applied, thank you.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-05-30  4:49 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28 17:38 [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 01/11] net: phy: Add phy_sysfs_create_links helper function Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 02/11] net: phy: Guard against the presence of a netdev Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 03/11] net: phy: Check against net_device being NULL Ioana Ciornei
2019-05-28 17:57   ` Florian Fainelli
2019-05-28 17:38 ` [PATCH v2 net-next 04/11] net: phy: Add phy_standalone sysfs entry Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 05/11] net: phylink: Add phylink_mac_link_{up,down} wrapper functions Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 06/11] net: phylink: Add struct phylink_config to PHYLINK API Ioana Ciornei
2019-05-29  7:28   ` Maxime Chevallier
2019-05-29 16:18     ` Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 07/11] net: phylink: Add PHYLINK_DEV operation type Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 08/11] net: phylink: Add phylink_{printk,err,warn,info,dbg} macros Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 09/11] net: dsa: Move the phylink driver calls into port.c Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 10/11] net: dsa: Use PHYLINK for the CPU/DSA ports Ioana Ciornei
2019-05-28 17:38 ` [PATCH v2 net-next 11/11] net: dsa: sja1105: Fix broken fixed-link interfaces on user ports Ioana Ciornei
2019-05-30  4:49 ` [PATCH v2 net-next 00/11] Decoupling PHYLINK from struct net_device David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.