[PATCH 0/4] i2c-xlp9xx Add support for SMBAlert and minor fixes

[PATCH 0/4] i2c-xlp9xx Add support for SMBAlert and minor fixes

[George Cherian]

This series adds the SMBAlert support for i2c-xlp9xx driver and the following
fixes.

Patch 2: Make sure we update the transfer length to a future length.

Patch 3: Restrict the transfer size to I2C_SMBUS_BLOCK_SIZE for transfers
with I2C_M_RECV_LEN is set.

Patch 4: While at that update the MAINATINERS file to reflect the current
maintainers of the driver.

George Cherian (4):
  i2c: xlp9xx: Add support for SMBAlert
  i2c: xlp9xx: Fix issue seen when updating receive length
  i2c: xlp9xx: Make sure the transfer size is not more than
    I2C_SMBUS_BLOCK_SIZE
  i2c: xlp9xx: Add MAINTAINERS entry

 MAINTAINERS                     |  8 ++++
 drivers/i2c/busses/i2c-xlp9xx.c | 89 +++++++++++++++++++++++++++++++----------
 2 files changed, 76 insertions(+), 21 deletions(-)

-- 
1.8.3.1

[PATCH 1/4] i2c: xlp9xx: Add support for SMBAlert

[George Cherian]

Add support for SMBus alert mechanism to i2c-xlp9xx driver.
The second interrupt is parsed to use for SMBus alert.
The first interrupt is the i2c controller main interrupt.

Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
Signed-off-by: George Cherian <george.cherian@cavium.com>
Reviewed-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index eb8913e..fe54512 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -10,6 +10,7 @@
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -84,6 +85,8 @@ struct xlp9xx_i2c_dev {
 	struct device *dev;
 	struct i2c_adapter adapter;
 	struct completion msg_complete;
+	struct i2c_smbus_alert_setup alert_data;
+	struct i2c_client *ara;
 	int irq;
 	bool msg_read;
 	bool len_recv;
@@ -447,6 +450,19 @@ static int xlp9xx_i2c_get_frequency(struct platform_device *pdev,
 	return 0;
 }
 
+static int xlp9xx_i2c_smbus_setup(struct xlp9xx_i2c_dev *priv,
+				  struct platform_device *pdev)
+{
+	if (!priv->alert_data.irq)
+		return -EINVAL;
+
+	priv->ara = i2c_setup_smbus_alert(&priv->adapter, &priv->alert_data);
+	if (!priv->ara)
+		return -ENODEV;
+
+	return 0;
+}
+
 static int xlp9xx_i2c_probe(struct platform_device *pdev)
 {
 	struct xlp9xx_i2c_dev *priv;
@@ -467,6 +483,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "invalid irq!\n");
 		return priv->irq;
 	}
+	/* SMBAlert irq */
+	priv->alert_data.irq = platform_get_irq(pdev, 1);
+	if (priv->alert_data.irq <= 0)
+		priv->alert_data.irq = 0;
 
 	xlp9xx_i2c_get_frequency(pdev, priv);
 	xlp9xx_i2c_init(priv);
@@ -493,6 +513,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	err = xlp9xx_i2c_smbus_setup(priv, pdev);
+	if (err)
+		dev_dbg(&pdev->dev, "No active SMBus alert %d\n", err);
+
 	platform_set_drvdata(pdev, priv);
 	dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
 
-- 
1.8.3.1

Re: [PATCH 1/4] i2c: xlp9xx: Add support for SMBAlert

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]

On Wed, May 16, 2018 at 12:00:16AM -0700, George Cherian wrote:
> Add support for SMBus alert mechanism to i2c-xlp9xx driver.
> The second interrupt is parsed to use for SMBus alert.
> The first interrupt is the i2c controller main interrupt.
> 
> Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
> Signed-off-by: George Cherian <george.cherian@cavium.com>
> Reviewed-by: Jan Glauber <jglauber@cavium.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/4] i2c: xlp9xx: Fix issue seen when updating receive length

[George Cherian]

The hardware does not handle updates to the length register gracefully
if the new value is less than the number of bytes received so far. If
this happens, the i2c controller will not stop the receive transaction
properly.

Fix this by ensuring that the updated length is ok. This is done by
making sure that the new length written to hardware is at least few
bytes more than the bytes received so far.

While at that refactor the length updation to a new function.

Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
Signed-off-by: George Cherian <george.cherian@cavium.com>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index fe54512..c268fde 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -158,9 +158,28 @@ static void xlp9xx_i2c_fill_tx_fifo(struct xlp9xx_i2c_dev *priv)
 	priv->msg_buf += len;
 }
 
