* [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts
@ 2020-12-18 17:53 Kevin Herbert
2020-12-19 14:47 ` Fabio Estevam
0 siblings, 1 reply; 7+ messages in thread
From: Kevin Herbert @ 2020-12-18 17:53 UTC (permalink / raw)
To: Biwen Li, Leo Li, linux, kernel, Wolfram Sang, shawnguo, s.hauer,
festevam, Aisheng Dong, Clark Wang, o.rempel, linux-i2c,
linux-kernel, Jiafei Pan, Xiaobo Xie, linux-arm-kernel, Biwen Li
From feaf638fb9b9a483c0d6090b277f34db21160885 Mon Sep 17 00:00:00 2001
From: Kevin Paul Herbert <kph@platinasystems.com>
Date: Tue, 15 Dec 2020 16:50:34 -0800
Subject: [PATCH] i2c-imx.c: Synthesize end of transaction events without idle
interrupts
Only the Layerscape SoCs have interrupts on bus idle, which facilitate
sending events which complete slave bus transactions.
Add support for synthesizing missing events. If we see a master request,
or a newly addressed slave request, if the last event sent to the backend
was I2C_SLAVE_READ_REQUESTED, send the backend a I2C_SLAVE_READ_PROCESSED
followed by I2C_SLAVE_STOP. For all other events, send an I2C_SLAVE_STOP.
Signed-off-by: Kevin Paul Herbert <kph@platinasystems.com>
---
drivers/i2c/busses/i2c-imx.c | 59 +++++++++++++++++++++++++++++++-----
1 file changed, 52 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 7137bae770ea..7255e1dabde4 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -211,6 +211,7 @@ struct imx_i2c_struct {
struct imx_i2c_dma *dma;
struct i2c_client *slave;
+ enum i2c_slave_event last_slave_event;
};
static const struct imx_i2c_hwdata imx1_i2c_hwdata = {
@@ -637,6 +638,36 @@ static void i2c_imx_enable_bus_idle(struct
imx_i2c_struct *i2c_imx)
}
}
+static void i2c_imx_slave_event(struct imx_i2c_struct *i2c_imx,
+ enum i2c_slave_event event, u8 *val)
+{
+ i2c_slave_event(i2c_imx->slave, event, val);
+ i2c_imx->last_slave_event = event;
+}
+
+static void i2c_imx_slave_finish_op(struct imx_i2c_struct *i2c_imx)
+{
+ u8 val;
+
+ while (i2c_imx->last_slave_event != I2C_SLAVE_STOP) {
+ switch (i2c_imx->last_slave_event) {
+ case I2C_SLAVE_READ_REQUESTED:
+ i2c_imx_slave_event(i2c_imx, I2C_SLAVE_READ_PROCESSED,
+ &val);
+ break;
+
+ case I2C_SLAVE_WRITE_REQUESTED:
+ case I2C_SLAVE_READ_PROCESSED:
+ case I2C_SLAVE_WRITE_RECEIVED:
+ i2c_imx_slave_event(i2c_imx, I2C_SLAVE_STOP, &val);
+ break;
+
+ case I2C_SLAVE_STOP:
+ break;
+ }
+ }
+}
+
static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
unsigned int status, unsigned int ctl)
{
@@ -649,9 +680,11 @@ static irqreturn_t i2c_imx_slave_isr(struct
imx_i2c_struct *i2c_imx,
}
if (status & I2SR_IAAS) { /* Addressed as a slave */
+ i2c_imx_slave_finish_op(i2c_imx);
if (status & I2SR_SRW) { /* Master wants to read from us*/
dev_dbg(&i2c_imx->adapter.dev, "read requested");
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_REQUESTED, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_READ_REQUESTED, &value);
/* Slave transmit */
ctl |= I2CR_MTX;
@@ -661,7 +694,8 @@ static irqreturn_t i2c_imx_slave_isr(struct
imx_i2c_struct *i2c_imx,
imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
} else { /* Master wants to write to us */
dev_dbg(&i2c_imx->adapter.dev, "write requested");
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_WRITE_REQUESTED, &value);
/* Slave receive */
ctl &= ~I2CR_MTX;
@@ -672,17 +706,20 @@ static irqreturn_t i2c_imx_slave_isr(struct
imx_i2c_struct *i2c_imx,
} else if (!(ctl & I2CR_MTX)) { /* Receive mode */
if (status & I2SR_IBB) { /* No STOP signal detected */
value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_WRITE_RECEIVED, &value);
} else { /* STOP signal is detected */
dev_dbg(&i2c_imx->adapter.dev,
"STOP signal detected");
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_STOP, &value);
}
} else if (!(status & I2SR_RXAK)) { /* Transmit mode received ACK */
ctl |= I2CR_MTX;
imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_PROCESSED, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_READ_PROCESSED, &value);
imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
} else { /* Transmit mode received NAK */
@@ -723,6 +760,7 @@ static int i2c_imx_reg_slave(struct i2c_client *client)
return -EBUSY;
i2c_imx->slave = client;
+ i2c_imx->last_slave_event = I2C_SLAVE_STOP;
/* Resume */
ret = pm_runtime_get_sync(i2c_imx->adapter.dev.parent);
@@ -775,10 +813,17 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+
if (status & I2SR_IIF) {
i2c_imx_clear_irq(i2c_imx, I2SR_IIF);
- if (i2c_imx->slave && !(ctl & I2CR_MSTA))
- return i2c_imx_slave_isr(i2c_imx, status, ctl);
+ if (i2c_imx->slave) {
+ if (!(ctl & I2CR_MSTA)) {
+ return i2c_imx_slave_isr(i2c_imx, status, ctl);
+ } else if (i2c_imx->last_slave_event !=
+ I2C_SLAVE_STOP) {
+ i2c_imx_slave_finish_op(i2c_imx);
+ }
+ }
return i2c_imx_master_isr(i2c_imx, status);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts
2020-12-18 17:53 [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts Kevin Herbert
@ 2020-12-19 14:47 ` Fabio Estevam
0 siblings, 0 replies; 7+ messages in thread
From: Fabio Estevam @ 2020-12-19 14:47 UTC (permalink / raw)
To: Kevin Herbert
Cc: Biwen Li, Leo Li, Oleksij Rempel, Sascha Hauer, Wolfram Sang,
Shawn Guo, Sascha Hauer, Aisheng Dong, Clark Wang,
Oleksij Rempel, linux-i2c, linux-kernel, Jiafei Pan, Xiaobo Xie,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Biwen Li
Hi Kevin,
On Fri, Dec 18, 2020 at 2:53 PM Kevin Herbert <kph@platinasystems.com> wrote:
>
> From feaf638fb9b9a483c0d6090b277f34db21160885 Mon Sep 17 00:00:00 2001
> From: Kevin Paul Herbert <kph@platinasystems.com>
> Date: Tue, 15 Dec 2020 16:50:34 -0800
> Subject: [PATCH] i2c-imx.c: Synthesize end of transaction events without idle
> interrupts
Your patch is corrupted. Please use 'git send-mail' and submit it again.
Thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts
2020-12-22 19:48 Kevin Paul Herbert
2021-01-08 8:05 ` Oleksij Rempel
@ 2021-02-01 22:13 ` Wolfram Sang
1 sibling, 0 replies; 7+ messages in thread
From: Wolfram Sang @ 2021-02-01 22:13 UTC (permalink / raw)
To: Kevin Paul Herbert
Cc: biwen.li, leoyang.li, linux, kernel, shawnguo, s.hauer, festevam,
aisheng.dong, xiaoning.wang, o.rempel, linux-i2c, linux-kernel,
jiafei.pan, xiaobo.xie, linux-arm-kernel, biwen.li
[-- Attachment #1: Type: text/plain, Size: 611 bytes --]
On Tue, Dec 22, 2020 at 11:48:50AM -0800, Kevin Paul Herbert wrote:
> Only the Layerscape SoCs have interrupts on bus idle, which facilitate
> sending events which complete slave bus transactions.
>
> Add support for synthesizing missing events. If we see a master request,
> or a newly addressed slave request, if the last event sent to the backend
> was I2C_SLAVE_READ_REQUESTED, send the backend a I2C_SLAVE_READ_PROCESSED
> followed by I2C_SLAVE_STOP. For all other events, send an I2C_SLAVE_STOP.
>
> Signed-off-by: Kevin Paul Herbert <kph@platinasystems.com>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts
2021-01-15 1:27 ` Kevin Herbert
@ 2021-01-28 9:20 ` Wolfram Sang
0 siblings, 0 replies; 7+ messages in thread
From: Wolfram Sang @ 2021-01-28 9:20 UTC (permalink / raw)
To: Kevin Herbert
Cc: Oleksij Rempel, Biwen Li, Leo Li, linux, kernel, shawnguo,
s.hauer, festevam, Aisheng Dong, Clark Wang, linux-i2c,
linux-kernel, Jiafei Pan, Xiaobo Xie, linux-arm-kernel, Biwen Li
[-- Attachment #1: Type: text/plain, Size: 571 bytes --]
On Thu, Jan 14, 2021 at 05:27:11PM -0800, Kevin Herbert wrote:
> The loop sends either one or two events to the slave driver. If the
> state is I2C_SLAVE_READ_REQUESTED, we synthesize the
> I2C_SLAVE_READ_PROCESSED event, and then our state becomes
> I2C_SLAVE_READ_PROCESSED. In all other states, we transition to
> I2C_SLAVE_STOP and exit the loop.
>
> It is not a busy loop at all. It is calling the callback handler (that
> already expects to be in IRQ context) one or two times, and then it
> exits the loop.
Oleksij, does this answer your question?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts
2021-01-08 8:05 ` Oleksij Rempel
@ 2021-01-15 1:27 ` Kevin Herbert
2021-01-28 9:20 ` Wolfram Sang
0 siblings, 1 reply; 7+ messages in thread
From: Kevin Herbert @ 2021-01-15 1:27 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Biwen Li, Leo Li, linux, kernel, Wolfram Sang, shawnguo, s.hauer,
festevam, Aisheng Dong, Clark Wang, linux-i2c, linux-kernel,
Jiafei Pan, Xiaobo Xie, linux-arm-kernel, Biwen Li
The loop sends either one or two events to the slave driver. If the
state is I2C_SLAVE_READ_REQUESTED, we synthesize the
I2C_SLAVE_READ_PROCESSED event, and then our state becomes
I2C_SLAVE_READ_PROCESSED. In all other states, we transition to
I2C_SLAVE_STOP and exit the loop.
It is not a busy loop at all. It is calling the callback handler (that
already expects to be in IRQ context) one or two times, and then it
exits the loop.
Kevin
On Fri, Jan 8, 2021 at 12:05 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
>
> Hi Kevin Paul,
>
> On Tue, Dec 22, 2020 at 11:48:50AM -0800, Kevin Paul Herbert wrote:
> > Only the Layerscape SoCs have interrupts on bus idle, which facilitate
> > sending events which complete slave bus transactions.
> >
> > Add support for synthesizing missing events. If we see a master request,
> > or a newly addressed slave request, if the last event sent to the backend
> > was I2C_SLAVE_READ_REQUESTED, send the backend a I2C_SLAVE_READ_PROCESSED
> > followed by I2C_SLAVE_STOP. For all other events, send an I2C_SLAVE_STOP.
> >
> > Signed-off-by: Kevin Paul Herbert <kph@platinasystems.com>
> > ---
> > drivers/i2c/busses/i2c-imx.c | 59 +++++++++++++++++++++++++++++++-----
> > 1 file changed, 52 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> > index b444fbf1a262..b3e2a6a7fc19 100644
> > --- a/drivers/i2c/busses/i2c-imx.c
> > +++ b/drivers/i2c/busses/i2c-imx.c
> > @@ -209,6 +209,7 @@ struct imx_i2c_struct {
> >
> > struct imx_i2c_dma *dma;
> > struct i2c_client *slave;
> > + enum i2c_slave_event last_slave_event;
> > };
> >
> > static const struct imx_i2c_hwdata imx1_i2c_hwdata = {
> > @@ -662,6 +663,36 @@ static void i2c_imx_enable_bus_idle(struct imx_i2c_struct *i2c_imx)
> > }
> > }
> >
> > +static void i2c_imx_slave_event(struct imx_i2c_struct *i2c_imx,
> > + enum i2c_slave_event event, u8 *val)
> > +{
> > + i2c_slave_event(i2c_imx->slave, event, val);
> > + i2c_imx->last_slave_event = event;
> > +}
> > +
> > +static void i2c_imx_slave_finish_op(struct imx_i2c_struct *i2c_imx)
> > +{
> > + u8 val;
> > +
> > + while (i2c_imx->last_slave_event != I2C_SLAVE_STOP) {
> > + switch (i2c_imx->last_slave_event) {
> > + case I2C_SLAVE_READ_REQUESTED:
> > + i2c_imx_slave_event(i2c_imx, I2C_SLAVE_READ_PROCESSED,
> > + &val);
> > + break;
> > +
> > + case I2C_SLAVE_WRITE_REQUESTED:
> > + case I2C_SLAVE_READ_PROCESSED:
> > + case I2C_SLAVE_WRITE_RECEIVED:
> > + i2c_imx_slave_event(i2c_imx, I2C_SLAVE_STOP, &val);
> > + break;
> > +
> > + case I2C_SLAVE_STOP:
> > + break;
> > + }
> > + }
> > +}
>
> Is it really working code?
>
> The i2c_imx_slave_finish_op() is a waiting busy loop withing an IRQ! And
> it is waiting for an event, which is updated in the same IRQ callback.
> Or am i missing some thing?
>
> Regards,
> Oleksij
>
> > static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
> > unsigned int status, unsigned int ctl)
> > {
> > @@ -674,9 +705,11 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
> > }
> >
> > if (status & I2SR_IAAS) { /* Addressed as a slave */
> > + i2c_imx_slave_finish_op(i2c_imx);
> > if (status & I2SR_SRW) { /* Master wants to read from us*/
> > dev_dbg(&i2c_imx->adapter.dev, "read requested");
> > - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_REQUESTED, &value);
> > + i2c_imx_slave_event(i2c_imx,
> > + I2C_SLAVE_READ_REQUESTED, &value);
> >
> > /* Slave transmit */
> > ctl |= I2CR_MTX;
> > @@ -686,7 +719,8 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
> > imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
> > } else { /* Master wants to write to us */
> > dev_dbg(&i2c_imx->adapter.dev, "write requested");
> > - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_WRITE_REQUESTED, &value);
> > + i2c_imx_slave_event(i2c_imx,
> > + I2C_SLAVE_WRITE_REQUESTED, &value);
> >
> > /* Slave receive */
> > ctl &= ~I2CR_MTX;
> > @@ -697,17 +731,20 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
> > } else if (!(ctl & I2CR_MTX)) { /* Receive mode */
> > if (status & I2SR_IBB) { /* No STOP signal detected */
> > value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> > - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_WRITE_RECEIVED, &value);
> > + i2c_imx_slave_event(i2c_imx,
> > + I2C_SLAVE_WRITE_RECEIVED, &value);
> > } else { /* STOP signal is detected */
> > dev_dbg(&i2c_imx->adapter.dev,
> > "STOP signal detected");
> > - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &value);
> > + i2c_imx_slave_event(i2c_imx,
> > + I2C_SLAVE_STOP, &value);
> > }
> > } else if (!(status & I2SR_RXAK)) { /* Transmit mode received ACK */
> > ctl |= I2CR_MTX;
> > imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
> >
> > - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_PROCESSED, &value);
> > + i2c_imx_slave_event(i2c_imx,
> > + I2C_SLAVE_READ_PROCESSED, &value);
> >
> > imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
> > } else { /* Transmit mode received NAK */
> > @@ -748,6 +785,7 @@ static int i2c_imx_reg_slave(struct i2c_client *client)
> > return -EBUSY;
> >
> > i2c_imx->slave = client;
> > + i2c_imx->last_slave_event = I2C_SLAVE_STOP;
> >
> > /* Resume */
> > ret = pm_runtime_get_sync(i2c_imx->adapter.dev.parent);
> > @@ -800,10 +838,17 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
> >
> > status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> > ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> > +
> > if (status & I2SR_IIF) {
> > i2c_imx_clear_irq(i2c_imx, I2SR_IIF);
> > - if (i2c_imx->slave && !(ctl & I2CR_MSTA))
> > - return i2c_imx_slave_isr(i2c_imx, status, ctl);
> > + if (i2c_imx->slave) {
> > + if (!(ctl & I2CR_MSTA)) {
> > + return i2c_imx_slave_isr(i2c_imx, status, ctl);
> > + } else if (i2c_imx->last_slave_event !=
> > + I2C_SLAVE_STOP) {
> > + i2c_imx_slave_finish_op(i2c_imx);
> > + }
> > + }
> > return i2c_imx_master_isr(i2c_imx, status);
> > }
> >
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts
2020-12-22 19:48 Kevin Paul Herbert
@ 2021-01-08 8:05 ` Oleksij Rempel
2021-01-15 1:27 ` Kevin Herbert
2021-02-01 22:13 ` Wolfram Sang
1 sibling, 1 reply; 7+ messages in thread
From: Oleksij Rempel @ 2021-01-08 8:05 UTC (permalink / raw)
To: Kevin Paul Herbert
Cc: biwen.li, leoyang.li, linux, kernel, wsa, shawnguo, s.hauer,
festevam, aisheng.dong, xiaoning.wang, linux-i2c, linux-kernel,
jiafei.pan, xiaobo.xie, linux-arm-kernel, biwen.li
Hi Kevin Paul,
On Tue, Dec 22, 2020 at 11:48:50AM -0800, Kevin Paul Herbert wrote:
> Only the Layerscape SoCs have interrupts on bus idle, which facilitate
> sending events which complete slave bus transactions.
>
> Add support for synthesizing missing events. If we see a master request,
> or a newly addressed slave request, if the last event sent to the backend
> was I2C_SLAVE_READ_REQUESTED, send the backend a I2C_SLAVE_READ_PROCESSED
> followed by I2C_SLAVE_STOP. For all other events, send an I2C_SLAVE_STOP.
>
> Signed-off-by: Kevin Paul Herbert <kph@platinasystems.com>
> ---
> drivers/i2c/busses/i2c-imx.c | 59 +++++++++++++++++++++++++++++++-----
> 1 file changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index b444fbf1a262..b3e2a6a7fc19 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -209,6 +209,7 @@ struct imx_i2c_struct {
>
> struct imx_i2c_dma *dma;
> struct i2c_client *slave;
> + enum i2c_slave_event last_slave_event;
> };
>
> static const struct imx_i2c_hwdata imx1_i2c_hwdata = {
> @@ -662,6 +663,36 @@ static void i2c_imx_enable_bus_idle(struct imx_i2c_struct *i2c_imx)
> }
> }
>
> +static void i2c_imx_slave_event(struct imx_i2c_struct *i2c_imx,
> + enum i2c_slave_event event, u8 *val)
> +{
> + i2c_slave_event(i2c_imx->slave, event, val);
> + i2c_imx->last_slave_event = event;
> +}
> +
> +static void i2c_imx_slave_finish_op(struct imx_i2c_struct *i2c_imx)
> +{
> + u8 val;
> +
> + while (i2c_imx->last_slave_event != I2C_SLAVE_STOP) {
> + switch (i2c_imx->last_slave_event) {
> + case I2C_SLAVE_READ_REQUESTED:
> + i2c_imx_slave_event(i2c_imx, I2C_SLAVE_READ_PROCESSED,
> + &val);
> + break;
> +
> + case I2C_SLAVE_WRITE_REQUESTED:
> + case I2C_SLAVE_READ_PROCESSED:
> + case I2C_SLAVE_WRITE_RECEIVED:
> + i2c_imx_slave_event(i2c_imx, I2C_SLAVE_STOP, &val);
> + break;
> +
> + case I2C_SLAVE_STOP:
> + break;
> + }
> + }
> +}
Is it really working code?
The i2c_imx_slave_finish_op() is a waiting busy loop withing an IRQ! And
it is waiting for an event, which is updated in the same IRQ callback.
Or am i missing some thing?
Regards,
Oleksij
> static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
> unsigned int status, unsigned int ctl)
> {
> @@ -674,9 +705,11 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
> }
>
> if (status & I2SR_IAAS) { /* Addressed as a slave */
> + i2c_imx_slave_finish_op(i2c_imx);
> if (status & I2SR_SRW) { /* Master wants to read from us*/
> dev_dbg(&i2c_imx->adapter.dev, "read requested");
> - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_REQUESTED, &value);
> + i2c_imx_slave_event(i2c_imx,
> + I2C_SLAVE_READ_REQUESTED, &value);
>
> /* Slave transmit */
> ctl |= I2CR_MTX;
> @@ -686,7 +719,8 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
> imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
> } else { /* Master wants to write to us */
> dev_dbg(&i2c_imx->adapter.dev, "write requested");
> - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_WRITE_REQUESTED, &value);
> + i2c_imx_slave_event(i2c_imx,
> + I2C_SLAVE_WRITE_REQUESTED, &value);
>
> /* Slave receive */
> ctl &= ~I2CR_MTX;
> @@ -697,17 +731,20 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
> } else if (!(ctl & I2CR_MTX)) { /* Receive mode */
> if (status & I2SR_IBB) { /* No STOP signal detected */
> value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
> - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_WRITE_RECEIVED, &value);
> + i2c_imx_slave_event(i2c_imx,
> + I2C_SLAVE_WRITE_RECEIVED, &value);
> } else { /* STOP signal is detected */
> dev_dbg(&i2c_imx->adapter.dev,
> "STOP signal detected");
> - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &value);
> + i2c_imx_slave_event(i2c_imx,
> + I2C_SLAVE_STOP, &value);
> }
> } else if (!(status & I2SR_RXAK)) { /* Transmit mode received ACK */
> ctl |= I2CR_MTX;
> imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
>
> - i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_PROCESSED, &value);
> + i2c_imx_slave_event(i2c_imx,
> + I2C_SLAVE_READ_PROCESSED, &value);
>
> imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
> } else { /* Transmit mode received NAK */
> @@ -748,6 +785,7 @@ static int i2c_imx_reg_slave(struct i2c_client *client)
> return -EBUSY;
>
> i2c_imx->slave = client;
> + i2c_imx->last_slave_event = I2C_SLAVE_STOP;
>
> /* Resume */
> ret = pm_runtime_get_sync(i2c_imx->adapter.dev.parent);
> @@ -800,10 +838,17 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
>
> status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
> ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> +
> if (status & I2SR_IIF) {
> i2c_imx_clear_irq(i2c_imx, I2SR_IIF);
> - if (i2c_imx->slave && !(ctl & I2CR_MSTA))
> - return i2c_imx_slave_isr(i2c_imx, status, ctl);
> + if (i2c_imx->slave) {
> + if (!(ctl & I2CR_MSTA)) {
> + return i2c_imx_slave_isr(i2c_imx, status, ctl);
> + } else if (i2c_imx->last_slave_event !=
> + I2C_SLAVE_STOP) {
> + i2c_imx_slave_finish_op(i2c_imx);
> + }
> + }
> return i2c_imx_master_isr(i2c_imx, status);
> }
>
> --
> 2.25.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts
@ 2020-12-22 19:48 Kevin Paul Herbert
2021-01-08 8:05 ` Oleksij Rempel
2021-02-01 22:13 ` Wolfram Sang
0 siblings, 2 replies; 7+ messages in thread
From: Kevin Paul Herbert @ 2020-12-22 19:48 UTC (permalink / raw)
To: biwen.li, leoyang.li, linux, kernel, wsa, shawnguo, s.hauer,
festevam, aisheng.dong, xiaoning.wang, o.rempel, linux-i2c,
linux-kernel, jiafei.pan, xiaobo.xie, linux-arm-kernel, biwen.li,
kph
Only the Layerscape SoCs have interrupts on bus idle, which facilitate
sending events which complete slave bus transactions.
Add support for synthesizing missing events. If we see a master request,
or a newly addressed slave request, if the last event sent to the backend
was I2C_SLAVE_READ_REQUESTED, send the backend a I2C_SLAVE_READ_PROCESSED
followed by I2C_SLAVE_STOP. For all other events, send an I2C_SLAVE_STOP.
Signed-off-by: Kevin Paul Herbert <kph@platinasystems.com>
---
drivers/i2c/busses/i2c-imx.c | 59 +++++++++++++++++++++++++++++++-----
1 file changed, 52 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index b444fbf1a262..b3e2a6a7fc19 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -209,6 +209,7 @@ struct imx_i2c_struct {
struct imx_i2c_dma *dma;
struct i2c_client *slave;
+ enum i2c_slave_event last_slave_event;
};
static const struct imx_i2c_hwdata imx1_i2c_hwdata = {
@@ -662,6 +663,36 @@ static void i2c_imx_enable_bus_idle(struct imx_i2c_struct *i2c_imx)
}
}
+static void i2c_imx_slave_event(struct imx_i2c_struct *i2c_imx,
+ enum i2c_slave_event event, u8 *val)
+{
+ i2c_slave_event(i2c_imx->slave, event, val);
+ i2c_imx->last_slave_event = event;
+}
+
+static void i2c_imx_slave_finish_op(struct imx_i2c_struct *i2c_imx)
+{
+ u8 val;
+
+ while (i2c_imx->last_slave_event != I2C_SLAVE_STOP) {
+ switch (i2c_imx->last_slave_event) {
+ case I2C_SLAVE_READ_REQUESTED:
+ i2c_imx_slave_event(i2c_imx, I2C_SLAVE_READ_PROCESSED,
+ &val);
+ break;
+
+ case I2C_SLAVE_WRITE_REQUESTED:
+ case I2C_SLAVE_READ_PROCESSED:
+ case I2C_SLAVE_WRITE_RECEIVED:
+ i2c_imx_slave_event(i2c_imx, I2C_SLAVE_STOP, &val);
+ break;
+
+ case I2C_SLAVE_STOP:
+ break;
+ }
+ }
+}
+
static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
unsigned int status, unsigned int ctl)
{
@@ -674,9 +705,11 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
}
if (status & I2SR_IAAS) { /* Addressed as a slave */
+ i2c_imx_slave_finish_op(i2c_imx);
if (status & I2SR_SRW) { /* Master wants to read from us*/
dev_dbg(&i2c_imx->adapter.dev, "read requested");
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_REQUESTED, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_READ_REQUESTED, &value);
/* Slave transmit */
ctl |= I2CR_MTX;
@@ -686,7 +719,8 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
} else { /* Master wants to write to us */
dev_dbg(&i2c_imx->adapter.dev, "write requested");
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_WRITE_REQUESTED, &value);
/* Slave receive */
ctl &= ~I2CR_MTX;
@@ -697,17 +731,20 @@ static irqreturn_t i2c_imx_slave_isr(struct imx_i2c_struct *i2c_imx,
} else if (!(ctl & I2CR_MTX)) { /* Receive mode */
if (status & I2SR_IBB) { /* No STOP signal detected */
value = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR);
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_WRITE_RECEIVED, &value);
} else { /* STOP signal is detected */
dev_dbg(&i2c_imx->adapter.dev,
"STOP signal detected");
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_STOP, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_STOP, &value);
}
} else if (!(status & I2SR_RXAK)) { /* Transmit mode received ACK */
ctl |= I2CR_MTX;
imx_i2c_write_reg(ctl, i2c_imx, IMX_I2C_I2CR);
- i2c_slave_event(i2c_imx->slave, I2C_SLAVE_READ_PROCESSED, &value);
+ i2c_imx_slave_event(i2c_imx,
+ I2C_SLAVE_READ_PROCESSED, &value);
imx_i2c_write_reg(value, i2c_imx, IMX_I2C_I2DR);
} else { /* Transmit mode received NAK */
@@ -748,6 +785,7 @@ static int i2c_imx_reg_slave(struct i2c_client *client)
return -EBUSY;
i2c_imx->slave = client;
+ i2c_imx->last_slave_event = I2C_SLAVE_STOP;
/* Resume */
ret = pm_runtime_get_sync(i2c_imx->adapter.dev.parent);
@@ -800,10 +838,17 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
ctl = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
+
if (status & I2SR_IIF) {
i2c_imx_clear_irq(i2c_imx, I2SR_IIF);
- if (i2c_imx->slave && !(ctl & I2CR_MSTA))
- return i2c_imx_slave_isr(i2c_imx, status, ctl);
+ if (i2c_imx->slave) {
+ if (!(ctl & I2CR_MSTA)) {
+ return i2c_imx_slave_isr(i2c_imx, status, ctl);
+ } else if (i2c_imx->last_slave_event !=
+ I2C_SLAVE_STOP) {
+ i2c_imx_slave_finish_op(i2c_imx);
+ }
+ }
return i2c_imx_master_isr(i2c_imx, status);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-01 22:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 17:53 [PATCH] i2c-imx.c: Synthesize end of transaction events without idle interrupts Kevin Herbert
2020-12-19 14:47 ` Fabio Estevam
2020-12-22 19:48 Kevin Paul Herbert
2021-01-08 8:05 ` Oleksij Rempel
2021-01-15 1:27 ` Kevin Herbert
2021-01-28 9:20 ` Wolfram Sang
2021-02-01 22:13 ` 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).