[PATCH net-next v1 4/7] net: pse-pd: add generic PSE driver

Oleksij Rempel posted 7 patches 3 years, 7 months ago
There is a newer version of this series
[PATCH net-next v1 4/7] net: pse-pd: add generic PSE driver
Posted by Oleksij Rempel 3 years, 7 months ago
Add generic driver to support simple Power Sourcing Equipment without
automatic classification support.

This driver was tested on 10Bast-T1L switch with regulator based PoDL PSE.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/pse-pd/Kconfig       |  11 +++
 drivers/net/pse-pd/Makefile      |   2 +
 drivers/net/pse-pd/pse_generic.c | 146 +++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)
 create mode 100644 drivers/net/pse-pd/pse_generic.c

diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
index 49c7f0bcff526..a804c9f1af2bc 100644
--- a/drivers/net/pse-pd/Kconfig
+++ b/drivers/net/pse-pd/Kconfig
@@ -9,3 +9,14 @@ menuconfig PSE_CONTROLLER
 	  Generic Power Sourcing Equipment Controller support.
 
 	  If unsure, say no.
+
+if PSE_CONTROLLER
+
+config PSE_GENERIC
+	tristate "Generic PSE driver"
+	help
+	  This module provides support for simple Ethernet Power Sourcing
+	  Equipment without automatic classification support. For example for
+	  PoDL (802.3bu) specification.
+
+endif
diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
index cbb79fc2e2706..80ef39ad68f10 100644
--- a/drivers/net/pse-pd/Makefile
+++ b/drivers/net/pse-pd/Makefile
@@ -2,3 +2,5 @@
 # Makefile for Linux PSE drivers
 
 obj-$(CONFIG_PSE_CONTROLLER) += pse-core.o