+static void xlp9xx_i2c_update_rlen(struct xlp9xx_i2c_dev *priv)
+{
+	u32 val, len;
+
+	/*
+	 * Update receive length. Re-read len to get the latest value,
+	 * and then add 4 to have a minimum value that can be safely
+	 * written. This is to account for the byte read above, the
+	 * transfer in progress and any delays in the register I/O
+	 */
+	val = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_CTRL);
+	len = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_FIFOWCNT) &
+				  XLP9XX_I2C_FIFO_WCNT_MASK;
+	len = max_t(u32, priv->msg_len, len + 4);
+	val = (val & ~XLP9XX_I2C_CTRL_MCTLEN_MASK) |
+			(len << XLP9XX_I2C_CTRL_MCTLEN_SHIFT);
+	xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CTRL, val);
+}
+
 static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
 {
-	u32 len, i, val;
+	u32 len, i;
 	u8 rlen, *buf = priv->msg_buf;
 
 	len = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_FIFOWCNT) &
@@ -171,20 +190,13 @@ static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
 		/* read length byte */
 		rlen = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
 		*buf++ = rlen;
-		len--;
-
 		if (priv->client_pec)
 			++rlen;
 		/* update remaining bytes and message length */
 		priv->msg_buf_remaining = rlen;
 		priv->msg_len = rlen + 1;
 		priv->len_recv = false;
-
-		/* Update transfer length to read only actual data */
-		val = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_CTRL);
-		val = (val & ~XLP9XX_I2C_CTRL_MCTLEN_MASK) |
-			((rlen + 1) << XLP9XX_I2C_CTRL_MCTLEN_SHIFT);
-		xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CTRL, val);
+		xlp9xx_i2c_update_rlen(priv);
 	} else {
 		len = min(priv->msg_buf_remaining, len);
 		for (i = 0; i < len; i++, buf++)
-- 
1.8.3.1

Re: [PATCH 2/4] i2c: xlp9xx: Fix issue seen when updating receive length

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

On Wed, May 16, 2018 at 12:00:17AM -0700, George Cherian wrote:
> The hardware does not handle updates to the length register gracefully
> if the new value is less than the number of bytes received so far. If
> this happens, the i2c controller will not stop the receive transaction
> properly.
> 
> Fix this by ensuring that the updated length is ok. This is done by
> making sure that the new length written to hardware is at least few
> bytes more than the bytes received so far.
> 
> While at that refactor the length updation to a new function.
> 
> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
> Signed-off-by: George Cherian <george.cherian@cavium.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/4] i2c: xlp9xx: Make sure the transfer size is not more than I2C_SMBUS_BLOCK_SIZE

[George Cherian]

For SMBus transactions the max permissible transfer size is
I2C_SMBUS_BLOCK_SIZE. It is possible that some clients might
not follow it strictly occasionally.
This would lead to stack corruption if the driver copies more than
I2C_SMBUS_BLOCK_SIZE bytes. Add a check to avoid such conditions.

Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
Signed-off-by: George Cherian <george.cherian@cavium.com>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index c268fde..1f41a4f 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -172,6 +172,8 @@ static void xlp9xx_i2c_update_rlen(struct xlp9xx_i2c_dev *priv)
 	len = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_FIFOWCNT) &
 				  XLP9XX_I2C_FIFO_WCNT_MASK;
 	len = max_t(u32, priv->msg_len, len + 4);
+	if (len >= I2C_SMBUS_BLOCK_MAX + 2)
+		return;
 	val = (val & ~XLP9XX_I2C_CTRL_MCTLEN_MASK) |
 			(len << XLP9XX_I2C_CTRL_MCTLEN_SHIFT);
 	xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CTRL, val);
@@ -189,14 +191,20 @@ static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
 	if (priv->len_recv) {
 		/* read length byte */
 		rlen = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
-		*buf++ = rlen;
-		if (priv->client_pec)
-			++rlen;
-		/* update remaining bytes and message length */
-		priv->msg_buf_remaining = rlen;
-		priv->msg_len = rlen + 1;
-		priv->len_recv = false;
+		if (rlen > I2C_SMBUS_BLOCK_MAX || rlen == 0) {
+			rlen = 0;	/*abort transfer */
+			priv->msg_buf_remaining = 0;
+			priv->msg_len = 0;
+		} else {
+			*buf++ = rlen;
+			if (priv->client_pec)
+				++rlen; /* account for error check byte */
+			/* update remaining bytes and message length */
+			priv->msg_buf_remaining = rlen;
+			priv->msg_len = rlen + 1;
+		}
 		xlp9xx_i2c_update_rlen(priv);
+		priv->len_recv = false;
 	} else {
 		len = min(priv->msg_buf_remaining, len);
 		for (i = 0; i < len; i++, buf++)
@@ -315,10 +323,6 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, struct i2c_msg *msg,
 	xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_MFIFOCTRL,
 			     XLP9XX_I2C_MFIFOCTRL_RST);
 
-	/* set FIFO threshold if reading */
-	if (priv->msg_read)
-		xlp9xx_i2c_update_rx_fifo_thres(priv);
-
 	/* set slave addr */
 	xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_SLAVEADDR,
 			     (msg->addr << XLP9XX_I2C_SLAVEADDR_ADDR_SHIFT) |
@@ -337,9 +341,13 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, struct i2c_msg *msg,
 		val &= ~XLP9XX_I2C_CTRL_ADDMODE;
 
 	priv->len_recv = msg->flags & I2C_M_RECV_LEN;
