1. Patchew
  2. linux
  3. [PATCH net-next 00/12] Add support for PSE port priority
  4. View patch

[PATCH net-next 12/12] net: pse-pd: tps23881: Add support for PSE events and interrupts

Kory Maincent posted 12 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH net-next 12/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
Posted by Kory Maincent 1 month, 3 weeks ago 
From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>

Add support for PSE event reporting through interrupts. Set up the newly
introduced devm_pse_irq_helper helper to register the interrupt. Events are
reported for over-current and over-temperature conditions.

This patch also adds support for an OSS GPIO line to turn off all low
priority ports in case of an over-current event.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 drivers/net/pse-pd/tps23881.c | 123 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 1 deletion(-)

diff --git a/drivers/net/pse-pd/tps23881.c b/drivers/net/pse-pd/tps23881.c
index ddb44a17218a..03f36b641bb4 100644
--- a/drivers/net/pse-pd/tps23881.c
+++ b/drivers/net/pse-pd/tps23881.c
@@ -17,6 +17,13 @@
 
 #define TPS23881_MAX_CHANS 8
 
+#define TPS23881_REG_IT		0x0
+#define TPS23881_REG_IT_MASK	0x1
+#define TPS23881_REG_IT_IFAULT	BIT(5)
+#define TPS23881_REG_IT_SUPF	BIT(7)
+#define TPS23881_REG_FAULT	0x7
+#define TPS23881_REG_SUPF_EVENT	0xb
+#define TPS23881_REG_TSD	BIT(7)
 #define TPS23881_REG_PW_STATUS	0x10
 #define TPS23881_REG_OP_MODE	0x12
 #define TPS23881_OP_MODE_SEMIAUTO	0xaaaa
@@ -25,6 +32,7 @@
 #define TPS23881_REG_PW_PRIO	0x15
 #define TPS23881_REG_GEN_MASK	0x17
 #define TPS23881_REG_NBITACC	BIT(5)
+#define TPS23881_REG_INTEN	BIT(7)
 #define TPS23881_REG_PW_EN	0x19
 #define TPS23881_REG_2PAIR_POL1	0x1e
 #define TPS23881_REG_PORT_MAP	0x26
@@ -59,6 +67,7 @@ struct tps23881_priv {
 	struct pse_controller_dev pcdev;
 	struct device_node *np;
 	struct tps23881_port_desc port[TPS23881_MAX_CHANS];
+	struct gpio_desc *oss;
 };
 
 static struct tps23881_priv *to_tps23881_priv(struct pse_controller_dev *pcdev)
@@ -1088,11 +1097,112 @@ static int tps23881_flash_sram_fw(struct i2c_client *client)
 	return 0;
 }
 