+
+obj-$(CONFIG_PSE_GENERIC) += pse_generic.o
diff --git a/drivers/net/pse-pd/pse_generic.c b/drivers/net/pse-pd/pse_generic.c
new file mode 100644
index 0000000000000..f264d4d589f59
--- /dev/null
+++ b/drivers/net/pse-pd/pse_generic.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Driver for the Generic Ethernet Power Sourcing Equipment, without
+// auto classification support.
+//
+// Copyright (c) 2022 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
+//
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pse-pd/pse.h>
+#include <linux/regulator/consumer.h>
+
+struct gen_pse_priv {
+	struct pse_controller_dev pcdev;
+	struct regulator *ps; /*power source */
+	enum ethtool_podl_pse_admin_state admin_state;
+};
+
+static struct gen_pse_priv *to_gen_pse(struct pse_controller_dev *pcdev)
+{
+	return container_of(pcdev, struct gen_pse_priv, pcdev);
+}
+
+static int
+gen_pse_podl_get_admin_sate(struct pse_controller_dev *pcdev, unsigned long id)
+{
+	struct gen_pse_priv *priv = to_gen_pse(pcdev);
+
+	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
+	 * which is provided by the regulator.
+	 */
+	return priv->admin_state;
+}
+
+static int
+gen_pse_podl_set_admin_control(struct pse_controller_dev *pcdev,
+			       unsigned long id,
+			       enum ethtool_podl_pse_admin_state state)
+{
+	struct gen_pse_priv *priv = to_gen_pse(pcdev);
+	int ret;
+
+	if (priv->admin_state == state)
+		goto set_state;
+
+	switch (state) {
+	case ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED:
+		ret = regulator_enable(priv->ps);
+		break;
+	case ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED:
+		ret = regulator_disable(priv->ps);
+		break;
+	default:
+		dev_err(pcdev->dev, "Unknown admin state %i\n", state);
+		ret = -ENOTSUPP;
+	}
+
+	if (ret)
+		return ret;
+
+set_state:
+	priv->admin_state = state;
+
+	return 0;
+}
+
+static int
+gen_pse_podl_get_pw_d_status(struct pse_controller_dev *pcdev, unsigned long id)
+{
+	struct gen_pse_priv *priv = to_gen_pse(pcdev);
+	int ret;
+
+	ret = regulator_is_enabled(priv->ps);
+	if (ret < 0)
+		return ret;
+
+	if (!ret)
+		return ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED;
+
+	return ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING;
+}
+
+static const struct pse_control_ops gen_pse_ops = {
+	.get_podl_pse_admin_sate = gen_pse_podl_get_admin_sate,
+	.set_podl_pse_admin_control = gen_pse_podl_set_admin_control,
+	.get_podl_pse_pw_d_status = gen_pse_podl_get_pw_d_status,
+};
+
+static int
+gen_pse_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct gen_pse_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (!pdev->dev.of_node)
+		return -ENOENT;
+
+	priv->ps = devm_regulator_get(dev, "ieee802.3-podl-pse");
+	if (IS_ERR(priv->ps)) {
+		dev_err(dev, "failed to get PSE regulator (%pe)\n", priv->ps);
+		return PTR_ERR(priv->ps);
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->admin_state = ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED;
+
+	priv->pcdev.owner = THIS_MODULE;
+	priv->pcdev.ops = &gen_pse_ops;
+	priv->pcdev.dev = dev;
+	ret = devm_pse_controller_register(dev, &priv->pcdev);
+	if (ret) {
+		dev_err(dev, "failed to register PSE controller (%pe)\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id gen_pse_of_match[] = {
+	{ .compatible = "ieee802.3-podl-pse-generic", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gen_pse_of_match);
+
+static struct platform_driver gen_pse_driver = {
+	.probe		= gen_pse_probe,
+	.driver		= {
+		.name		= "PSE Generic",
+		.of_match_table = of_match_ptr(gen_pse_of_match),
+	},
+};
+module_platform_driver(gen_pse_driver);
+
+MODULE_AUTHOR("Oleksij Rempel <kernel@pengutronix.de>");
+MODULE_DESCRIPTION("Generic Ethernet Power Sourcing Equipment");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:pse-generic");
-- 
2.30.2
Re: [PATCH net-next v1 4/7] net: pse-pd: add generic PSE driver
Posted by Andrew Lunn 3 years, 7 months ago
On Fri, Aug 19, 2022 at 02:01:06PM +0200, Oleksij Rempel wrote:
> Add generic driver to support simple Power Sourcing Equipment without
> automatic classification support.
> 
> This driver was tested on 10Bast-T1L switch with regulator based PoDL PSE.

Do you have access to a PHY which implements clause 45.2.9? That seems
like a better reference implementation?

I don't know the market, what is more likely, a simple regulator, or
something more capable with an interface like 45.2.9?

netlink does allow us to keep adding more attributes, so we don't need
to be perfect first time, but it seems like 45.2.9 is what IEEE expect
vendors to provide, so at some point Linux should implement it.

	  Andrew
Re: [PATCH net-next v1 4/7] net: pse-pd: add generic PSE driver
Posted by Oleksij Rempel 3 years, 7 months ago
On Fri, Aug 19, 2022 at 11:32:00PM +0200, Andrew Lunn wrote:
> On Fri, Aug 19, 2022 at 02:01:06PM +0200, Oleksij Rempel wrote:
> > Add generic driver to support simple Power Sourcing Equipment without
> > automatic classification support.
> > 
> > This driver was tested on 10Bast-T1L switch with regulator based PoDL PSE.
> 
> Do you have access to a PHY which implements clause 45.2.9? That seems
> like a better reference implementation?

Suddenly I do not have access to any of them. 

> I don't know the market, what is more likely, a simple regulator, or
> something more capable with an interface like 45.2.9?

So far I was not able to find any PoDL PES IC (with classification
support). Or PHY with PoDL PSE on one package. If some one know any,
please let me know.

> netlink does allow us to keep adding more attributes, so we don't need
> to be perfect first time, but it seems like 45.2.9 is what IEEE expect
> vendors to provide, so at some point Linux should implement it.

ack.

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 v1 4/7] net: pse-pd: add generic PSE driver
Posted by Andrew Lunn 3 years, 7 months ago
On Fri, Aug 19, 2022 at 02:01:06PM +0200, Oleksij Rempel wrote:
> Add generic driver to support simple Power Sourcing Equipment without
> automatic classification support.
> 
> This driver was tested on 10Bast-T1L switch with regulator based PoDL PSE.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/pse-pd/Kconfig       |  11 +++
>  drivers/net/pse-pd/Makefile      |   2 +
>  drivers/net/pse-pd/pse_generic.c | 146 +++++++++++++++++++++++++++++++
>  3 files changed, 159 insertions(+)
>  create mode 100644 drivers/net/pse-pd/pse_generic.c
> 
> diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
> index 49c7f0bcff526..a804c9f1af2bc 100644
> --- a/drivers/net/pse-pd/Kconfig
> +++ b/drivers/net/pse-pd/Kconfig
> @@ -9,3 +9,14 @@ menuconfig PSE_CONTROLLER
>  	  Generic Power Sourcing Equipment Controller support.
>  
>  	  If unsure, say no.
> +
> +if PSE_CONTROLLER
> +
> +config PSE_GENERIC
> +	tristate "Generic PSE driver"
> +	help
> +	  This module provides support for simple Ethernet Power Sourcing
> +	  Equipment without automatic classification support. For example for
> +	  PoDL (802.3bu) specification.
> +
> +endif
> diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
> index cbb79fc2e2706..80ef39ad68f10 100644
> --- a/drivers/net/pse-pd/Makefile
> +++ b/drivers/net/pse-pd/Makefile
> @@ -2,3 +2,5 @@
>  # Makefile for Linux PSE drivers
>  
>  obj-$(CONFIG_PSE_CONTROLLER) += pse-core.o
> +
> +obj-$(CONFIG_PSE_GENERIC) += pse_generic.o
> diff --git a/drivers/net/pse-pd/pse_generic.c b/drivers/net/pse-pd/pse_generic.c
> new file mode 100644
> index 0000000000000..f264d4d589f59
> --- /dev/null
> +++ b/drivers/net/pse-pd/pse_generic.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Driver for the Generic Ethernet Power Sourcing Equipment, without
> +// auto classification support.
> +//
> +// Copyright (c) 2022 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
> +//
> +
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pse-pd/pse.h>
> +#include <linux/regulator/consumer.h>
> +
> +struct gen_pse_priv {
> +	struct pse_controller_dev pcdev;
> +	struct regulator *ps; /*power source */
> +	enum ethtool_podl_pse_admin_state admin_state;
> +};
> +
> +static struct gen_pse_priv *to_gen_pse(struct pse_controller_dev *pcdev)
> +{
> +	return container_of(pcdev, struct gen_pse_priv, pcdev);
> +}
> +
> +static int
> +gen_pse_podl_get_admin_sate(struct pse_controller_dev *pcdev, unsigned long id)

Should that be state?

> +{
> +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> +
> +	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
> +	 * which is provided by the regulator.
> +	 */
> +	return priv->admin_state;
> +}
> +
> +static int
> +gen_pse_podl_set_admin_control(struct pse_controller_dev *pcdev,
> +			       unsigned long id,
> +			       enum ethtool_podl_pse_admin_state state)
> +{
> +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> +	int ret;
> +
> +	if (priv->admin_state == state)
> +		goto set_state;

return 0; ?

> +
> +	switch (state) {
> +	case ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED:
> +		ret = regulator_enable(priv->ps);
> +		break;
> +	case ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED:
> +		ret = regulator_disable(priv->ps);
> +		break;
> +	default:
> +		dev_err(pcdev->dev, "Unknown admin state %i\n", state);
> +		ret = -ENOTSUPP;
> +	}
> +
> +	if (ret)
> +		return ret;
> +
> +set_state:
> +	priv->admin_state = state;
> +
> +	return 0;
> +}
> +
> +static int
> +gen_pse_podl_get_pw_d_status(struct pse_controller_dev *pcdev, unsigned long id)
> +{
> +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> +	int ret;
> +
> +	ret = regulator_is_enabled(priv->ps);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!ret)
> +		return ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED;
> +
> +	return ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING;
> +}
> +
> +static const struct pse_control_ops gen_pse_ops = {
> +	.get_podl_pse_admin_sate = gen_pse_podl_get_admin_sate,
> +	.set_podl_pse_admin_control = gen_pse_podl_set_admin_control,
> +	.get_podl_pse_pw_d_status = gen_pse_podl_get_pw_d_status,
> +};
> +
> +static int
> +gen_pse_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct gen_pse_priv *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENOENT;
> +
> +	priv->ps = devm_regulator_get(dev, "ieee802.3-podl-pse");
> +	if (IS_ERR(priv->ps)) {
> +		dev_err(dev, "failed to get PSE regulator (%pe)\n", priv->ps);
> +		return PTR_ERR(priv->ps);
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->admin_state = ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED;

There is the comment earlier:

	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
	 * which is provided by the regulator.

Is this because the regulator might of been turned on, but it has
detected a short and turned itself off? So it is administratively on,
but off in order to stop the magic smoke escaping?

But what about the other way around? Something has already turned the
regulator on, e.g. the bootloader. Should the default be
ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED even thought power is being
delivered? Do we want to put the regulator into the off state at
probe, so it is in a well defined state? Or set priv->admin_state to
whatever regulator_is_enabled() indicates?

	 Andrew
Re: [PATCH net-next v1 4/7] net: pse-pd: add generic PSE driver
Posted by Oleksij Rempel 3 years, 7 months ago
On Fri, Aug 19, 2022 at 10:54:14PM +0200, Andrew Lunn wrote:
> > +static int
> > +gen_pse_podl_get_admin_sate(struct pse_controller_dev *pcdev, unsigned long id)
> 
> Should that be state?

ack. fixed.

> > +{
> > +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> > +
> > +	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
> > +	 * which is provided by the regulator.
> > +	 */
> > +	return priv->admin_state;
> > +}
> > +
> > +static int
> > +gen_pse_podl_set_admin_control(struct pse_controller_dev *pcdev,
> > +			       unsigned long id,
> > +			       enum ethtool_podl_pse_admin_state state)
> > +{
> > +	struct gen_pse_priv *priv = to_gen_pse(pcdev);
> > +	int ret;
> > +
> > +	if (priv->admin_state == state)
> > +		goto set_state;
> 
> return 0; ?

ack. done

> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	priv->admin_state = ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED;
> 
> There is the comment earlier:
> 
> 	/* aPoDLPSEAdminState can be different to aPoDLPSEPowerDetectionStatus
> 	 * which is provided by the regulator.
> 
> Is this because the regulator might of been turned on, but it has
> detected a short and turned itself off? So it is administratively on,
> but off in order to stop the magic smoke escaping?

ack. According to 30.15.1.1.3 aPoDLPSEPowerDetectionStatus, we may have
following:
/**
 * enum ethtool_podl_pse_pw_d_status - power detection status of the PoDL PSE.
 *	IEEE 802.3-2018 30.15.1.1.3 aPoDLPSEPowerDetectionStatus:
 * @ETHTOOL_PODL_PSE_PW_D_STATUS_UNKNOWN: PoDL PSE
 * @ETHTOOL_PODL_PSE_PW_D_STATUS_DISABLED: "The enumeration “disabled” is
 *	asserted true when the PoDL PSE state diagram variable mr_pse_enable is
 *	false"
 * @ETHTOOL_PODL_PSE_PW_D_STATUS_SEARCHING: "The enumeration “searching” is
 *	asserted true when either of the PSE state diagram variables
 *	pi_detecting or pi_classifying is true."
 * @ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING: "The enumeration “deliveringPower”
 *	is asserted true when the PoDL PSE state diagram variable pi_powered is
 *	true."
 * @ETHTOOL_PODL_PSE_PW_D_STATUS_SLEEP: "The enumeration “sleep” is asserted
 *	true when the PoDL PSE state diagram variable pi_sleeping is true."
 * @ETHTOOL_PODL_PSE_PW_D_STATUS_IDLE: "The enumeration “idle” is asserted true
 *	when the logical combination of the PoDL PSE state diagram variables
 *	pi_prebiased*!pi_sleeping is true."
 * @ETHTOOL_PODL_PSE_PW_D_STATUS_ERROR: "The enumeration “error” is asserted
 *	true when the PoDL PSE state diagram variable overload_held is true."
 */

Probably all of them, except of ETHTOOL_PODL_PSE_PW_D_STATUS_SEARCHING can be
potentially implemented in this driver on top of regulator framework.

> But what about the other way around? Something has already turned the
> regulator on, e.g. the bootloader. Should the default be
> ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED even thought power is being
> delivered? Do we want to put the regulator into the off state at
> probe, so it is in a well defined state? Or set priv->admin_state to
> whatever regulator_is_enabled() indicates?

Good question. I assume, automotive folks would love be able to enable
regulator in the boot loader on the PSE to let the Powered Device boot parallel
to the PSE.

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 |