Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Sean Anderson <sean.anderson@seco.com>
To:     netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Sean Anderson <sean.anderson@seco.com>,
        Saravana Kannan <saravanak@google.com>
Subject: [RFC net-next PATCH 05/16] net: phylink: Automatically attach PCS devices
Date:   Mon,  4 Oct 2021 15:15:16 -0400
Message-Id: <20211004191527.1610759-6-sean.anderson@seco.com>
In-Reply-To: <20211004191527.1610759-1-sean.anderson@seco.com>
References: <20211004191527.1610759-1-sean.anderson@seco.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?8CJX875Y8Z9KZpNE9SaMSBkbrMrOVhZIyiPjCHeNn4BjDF7PJZ3M82rBeROp?=
 =?us-ascii?Q?fvKeyTi8/r1I5sXl5iNvdbJYNgvlLLdHQYXHdxX+RbozhJvdd333e5NzMFRD?=
 =?us-ascii?Q?P668z4uFeN6ezTwPB8Cv15bp92sXwftfUf6wWVQTxiO8OkXn+tGH2xh/xHh9?=
 =?us-ascii?Q?wXqywRS6wGRyvezQfwkOnsMMtUPEhjqprcdfJ3YX97nPVCZmRyJRPrhTDech?=
 =?us-ascii?Q?WFL0cuIrfyL0DagznDnZd/7t+3hDwKhblOSudqQQpc2uHG+pEGSdDclVncOv?=
 =?us-ascii?Q?7r0WZQOnDysMoGFpsiJx4vPiSS5COpfDnJqaRCXvnbZCMSm0iw67TTU5u66H?=
 =?us-ascii?Q?0My3RgZ9eJgAmn7SiMtGZcfbHvyah2HSfMBlmgQEIF4m4fqR07vYuVJDVyVC?=
 =?us-ascii?Q?mAZxevyr0CfjFxenn3v5LdVXebTx7Q2bbGbge3b8l3Rhl8or2pLsKn3BKEIa?=
 =?us-ascii?Q?55HKDVQBTWR1f2kkg3wp7rPxkGcBzFYxM0jRoH6DKnDbVfuT9E+ObQvtABqt?=
 =?us-ascii?Q?lRbs6Q42rCh6ZhSQe98Yt1oJptVuQap5rOVL9m/f//3jE6q1cM+wQUmr2nJd?=
 =?us-ascii?Q?ytYjiea8r3wAIbzPK8Xaaatr50jX+A8pKxxVfsC1TpiuhVCqLo+vjGcaqPBu?=
 =?us-ascii?Q?z5niHIbvOVfeeP+k5Pg3JehNLXglUKuzubnEGiuathsn7ZP/Kbj4RArTFMS7?=
 =?us-ascii?Q?nk+EH9Agfp9fz/QclFuXUy/e9Ej7ysWSYUy1Nb7Vr6qrWdLWIwUGV2TvxUpV?=
 =?us-ascii?Q?aj+QswJLEBjLbXP41nhQHFUf6tkEVP/DAymfLrFnCF5a8wpryLmK2SLshFZC?=
 =?us-ascii?Q?uaNQFJ62Ox2SiW7SJEq45oMa9HpgCTqd1WPlCQi0CJMA9ARxRJJH3bDOMrdW?=
 =?us-ascii?Q?pDJp/hNbbLBudShlNElal9gCe2XYQS8+CgSkTDm4gqhsLMsSiUzxNTSmY9p9?=
 =?us-ascii?Q?cyRXRKesdcz6dMePBUT4B9Q8I41y8Z8BxQ90/8k/iWUrinjQHdoVUTaTrT0/?=
 =?us-ascii?Q?+K9uGmiJN4v7xN5Xj6l9ygiKD+9jmqAwH34ksI+rAGFMF1hglVyXFWY4xhhY?=
 =?us-ascii?Q?/UddjHBnt+Mna3oH+CVfwfynhS+es3G9fz49OmIn6Kzs+fFLSYe4c6uz5qCS?=
 =?us-ascii?Q?qW71y3TzgdwQl7AhKVSPjJ5FMH7c1gidWIfYzDOxPE9uyEHGDm9OTenBTPX1?=
 =?us-ascii?Q?ni/DY4HCF7o6/+OvoqAp7/hROkZAVeRPtPLRXobejOct2ZexffCMmh6xdyMq?=
 =?us-ascii?Q?iLLkh5/o1cpQ9IJgppJdS4p7ZuMpKuenkgXdk9yhqn1W55Glv0XkwQ8BXZvC?=
 =?us-ascii?Q?zYYLBPmDfoz3yJIaZuNflqUJ?=
