diff mbox series

[RFC,v2,5/7] i2c: busses: omap: Add the master_xfer_irqless hook

Message ID 20190302134735.4393-6-wsa+renesas@sang-engineering.com (mailing list archive)
State RFC
Headers show
Series i2c: core: introduce atomic transfers | expand

Commit Message

Wolfram Sang March 2, 2019, 1:47 p.m. UTC 
From: Tero Kristo <t-kristo@ti.com>

Add the master_xfer_irqless hook to enable i2c transactions
in irq disabled contexts like the poweroff case.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Signed-off-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-omap.c | 79 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 66 insertions(+), 13 deletions(-)

Comments

Simon Horman March 15, 2019, 12:47 p.m. UTC | #1 
On Sat, Mar 02, 2019 at 02:47:33PM +0100, Wolfram Sang wrote:
> From: Tero Kristo <t-kristo@ti.com>
> 
> Add the master_xfer_irqless hook to enable i2c transactions
> in irq disabled contexts like the poweroff case.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Minor nit below not withstanding

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>


> ---
>  drivers/i2c/busses/i2c-omap.c | 79 ++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 66 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index cd9c65f3d404..bf48126faf94 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -269,6 +269,8 @@ static const u8 reg_map_ip_v2[] = {
>  	[OMAP_I2C_IP_V2_IRQENABLE_CLR] = 0x30,
>  };
>  
> +static int omap_i2c_xfer_data(struct omap_i2c_dev *omap);
> +
>  static inline void omap_i2c_write_reg(struct omap_i2c_dev *omap,
>  				      int reg, u16 val)
>  {
> @@ -648,15 +650,28 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *omap, u8 size, bool is_rx)
>  			(1000 * omap->speed / 8);
>  }
>  
> +static void omap_i2c_wait(struct omap_i2c_dev *omap)
> +{
> +	u16 stat;
> +	u16 mask = omap_i2c_read_reg(omap, OMAP_I2C_IE_REG);
> +	int count = 0;
> +
> +	do {
> +		stat = omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG);
> +		count++;
> +	} while (!(stat & mask) && count < 5);
> +}
> +
>  /*
>   * Low level master read/write transaction.
>   */
>  static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> -			     struct i2c_msg *msg, int stop)
> +			     struct i2c_msg *msg, int stop, bool polling)
>  {
>  	struct omap_i2c_dev *omap = i2c_get_adapdata(adap);
>  	unsigned long timeout;
>  	u16 w;
> +	int ret;
>  
>  	dev_dbg(omap->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
>  		msg->addr, msg->len, msg->flags, stop);
> @@ -680,7 +695,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>  	w |= OMAP_I2C_BUF_RXFIF_CLR | OMAP_I2C_BUF_TXFIF_CLR;
>  	omap_i2c_write_reg(omap, OMAP_I2C_BUF_REG, w);
>  
> -	reinit_completion(&omap->cmd_complete);
> +	if (!polling)
> +		reinit_completion(&omap->cmd_complete);
>  	omap->cmd_err = 0;
>  
>  	w = OMAP_I2C_CON_EN | OMAP_I2C_CON_MST | OMAP_I2C_CON_STT;
> @@ -732,8 +748,21 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>  	 * REVISIT: We should abort the transfer on signals, but the bus goes
>  	 * into arbitration and we're currently unable to recover from it.
>  	 */
> -	timeout = wait_for_completion_timeout(&omap->cmd_complete,
> -						OMAP_I2C_TIMEOUT);
> +	if (!polling) {
> +		timeout = wait_for_completion_timeout(&omap->cmd_complete,
> +						      OMAP_I2C_TIMEOUT);
> +	} else {
> +		do {
> +			omap_i2c_wait(omap);
> +			ret = omap_i2c_xfer_data(omap);
> +		} while (ret == -EAGAIN);
> +
> +		if (!ret)
> +			timeout = 1;
> +		else
> +			timeout = 0;


nit: the following might be cleaner

		ret = !!timeout;

> +	}
> +
>  	if (timeout == 0) {
>  		dev_err(omap->dev, "controller timed out\n");
>  		omap_i2c_reset(omap);
> @@ -772,7 +801,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>   * to do the work during IRQ processing.
>   */
>  static int
> -omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> +omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msgs[], int num,
> +		     bool polling)
>  {
>  	struct omap_i2c_dev *omap = i2c_get_adapdata(adap);
>  	int i;
> @@ -794,7 +824,8 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  		omap->set_mpu_wkup_lat(omap->dev, omap->latency);
>  
>  	for (i = 0; i < num; i++) {
> -		r = omap_i2c_xfer_msg(adap, &msgs[i], (i == (num - 1)));
> +		r = omap_i2c_xfer_msg(adap, &msgs[i], (i == (num - 1)),
> +				      polling);
>  		if (r != 0)
>  			break;
>  	}
> @@ -813,6 +844,18 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
>  	return r;
>  }
>  
> +static int
> +omap_i2c_xfer_irq(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> +{
> +	return omap_i2c_xfer_common(adap, msgs, num, false);
> +}
> +
> +static int
> +omap_i2c_xfer_polling(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> +{
> +	return omap_i2c_xfer_common(adap, msgs, num, true);
> +}
> +
>  static u32
>  omap_i2c_func(struct i2c_adapter *adap)
>  {
> @@ -1035,10 +1078,8 @@ omap_i2c_isr(int irq, void *dev_id)
>  	return ret;
>  }
>  
> -static irqreturn_t
> -omap_i2c_isr_thread(int this_irq, void *dev_id)
> +static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
>  {
> -	struct omap_i2c_dev *omap = dev_id;
>  	u16 bits;
>  	u16 stat;
>  	int err = 0, count = 0;
> @@ -1056,7 +1097,8 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
>  
>  		if (!stat) {
>  			/* my work here is done */
> -			goto out;
> +			err = -EAGAIN;
> +			break;
>  		}
>  
>  		dev_dbg(omap->dev, "IRQ (ISR = 0x%04x)\n", stat);
> @@ -1165,14 +1207,25 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
>  		}
>  	} while (stat);
>  
> -	omap_i2c_complete_cmd(omap, err);
> +	return err;
> +}
> +
> +static irqreturn_t
> +omap_i2c_isr_thread(int this_irq, void *dev_id)
> +{
> +	int ret;
> +	struct omap_i2c_dev *omap = dev_id;
> +
> +	ret = omap_i2c_xfer_data(omap);
> +	if (ret != -EAGAIN)
> +		omap_i2c_complete_cmd(omap, ret);
>  
> -out:
>  	return IRQ_HANDLED;
>  }
>  
>  static const struct i2c_algorithm omap_i2c_algo = {
> -	.master_xfer	= omap_i2c_xfer,
> +	.master_xfer	= omap_i2c_xfer_irq,
> +	.master_xfer_atomic	= omap_i2c_xfer_polling,
>  	.functionality	= omap_i2c_func,
>  };
>  
> -- 
> 2.11.0
>
Andy Shevchenko March 15, 2019, 1:14 p.m. UTC | #2 
On Fri, Mar 15, 2019 at 01:47:18PM +0100, Simon Horman wrote:
> On Sat, Mar 02, 2019 at 02:47:33PM +0100, Wolfram Sang wrote:

> > +	if (!polling) {
> > +		timeout = wait_for_completion_timeout(&omap->cmd_complete,
> > +						      OMAP_I2C_TIMEOUT);
> > +	} else {
> > +		do {
> > +			omap_i2c_wait(omap);
> > +			ret = omap_i2c_xfer_data(omap);
> > +		} while (ret == -EAGAIN);
> > +
> > +		if (!ret)
> > +			timeout = 1;
> > +		else
> > +			timeout = 0;
> 
> 
> nit: the following might be cleaner
> 
> 		ret = !!timeout;
> 

Other way around, perhaps,

      timeout = !ret;

> > +	}
Wolfram Sang March 27, 2019, 1:50 p.m. UTC | #3 
> > > +		if (!ret)
> > > +			timeout = 1;
> > > +		else
> > > +			timeout = 0;
> > 
> > 
> > nit: the following might be cleaner
> > 
> > 		ret = !!timeout;
> > 
> 
> Other way around, perhaps,
> 
>       timeout = !ret;

I saw that but didn't want to modify this patch. But yes, this is easy
enough and looks a lot better.
Simon Horman March 29, 2019, 9:45 a.m. UTC | #4 
On Wed, Mar 27, 2019 at 02:50:45PM +0100, Wolfram Sang wrote:
> 
> > > > +		if (!ret)
> > > > +			timeout = 1;
> > > > +		else
> > > > +			timeout = 0;
> > > 
> > > 
> > > nit: the following might be cleaner
> > > 
> > > 		ret = !!timeout;
> > > 
> > 
> > Other way around, perhaps,
> > 
> >       timeout = !ret;
> 
> I saw that but didn't want to modify this patch. But yes, this is easy
> enough and looks a lot better.

Thanks, I don't think this needs to block progress.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index cd9c65f3d404..bf48126faf94 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -269,6 +269,8 @@  static const u8 reg_map_ip_v2[] = {
 	[OMAP_I2C_IP_V2_IRQENABLE_CLR] = 0x30,
 };
 
