[PATCH 0/4][Bionic][SRU Artful] i2c: xlp9xx: driver stability fixes

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/4][Bionic][SRU Artful] i2c: xlp9xx: driver stability fixes

dann frazier-4
BugLink: https://bugs.launchpad.net/bugs/1762812

All clean cherry-picks from upstream.

Dmitry Bazhenov (1):
  i2c: xlp9xx: return ENXIO on slave address NACK

George Cherian (3):
  i2c: xlp9xx: Handle transactions with I2C_M_RECV_LEN properly
  i2c: xlp9xx: Check for Bus state before every transfer
  i2c: xlp9xx: Handle NACK on DATA properly

 drivers/i2c/busses/i2c-xlp9xx.c | 78 +++++++++++++++++++++++++++------
 1 file changed, 65 insertions(+), 13 deletions(-)

--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/4][Bionic][SRU Artful] i2c: xlp9xx: return ENXIO on slave address NACK

dann frazier-4
From: Dmitry Bazhenov <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1762812

Fix the driver violation of the common practice to return
ENXIO error on a slave address NACK.

Signed-off-by: Dmitry Bazhenov <[hidden email]>
Signed-off-by: George Cherian <[hidden email]>
Tested-by: dann frazier <[hidden email]>
Signed-off-by: Wolfram Sang <[hidden email]>
(cherry picked from commit c2a3b3cce8df1cafeda2ab03563d7e703c51a4ac)
Signed-off-by: dann frazier <[hidden email]>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index b970bf8f38e5..6d78cdc5cf91 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -324,7 +324,8 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, struct i2c_msg *msg,
  dev_dbg(priv->dev, "transfer error %x!\n", priv->msg_err);
  if (priv->msg_err & XLP9XX_I2C_INTEN_BUSERR)
  xlp9xx_i2c_init(priv);
- return -EIO;
+ return (priv->msg_err & XLP9XX_I2C_INTEN_NACKADDR) ?
+ -ENXIO : -EIO;
  }
 
  if (timeleft == 0) {
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/4][Bionic][SRU Artful] i2c: xlp9xx: Handle transactions with I2C_M_RECV_LEN properly

dann frazier-4
In reply to this post by dann frazier-4
From: George Cherian <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1762812

In case of transaction with I2C_M_RECV_LEN set, make sure the driver reads
the first byte and then updates the RX fifo with the expected length. Set
threshold to 1 byte so that driver gets an interrupt on receiving the first byte.
After which the transfer length is updated depending on the received length.
Also report SMBus block read functionality.

Signed-off-by: George Cherian <[hidden email]>
Tested-by: dann frazier <[hidden email]>
Signed-off-by: Wolfram Sang <[hidden email]>
(cherry picked from commit 41b1d4de96323e84c0a902e7e4b2c0f367e77f92)
Signed-off-by: dann frazier <[hidden email]>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 35 ++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index 6d78cdc5cf91..1f6d78087af9 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -125,7 +125,16 @@ static void xlp9xx_i2c_update_rx_fifo_thres(struct xlp9xx_i2c_dev *priv)
 {
  u32 thres;
 
- thres = min(priv->msg_buf_remaining, XLP9XX_I2C_FIFO_SIZE);
+ if (priv->len_recv)
+ /* interrupt after the first read to examine
+ * the length byte before proceeding further
+ */
+ thres = 1;
+ else if (priv->msg_buf_remaining > XLP9XX_I2C_FIFO_SIZE)
+ thres = XLP9XX_I2C_FIFO_SIZE;
+ else
+ thres = priv->msg_buf_remaining;
+
  xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_MFIFOCTRL,
      thres << XLP9XX_I2C_MFIFOCTRL_HITH_SHIFT);
 }
@@ -144,7 +153,7 @@ static void xlp9xx_i2c_fill_tx_fifo(struct xlp9xx_i2c_dev *priv)
 
 static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
 {
- u32 len, i;
+ u32 len, i, val;
  u8 rlen, *buf = priv->msg_buf;
 
  len = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_FIFOWCNT) &
@@ -156,19 +165,27 @@ static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
  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;
- }
 
