linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Improve Actions Semi Owl I2C driver
@ 2020-10-08 21:44 Cristian Ciocaltea
  2020-10-08 21:44 ` [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits Cristian Ciocaltea
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Cristian Ciocaltea @ 2020-10-08 21:44 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Andreas Färber, Wolfram Sang, Peter Rosin
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, linux-actions

This patchset brings a few improvements to the Actions Semiconductor
Owl I2C driver driver:

- Fixes an issue reported by Mani related to the error handling
- Adds support for atomic transfers
- Enables asynchronous probing, per Mani's suggestion

Please note the first two patches incorporate the review received for
the following patch (which became obsolete now):
https://lore.kernel.org/lkml/b6c56858854805b0f03e29b7dde40b20796d5c93.1599561278.git.cristian.ciocaltea@gmail.com/

Kind regards,
Cristi

Cristian Ciocaltea (3):
  i2c: owl: Clear NACK and BUS error bits
  i2c: owl: Add support for atomic transfers
  i2c: owl: Enable asynchronous probing

 drivers/i2c/busses/i2c-owl.c | 83 +++++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 20 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits
  2020-10-08 21:44 [PATCH 0/3] Improve Actions Semi Owl I2C driver Cristian Ciocaltea
@ 2020-10-08 21:44 ` Cristian Ciocaltea
  2020-10-10 11:16   ` Wolfram Sang
  2020-10-11 14:09   ` Manivannan Sadhasivam
  2020-10-08 21:44 ` [PATCH 2/3] i2c: owl: Add support for atomic transfers Cristian Ciocaltea
  2020-10-08 21:44 ` [PATCH 3/3] i2c: owl: Enable asynchronous probing Cristian Ciocaltea
  2 siblings, 2 replies; 13+ messages in thread
From: Cristian Ciocaltea @ 2020-10-08 21:44 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Andreas Färber, Wolfram Sang, Peter Rosin
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, linux-actions

When the NACK and BUS error bits are set by the hardware, the driver is
responsible for clearing them by writing "1" into the corresponding
status registers.

Hence perform the necessary operations in owl_i2c_interrupt().

Fixes: d211e62af466 ("i2c: Add Actions Semiconductor Owl family S900 I2C driver")
Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
---
 drivers/i2c/busses/i2c-owl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/i2c/busses/i2c-owl.c b/drivers/i2c/busses/i2c-owl.c
index 672f1f239bd6..a163b8f308c1 100644
--- a/drivers/i2c/busses/i2c-owl.c
+++ b/drivers/i2c/busses/i2c-owl.c
@@ -176,6 +176,9 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
 	fifostat = readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT);
 	if (fifostat & OWL_I2C_FIFOSTAT_RNB) {
 		i2c_dev->err = -ENXIO;
+		/* Clear NACK error bit by writing "1" */
+		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOSTAT,
+				   OWL_I2C_FIFOSTAT_RNB, true);
 		goto stop;
 	}
 