+static void tps23881_turn_off_low_prio(struct tps23881_priv *priv)
+{
+	dev_info(&priv->client->dev,
+		 "turn off low priority ports due to over current event.\n");
+	gpiod_set_value_cansleep(priv->oss, 1);
+
+	/* TPS23880 datasheet (Rev G) indicates minimum OSS pulse is 5us */
+	usleep_range(5, 10);
+	gpiod_set_value_cansleep(priv->oss, 0);
+}
+
+static int tps23881_irq_handler(int irq, struct pse_irq_data *pid,
+				unsigned long *dev_mask)
+{
+	struct tps23881_priv *priv = (struct tps23881_priv *)pid->data;
+	struct i2c_client *client = priv->client;
+	struct pse_err_state *stat;
+	int ret, i;
+	u16 val;
+
+	*dev_mask = 0;
+	for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+		stat = &pid->states[i];
+		stat->notifs = 0;
+		stat->errors = 0;
+	}
+
+	ret = i2c_smbus_read_word_data(client, TPS23881_REG_IT);
+	if (ret < 0)
+		return PSE_FAILED_RETRY;
+
+	val = (u16)ret;
+	if (val & TPS23881_REG_IT_SUPF) {
+		ret = i2c_smbus_read_word_data(client, TPS23881_REG_SUPF_EVENT);
+		if (ret < 0)
+			return PSE_FAILED_RETRY;
+
+		if (ret & TPS23881_REG_TSD) {
+			for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+				stat = &pid->states[i];
+				*dev_mask |= 1 << i;
+				stat->notifs = PSE_EVENT_OVER_TEMP;
+				stat->errors = PSE_ERROR_OVER_TEMP;
+			}
+		}
+	}
+
+	if (val & (TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_IFAULT << 8)) {
+		ret = i2c_smbus_read_word_data(client, TPS23881_REG_FAULT);
+		if (ret < 0)
+			return PSE_FAILED_RETRY;
+
+		val = (u16)(ret & 0xf0f);
+
+		/* Power cut detected, shutdown low priority port */
+		if (val && priv->oss)
+			tps23881_turn_off_low_prio(priv);
+
+		*dev_mask |= val;
+		for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+			if (val & BIT(i)) {
+				stat = &pid->states[i];
+				stat->notifs = PSE_EVENT_OVER_CURRENT;
+				stat->errors = PSE_ERROR_OVER_CURRENT;
+			}
+		}
+	}
+
+	return PSE_ERROR_CLEARED;
+}
+
+static int tps23881_setup_irq(struct tps23881_priv *priv, int irq)
+{
+	int errs = PSE_ERROR_OVER_CURRENT | PSE_ERROR_OVER_TEMP;
+	struct i2c_client *client = priv->client;
+	struct pse_irq_desc irq_desc = {
+		.name = "tps23881-irq",
+		.map_event = tps23881_irq_handler,
+		.data = priv,
+	};
+	int ret;
+	u16 val;
+
+	val = TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_SUPF |
+	      TPS23881_REG_IT_IFAULT << 8 | TPS23881_REG_IT_SUPF << 8;
+	ret = i2c_smbus_write_word_data(client, TPS23881_REG_IT_MASK, val);
+	if (ret)
+		return ret;
+
+	ret = i2c_smbus_read_word_data(client, TPS23881_REG_GEN_MASK);
+	if (ret < 0)
+		return ret;
+
+	val = (u16)(ret | TPS23881_REG_INTEN | TPS23881_REG_INTEN << 8);
+	ret = i2c_smbus_write_word_data(client, TPS23881_REG_GEN_MASK, val);
+	if (ret < 0)
+		return ret;
+
+	return devm_pse_irq_helper(&priv->pcdev, irq, 0, errs, &irq_desc);
+}
+
 static int tps23881_i2c_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct tps23881_priv *priv;
-	struct gpio_desc *reset;
+	struct gpio_desc *reset, *oss;
 	int ret;
 	u8 val;
 
@@ -1169,6 +1279,17 @@ static int tps23881_i2c_probe(struct i2c_client *client)
 				     "failed to register PSE controller\n");
 	}
 
+	oss = devm_gpiod_get_optional(dev, "oss", GPIOD_OUT_LOW);
+	if (IS_ERR(oss))
+		return dev_err_probe(&client->dev, PTR_ERR(oss), "Failed to get OSS GPIO\n");
+	priv->oss = oss;
+
+	if (client->irq) {
+		ret = tps23881_setup_irq(priv, client->irq);
+		if (ret)
+			return ret;
+	}
+
 	return ret;
 }
 

