| Message ID | 20230929113841.1272625-1-m.szyprowski@samsung.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: exynos5: add support for atomic transfers | expand |
Hi Marek, > @@ -178,6 +179,7 @@ struct exynos5_i2c { > unsigned int msg_ptr; > > unsigned int irq; > + unsigned int polled; Is this supposed to be called polling? > void __iomem *regs; > struct clk *clk; /* operating clock */ > @@ -711,6 +713,24 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop) > spin_unlock_irqrestore(&i2c->lock, flags); > } > > +static unsigned long exynos5_i2c_polled_irqs_timeout(struct exynos5_i2c *i2c, > + unsigned long timeout_ms) > +{ > + ktime_t start, now; > + > + start = now = ktime_get(); > + while (ktime_ms_delta(now, start) < timeout_ms && > + !((i2c->trans_done && (i2c->msg->len == i2c->msg_ptr)) || > + (i2c->state < 0))) { > + while (readl(i2c->regs + HSI2C_INT_ENABLE) & > + readl(i2c->regs + HSI2C_INT_STATUS)) > + exynos5_i2c_irq(i2c->irq, i2c); > + usleep_range(100, 200); > + now = ktime_get(); > + } > + return ktime_ms_delta(now, start) < timeout_ms; what are you returning here? Andi
On 30.09.2023 00:57, Andi Shyti wrote: >> @@ -178,6 +179,7 @@ struct exynos5_i2c { >> unsigned int msg_ptr; >> >> unsigned int irq; >> + unsigned int polled; > Is this supposed to be called polling? Yea, a bit better name. >> void __iomem *regs; >> struct clk *clk; /* operating clock */ >> @@ -711,6 +713,24 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop) >> spin_unlock_irqrestore(&i2c->lock, flags); >> } >> >> +static unsigned long exynos5_i2c_polled_irqs_timeout(struct exynos5_i2c *i2c, >> + unsigned long timeout_ms) >> +{ >> + ktime_t start, now; >> + >> + start = now = ktime_get(); >> + while (ktime_ms_delta(now, start) < timeout_ms && >> + !((i2c->trans_done && (i2c->msg->len == i2c->msg_ptr)) || >> + (i2c->state < 0))) { >> + while (readl(i2c->regs + HSI2C_INT_ENABLE) & >> + readl(i2c->regs + HSI2C_INT_STATUS)) >> + exynos5_i2c_irq(i2c->irq, i2c); >> + usleep_range(100, 200); >> + now = ktime_get(); >> + } >> + return ktime_ms_delta(now, start) < timeout_ms; > what are you returning here? Values similar to wait_for_completion_timeout(); 0 means timeout and non-zero that the waiting condition has been reached, please check how it is used in exynos5_i2c_xfer_msg(). Maybe the function should be named a bit different, but I had no good idea so far. Best regards
Hi Marek, > >> +static unsigned long exynos5_i2c_polled_irqs_timeout(struct exynos5_i2c *i2c, > >> + unsigned long timeout_ms) > >> +{ > >> + ktime_t start, now; > >> + > >> + start = now = ktime_get(); > >> + while (ktime_ms_delta(now, start) < timeout_ms && > >> + !((i2c->trans_done && (i2c->msg->len == i2c->msg_ptr)) || > >> + (i2c->state < 0))) { > >> + while (readl(i2c->regs + HSI2C_INT_ENABLE) & > >> + readl(i2c->regs + HSI2C_INT_STATUS)) > >> + exynos5_i2c_irq(i2c->irq, i2c); > >> + usleep_range(100, 200); > >> + now = ktime_get(); > >> + } > >> + return ktime_ms_delta(now, start) < timeout_ms; > > what are you returning here? > > Values similar to wait_for_completion_timeout(); 0 means timeout and > non-zero that the waiting condition has been reached, please check how > it is used in exynos5_i2c_xfer_msg(). Maybe the function should be named > a bit different, but I had no good idea so far. but you are returning a boolean here. Make it a boolean then, no? It's true that the timeout is treated as a 1/0, but still it's a bit misleading. Andi
diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c index 2b0b9cdffa86..4b012d86d811 100644 --- a/drivers/i2c/busses/i2c-exynos5.c +++ b/drivers/i2c/busses/i2c-exynos5.c @@ -162,7 +162,8 @@ #define HSI2C_MASTER_ID(x) ((x & 0xff) << 24) #define MASTER_ID(x) ((x & 0x7) + 0x08) -#define EXYNOS5_I2C_TIMEOUT (msecs_to_jiffies(100)) +#define EXYNOS5_I2C_TIMEOUT_MS (100) +#define EXYNOS5_I2C_TIMEOUT (msecs_to_jiffies(EXYNOS5_I2C_TIMEOUT_MS)) enum i2c_type_exynos { I2C_TYPE_EXYNOS5, @@ -178,6 +179,7 @@ struct exynos5_i2c { unsigned int msg_ptr; unsigned int irq; + unsigned int polled; void __iomem *regs; struct clk *clk; /* operating clock */ @@ -711,6 +713,24 @@ static void exynos5_i2c_message_start(struct exynos5_i2c *i2c, int stop) spin_unlock_irqrestore(&i2c->lock, flags); } +static unsigned long exynos5_i2c_polled_irqs_timeout(struct exynos5_i2c *i2c, + unsigned long timeout_ms) +{ + ktime_t start, now; + + start = now = ktime_get(); + while (ktime_ms_delta(now, start) < timeout_ms && + !((i2c->trans_done && (i2c->msg->len == i2c->msg_ptr)) || + (i2c->state < 0))) { + while (readl(i2c->regs + HSI2C_INT_ENABLE) & + readl(i2c->regs + HSI2C_INT_STATUS)) + exynos5_i2c_irq(i2c->irq, i2c); + usleep_range(100, 200); + now = ktime_get(); + } + return ktime_ms_delta(now, start) < timeout_ms; +} + static int exynos5_i2c_xfer_msg(struct exynos5_i2c *i2c, struct i2c_msg *msgs, int stop) { @@ -725,8 +745,13 @@ static int exynos5_i2c_xfer_msg(struct exynos5_i2c *i2c, exynos5_i2c_message_start(i2c, stop); - timeout = wait_for_completion_timeout(&i2c->msg_complete, - EXYNOS5_I2C_TIMEOUT); + if (!i2c->polled) + timeout = wait_for_completion_timeout(&i2c->msg_complete, + EXYNOS5_I2C_TIMEOUT); + else + timeout = exynos5_i2c_polled_irqs_timeout(i2c, + EXYNOS5_I2C_TIMEOUT_MS); + if (timeout == 0) ret = -ETIMEDOUT; else @@ -777,6 +802,21 @@ static int exynos5_i2c_xfer(struct i2c_adapter *adap, return ret ?: num; } +static int exynos5_i2c_xfer_atomic(struct i2c_adapter *adap, + struct i2c_msg *msgs, int num) +{ + struct exynos5_i2c *i2c = adap->algo_data; + int ret; + + disable_irq(i2c->irq); + i2c->polled = true; + ret = exynos5_i2c_xfer(adap, msgs, num); + i2c->polled = false; + enable_irq(i2c->irq); + + return ret; +} + static u32 exynos5_i2c_func(struct i2c_adapter *adap) { return I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK); @@ -784,6 +824,7 @@ static u32 exynos5_i2c_func(struct i2c_adapter *adap) static const struct i2c_algorithm exynos5_i2c_algorithm = { .master_xfer = exynos5_i2c_xfer, + .master_xfer_atomic = exynos5_i2c_xfer_atomic, .functionality = exynos5_i2c_func, };
Add support for atomic transfers using polled mode with interrupts intentionally disabled. This removes the warning introduced by commit 63b96983a5dd ("i2c: core: introduce callbacks for atomic transfers") during system reboot and power off. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/i2c/busses/i2c-exynos5.c | 47 ++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-)