-	len = priv->len_recv ? XLP9XX_I2C_FIFO_SIZE : msg->len;
+	len = priv->len_recv ? I2C_SMBUS_BLOCK_MAX + 2 : msg->len;
 	priv->client_pec = msg->flags & I2C_CLIENT_PEC;
 
+	/* set FIFO threshold if reading */
+	if (priv->msg_read)
+		xlp9xx_i2c_update_rx_fifo_thres(priv);
+
 	/* set data length to be transferred */
 	val = (val & ~XLP9XX_I2C_CTRL_MCTLEN_MASK) |
 	      (len << XLP9XX_I2C_CTRL_MCTLEN_SHIFT);
@@ -393,8 +401,11 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, struct i2c_msg *msg,
 	}
 
 	/* update msg->len with actual received length */
-	if (msg->flags & I2C_M_RECV_LEN)
+	if (msg->flags & I2C_M_RECV_LEN) {
+		if (!priv->msg_len)
+			return -EPROTO;
 		msg->len = priv->msg_len;
+	}
 	return 0;
 }
 
-- 
1.8.3.1

Re: [PATCH 3/4] i2c: xlp9xx: Make sure the transfer size is not more than I2C_SMBUS_BLOCK_SIZE

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

On Wed, May 16, 2018 at 12:00:18AM -0700, George Cherian wrote:
> For SMBus transactions the max permissible transfer size is
> I2C_SMBUS_BLOCK_SIZE. It is possible that some clients might
> not follow it strictly occasionally.
> This would lead to stack corruption if the driver copies more than
> I2C_SMBUS_BLOCK_SIZE bytes. Add a check to avoid such conditions.
> 
> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
> Signed-off-by: George Cherian <george.cherian@cavium.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/4] i2c: xlp9xx: Add MAINTAINERS entry

[George Cherian]

The i2c XLP9xx driver is maintained by Cavium.
Add George Cherian and Jan Glauber as the Maintainers.

Signed-off-by: George Cherian <george.cherian@cavium.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index df6e9bb..68da265 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15509,6 +15509,14 @@ L:	linux-kernel@vger.kernel.org
 S:	Supported
 F:	drivers/char/xillybus/
 
+XLP9XX I2C DRIVER
+M:	George Cherian <george.cherian@cavium.com>
+M:	Jan Glauber <jglauber@cavium.com>
+L:	linux-i2c@vger.kernel.org
+W:	http://www.cavium.com
+S:	Supported
+F:	drivers/i2c/busses/i2c-xlp9xx.c
+
 XRA1403 GPIO EXPANDER
 M:	Nandor Han <nandor.han@ge.com>
 M:	Semi Malinen <semi.malinen@ge.com>
-- 
1.8.3.1

Re: [PATCH 4/4] i2c: xlp9xx: Add MAINTAINERS entry

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 715 bytes --]

On Wed, May 16, 2018 at 12:00:19AM -0700, George Cherian wrote:
> The i2c XLP9xx driver is maintained by Cavium.
> Add George Cherian and Jan Glauber as the Maintainers.
> 
> Signed-off-by: George Cherian <george.cherian@cavium.com>
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index df6e9bb..68da265 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15509,6 +15509,14 @@ L:	linux-kernel@vger.kernel.org
>  S:	Supported
>  F:	drivers/char/xillybus/
>  
> +XLP9XX I2C DRIVER
> +M:	George Cherian <george.cherian@cavium.com>
> +M:	Jan Glauber <jglauber@cavium.com>

Jan, I need an explicit ACK from you for this change.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] i2c: xlp9xx: Add MAINTAINERS entry

[Jan Glauber]

On Tue, May 22, 2018 at 01:54:14PM +0200, Wolfram Sang wrote:
> On Wed, May 16, 2018 at 12:00:19AM -0700, George Cherian wrote:
> > The i2c XLP9xx driver is maintained by Cavium.
> > Add George Cherian and Jan Glauber as the Maintainers.
> > 
> > Signed-off-by: George Cherian <george.cherian@cavium.com>
> > ---
> >  MAINTAINERS | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index df6e9bb..68da265 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -15509,6 +15509,14 @@ L:	linux-kernel@vger.kernel.org
> >  S:	Supported
> >  F:	drivers/char/xillybus/
> >  
> > +XLP9XX I2C DRIVER
> > +M:	George Cherian <george.cherian@cavium.com>
> > +M:	Jan Glauber <jglauber@cavium.com>
> 
> Jan, I need an explicit ACK from you for this change.
> 

Sure:
Acked-by: Jan Glauber <jglauber@cavium.com>

Cheers,
Jan

Re: [PATCH 4/4] i2c: xlp9xx: Add MAINTAINERS entry

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 275 bytes --]

On Wed, May 16, 2018 at 12:00:19AM -0700, George Cherian wrote:
> The i2c XLP9xx driver is maintained by Cavium.
> Add George Cherian and Jan Glauber as the Maintainers.
> 
> Signed-off-by: George Cherian <george.cherian@cavium.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]