X-OriginatorOrg: seco.com
X-MS-Exchange-CrossTenant-Network-Message-Id: efc22294-5544-42ab-2001-08d9876b634a
X-MS-Exchange-CrossTenant-AuthSource: DB7PR03MB4523.eurprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Oct 2021 19:15:54.7831
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bebe97c3-6438-442e-ade3-ff17aa50e733
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NgyMAEOXYg89VNqdW1gjyHJuoapT1wj/4s25HatiR6Ynm/P1WL4OsgUo/UlaW7dsmdVf044DazclJ/mqssNc9A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR03MB7434
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

This adds support for automatically attaching PCS devices when creating
a phylink. To do this, drivers must first register with
phylink_register_pcs. After that, new phylinks will attach the PCS
device specified by the "pcs" property.

At the moment there is no support for specifying the interface used to
talk to the PCS. The MAC driver is expected to know how to talk to the
PCS. This is not a change, but it is perhaps an area for improvement.

I believe this is mostly correct with regard to registering/
unregistering. However I am not too familiar with the guts of Linux's
device subsystem. It is possible (likely, even) that the current system
is insufficient to prevent removing PCS devices which are still in-use.
I would really appreciate any feedback, or suggestions of subsystems to
use as reference. In particular: do I need to manually create device
links? Should I instead add an entry to of_supplier_bindings? Do I need
a call to try_module_get?

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 drivers/net/phy/phylink.c | 115 ++++++++++++++++++++++++++++++++++++--
 include/linux/phylink.h   |  11 +++-
 2 files changed, 120 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 6387c40c5592..046fdac3597d 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -795,6 +795,42 @@ static int phylink_register_sfp(struct phylink *pl,
 	return ret;
 }
 
+static LIST_HEAD(pcs_devices);
+static DEFINE_MUTEX(pcs_mutex);
+
+/**
+ * phylink_register_pcs() - register a new PCS
+ * @pcs: the PCS to register
+ *
+ * Registers a new PCS which can be automatically attached to a phylink.
+ *
+ * Return: 0 on success, or -errno on error
+ */
+int phylink_register_pcs(struct phylink_pcs *pcs)
+{
+	if (!pcs->dev || !pcs->ops)
+		return -EINVAL;
+
+	INIT_LIST_HEAD(&pcs->list);
+	mutex_lock(&pcs_mutex);
+	list_add(&pcs->list, &pcs_devices);
+	mutex_unlock(&pcs_mutex);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(phylink_register_pcs);
+
+/**
+ * phylink_unregister_pcs() - unregister a PCS
+ * @pcs: a PCS previously registered with phylink_register_pcs()
+ */
+void phylink_unregister_pcs(struct phylink_pcs *pcs)
+{
+	mutex_lock(&pcs_mutex);
+	list_del(&pcs->list);
+	mutex_unlock(&pcs_mutex);
+}
+EXPORT_SYMBOL_GPL(phylink_unregister_pcs);
+
 /**
  * phylink_set_pcs() - set the current PCS for phylink to use
  * @pl: a pointer to a &struct phylink returned from phylink_create()
@@ -808,14 +844,72 @@ static int phylink_register_sfp(struct phylink *pl,
  * callback if a PCS is present (denoting a newer setup) so removing a PCS
  * is not supported, and if a PCS is going to be used, it must be registered
  * by calling phylink_set_pcs() at the latest in the first mac_config() call.
+ *
+ * Context: may sleep.
+ * Return: 0 on success or -errno on failure.
  */
-void phylink_set_pcs(struct phylink *pl, struct phylink_pcs *pcs)
+int phylink_set_pcs(struct phylink *pl, struct phylink_pcs *pcs)
 {
+	if (pl->pcs && pl->pcs->dev)
+		device_link_remove(pl->dev, pl->pcs->dev);
+
+	if (pcs->dev) {
+		struct device_link *dl =
+			device_link_add(pl->dev, pcs->dev, 0);
+
+		if (IS_ERR(dl)) {
+			dev_err(pl->dev,
+				"failed to create device link to %s\n",
+				dev_name(pcs->dev));
+			return PTR_ERR(dl);
+		}
+	}
+
 	pl->pcs = pcs;
 	pl->pcs_ops = pcs->ops;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(phylink_set_pcs);
 
+static struct phylink_pcs *phylink_find_pcs(struct fwnode_handle *fwnode)
+{
+	struct phylink_pcs *pcs;
+
+	mutex_lock(&pcs_mutex);
+	list_for_each_entry(pcs, &pcs_devices, list) {
+		if (pcs->dev && pcs->dev->fwnode == fwnode) {
+			mutex_unlock(&pcs_mutex);
+			return pcs;
+		}
+	}
+	mutex_unlock(&pcs_mutex);
+
+	return NULL;
+}
+
+static int phylink_attach_pcs(struct phylink *pl, struct fwnode_handle *fwnode)
+{
+	int ret;
+	struct phylink_pcs *pcs;
+	struct fwnode_reference_args ref;
+
+	ret = fwnode_property_get_reference_args(fwnode, "pcs", NULL,
+						 0, 0, &ref);
+	if (ret == -ENOENT)
+		return 0;
+	else if (ret)
+		return ret;
+
+	pcs = phylink_find_pcs(ref.fwnode);
+	if (pcs)
+		ret = phylink_set_pcs(pl, pcs);
+	else
+		ret = -EPROBE_DEFER;
+
+	fwnode_handle_put(ref.fwnode);
+	return ret;
+}
+
 /**
  * phylink_create() - create a phylink instance
  * @config: a pointer to the target &struct phylink_config
@@ -893,12 +987,20 @@ struct phylink *phylink_create(struct phylink_config *config,
 	pl->cur_link_an_mode = pl->cfg_link_an_mode;
 
 	ret = phylink_register_sfp(pl, fwnode);
-	if (ret < 0) {
-		kfree(pl);
-		return ERR_PTR(ret);
-	}
+	if (ret < 0)
+		goto err_sfp;
+
+	ret = phylink_attach_pcs(pl, fwnode);
+	if (ret)
+		goto err_pcs;
 
 	return pl;
+
+err_pcs:
+	sfp_bus_del_upstream(pl->sfp_bus);
+err_sfp:
+	kfree(pl);
+	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(phylink_create);
 
@@ -913,6 +1015,9 @@ EXPORT_SYMBOL_GPL(phylink_create);
  */
 void phylink_destroy(struct phylink *pl)
 {
+	if (pl->pcs && pl->pcs->dev)
+		device_link_remove(pl->dev, pl->pcs->dev);
+
 	sfp_bus_del_upstream(pl->sfp_bus);
 	if (pl->link_gpio)
 		gpiod_put(pl->link_gpio);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 237291196ce2..d60756b36ad3 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -331,14 +331,20 @@ struct phylink_pcs_ops;
 
 /**
  * struct phylink_pcs - PHYLINK PCS instance
+ * @dev: the device associated with this PCS, or %NULL if the PCS doesn't have
+ *       a device of its own. Typically, @dev should only be %NULL for internal
+ *       PCS devices which do not need to be looked up via phandle.
  * @ops: a pointer to the &struct phylink_pcs_ops structure
+ * @list: internal list of PCS devices
  * @poll: poll the PCS for link changes
  *
  * This structure is designed to be embedded within the PCS private data,
  * and will be passed between phylink and the PCS.
  */
 struct phylink_pcs {
+	struct device *dev;
 	const struct phylink_pcs_ops *ops;
+	struct list_head list;
 	bool poll;
 };
 
@@ -433,10 +439,13 @@ void pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
 		 phy_interface_t interface, int speed, int duplex);
 #endif
 
+int phylink_register_pcs(struct phylink_pcs *pcs);
+void phylink_unregister_pcs(struct phylink_pcs *pcs);
+int phylink_set_pcs(struct phylink *pl, struct phylink_pcs *pcs);
+
 struct phylink *phylink_create(struct phylink_config *, struct fwnode_handle *,
 			       phy_interface_t iface,
 			       const struct phylink_mac_ops *mac_ops);
-void phylink_set_pcs(struct phylink *, struct phylink_pcs *pcs);
 void phylink_destroy(struct phylink *);
 
 int phylink_connect_phy(struct phylink *, struct phy_device *);
-- 
2.25.1