@@ -183,6 +186,9 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
 	stat = readl(i2c_dev->base + OWL_I2C_REG_STAT);
 	if (stat & OWL_I2C_STAT_BEB) {
 		i2c_dev->err = -EIO;
+		/* Clear BUS error bit by writing "1" */
+		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT,
+				   OWL_I2C_STAT_BEB, true);
 		goto stop;
 	}
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] i2c: owl: Add support for atomic transfers
  2020-10-08 21:44 [PATCH 0/3] Improve Actions Semi Owl I2C driver Cristian Ciocaltea
  2020-10-08 21:44 ` [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits Cristian Ciocaltea
@ 2020-10-08 21:44 ` Cristian Ciocaltea
  2020-10-11 14:09   ` Manivannan Sadhasivam
  2020-11-03 20:57   ` Wolfram Sang
  2020-10-08 21:44 ` [PATCH 3/3] i2c: owl: Enable asynchronous probing Cristian Ciocaltea
  2 siblings, 2 replies; 13+ messages in thread
From: Cristian Ciocaltea @ 2020-10-08 21:44 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Andreas Färber, Wolfram Sang, Peter Rosin
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, linux-actions

Atomic transfers are required to properly power off a machine through
an I2C controlled PMIC, such as the Actions Semi ATC260x series.

System shutdown may happen with interrupts being disabled and, as a
consequence, the kernel may hang if the driver does not support atomic
transfers.

This functionality is essentially implemented by polling the FIFO
Status register until either Command Execute Completed or NACK Error
bits are set.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
---
 drivers/i2c/busses/i2c-owl.c | 76 ++++++++++++++++++++++++++----------
 1 file changed, 56 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-owl.c b/drivers/i2c/busses/i2c-owl.c
index a163b8f308c1..547132768119 100644
--- a/drivers/i2c/busses/i2c-owl.c
+++ b/drivers/i2c/busses/i2c-owl.c
@@ -14,6 +14,7 @@
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 
@@ -76,6 +77,7 @@
 #define OWL_I2C_FIFOCTL_TFR	BIT(2)
 
 /* I2Cc_FIFOSTAT Bit Mask */
+#define OWL_I2C_FIFOSTAT_CECB	BIT(0)
 #define OWL_I2C_FIFOSTAT_RNB	BIT(1)
 #define OWL_I2C_FIFOSTAT_RFE	BIT(2)
 #define OWL_I2C_FIFOSTAT_TFF	BIT(5)
@@ -83,7 +85,8 @@
 #define OWL_I2C_FIFOSTAT_RFD	GENMASK(15, 8)
 
 /* I2C bus timeout */
-#define OWL_I2C_TIMEOUT		msecs_to_jiffies(4 * 1000)
+#define OWL_I2C_TIMEOUT_MS	(4 * 1000)
+#define OWL_I2C_TIMEOUT		msecs_to_jiffies(OWL_I2C_TIMEOUT_MS)
 
 #define OWL_I2C_MAX_RETRIES	50
 
@@ -161,15 +164,11 @@ static void owl_i2c_set_freq(struct owl_i2c_dev *i2c_dev)
 	writel(OWL_I2C_DIV_FACTOR(val), i2c_dev->base + OWL_I2C_REG_CLKDIV);
 }
 
-static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
+static void owl_i2c_xfer_data(struct owl_i2c_dev *i2c_dev)
 {
-	struct owl_i2c_dev *i2c_dev = _dev;
 	struct i2c_msg *msg = i2c_dev->msg;
-	unsigned long flags;
 	unsigned int stat, fifostat;
 
-	spin_lock_irqsave(&i2c_dev->lock, flags);
-
 	i2c_dev->err = 0;
 
 	/* Handle NACK from slave */
@@ -179,7 +178,7 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
 		/* Clear NACK error bit by writing "1" */
 		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOSTAT,
 				   OWL_I2C_FIFOSTAT_RNB, true);
-		goto stop;
+		return;
 	}
 
 	/* Handle bus error */
@@ -189,7 +188,7 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
 		/* Clear BUS error bit by writing "1" */
 		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT,
 				   OWL_I2C_STAT_BEB, true);
-		goto stop;
+		return;
 	}
 
 	/* Handle FIFO read */
@@ -207,13 +206,23 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
 			       i2c_dev->base + OWL_I2C_REG_TXDAT);
 		}
 	}
+}
+
+static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
+{
+	struct owl_i2c_dev *i2c_dev = _dev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&i2c_dev->lock, flags);
+
+	owl_i2c_xfer_data(i2c_dev);
 
-stop:
 	/* Clear pending interrupts */
 	owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT,
 			   OWL_I2C_STAT_IRQP, true);
 
 	complete_all(&i2c_dev->msg_complete);
+
 	spin_unlock_irqrestore(&i2c_dev->lock, flags);
 
 	return IRQ_HANDLED;
@@ -241,8 +250,8 @@ static int owl_i2c_check_bus_busy(struct i2c_adapter *adap)
 	return 0;
 }
 
-static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
-			       int num)
+static int owl_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			       int num, bool atomic)
 {
 	struct owl_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
 	struct i2c_msg *msg;
@@ -286,11 +295,12 @@ static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 		goto err_exit;
 	}
 
-	reinit_completion(&i2c_dev->msg_complete);
+	if (!atomic)
+		reinit_completion(&i2c_dev->msg_complete);
 
-	/* Enable I2C controller interrupt */
+	/* Enable/disable I2C controller interrupt */
 	owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
-			   OWL_I2C_CTL_IRQE, true);
+			   OWL_I2C_CTL_IRQE, !atomic);
 
 	/*
 	 * Select: FIFO enable, Master mode, Stop enable, Data count enable,
@@ -358,20 +368,33 @@ static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 
 	spin_unlock_irqrestore(&i2c_dev->lock, flags);
 
-	time_left = wait_for_completion_timeout(&i2c_dev->msg_complete,
-						adap->timeout);
+	if (atomic) {
+		/* Wait for Command Execute Completed or NACK Error bits */
+		ret = readl_poll_timeout_atomic(i2c_dev->base + OWL_I2C_REG_FIFOSTAT,
+						val, val & (OWL_I2C_FIFOSTAT_CECB |
+							    OWL_I2C_FIFOSTAT_RNB),
+						10, OWL_I2C_TIMEOUT_MS * 1000);
+	} else {
+		time_left = wait_for_completion_timeout(&i2c_dev->msg_complete,
+							adap->timeout);
+		if (!time_left)
+			ret = -ETIMEDOUT;
+	}
 
 	spin_lock_irqsave(&i2c_dev->lock, flags);
-	if (time_left == 0) {
+
+	if (ret) {
 		dev_err(&adap->dev, "Transaction timed out\n");
 		/* Send stop condition and release the bus */
 		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
 				   OWL_I2C_CTL_GBCC_STOP | OWL_I2C_CTL_RB,
 				   true);
-		ret = -ETIMEDOUT;
 		goto err_exit;
 	}
 
+	if (atomic)
+		owl_i2c_xfer_data(i2c_dev);
+
 	ret = i2c_dev->err < 0 ? i2c_dev->err : num;
 
 err_exit:
@@ -385,9 +408,22 @@ static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
 	return ret;
 }
 
+static int owl_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+			int num)
+{
+	return owl_i2c_xfer_common(adap, msgs, num, false);
+}
+
+static int owl_i2c_xfer_atomic(struct i2c_adapter *adap,
+			       struct i2c_msg *msgs, int num)
+{
+	return owl_i2c_xfer_common(adap, msgs, num, true);
+}
+
 static const struct i2c_algorithm owl_i2c_algorithm = {
-	.master_xfer    = owl_i2c_master_xfer,
-	.functionality  = owl_i2c_func,
+	.master_xfer	     = owl_i2c_xfer,
+	.master_xfer_atomic  = owl_i2c_xfer_atomic,
+	.functionality	     = owl_i2c_func,
 };
 
 static const struct i2c_adapter_quirks owl_i2c_quirks = {
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] i2c: owl: Enable asynchronous probing
  2020-10-08 21:44 [PATCH 0/3] Improve Actions Semi Owl I2C driver Cristian Ciocaltea
  2020-10-08 21:44 ` [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits Cristian Ciocaltea
  2020-10-08 21:44 ` [PATCH 2/3] i2c: owl: Add support for atomic transfers Cristian Ciocaltea
@ 2020-10-08 21:44 ` Cristian Ciocaltea
  2020-10-11 14:06   ` Manivannan Sadhasivam
  2020-11-03 20:57   ` Wolfram Sang
  2 siblings, 2 replies; 13+ messages in thread
From: Cristian Ciocaltea @ 2020-10-08 21:44 UTC (permalink / raw)
  To: Manivannan Sadhasivam, Andreas Färber, Wolfram Sang, Peter Rosin
  Cc: linux-i2c, linux-arm-kernel, linux-kernel, linux-actions

Speed up the boot process by using the asynchronous probing feature
supported by the recent kernels.

For SBCs based on the Actions Semi S500 SoC, the overall boot time is
expected to be reduced by 200-300 ms.

Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
---
 drivers/i2c/busses/i2c-owl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-owl.c b/drivers/i2c/busses/i2c-owl.c
index 547132768119..ed3942051845 100644
--- a/drivers/i2c/busses/i2c-owl.c
+++ b/drivers/i2c/busses/i2c-owl.c
@@ -521,6 +521,7 @@ static struct platform_driver owl_i2c_driver = {
 	.driver		= {
 		.name	= "owl-i2c",
 		.of_match_table = of_match_ptr(owl_i2c_of_match),
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	},
 };
 module_platform_driver(owl_i2c_driver);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits
  2020-10-08 21:44 ` [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits Cristian Ciocaltea
@ 2020-10-10 11:16   ` Wolfram Sang
  2020-10-11 14:09   ` Manivannan Sadhasivam
  1 sibling, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2020-10-10 11:16 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Manivannan Sadhasivam, Andreas Färber, Peter Rosin,
	linux-i2c, linux-arm-kernel, linux-kernel, linux-actions

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

On Fri, Oct 09, 2020 at 12:44:39AM +0300, Cristian Ciocaltea wrote:
> When the NACK and BUS error bits are set by the hardware, the driver is
> responsible for clearing them by writing "1" into the corresponding
> status registers.
> 
> Hence perform the necessary operations in owl_i2c_interrupt().
> 
> Fixes: d211e62af466 ("i2c: Add Actions Semiconductor Owl family S900 I2C driver")
> Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>

Applied to for-current, thanks!


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] i2c: owl: Enable asynchronous probing
  2020-10-08 21:44 ` [PATCH 3/3] i2c: owl: Enable asynchronous probing Cristian Ciocaltea
@ 2020-10-11 14:06   ` Manivannan Sadhasivam
  2020-10-12  9:08     ` Cristian Ciocaltea
  2020-11-03 20:57   ` Wolfram Sang
  1 sibling, 1 reply; 13+ messages in thread
From: Manivannan Sadhasivam @ 2020-10-11 14:06 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Andreas Färber, Wolfram Sang, Peter Rosin, linux-i2c,
	linux-arm-kernel, linux-kernel, linux-actions

On Fri, Oct 09, 2020 at 12:44:41AM +0300, Cristian Ciocaltea wrote:
> Speed up the boot process by using the asynchronous probing feature
> supported by the recent kernels.
> 
> For SBCs based on the Actions Semi S500 SoC, the overall boot time is
> expected to be reduced by 200-300 ms.
> 
> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/i2c/busses/i2c-owl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/i2c-owl.c b/drivers/i2c/busses/i2c-owl.c
> index 547132768119..ed3942051845 100644
> --- a/drivers/i2c/busses/i2c-owl.c
> +++ b/drivers/i2c/busses/i2c-owl.c
> @@ -521,6 +521,7 @@ static struct platform_driver owl_i2c_driver = {
>  	.driver		= {
>  		.name	= "owl-i2c",
>  		.of_match_table = of_match_ptr(owl_i2c_of_match),
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>  	},
>  };
>  module_platform_driver(owl_i2c_driver);
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] i2c: owl: Add support for atomic transfers
  2020-10-08 21:44 ` [PATCH 2/3] i2c: owl: Add support for atomic transfers Cristian Ciocaltea
@ 2020-10-11 14:09   ` Manivannan Sadhasivam
  2020-10-12  9:07     ` Cristian Ciocaltea
  2020-11-03 20:57   ` Wolfram Sang
  1 sibling, 1 reply; 13+ messages in thread
From: Manivannan Sadhasivam @ 2020-10-11 14:09 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Andreas Färber, Wolfram Sang, Peter Rosin, linux-i2c,
	linux-arm-kernel, linux-kernel, linux-actions

On Fri, Oct 09, 2020 at 12:44:40AM +0300, Cristian Ciocaltea wrote:
> Atomic transfers are required to properly power off a machine through
> an I2C controlled PMIC, such as the Actions Semi ATC260x series.
> 
> System shutdown may happen with interrupts being disabled and, as a
> consequence, the kernel may hang if the driver does not support atomic
> transfers.
> 
> This functionality is essentially implemented by polling the FIFO
> Status register until either Command Execute Completed or NACK Error
> bits are set.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/i2c/busses/i2c-owl.c | 76 ++++++++++++++++++++++++++----------
>  1 file changed, 56 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-owl.c b/drivers/i2c/busses/i2c-owl.c
> index a163b8f308c1..547132768119 100644
> --- a/drivers/i2c/busses/i2c-owl.c
> +++ b/drivers/i2c/busses/i2c-owl.c
> @@ -14,6 +14,7 @@
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> +#include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
>  
> @@ -76,6 +77,7 @@
>  #define OWL_I2C_FIFOCTL_TFR	BIT(2)
>  
>  /* I2Cc_FIFOSTAT Bit Mask */
> +#define OWL_I2C_FIFOSTAT_CECB	BIT(0)
>  #define OWL_I2C_FIFOSTAT_RNB	BIT(1)
>  #define OWL_I2C_FIFOSTAT_RFE	BIT(2)
>  #define OWL_I2C_FIFOSTAT_TFF	BIT(5)
> @@ -83,7 +85,8 @@
>  #define OWL_I2C_FIFOSTAT_RFD	GENMASK(15, 8)
>  
>  /* I2C bus timeout */
> -#define OWL_I2C_TIMEOUT		msecs_to_jiffies(4 * 1000)
> +#define OWL_I2C_TIMEOUT_MS	(4 * 1000)
> +#define OWL_I2C_TIMEOUT		msecs_to_jiffies(OWL_I2C_TIMEOUT_MS)
>  
>  #define OWL_I2C_MAX_RETRIES	50
>  
> @@ -161,15 +164,11 @@ static void owl_i2c_set_freq(struct owl_i2c_dev *i2c_dev)
>  	writel(OWL_I2C_DIV_FACTOR(val), i2c_dev->base + OWL_I2C_REG_CLKDIV);
>  }
>  
> -static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
> +static void owl_i2c_xfer_data(struct owl_i2c_dev *i2c_dev)
>  {
> -	struct owl_i2c_dev *i2c_dev = _dev;
>  	struct i2c_msg *msg = i2c_dev->msg;
> -	unsigned long flags;
>  	unsigned int stat, fifostat;
>  
> -	spin_lock_irqsave(&i2c_dev->lock, flags);
> -
>  	i2c_dev->err = 0;
>  
>  	/* Handle NACK from slave */
> @@ -179,7 +178,7 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
>  		/* Clear NACK error bit by writing "1" */
>  		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOSTAT,
>  				   OWL_I2C_FIFOSTAT_RNB, true);
> -		goto stop;
> +		return;
>  	}
>  
>  	/* Handle bus error */
> @@ -189,7 +188,7 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
>  		/* Clear BUS error bit by writing "1" */
>  		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT,
>  				   OWL_I2C_STAT_BEB, true);
> -		goto stop;
> +		return;
>  	}
>  
>  	/* Handle FIFO read */
> @@ -207,13 +206,23 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
>  			       i2c_dev->base + OWL_I2C_REG_TXDAT);
>  		}
>  	}
> +}
> +
> +static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
> +{
> +	struct owl_i2c_dev *i2c_dev = _dev;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&i2c_dev->lock, flags);
> +
> +	owl_i2c_xfer_data(i2c_dev);
>  
> -stop:
>  	/* Clear pending interrupts */
>  	owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT,
>  			   OWL_I2C_STAT_IRQP, true);
>  
>  	complete_all(&i2c_dev->msg_complete);
> +
>  	spin_unlock_irqrestore(&i2c_dev->lock, flags);
>  
>  	return IRQ_HANDLED;
> @@ -241,8 +250,8 @@ static int owl_i2c_check_bus_busy(struct i2c_adapter *adap)
>  	return 0;
>  }
>  
> -static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> -			       int num)
> +static int owl_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +			       int num, bool atomic)
>  {
>  	struct owl_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
>  	struct i2c_msg *msg;
> @@ -286,11 +295,12 @@ static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  		goto err_exit;
>  	}
>  
> -	reinit_completion(&i2c_dev->msg_complete);
> +	if (!atomic)
> +		reinit_completion(&i2c_dev->msg_complete);
>  
> -	/* Enable I2C controller interrupt */
> +	/* Enable/disable I2C controller interrupt */
>  	owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
> -			   OWL_I2C_CTL_IRQE, true);
> +			   OWL_I2C_CTL_IRQE, !atomic);
>  
>  	/*
>  	 * Select: FIFO enable, Master mode, Stop enable, Data count enable,
> @@ -358,20 +368,33 @@ static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  
>  	spin_unlock_irqrestore(&i2c_dev->lock, flags);
>  
> -	time_left = wait_for_completion_timeout(&i2c_dev->msg_complete,
> -						adap->timeout);
> +	if (atomic) {
> +		/* Wait for Command Execute Completed or NACK Error bits */
> +		ret = readl_poll_timeout_atomic(i2c_dev->base + OWL_I2C_REG_FIFOSTAT,
> +						val, val & (OWL_I2C_FIFOSTAT_CECB |
> +							    OWL_I2C_FIFOSTAT_RNB),
> +						10, OWL_I2C_TIMEOUT_MS * 1000);
> +	} else {
> +		time_left = wait_for_completion_timeout(&i2c_dev->msg_complete,
> +							adap->timeout);
> +		if (!time_left)
> +			ret = -ETIMEDOUT;
> +	}
>  
>  	spin_lock_irqsave(&i2c_dev->lock, flags);
> -	if (time_left == 0) {
> +
> +	if (ret) {
>  		dev_err(&adap->dev, "Transaction timed out\n");
>  		/* Send stop condition and release the bus */
>  		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_CTL,
>  				   OWL_I2C_CTL_GBCC_STOP | OWL_I2C_CTL_RB,
>  				   true);
> -		ret = -ETIMEDOUT;
>  		goto err_exit;
>  	}
>  
> +	if (atomic)
> +		owl_i2c_xfer_data(i2c_dev);
> +
>  	ret = i2c_dev->err < 0 ? i2c_dev->err : num;
>  
>  err_exit:
> @@ -385,9 +408,22 @@ static int owl_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  	return ret;
>  }
>  
> +static int owl_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +			int num)
> +{
> +	return owl_i2c_xfer_common(adap, msgs, num, false);
> +}
> +
> +static int owl_i2c_xfer_atomic(struct i2c_adapter *adap,
> +			       struct i2c_msg *msgs, int num)
> +{
> +	return owl_i2c_xfer_common(adap, msgs, num, true);
> +}
> +
>  static const struct i2c_algorithm owl_i2c_algorithm = {
> -	.master_xfer    = owl_i2c_master_xfer,
> -	.functionality  = owl_i2c_func,
> +	.master_xfer	     = owl_i2c_xfer,
> +	.master_xfer_atomic  = owl_i2c_xfer_atomic,
> +	.functionality	     = owl_i2c_func,
>  };
>  
>  static const struct i2c_adapter_quirks owl_i2c_quirks = {
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits
  2020-10-08 21:44 ` [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits Cristian Ciocaltea
  2020-10-10 11:16   ` Wolfram Sang
@ 2020-10-11 14:09   ` Manivannan Sadhasivam
  2020-10-12  9:05     ` Cristian Ciocaltea
  1 sibling, 1 reply; 13+ messages in thread
From: Manivannan Sadhasivam @ 2020-10-11 14:09 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Andreas Färber, Wolfram Sang, Peter Rosin, linux-i2c,
	linux-arm-kernel, linux-kernel, linux-actions

On Fri, Oct 09, 2020 at 12:44:39AM +0300, Cristian Ciocaltea wrote:
> When the NACK and BUS error bits are set by the hardware, the driver is
> responsible for clearing them by writing "1" into the corresponding
> status registers.
> 
> Hence perform the necessary operations in owl_i2c_interrupt().
> 
> Fixes: d211e62af466 ("i2c: Add Actions Semiconductor Owl family S900 I2C driver")
> Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/i2c/busses/i2c-owl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-owl.c b/drivers/i2c/busses/i2c-owl.c
> index 672f1f239bd6..a163b8f308c1 100644
> --- a/drivers/i2c/busses/i2c-owl.c
> +++ b/drivers/i2c/busses/i2c-owl.c
> @@ -176,6 +176,9 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
>  	fifostat = readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT);
>  	if (fifostat & OWL_I2C_FIFOSTAT_RNB) {
>  		i2c_dev->err = -ENXIO;
> +		/* Clear NACK error bit by writing "1" */
> +		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_FIFOSTAT,
> +				   OWL_I2C_FIFOSTAT_RNB, true);
>  		goto stop;
>  	}
>  
> @@ -183,6 +186,9 @@ static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
>  	stat = readl(i2c_dev->base + OWL_I2C_REG_STAT);
>  	if (stat & OWL_I2C_STAT_BEB) {
>  		i2c_dev->err = -EIO;
> +		/* Clear BUS error bit by writing "1" */
> +		owl_i2c_update_reg(i2c_dev->base + OWL_I2C_REG_STAT,
> +				   OWL_I2C_STAT_BEB, true);
>  		goto stop;
>  	}
>  
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits
  2020-10-11 14:09   ` Manivannan Sadhasivam
@ 2020-10-12  9:05     ` Cristian Ciocaltea
  0 siblings, 0 replies; 13+ messages in thread
From: Cristian Ciocaltea @ 2020-10-12  9:05 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Andreas Färber, Wolfram Sang, Peter Rosin, linux-i2c,
	linux-arm-kernel, linux-kernel, linux-actions

On Sun, Oct 11, 2020 at 07:39:48PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Oct 09, 2020 at 12:44:39AM +0300, Cristian Ciocaltea wrote:
> > When the NACK and BUS error bits are set by the hardware, the driver is
> > responsible for clearing them by writing "1" into the corresponding
> > status registers.
> > 
> > Hence perform the necessary operations in owl_i2c_interrupt().
> > 
> > Fixes: d211e62af466 ("i2c: Add Actions Semiconductor Owl family S900 I2C driver")
> > Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Thanks,
> Mani

Thanks for the review,
Cristi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] i2c: owl: Add support for atomic transfers
  2020-10-11 14:09   ` Manivannan Sadhasivam
@ 2020-10-12  9:07     ` Cristian Ciocaltea
  0 siblings, 0 replies; 13+ messages in thread
From: Cristian Ciocaltea @ 2020-10-12  9:07 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Andreas Färber, Wolfram Sang, Peter Rosin, linux-i2c,
	linux-arm-kernel, linux-kernel, linux-actions

On Sun, Oct 11, 2020 at 07:39:20PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Oct 09, 2020 at 12:44:40AM +0300, Cristian Ciocaltea wrote:
> > Atomic transfers are required to properly power off a machine through
> > an I2C controlled PMIC, such as the Actions Semi ATC260x series.
> > 
> > System shutdown may happen with interrupts being disabled and, as a
> > consequence, the kernel may hang if the driver does not support atomic
> > transfers.
> > 
> > This functionality is essentially implemented by polling the FIFO
> > Status register until either Command Execute Completed or NACK Error
> > bits are set.
> > 
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Thanks,
> Mani

Thanks for reviewing,
Cristi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] i2c: owl: Enable asynchronous probing
  2020-10-11 14:06   ` Manivannan Sadhasivam
@ 2020-10-12  9:08     ` Cristian Ciocaltea
  0 siblings, 0 replies; 13+ messages in thread
From: Cristian Ciocaltea @ 2020-10-12  9:08 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Andreas Färber, Wolfram Sang, Peter Rosin, linux-i2c,
	linux-arm-kernel, linux-kernel, linux-actions

On Sun, Oct 11, 2020 at 07:36:45PM +0530, Manivannan Sadhasivam wrote:
> On Fri, Oct 09, 2020 at 12:44:41AM +0300, Cristian Ciocaltea wrote:
> > Speed up the boot process by using the asynchronous probing feature
> > supported by the recent kernels.
> > 
> > For SBCs based on the Actions Semi S500 SoC, the overall boot time is
> > expected to be reduced by 200-300 ms.
> > 
> > Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Thanks,
> Mani

Thanks for reviewing,
Cristi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] i2c: owl: Add support for atomic transfers
  2020-10-08 21:44 ` [PATCH 2/3] i2c: owl: Add support for atomic transfers Cristian Ciocaltea
  2020-10-11 14:09   ` Manivannan Sadhasivam
@ 2020-11-03 20:57   ` Wolfram Sang
  1 sibling, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2020-11-03 20:57 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Manivannan Sadhasivam, Andreas Färber, Peter Rosin,
	linux-i2c, linux-arm-kernel, linux-kernel, linux-actions

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

On Fri, Oct 09, 2020 at 12:44:40AM +0300, Cristian Ciocaltea wrote:
> Atomic transfers are required to properly power off a machine through
> an I2C controlled PMIC, such as the Actions Semi ATC260x series.
> 
> System shutdown may happen with interrupts being disabled and, as a
> consequence, the kernel may hang if the driver does not support atomic
> transfers.
> 
> This functionality is essentially implemented by polling the FIFO
> Status register until either Command Execute Completed or NACK Error
> bits are set.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>

Applied to for-next, thanks!


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] i2c: owl: Enable asynchronous probing
  2020-10-08 21:44 ` [PATCH 3/3] i2c: owl: Enable asynchronous probing Cristian Ciocaltea
  2020-10-11 14:06   ` Manivannan Sadhasivam
@ 2020-11-03 20:57   ` Wolfram Sang
  1 sibling, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2020-11-03 20:57 UTC (permalink / raw)
  To: Cristian Ciocaltea
  Cc: Manivannan Sadhasivam, Andreas Färber, Peter Rosin,
	linux-i2c, linux-arm-kernel, linux-kernel, linux-actions

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

On Fri, Oct 09, 2020 at 12:44:41AM +0300, Cristian Ciocaltea wrote:
> Speed up the boot process by using the asynchronous probing feature
> supported by the recent kernels.
> 
> For SBCs based on the Actions Semi S500 SoC, the overall boot time is
> expected to be reduced by 200-300 ms.
> 
> Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@gmail.com>

Applied to for-next, thanks!


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-11-03 21:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-08 21:44 [PATCH 0/3] Improve Actions Semi Owl I2C driver Cristian Ciocaltea
2020-10-08 21:44 ` [PATCH 1/3] i2c: owl: Clear NACK and BUS error bits Cristian Ciocaltea
2020-10-10 11:16   ` Wolfram Sang
2020-10-11 14:09   ` Manivannan Sadhasivam
2020-10-12  9:05     ` Cristian Ciocaltea
2020-10-08 21:44 ` [PATCH 2/3] i2c: owl: Add support for atomic transfers Cristian Ciocaltea
2020-10-11 14:09   ` Manivannan Sadhasivam
2020-10-12  9:07     ` Cristian Ciocaltea
2020-11-03 20:57   ` Wolfram Sang
2020-10-08 21:44 ` [PATCH 3/3] i2c: owl: Enable asynchronous probing Cristian Ciocaltea
2020-10-11 14:06   ` Manivannan Sadhasivam
2020-10-12  9:08     ` Cristian Ciocaltea
2020-11-03 20:57   ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).