[PATCH net-next 11/14] net: pse-pd: Add support for PSE device index

Kory Maincent posted 14 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH net-next 11/14] net: pse-pd: Add support for PSE device index
Posted by Kory Maincent 1 year, 1 month ago
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

Add support for a PSE device index to report the PSE controller index to
the user through ethtool. This will be useful for future support of power
domains and port priority management.

Acked-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 drivers/net/pse-pd/pse_core.c | 24 +++++++++++++++++++-----
 include/linux/ethtool.h       |  2 ++
 include/linux/pse-pd/pse.h    |  2 ++
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 887a477197a6..830e8d567d4d 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -13,6 +13,7 @@
 
 static DEFINE_MUTEX(pse_list_mutex);
 static LIST_HEAD(pse_controller_list);
+static DEFINE_IDA(pse_ida);
 
 /**
  * struct pse_control - a PSE control
@@ -448,6 +449,10 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 
 	mutex_init(&pcdev->lock);
 	INIT_LIST_HEAD(&pcdev->pse_control_head);
+	ret = ida_alloc_max(&pse_ida, INT_MAX, GFP_KERNEL);
+	if (ret < 0)
+		return ret;
+	pcdev->id = ret;
 
 	if (!pcdev->nr_lines)
 		pcdev->nr_lines = 1;
@@ -461,12 +466,12 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 
 	ret = of_load_pse_pis(pcdev);
 	if (ret)
-		return ret;
+		goto free_pse_ida;
 
 	if (pcdev->ops->setup_pi_matrix) {
 		ret = pcdev->ops->setup_pi_matrix(pcdev);
 		if (ret)
-			return ret;
+			goto free_pse_ida;
 	}
 
 	/* Each regulator name len is pcdev dev name + 7 char +
@@ -483,15 +488,17 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 			continue;
 
 		reg_name = devm_kzalloc(pcdev->dev, reg_name_len, GFP_KERNEL);
-		if (!reg_name)
-			return -ENOMEM;
+		if (!reg_name) {
+			ret = -ENOMEM;
+			goto free_pse_ida;
+		}
 
 		snprintf(reg_name, reg_name_len, "pse-%s_pi%d",
 			 dev_name(pcdev->dev), i);
 
 		ret = devm_pse_pi_regulator_register(pcdev, reg_name, i);
 		if (ret)
-			return ret;
+			goto free_pse_ida;
 	}
 
 	mutex_lock(&pse_list_mutex);
@@ -499,6 +506,10 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 	mutex_unlock(&pse_list_mutex);
 
 	return 0;
+
+free_pse_ida:
+	ida_free(&pse_ida, pcdev->id);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(pse_controller_register);
 
@@ -509,6 +520,7 @@ EXPORT_SYMBOL_GPL(pse_controller_register);
 void pse_controller_unregister(struct pse_controller_dev *pcdev)
 {
 	pse_release_pis(pcdev);
+	ida_free(&pse_ida, pcdev->id);
 	mutex_lock(&pse_list_mutex);
 	list_del(&pcdev->list);
 	mutex_unlock(&pse_list_mutex);
@@ -771,6 +783,8 @@ int pse_ethtool_get_status(struct pse_control *psec,
 
 	pcdev = psec->pcdev;
 	ops = pcdev->ops;
+	status->pse_id = pcdev->id;
+
 	mutex_lock(&pcdev->lock);
 	ret = ops->pi_get_admin_state(pcdev, psec->id, &admin_state);
 	if (ret)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 2bdf7e72ee50..d5d13a3d4447 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -1327,6 +1327,7 @@ struct ethtool_c33_pse_pw_limit_range {
 /**
  * struct ethtool_pse_control_status - PSE control/channel status.
  *
+ * @pse_id: index number of the PSE.
  * @podl_admin_state: operational state of the PoDL PSE
  *	functions. IEEE 802.3-2018 30.15.1.1.2 aPoDLPSEAdminState
  * @podl_pw_status: power detection status of the PoDL PSE.
@@ -1348,6 +1349,7 @@ struct ethtool_c33_pse_pw_limit_range {
  *	ranges
  */
 struct ethtool_pse_control_status {
+	u32 pse_id;
 	enum ethtool_podl_pse_admin_state podl_admin_state;
 	enum ethtool_podl_pse_pw_d_status podl_pw_status;
 	enum ethtool_c33_pse_admin_state c33_admin_state;
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 09e62d8c3271..8780a4160d3c 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -172,6 +172,7 @@ struct pse_pi {
  * @types: types of the PSE controller
  * @pi: table of PSE PIs described in this controller device
  * @no_of_pse_pi: flag set if the pse_pis devicetree node is not used
+ * @id: Index of the PSE
  */
 struct pse_controller_dev {
 	const struct pse_controller_ops *ops;
@@ -185,6 +186,7 @@ struct pse_controller_dev {
 	enum ethtool_pse_types types;
 	struct pse_pi *pi;
 	bool no_of_pse_pi;
+	u32 id;
 };
 
 #if IS_ENABLED(CONFIG_PSE_CONTROLLER)

-- 
2.34.1
Re: [PATCH net-next 11/14] net: pse-pd: Add support for PSE device index
Posted by Jakub Kicinski 1 year, 1 month ago
On Sat, 04 Jan 2025 23:27:36 +0100 Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> 
> Add support for a PSE device index to report the PSE controller index to
> the user through ethtool. This will be useful for future support of power
> domains and port priority management.

Can you say more? How do the PSE controllers relate to netdevs?
ethtool is primarily driven by netdev / ifindex.
If you're starting to build your own object hierarchy you may be
better off with a separate genl family.
Re: [PATCH net-next 11/14] net: pse-pd: Add support for PSE device index
Posted by Oleksij Rempel 1 year, 1 month ago
On Tue, Jan 07, 2025 at 05:18:34PM -0800, Jakub Kicinski wrote:
> On Sat, 04 Jan 2025 23:27:36 +0100 Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> > 
> > Add support for a PSE device index to report the PSE controller index to
> > the user through ethtool. This will be useful for future support of power
> > domains and port priority management.
> 
> Can you say more? How do the PSE controllers relate to netdevs?
> ethtool is primarily driven by netdev / ifindex.
> If you're starting to build your own object hierarchy you may be
> better off with a separate genl family.
            
I hope this schema may help to explain the topology:

	                              +--- netdev / ifindex 0
	    +--- PSE power domain 0 --+--- netdev / ifindex 1
            |                         +--- netdev / ifindex 2
PSE ctrl 0 -+
            |                         +--- netdev / ifindex 3
            +--- PSE power domain 1 --+--- netdev / ifindex 4
	                              +--- netdev / ifindex 5

	                              +--- netdev / ifindex 6
	    +--- PSE power domain 2 --+--- netdev / ifindex 7
            |                         +--- netdev / ifindex 8
PSE ctrl 1 -+
            |                         +--- netdev / ifindex 9
            +--- PSE power domain 3 --+--- netdev / ifindex 10
	                              +--- netdev / ifindex 11

PSE device index is needed to find actually PSE controller related to
specific netdev / ifindex.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Re: [PATCH net-next 11/14] net: pse-pd: Add support for PSE device index
Posted by Jakub Kicinski 1 year, 1 month ago
On Wed, 8 Jan 2025 06:47:10 +0100 Oleksij Rempel wrote:
> On Tue, Jan 07, 2025 at 05:18:34PM -0800, Jakub Kicinski wrote:
> > On Sat, 04 Jan 2025 23:27:36 +0100 Kory Maincent wrote:  
> > > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> > > 
> > > Add support for a PSE device index to report the PSE controller index to
> > > the user through ethtool. This will be useful for future support of power
> > > domains and port priority management.  
> > 
> > Can you say more? How do the PSE controllers relate to netdevs?
> > ethtool is primarily driven by netdev / ifindex.
> > If you're starting to build your own object hierarchy you may be
> > better off with a separate genl family.  
>             
> I hope this schema may help to explain the topology:
> 
> 	                              +--- netdev / ifindex 0
> 	    +--- PSE power domain 0 --+--- netdev / ifindex 1
>             |                         +--- netdev / ifindex 2
> PSE ctrl 0 -+
>             |                         +--- netdev / ifindex 3
>             +--- PSE power domain 1 --+--- netdev / ifindex 4
> 	                              +--- netdev / ifindex 5
> 
> 	                              +--- netdev / ifindex 6
> 	    +--- PSE power domain 2 --+--- netdev / ifindex 7
>             |                         +--- netdev / ifindex 8
> PSE ctrl 1 -+
>             |                         +--- netdev / ifindex 9
>             +--- PSE power domain 3 --+--- netdev / ifindex 10
> 	                              +--- netdev / ifindex 11
> 
> PSE device index is needed to find actually PSE controller related to
> specific netdev / ifindex.

Makes sense. So how does it end up looking in terms of APIs 
and attributes? Will we need much more than power limits at
the domain and ctrl level?
Re: [PATCH net-next 11/14] net: pse-pd: Add support for PSE device index
Posted by Oleksij Rempel 1 year, 1 month ago
On Wed, Jan 08, 2025 at 09:42:01AM -0800, Jakub Kicinski wrote:
> On Wed, 8 Jan 2025 06:47:10 +0100 Oleksij Rempel wrote:
> > On Tue, Jan 07, 2025 at 05:18:34PM -0800, Jakub Kicinski wrote:
> > > On Sat, 04 Jan 2025 23:27:36 +0100 Kory Maincent wrote:  
> > > > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> > > > 
> > > > Add support for a PSE device index to report the PSE controller index to
> > > > the user through ethtool. This will be useful for future support of power
> > > > domains and port priority management.  
> > > 
> > > Can you say more? How do the PSE controllers relate to netdevs?
> > > ethtool is primarily driven by netdev / ifindex.
> > > If you're starting to build your own object hierarchy you may be
> > > better off with a separate genl family.  
> >             
> > I hope this schema may help to explain the topology:
> > 
> > 	                              +--- netdev / ifindex 0
> > 	    +--- PSE power domain 0 --+--- netdev / ifindex 1
> >             |                         +--- netdev / ifindex 2
> > PSE ctrl 0 -+
> >             |                         +--- netdev / ifindex 3
> >             +--- PSE power domain 1 --+--- netdev / ifindex 4
> > 	                              +--- netdev / ifindex 5
> > 
> > 	                              +--- netdev / ifindex 6
> > 	    +--- PSE power domain 2 --+--- netdev / ifindex 7
> >             |                         +--- netdev / ifindex 8
> > PSE ctrl 1 -+
> >             |                         +--- netdev / ifindex 9
> >             +--- PSE power domain 3 --+--- netdev / ifindex 10
> > 	                              +--- netdev / ifindex 11
> > 
> > PSE device index is needed to find actually PSE controller related to
> > specific netdev / ifindex.
> 
> Makes sense. So how does it end up looking in terms of APIs 
> and attributes? Will we need much more than power limits at
> the domain and ctrl level?

The PSE power domains are based on regulator framework. So, we will get
some diagnostic and may be control API on this side.

The PSE controller will need some configuration and diagnostic
interfaces. For example:
- not port specific configurations
- not port specific diagnostics

Every thing port related can be passed to the netdev

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |