[net-next,v2,3/8] net: pse-pd: Introduce PSE types enumeration

Message ID 20231201-feature_poe-v2-3-56d8cac607fa@bootlin.com
State New
Headers
Series net: Add support for Power over Ethernet (PoE) |

Commit Message

Köry Maincent Dec. 1, 2023, 5:10 p.m. UTC
  Introduce an enumeration to define PSE types (C33 or PoDL),
utilizing a bitfield for potential future support of both types.
Include 'pse_get_types' helper for external access to PSE type info.

Sponsored-by: Dent Project <dentproject@linuxfoundation.org>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Changes in v2:
- Rename PSE_POE to PSE_C33 to have naming consistency.
- Use "static inline" instead of simple static in the header
---
 drivers/net/pse-pd/pse_core.c      |  9 +++++++++
 drivers/net/pse-pd/pse_regulator.c |  1 +
 include/linux/pse-pd/pse.h         | 22 ++++++++++++++++++++++
 3 files changed, 32 insertions(+)
  

Comments

Andrew Lunn Dec. 3, 2023, 6:31 p.m. UTC | #1
> +u32 pse_get_types(struct pse_control *psec)
> +{
> +	if (!psec->pcdev)

Is that possible?

   Andrew
  
Andrew Lunn Dec. 3, 2023, 6:38 p.m. UTC | #2
> +/**
> + * enum - Types of PSE controller.
> + *
> + * @PSE_UNKNOWN: Type of PSE controller is unknown
> + * @PSE_PODL: PSE controller which support PoDL
> + * @PSE_C33: PSE controller which support Clause 33 (PoE)
> + */
> +enum {
> +	PSE_UNKNOWN = BIT(0),
> +	PSE_PODL = BIT(1),
> +	PSE_C33 = BIT(2),
> +};

Maybe this should be in the netlink API?

I think you can imply it by looking at what properties are in the
netlink reply, but having it explicitly is probably better.
ethtool(1) can default to PSE_PODL if the property is missing for an
older kernel.

      Andrew
  
Oleksij Rempel Dec. 4, 2023, 12:43 p.m. UTC | #3
On Sun, Dec 03, 2023 at 07:31:01PM +0100, Andrew Lunn wrote:
> > +u32 pse_get_types(struct pse_control *psec)
> > +{
> > +	if (!psec->pcdev)
> 
> Is that possible?

pcdev is automatically linked within pse_control_get_internal()
  
Oleksij Rempel Dec. 4, 2023, 12:51 p.m. UTC | #4
On Fri, Dec 01, 2023 at 06:10:25PM +0100, Kory Maincent wrote:
> +u32 pse_get_types(struct pse_control *psec);

I would add here some helper function. Something like:
pse_has_podl() or pse_has_c33()
  
Köry Maincent Dec. 4, 2023, 1:55 p.m. UTC | #5
Thanks for your review!

On Mon, 4 Dec 2023 13:51:31 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> On Fri, Dec 01, 2023 at 06:10:25PM +0100, Kory Maincent wrote:
> > +u32 pse_get_types(struct pse_control *psec);  
> 
> I would add here some helper function. Something like:
> pse_has_podl() or pse_has_c33()

Instead of pse_get_types function? It is indeed maybe cleaner to use such
helper functions instead.

Regards,
  
Köry Maincent Dec. 4, 2023, 2 p.m. UTC | #6
Thanks for your review:

On Sun, 3 Dec 2023 19:38:04 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> > +/**
> > + * enum - Types of PSE controller.
> > + *
> > + * @PSE_UNKNOWN: Type of PSE controller is unknown
> > + * @PSE_PODL: PSE controller which support PoDL
> > + * @PSE_C33: PSE controller which support Clause 33 (PoE)
> > + */
> > +enum {
> > +	PSE_UNKNOWN = BIT(0),
> > +	PSE_PODL = BIT(1),
> > +	PSE_C33 = BIT(2),
> > +};  
> 
> Maybe this should be in the netlink API?
> 
> I think you can imply it by looking at what properties are in the
> netlink reply, but having it explicitly is probably better.
> ethtool(1) can default to PSE_PODL if the property is missing for an
> older kernel.

Ok, I will add it to netlink API in v3 then.

Regards,
  

Patch

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 146b81f08a89..2959c94f7798 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -312,3 +312,12 @@  int pse_ethtool_set_config(struct pse_control *psec,
 	return err;
 }
 EXPORT_SYMBOL_GPL(pse_ethtool_set_config);
+
+u32 pse_get_types(struct pse_control *psec)
+{
+	if (!psec->pcdev)
+		return PSE_UNKNOWN;
+	else
+		return psec->pcdev->types;
+}
+EXPORT_SYMBOL_GPL(pse_get_types);
diff --git a/drivers/net/pse-pd/pse_regulator.c b/drivers/net/pse-pd/pse_regulator.c
index 1dedf4de296e..e34ab8526067 100644
--- a/drivers/net/pse-pd/pse_regulator.c
+++ b/drivers/net/pse-pd/pse_regulator.c
@@ -116,6 +116,7 @@  pse_reg_probe(struct platform_device *pdev)
 	priv->pcdev.owner = THIS_MODULE;
 	priv->pcdev.ops = &pse_reg_ops;
 	priv->pcdev.dev = dev;
+	priv->pcdev.types = PSE_PODL;
 	ret = devm_pse_controller_register(dev, &priv->pcdev);
 	if (ret) {
 		dev_err(dev, "failed to register PSE controller (%pe)\n",
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index be4e5754eb24..4a0ae3097bb2 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -44,6 +44,19 @@  struct pse_control_status {
 	enum ethtool_c33_pse_pw_d_status c33_pw_status;
 };
 
+/**
+ * enum - Types of PSE controller.
+ *
+ * @PSE_UNKNOWN: Type of PSE controller is unknown
+ * @PSE_PODL: PSE controller which support PoDL
+ * @PSE_C33: PSE controller which support Clause 33 (PoE)
+ */
+enum {
+	PSE_UNKNOWN = BIT(0),
+	PSE_PODL = BIT(1),
+	PSE_C33 = BIT(2),
+};
+
 /**
  * struct pse_controller_ops - PSE controller driver callbacks
  *
@@ -77,6 +90,7 @@  struct pse_control;
  *            device tree to id as given to the PSE control ops
  * @nr_lines: number of PSE controls in this controller device
  * @lock: Mutex for serialization access to the PSE controller
+ * @types: types of the PSE controller
  */
 struct pse_controller_dev {
 	const struct pse_controller_ops *ops;
@@ -89,6 +103,7 @@  struct pse_controller_dev {
 			const struct of_phandle_args *pse_spec);
 	unsigned int nr_lines;
 	struct mutex lock;
+	u32 types;
 };
 
 #if IS_ENABLED(CONFIG_PSE_CONTROLLER)
@@ -108,6 +123,8 @@  int pse_ethtool_set_config(struct pse_control *psec,
 			   struct netlink_ext_ack *extack,
 			   const struct pse_control_config *config);
 
+u32 pse_get_types(struct pse_control *psec);
+
 #else
 
 static inline struct pse_control *of_pse_control_get(struct device_node *node)
@@ -133,6 +150,11 @@  static inline int pse_ethtool_set_config(struct pse_control *psec,
 	return -ENOTSUPP;
 }
 
+static inline u32 pse_get_types(struct pse_control *psec)
+{
+	return PSE_UNKNOWN;
+}
+
 #endif
 
 #endif