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
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 <[email protected]>
Signed-off-by: George Cherian <[email protected]>
---
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
The i2c XLP9xx driver is maintained by Cavium.
Add George Cherian and Jan Glauber as the Maintainers.
Signed-off-by: George Cherian <[email protected]>
---
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: [email protected]
S: Supported
F: drivers/char/xillybus/
+XLP9XX I2C DRIVER
+M: George Cherian <[email protected]>
+M: Jan Glauber <[email protected]>
+L: [email protected]
+W: http://www.cavium.com
+S: Supported
+F: drivers/i2c/busses/i2c-xlp9xx.c
+
XRA1403 GPIO EXPANDER
M: Nandor Han <[email protected]>
M: Semi Malinen <[email protected]>
--
1.8.3.1
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 <[email protected]>
Signed-off-by: George Cherian <[email protected]>
Reviewed-by: Jan Glauber <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: George Cherian <[email protected]>
---
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
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 <[email protected]>
> ---
> 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: [email protected]
> S: Supported
> F: drivers/char/xillybus/
>
> +XLP9XX I2C DRIVER
> +M: George Cherian <[email protected]>
> +M: Jan Glauber <[email protected]>
Jan, I need an explicit ACK from you for this change.
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 <[email protected]>
> > ---
> > 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: [email protected]
> > S: Supported
> > F: drivers/char/xillybus/
> >
> > +XLP9XX I2C DRIVER
> > +M: George Cherian <[email protected]>
> > +M: Jan Glauber <[email protected]>
>
> Jan, I need an explicit ACK from you for this change.
>
Sure:
Acked-by: Jan Glauber <[email protected]>
Cheers,
Jan
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 <[email protected]>
Applied to for-current, thanks!
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 <[email protected]>
> Signed-off-by: George Cherian <[email protected]>
Applied to for-next, thanks!
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 <[email protected]>
> Signed-off-by: George Cherian <[email protected]>
Applied to for-next, thanks!
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 <[email protected]>
> Signed-off-by: George Cherian <[email protected]>
> Reviewed-by: Jan Glauber <[email protected]>
Applied to for-next, thanks!