- len = min(priv->msg_buf_remaining, len);
- for (i = 0; i < len; i++, buf++)
- *buf = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
+ /* 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);
+ } else {
+ len = min(priv->msg_buf_remaining, len);
+ for (i = 0; i < len; i++, buf++)
+ *buf = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
+
+ priv->msg_buf_remaining -= len;
+ }
 
- priv->msg_buf_remaining -= len;
  priv->msg_buf = buf;
 
  if (priv->msg_buf_remaining)
@@ -357,8 +374,8 @@ static int xlp9xx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 
 static u32 xlp9xx_i2c_functionality(struct i2c_adapter *adapter)
 {
- return I2C_FUNC_SMBUS_EMUL | I2C_FUNC_I2C |
- I2C_FUNC_10BIT_ADDR;
+ return I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_READ_BLOCK_DATA |
+ I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR;
 }
 
 static const struct i2c_algorithm xlp9xx_i2c_algo = {
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/4][Bionic][SRU Artful] i2c: xlp9xx: Check for Bus state before every transfer

dann frazier-4
In reply to this post by dann frazier-4
From: George Cherian <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1762812

I2C bus enters the STOP condition after the DATA_DONE interrupt is raised.
Essentially the driver should be checking the bus state before sending
any transaction. In case a transaction is initiated while the
bus is busy, the prior transaction's stop condition is not achieved.
Add the check to make sure the bus is not busy before every transaction.

Signed-off-by: George Cherian <[hidden email]>
Reviewed-by: Jan Glauber <[hidden email]>
Signed-off-by: Wolfram Sang <[hidden email]>
(cherry picked from commit d3898a78521cd383d287b3ed5683f914c48c3be9)
Signed-off-by: dann frazier <[hidden email]>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index 1f6d78087af9..42dd1fa0b644 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/delay.h>
 
 #define XLP9XX_I2C_DIV 0x0
 #define XLP9XX_I2C_CTRL 0x1
@@ -36,6 +37,8 @@
 #define XLP9XX_I2C_TIMEOUT 0X10
 #define XLP9XX_I2C_GENCALLADDR 0x11
 
+#define XLP9XX_I2C_STATUS_BUSY BIT(0)
+
 #define XLP9XX_I2C_CMD_START BIT(7)
 #define XLP9XX_I2C_CMD_STOP BIT(6)
 #define XLP9XX_I2C_CMD_READ BIT(5)
@@ -71,6 +74,7 @@
 #define XLP9XX_I2C_HIGH_FREQ 400000
 #define XLP9XX_I2C_FIFO_SIZE 0x80U
 #define XLP9XX_I2C_TIMEOUT_MS 1000
+#define XLP9XX_I2C_BUSY_TIMEOUT 50
 
 #define XLP9XX_I2C_FIFO_WCNT_MASK 0xff
 #define XLP9XX_I2C_STATUS_ERRMASK (XLP9XX_I2C_INTEN_ARLOST | \
@@ -241,6 +245,26 @@ static irqreturn_t xlp9xx_i2c_isr(int irq, void *dev_id)
  return IRQ_HANDLED;
 }
 
+static int xlp9xx_i2c_check_bus_status(struct xlp9xx_i2c_dev *priv)
+{
+ u32 status;
+ u32 busy_timeout = XLP9XX_I2C_BUSY_TIMEOUT;
+
+ while (busy_timeout) {
+ status = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_STATUS);
+ if ((status & XLP9XX_I2C_STATUS_BUSY) == 0)
+ break;
+
+ busy_timeout--;
+ usleep_range(1000, 1100);
+ }
+
+ if (!busy_timeout)
+ return -EIO;
+
+ return 0;
+}
+
 static int xlp9xx_i2c_init(struct xlp9xx_i2c_dev *priv)
 {
  u32 prescale;
@@ -363,6 +387,14 @@ static int xlp9xx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
  int i, ret;
  struct xlp9xx_i2c_dev *priv = i2c_get_adapdata(adap);
 
+ ret = xlp9xx_i2c_check_bus_status(priv);
+ if (ret) {
+ xlp9xx_i2c_init(priv);
+ ret = xlp9xx_i2c_check_bus_status(priv);
+ if (ret)
+ return ret;
+ }
+
  for (i = 0; i < num; i++) {
  ret = xlp9xx_i2c_xfer_msg(priv, &msgs[i], i == num - 1);
  if (ret != 0)
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/4][Bionic][SRU Artful] i2c: xlp9xx: Handle NACK on DATA properly

dann frazier-4
In reply to this post by dann frazier-4
From: George Cherian <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1762812

In case we receive NACK on DATA we shouldn't be resetting the controller,
rather we should issue STOP command. This will terminate the current
transaction and -EIO is returned.

While at that handle the SMBus Quick Command properly.
We shouldn't be setting the XLP9XX_I2C_CMD_READ/WRITE for such
transactions.

Signed-off-by: George Cherian <[hidden email]>
Reviewed-by: Jan Glauber <[hidden email]>
Signed-off-by: Wolfram Sang <[hidden email]>
(cherry picked from commit e349d7d08e7044caf37a36409305edbd5af013c7)
Signed-off-by: dann frazier <[hidden email]>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index 42dd1fa0b644..eb8913eba0c5 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -352,7 +352,9 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, struct i2c_msg *msg,
 
  /* set cmd reg */
  cmd = XLP9XX_I2C_CMD_START;