+static int omap_i2c_xfer_data(struct omap_i2c_dev *omap);
+
 static inline void omap_i2c_write_reg(struct omap_i2c_dev *omap,
 				      int reg, u16 val)
 {
@@ -648,15 +650,28 @@  static void omap_i2c_resize_fifo(struct omap_i2c_dev *omap, u8 size, bool is_rx)
 			(1000 * omap->speed / 8);
 }
 
+static void omap_i2c_wait(struct omap_i2c_dev *omap)
+{
+	u16 stat;
+	u16 mask = omap_i2c_read_reg(omap, OMAP_I2C_IE_REG);
+	int count = 0;
+
+	do {
+		stat = omap_i2c_read_reg(omap, OMAP_I2C_STAT_REG);
+		count++;
+	} while (!(stat & mask) && count < 5);
+}
+
 /*
  * Low level master read/write transaction.
  */
 static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
-			     struct i2c_msg *msg, int stop)
+			     struct i2c_msg *msg, int stop, bool polling)
 {
 	struct omap_i2c_dev *omap = i2c_get_adapdata(adap);
 	unsigned long timeout;
 	u16 w;
+	int ret;
 
 	dev_dbg(omap->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n",
 		msg->addr, msg->len, msg->flags, stop);
@@ -680,7 +695,8 @@  static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	w |= OMAP_I2C_BUF_RXFIF_CLR | OMAP_I2C_BUF_TXFIF_CLR;
 	omap_i2c_write_reg(omap, OMAP_I2C_BUF_REG, w);
 
-	reinit_completion(&omap->cmd_complete);
+	if (!polling)
+		reinit_completion(&omap->cmd_complete);
 	omap->cmd_err = 0;
 
 	w = OMAP_I2C_CON_EN | OMAP_I2C_CON_MST | OMAP_I2C_CON_STT;
@@ -732,8 +748,21 @@  static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	 * REVISIT: We should abort the transfer on signals, but the bus goes
 	 * into arbitration and we're currently unable to recover from it.
 	 */
-	timeout = wait_for_completion_timeout(&omap->cmd_complete,
-						OMAP_I2C_TIMEOUT);
+	if (!polling) {
+		timeout = wait_for_completion_timeout(&omap->cmd_complete,
+						      OMAP_I2C_TIMEOUT);
+	} else {
+		do {
+			omap_i2c_wait(omap);
+			ret = omap_i2c_xfer_data(omap);
+		} while (ret == -EAGAIN);
+
+		if (!ret)
+			timeout = 1;
+		else
+			timeout = 0;
+	}
+
 	if (timeout == 0) {
 		dev_err(omap->dev, "controller timed out\n");
 		omap_i2c_reset(omap);
@@ -772,7 +801,8 @@  static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
  * to do the work during IRQ processing.
  */
 static int
-omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
+omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msgs[], int num,
+		     bool polling)
 {
 	struct omap_i2c_dev *omap = i2c_get_adapdata(adap);
 	int i;
@@ -794,7 +824,8 @@  omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		omap->set_mpu_wkup_lat(omap->dev, omap->latency);
 
 	for (i = 0; i < num; i++) {
-		r = omap_i2c_xfer_msg(adap, &msgs[i], (i == (num - 1)));
+		r = omap_i2c_xfer_msg(adap, &msgs[i], (i == (num - 1)),
+				      polling);
 		if (r != 0)
 			break;
 	}
@@ -813,6 +844,18 @@  omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	return r;
 }
 
+static int
+omap_i2c_xfer_irq(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
+{
+	return omap_i2c_xfer_common(adap, msgs, num, false);
+}
+
+static int
+omap_i2c_xfer_polling(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
+{
+	return omap_i2c_xfer_common(adap, msgs, num, true);
+}
+
 static u32
 omap_i2c_func(struct i2c_adapter *adap)
 {
@@ -1035,10 +1078,8 @@  omap_i2c_isr(int irq, void *dev_id)
 	return ret;
 }
 
-static irqreturn_t
-omap_i2c_isr_thread(int this_irq, void *dev_id)
+static int omap_i2c_xfer_data(struct omap_i2c_dev *omap)
 {
-	struct omap_i2c_dev *omap = dev_id;
 	u16 bits;
 	u16 stat;
 	int err = 0, count = 0;
@@ -1056,7 +1097,8 @@  omap_i2c_isr_thread(int this_irq, void *dev_id)
 
 		if (!stat) {
 			/* my work here is done */
-			goto out;
+			err = -EAGAIN;
+			break;
 		}
 
 		dev_dbg(omap->dev, "IRQ (ISR = 0x%04x)\n", stat);
@@ -1165,14 +1207,25 @@  omap_i2c_isr_thread(int this_irq, void *dev_id)
 		}
 	} while (stat);
 
-	omap_i2c_complete_cmd(omap, err);
+	return err;
+}
+
+static irqreturn_t
+omap_i2c_isr_thread(int this_irq, void *dev_id)
+{
+	int ret;
+	struct omap_i2c_dev *omap = dev_id;
+
+	ret = omap_i2c_xfer_data(omap);
+	if (ret != -EAGAIN)
+		omap_i2c_complete_cmd(omap, ret);
 
-out:
 	return IRQ_HANDLED;
 }
 
 static const struct i2c_algorithm omap_i2c_algo = {
-	.master_xfer	= omap_i2c_xfer,
+	.master_xfer	= omap_i2c_xfer_irq,
+	.master_xfer_atomic	= omap_i2c_xfer_polling,
 	.functionality	= omap_i2c_func,
 };