-- 
2.34.1
Re: [PATCH net-next 12/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
Posted by Oleksij Rempel 1 month, 2 weeks ago 
Hi Kory,

On Wed, Oct 02, 2024 at 06:28:08PM +0200, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> 
> Add support for PSE event reporting through interrupts. Set up the newly
> introduced devm_pse_irq_helper helper to register the interrupt. Events are
> reported for over-current and over-temperature conditions.
> 
> This patch also adds support for an OSS GPIO line to turn off all low
> priority ports in case of an over-current event.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

...
> +static int tps23881_irq_handler(int irq, struct pse_irq_data *pid,
> +				unsigned long *dev_mask)
> +{
> +	struct tps23881_priv *priv = (struct tps23881_priv *)pid->data;
> +	struct i2c_client *client = priv->client;
> +	struct pse_err_state *stat;
> +	int ret, i;
> +	u16 val;
> +
> +	*dev_mask = 0;
> +	for (i = 0; i < TPS23881_MAX_CHANS; i++) {
> +		stat = &pid->states[i];
> +		stat->notifs = 0;
> +		stat->errors = 0;
> +	}
> +
> +	ret = i2c_smbus_read_word_data(client, TPS23881_REG_IT);
> +	if (ret < 0)
> +		return PSE_FAILED_RETRY;
> +
> +	val = (u16)ret;
> +	if (val & TPS23881_REG_IT_SUPF) {
> +		ret = i2c_smbus_read_word_data(client, TPS23881_REG_SUPF_EVENT);
> +		if (ret < 0)
> +			return PSE_FAILED_RETRY;
> +
> +		if (ret & TPS23881_REG_TSD) {
> +			for (i = 0; i < TPS23881_MAX_CHANS; i++) {
> +				stat = &pid->states[i];
> +				*dev_mask |= 1 << i;
> +				stat->notifs = PSE_EVENT_OVER_TEMP;
> +				stat->errors = PSE_ERROR_OVER_TEMP;
> +			}
> +		}
> +	}
> +
> +	if (val & (TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_IFAULT << 8)) {
> +		ret = i2c_smbus_read_word_data(client, TPS23881_REG_FAULT);
> +		if (ret < 0)
> +			return PSE_FAILED_RETRY;
> +
> +		val = (u16)(ret & 0xf0f);
> +
> +		/* Power cut detected, shutdown low priority port */
> +		if (val && priv->oss)
> +			tps23881_turn_off_low_prio(priv);

Sorry, this is policy and even not the best one.
The priority concept is related to the power budget, but this
implementation will shutdown all low prios ports only if some
port/channel has over-current event. It means, in case high prio port
has over-current event, it will be not shut down.
 
I'll propose not to add prio support for this chip right now, it will
need more software infrastructure to handle it nearly in similar way as
it is done by pd692x0.
-- 
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 12/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
Posted by Kory Maincent 1 month, 2 weeks ago 
On Wed, 9 Oct 2024 09:25:10 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> > +
> > +	if (val & (TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_IFAULT << 8)) {
> > +		ret = i2c_smbus_read_word_data(client, TPS23881_REG_FAULT);
> > +		if (ret < 0)
> > +			return PSE_FAILED_RETRY;
> > +
> > +		val = (u16)(ret & 0xf0f);
> > +
> > +		/* Power cut detected, shutdown low priority port */
> > +		if (val && priv->oss)
> > +			tps23881_turn_off_low_prio(priv);  
> 
> Sorry, this is policy and even not the best one.
> The priority concept is related to the power budget, but this
> implementation will shutdown all low prios ports only if some
> port/channel has over-current event. It means, in case high prio port
> has over-current event, it will be not shut down.
>  
> I'll propose not to add prio support for this chip right now, it will
> need more software infrastructure to handle it nearly in similar way as
> it is done by pd692x0.

Yes, I have expected some debate around this support.
I was not sure of the policy while developing it.
Ok, let's remove it for now.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Re: [PATCH net-next 12/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
Posted by Oleksij Rempel 1 month, 2 weeks ago 
On Wed, Oct 02, 2024 at 06:28:08PM +0200, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> 
> Add support for PSE event reporting through interrupts. Set up the newly
> introduced devm_pse_irq_helper helper to register the interrupt. Events are
> reported for over-current and over-temperature conditions.
> 
> This patch also adds support for an OSS GPIO line to turn off all low
> priority ports in case of an over-current event.
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>  drivers/net/pse-pd/tps23881.c | 123 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 122 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/pse-pd/tps23881.c b/drivers/net/pse-pd/tps23881.c
> index ddb44a17218a..03f36b641bb4 100644
> --- a/drivers/net/pse-pd/tps23881.c
> +++ b/drivers/net/pse-pd/tps23881.c
> @@ -17,6 +17,13 @@
>  
>  #define TPS23881_MAX_CHANS 8
>  
> +#define TPS23881_REG_IT		0x0
> +#define TPS23881_REG_IT_MASK	0x1
> +#define TPS23881_REG_IT_IFAULT	BIT(5)
> +#define TPS23881_REG_IT_SUPF	BIT(7)
> +#define TPS23881_REG_FAULT	0x7
> +#define TPS23881_REG_SUPF_EVENT	0xb
> +#define TPS23881_REG_TSD	BIT(7)
>  #define TPS23881_REG_PW_STATUS	0x10
>  #define TPS23881_REG_OP_MODE	0x12
>  #define TPS23881_OP_MODE_SEMIAUTO	0xaaaa
> @@ -25,6 +32,7 @@
>  #define TPS23881_REG_PW_PRIO	0x15
>  #define TPS23881_REG_GEN_MASK	0x17
>  #define TPS23881_REG_NBITACC	BIT(5)
> +#define TPS23881_REG_INTEN	BIT(7)
>  #define TPS23881_REG_PW_EN	0x19
>  #define TPS23881_REG_2PAIR_POL1	0x1e
>  #define TPS23881_REG_PORT_MAP	0x26
> @@ -59,6 +67,7 @@ struct tps23881_priv {
>  	struct pse_controller_dev pcdev;
>  	struct device_node *np;
>  	struct tps23881_port_desc port[TPS23881_MAX_CHANS];
> +	struct gpio_desc *oss;
>  };
>  
>  static struct tps23881_priv *to_tps23881_priv(struct pse_controller_dev *pcdev)
> @@ -1088,11 +1097,112 @@ static int tps23881_flash_sram_fw(struct i2c_client *client)
>  	return 0;
>  }
>  
> +static void tps23881_turn_off_low_prio(struct tps23881_priv *priv)
> +{
> +	dev_info(&priv->client->dev,
> +		 "turn off low priority ports due to over current event.\n");
> +	gpiod_set_value_cansleep(priv->oss, 1);
> +
> +	/* TPS23880 datasheet (Rev G) indicates minimum OSS pulse is 5us */
> +	usleep_range(5, 10);
> +	gpiod_set_value_cansleep(priv->oss, 0);

Ah, now I understand why 1 bit priority mode is used. The "fast" shutdown
path is done over interrupt and gpio bitbang.

It is not your fault...

> +}
> +
> +static int tps23881_irq_handler(int irq, struct pse_irq_data *pid,
> +				unsigned long *dev_mask)
> +{
> +	struct tps23881_priv *priv = (struct tps23881_priv *)pid->data;
> +	struct i2c_client *client = priv->client;
> +	struct pse_err_state *stat;
> +	int ret, i;
> +	u16 val;
> +
> +	*dev_mask = 0;
> +	for (i = 0; i < TPS23881_MAX_CHANS; i++) {
> +		stat = &pid->states[i];
> +		stat->notifs = 0;
> +		stat->errors = 0;
> +	}
> +

Please add comment that two registers are read here in one run.

> +	ret = i2c_smbus_read_word_data(client, TPS23881_REG_IT);
> +	if (ret < 0)
> +		return PSE_FAILED_RETRY;
> +
> +	val = (u16)ret;
> +	if (val & TPS23881_REG_IT_SUPF) {
> +		ret = i2c_smbus_read_word_data(client, TPS23881_REG_SUPF_EVENT);
> +		if (ret < 0)
> +			return PSE_FAILED_RETRY;
> +
> +		if (ret & TPS23881_REG_TSD) {
> +			for (i = 0; i < TPS23881_MAX_CHANS; i++) {
> +				stat = &pid->states[i];
> +				*dev_mask |= 1 << i;
> +				stat->notifs = PSE_EVENT_OVER_TEMP;
> +				stat->errors = PSE_ERROR_OVER_TEMP;
> +			}
> +		}
> +	}
> +
> +	if (val & (TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_IFAULT << 8)) {

Ok, i see, you are reading two registers in one run and wont to test if
mask and status bits are active. In this code you will get true every
time the interrupt handler is executed.

> +		ret = i2c_smbus_read_word_data(client, TPS23881_REG_FAULT);
> +		if (ret < 0)
> +			return PSE_FAILED_RETRY;
> +
> +		val = (u16)(ret & 0xf0f);
> +
> +		/* Power cut detected, shutdown low priority port */
> +		if (val && priv->oss)
> +			tps23881_turn_off_low_prio(priv);
> +
> +		*dev_mask |= val;
> +		for (i = 0; i < TPS23881_MAX_CHANS; i++) {
> +			if (val & BIT(i)) {
> +				stat = &pid->states[i];
> +				stat->notifs = PSE_EVENT_OVER_CURRENT;
> +				stat->errors = PSE_ERROR_OVER_CURRENT;
> +			}
> +		}
> +	}
> +
> +	return PSE_ERROR_CLEARED;
> +}
> +
> +static int tps23881_setup_irq(struct tps23881_priv *priv, int irq)
> +{
> +	int errs = PSE_ERROR_OVER_CURRENT | PSE_ERROR_OVER_TEMP;
> +	struct i2c_client *client = priv->client;
> +	struct pse_irq_desc irq_desc = {
> +		.name = "tps23881-irq",
> +		.map_event = tps23881_irq_handler,
> +		.data = priv,
> +	};
> +	int ret;
> +	u16 val;
> +
> +	val = TPS23881_REG_IT_IFAULT | TPS23881_REG_IT_SUPF |
> +	      TPS23881_REG_IT_IFAULT << 8 | TPS23881_REG_IT_SUPF << 8;
> +	ret = i2c_smbus_write_word_data(client, TPS23881_REG_IT_MASK, val);
> +	if (ret)
> +		return ret;
> +
> +	ret = i2c_smbus_read_word_data(client, TPS23881_REG_GEN_MASK);
> +	if (ret < 0)
> +		return ret;
> +
> +	val = (u16)(ret | TPS23881_REG_INTEN | TPS23881_REG_INTEN << 8);
> +	ret = i2c_smbus_write_word_data(client, TPS23881_REG_GEN_MASK, val);
> +	if (ret < 0)
> +		return ret;
> +
> +	return devm_pse_irq_helper(&priv->pcdev, irq, 0, errs, &irq_desc);
> +}
> +
>  static int tps23881_i2c_probe(struct i2c_client *client)
>  {
>  	struct device *dev = &client->dev;
>  	struct tps23881_priv *priv;
> -	struct gpio_desc *reset;
> +	struct gpio_desc *reset, *oss;
>  	int ret;
>  	u8 val;
>  
> @@ -1169,6 +1279,17 @@ static int tps23881_i2c_probe(struct i2c_client *client)
>  				     "failed to register PSE controller\n");
>  	}
>  
> +	oss = devm_gpiod_get_optional(dev, "oss", GPIOD_OUT_LOW);
> +	if (IS_ERR(oss))
> +		return dev_err_probe(&client->dev, PTR_ERR(oss), "Failed to get OSS GPIO\n");
> +	priv->oss = oss;
> +
> +	if (client->irq) {
> +		ret = tps23881_setup_irq(priv, client->irq);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return ret;
>  }
>  
> 
> -- 
> 2.34.1
> 
> 

-- 
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 12/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
Posted by Andrew Lunn 1 month, 3 weeks ago 
> This patch also adds support for an OSS GPIO line to turn off all low
> priority ports in case of an over-current event.

Does this need a binding update? Or is the GPIO already listed?

	Andrew
Re: [PATCH net-next 12/12] net: pse-pd: tps23881: Add support for PSE events and interrupts
Posted by Kory Maincent 1 month, 3 weeks ago 
On Thu, 3 Oct 2024 01:57:08 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > This patch also adds support for an OSS GPIO line to turn off all low
> > priority ports in case of an over-current event.  
> 
> Does this need a binding update? Or is the GPIO already listed?

Indeed and it is missing. Oops! /o\

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.