- cmd |= (priv->msg_read ? XLP9XX_I2C_CMD_READ : XLP9XX_I2C_CMD_WRITE);
+ if (msg->len)
+ cmd |= (priv->msg_read ?
+ XLP9XX_I2C_CMD_READ : XLP9XX_I2C_CMD_WRITE);
  if (last_msg)
  cmd |= XLP9XX_I2C_CMD_STOP;
 
@@ -361,12 +363,12 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, struct i2c_msg *msg,
  timeleft = msecs_to_jiffies(XLP9XX_I2C_TIMEOUT_MS);
  timeleft = wait_for_completion_timeout(&priv->msg_complete, timeleft);
 
- if (priv->msg_err) {
+ if (priv->msg_err & XLP9XX_I2C_INTEN_BUSERR) {
  dev_dbg(priv->dev, "transfer error %x!\n", priv->msg_err);
- if (priv->msg_err & XLP9XX_I2C_INTEN_BUSERR)
- xlp9xx_i2c_init(priv);
- return (priv->msg_err & XLP9XX_I2C_INTEN_NACKADDR) ?
- -ENXIO : -EIO;
+ xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CMD, XLP9XX_I2C_CMD_STOP);
+ return -EIO;
+ } else if (priv->msg_err & XLP9XX_I2C_INTEN_NACKADDR) {
+ return -ENXIO;
  }
 
  if (timeleft == 0) {
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK / APPLIED[bionic]: [PATCH 0/4][Bionic][SRU Artful] i2c: xlp9xx: driver stability fixes

Seth Forshee
In reply to this post by dann frazier-4
On Tue, Apr 10, 2018 at 01:29:33PM -0600, dann frazier wrote:
> BugLink: https://bugs.launchpad.net/bugs/1762812
>
> All clean cherry-picks from upstream.

Clean cherry-picks, limited scope.

Acked-by: Seth Forshee <[hidden email]>

Applied to bionic/master-next and unstable/master, thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK[Artful]: [PATCH 0/4][Bionic][SRU Artful] i2c: xlp9xx: driver stability fixes

Kleber Sacilotto de Souza
In reply to this post by dann frazier-4
On 04/10/18 21:29, dann frazier wrote:

> BugLink: https://bugs.launchpad.net/bugs/1762812
>
> All clean cherry-picks from upstream.
>
> Dmitry Bazhenov (1):
>   i2c: xlp9xx: return ENXIO on slave address NACK
>
> George Cherian (3):
>   i2c: xlp9xx: Handle transactions with I2C_M_RECV_LEN properly
>   i2c: xlp9xx: Check for Bus state before every transfer
>   i2c: xlp9xx: Handle NACK on DATA properly
>
>  drivers/i2c/busses/i2c-xlp9xx.c | 78 +++++++++++++++++++++++++++------
>  1 file changed, 65 insertions(+), 13 deletions(-)
>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

Clean cherry-picks, limited to a platform-specific driver.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[A]: [PATCH 0/4][Bionic][SRU Artful] i2c: xlp9xx: driver stability fixes

Stefan Bader-2
In reply to this post by dann frazier-4
On 10.04.2018 21:29, dann frazier wrote:

> BugLink: https://bugs.launchpad.net/bugs/1762812
>
> All clean cherry-picks from upstream.
>
> Dmitry Bazhenov (1):
>   i2c: xlp9xx: return ENXIO on slave address NACK
>
> George Cherian (3):
>   i2c: xlp9xx: Handle transactions with I2C_M_RECV_LEN properly
>   i2c: xlp9xx: Check for Bus state before every transfer
>   i2c: xlp9xx: Handle NACK on DATA properly
>
>  drivers/i2c/busses/i2c-xlp9xx.c | 78 +++++++++++++++++++++++++++------
>  1 file changed, 65 insertions(+), 13 deletions(-)
>
Applied to artful/master-next.


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment