* [PATCH v0 0/4] Use complete() instead of complete_all()
@ 2016-08-03 12:03 Daniel Wagner
2016-08-03 12:03 ` [PATCH v0 1/4] i2c: bcm-iproc: " Daniel Wagner
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Daniel Wagner @ 2016-08-03 12:03 UTC (permalink / raw)
To: Wolfram Sang
Cc: Beniamino Galvani, Kamal Dasu, Ray Jui, Tim Kryger,
linux-amlogic, linux-arm-kernel, linux-i2c,
bcm-kernel-feedback-list, linux-kernel, Daniel Wagner
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
Hi,
Using complete_all() is not wrong per se but it suggest that there
might be more than one reader. For -rt I am reviewing all
complete_all() users and would like to leave only the real ones in the
tree. The main problem for -rt about complete_all() is that it can be
uses inside IRQ context and that can lead to unbounded amount work
inside the interrupt handler. That is a no no for -rt.
The patches grouped per subsystem and in small batches to allow
reviewing. Unfortanatly I am not so good in coming up with unique
commit message, so please bear with me in that regard. I could also
squash them together, although each patch containts a very short
reasoning why there is only one waiter. Let me know what you rather
prefer. One patch which updates all complete_all() users or those 4
patches with some reasoning.
It is only test compiled because I don't have the all the hardware.
cheers,
daniel
Daniel Wagner (4):
i2c: bcm-iproc: Use complete() instead of complete_all()
i2c: bcm-kona: Use complete() instead of complete_all()
i2c: brcmstb: Use complete() instead of complete_all()
i2c: meson: Use complete() instead of complete_all()
drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
drivers/i2c/busses/i2c-bcm-kona.c | 2 +-
drivers/i2c/busses/i2c-brcmstb.c | 2 +-
drivers/i2c/busses/i2c-meson.c | 6 +++---
4 files changed, 6 insertions(+), 6 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v0 1/4] i2c: bcm-iproc: Use complete() instead of complete_all()
2016-08-03 12:03 [PATCH v0 0/4] Use complete() instead of complete_all() Daniel Wagner
@ 2016-08-03 12:03 ` Daniel Wagner
2016-08-03 16:36 ` Ray Jui
2016-08-03 12:03 ` [PATCH v0 2/4] i2c: bcm-kona: " Daniel Wagner
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Daniel Wagner @ 2016-08-03 12:03 UTC (permalink / raw)
To: Wolfram Sang
Cc: Beniamino Galvani, Kamal Dasu, Ray Jui, Tim Kryger,
linux-amlogic, linux-arm-kernel, linux-i2c,
bcm-kernel-feedback-list, linux-kernel, Daniel Wagner
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
There is only one waiter for the completion, therefore there
is no need to use complete_all(). Let's make that clear by
using complete() instead of complete_all().
The usage pattern of the completion is:
bcm_iproc_i2c_xfer_single_msg()
reinit_completion()
...
(activate the transfer)
...
wait_for_completion_timeout()
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index 19c8438..95f7cac 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -158,7 +158,7 @@ static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
if (status & BIT(IS_M_START_BUSY_SHIFT)) {
iproc_i2c->xfer_is_done = 1;
- complete_all(&iproc_i2c->done);
+ complete(&iproc_i2c->done);
}
writel(status, iproc_i2c->base + IS_OFFSET);
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v0 2/4] i2c: bcm-kona: Use complete() instead of complete_all()
2016-08-03 12:03 [PATCH v0 0/4] Use complete() instead of complete_all() Daniel Wagner
2016-08-03 12:03 ` [PATCH v0 1/4] i2c: bcm-iproc: " Daniel Wagner
@ 2016-08-03 12:03 ` Daniel Wagner
2016-08-03 16:38 ` Ray Jui
2016-08-05 4:16 ` Tim Kryger
2016-08-03 12:03 ` [PATCH v0 3/4] i2c: brcmstb: " Daniel Wagner
2016-08-03 12:03 ` [PATCH v0 4/4] i2c: meson: " Daniel Wagner
3 siblings, 2 replies; 10+ messages in thread
From: Daniel Wagner @ 2016-08-03 12:03 UTC (permalink / raw)
To: Wolfram Sang
Cc: Beniamino Galvani, Kamal Dasu, Ray Jui, Tim Kryger,
linux-amlogic, linux-arm-kernel, linux-i2c,
bcm-kernel-feedback-list, linux-kernel, Daniel Wagner
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
There is only one waiter for the completion, therefore there
is no need to use complete_all(). Let's make that clear by
using complete() instead of complete_all().
The usage pattern of the completion is:
bcm_kona_send_i2c_cmd()
reinit_completion()
...
bcm_kona_i2c_send_cmd_to_ctrl()
...
wait_for_completion_timeout()
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
drivers/i2c/busses/i2c-bcm-kona.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-bcm-kona.c b/drivers/i2c/busses/i2c-bcm-kona.c
index ac9f476..f987432 100644
--- a/drivers/i2c/busses/i2c-bcm-kona.c
+++ b/drivers/i2c/busses/i2c-bcm-kona.c
@@ -229,7 +229,7 @@ static irqreturn_t bcm_kona_i2c_isr(int irq, void *devid)
dev->base + TXFCR_OFFSET);
writel(status & ~ISR_RESERVED_MASK, dev->base + ISR_OFFSET);
- complete_all(&dev->done);
+ complete(&dev->done);
return IRQ_HANDLED;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v0 3/4] i2c: brcmstb: Use complete() instead of complete_all()
2016-08-03 12:03 [PATCH v0 0/4] Use complete() instead of complete_all() Daniel Wagner
2016-08-03 12:03 ` [PATCH v0 1/4] i2c: bcm-iproc: " Daniel Wagner
2016-08-03 12:03 ` [PATCH v0 2/4] i2c: bcm-kona: " Daniel Wagner
@ 2016-08-03 12:03 ` Daniel Wagner
2016-08-03 16:00 ` Kamal Dasu
2016-08-03 12:03 ` [PATCH v0 4/4] i2c: meson: " Daniel Wagner
3 siblings, 1 reply; 10+ messages in thread
From: Daniel Wagner @ 2016-08-03 12:03 UTC (permalink / raw)
To: Wolfram Sang
Cc: Beniamino Galvani, Kamal Dasu, Ray Jui, Tim Kryger,
linux-amlogic, linux-arm-kernel, linux-i2c,
bcm-kernel-feedback-list, linux-kernel, Daniel Wagner
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
There is only one waiter for the completion, therefore there
is no need to use complete_all(). Let's make that clear by
using complete() instead of complete_all().
The usage pattern of the completion is:
brcmstb_send_i2c_cmd()
reinit_completion()
...
/* initiate transfer by setting iic_enable */
...
brcmstb_i2c_wait_for_completion()
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
drivers/i2c/busses/i2c-brcmstb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c
index 6a8cfc1..ab4ff96 100644
--- a/drivers/i2c/busses/i2c-brcmstb.c
+++ b/drivers/i2c/busses/i2c-brcmstb.c
@@ -228,7 +228,7 @@ static irqreturn_t brcmstb_i2c_isr(int irq, void *devid)
return IRQ_NONE;
brcmstb_i2c_enable_disable_irq(dev, INT_DISABLE);
- complete_all(&dev->done);
+ complete(&dev->done);
dev_dbg(dev->device, "isr handled");
return IRQ_HANDLED;
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v0 4/4] i2c: meson: Use complete() instead of complete_all()
2016-08-03 12:03 [PATCH v0 0/4] Use complete() instead of complete_all() Daniel Wagner
` (2 preceding siblings ...)
2016-08-03 12:03 ` [PATCH v0 3/4] i2c: brcmstb: " Daniel Wagner
@ 2016-08-03 12:03 ` Daniel Wagner
2016-08-16 13:12 ` Kevin Hilman
3 siblings, 1 reply; 10+ messages in thread
From: Daniel Wagner @ 2016-08-03 12:03 UTC (permalink / raw)
To: Wolfram Sang
Cc: Beniamino Galvani, Kamal Dasu, Ray Jui, Tim Kryger,
linux-amlogic, linux-arm-kernel, linux-i2c,
bcm-kernel-feedback-list, linux-kernel, Daniel Wagner
From: Daniel Wagner <daniel.wagner@bmw-carit.de>
There is only one waiter for the completion, therefore there
is no need to use complete_all(). Let's make that clear by
using complete() instead of complete_all().
The usage pattern of the completion is:
meson_i2c_xfer_msg()
reinit_completion()
...
/* Start the transfer */
...
wait_for_completion_timeout()
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
drivers/i2c/busses/i2c-meson.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 71d3929..76e2898 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -211,7 +211,7 @@ static void meson_i2c_stop(struct meson_i2c *i2c)
meson_i2c_add_token(i2c, TOKEN_STOP);
} else {
i2c->state = STATE_IDLE;
- complete_all(&i2c->done);
+ complete(&i2c->done);
}
}
@@ -238,7 +238,7 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
dev_dbg(i2c->dev, "error bit set\n");
i2c->error = -ENXIO;
i2c->state = STATE_IDLE;
- complete_all(&i2c->done);
+ complete(&i2c->done);
goto out;
}
@@ -269,7 +269,7 @@ static irqreturn_t meson_i2c_irq(int irqno, void *dev_id)
break;
case STATE_STOP:
i2c->state = STATE_IDLE;
- complete_all(&i2c->done);
+ complete(&i2c->done);
break;
case STATE_IDLE:
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v0 3/4] i2c: brcmstb: Use complete() instead of complete_all()
2016-08-03 12:03 ` [PATCH v0 3/4] i2c: brcmstb: " Daniel Wagner
@ 2016-08-03 16:00 ` Kamal Dasu
0 siblings, 0 replies; 10+ messages in thread
From: Kamal Dasu @ 2016-08-03 16:00 UTC (permalink / raw)
To: Daniel Wagner
Cc: Wolfram Sang, Beniamino Galvani, Ray Jui, Tim Kryger,
linux-amlogic, linux-arm-kernel, linux-i2c,
Jayachandran C <jchandra@broadcom.com>,
bcm-kernel-feedback-list, linux-kernel, Daniel Wagner
LGTM
On Wed, Aug 3, 2016 at 8:03 AM, Daniel Wagner <wagi@monom.org> wrote:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> There is only one waiter for the completion, therefore there
> is no need to use complete_all(). Let's make that clear by
> using complete() instead of complete_all().
>
> The usage pattern of the completion is:
>
> brcmstb_send_i2c_cmd()
> reinit_completion()
> ...
> /* initiate transfer by setting iic_enable */
> ...
> brcmstb_i2c_wait_for_completion()
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reviewed-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
> drivers/i2c/busses/i2c-brcmstb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c
> index 6a8cfc1..ab4ff96 100644
> --- a/drivers/i2c/busses/i2c-brcmstb.c
> +++ b/drivers/i2c/busses/i2c-brcmstb.c
> @@ -228,7 +228,7 @@ static irqreturn_t brcmstb_i2c_isr(int irq, void *devid)
> return IRQ_NONE;
>
> brcmstb_i2c_enable_disable_irq(dev, INT_DISABLE);
> - complete_all(&dev->done);
> + complete(&dev->done);
>
> dev_dbg(dev->device, "isr handled");
> return IRQ_HANDLED;
> --
> 2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v0 1/4] i2c: bcm-iproc: Use complete() instead of complete_all()
2016-08-03 12:03 ` [PATCH v0 1/4] i2c: bcm-iproc: " Daniel Wagner
@ 2016-08-03 16:36 ` Ray Jui
0 siblings, 0 replies; 10+ messages in thread
From: Ray Jui @ 2016-08-03 16:36 UTC (permalink / raw)
To: Daniel Wagner, Wolfram Sang
Cc: Beniamino Galvani, Kamal Dasu, Ray Jui, Tim Kryger,
linux-amlogic, linux-arm-kernel, linux-i2c,
bcm-kernel-feedback-list, linux-kernel, Daniel Wagner
Hi Daniel,
On 8/3/2016 5:03 AM, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> There is only one waiter for the completion, therefore there
> is no need to use complete_all(). Let's make that clear by
> using complete() instead of complete_all().
>
> The usage pattern of the completion is:
>
> bcm_iproc_i2c_xfer_single_msg()
> reinit_completion()
> ...
> (activate the transfer)
> ...
> wait_for_completion_timeout()
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> ---
> drivers/i2c/busses/i2c-bcm-iproc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index 19c8438..95f7cac 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -158,7 +158,7 @@ static irqreturn_t bcm_iproc_i2c_isr(int irq, void *data)
>
> if (status & BIT(IS_M_START_BUSY_SHIFT)) {
> iproc_i2c->xfer_is_done = 1;
> - complete_all(&iproc_i2c->done);
> + complete(&iproc_i2c->done);
> }
>
> writel(status, iproc_i2c->base + IS_OFFSET);
>
Yes each time there's only one waiter invoked from the main transfer
routine 'bcm_iproc_i2c_xfer_single_msg'.
This change looks good to me, thanks!
Acked-by: Ray Jui <ray.jui@broadcom.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v0 2/4] i2c: bcm-kona: Use complete() instead of complete_all()
2016-08-03 12:03 ` [PATCH v0 2/4] i2c: bcm-kona: " Daniel Wagner
@ 2016-08-03 16:38 ` Ray Jui
2016-08-05 4:16 ` Tim Kryger
1 sibling, 0 replies; 10+ messages in thread
From: Ray Jui @ 2016-08-03 16:38 UTC (permalink / raw)
To: Daniel Wagner, Wolfram Sang
Cc: Beniamino Galvani, Kamal Dasu, Ray Jui, Tim Kryger,
linux-amlogic, linux-arm-kernel, linux-i2c,
bcm-kernel-feedback-list, linux-kernel, Daniel Wagner
Hi Daniel,
On 8/3/2016 5:03 AM, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> There is only one waiter for the completion, therefore there
> is no need to use complete_all(). Let's make that clear by
> using complete() instead of complete_all().
>
> The usage pattern of the completion is:
>
> bcm_kona_send_i2c_cmd()
> reinit_completion()
> ...
> bcm_kona_i2c_send_cmd_to_ctrl()
> ...
> wait_for_completion_timeout()
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> ---
> drivers/i2c/busses/i2c-bcm-kona.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-kona.c b/drivers/i2c/busses/i2c-bcm-kona.c
> index ac9f476..f987432 100644
> --- a/drivers/i2c/busses/i2c-bcm-kona.c
> +++ b/drivers/i2c/busses/i2c-bcm-kona.c
> @@ -229,7 +229,7 @@ static irqreturn_t bcm_kona_i2c_isr(int irq, void *devid)
> dev->base + TXFCR_OFFSET);
>
> writel(status & ~ISR_RESERVED_MASK, dev->base + ISR_OFFSET);
> - complete_all(&dev->done);
> + complete(&dev->done);
>
> return IRQ_HANDLED;
> }
>
This change looks good me! Thanks.
Acked-by: Ray Jui <ray.jui@broadcom.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v0 2/4] i2c: bcm-kona: Use complete() instead of complete_all()
2016-08-03 12:03 ` [PATCH v0 2/4] i2c: bcm-kona: " Daniel Wagner
2016-08-03 16:38 ` Ray Jui
@ 2016-08-05 4:16 ` Tim Kryger
1 sibling, 0 replies; 10+ messages in thread
From: Tim Kryger @ 2016-08-05 4:16 UTC (permalink / raw)
To: Daniel Wagner
Cc: Wolfram Sang, Kamal Dasu, Tim Kryger, Ray Jui, Daniel Wagner,
Linux Kernel Mailing List, Beniamino Galvani,
Broadcom Kernel Feedback, linux-i2c, linux-amlogic,
linux-arm-kernel
On Wed, Aug 3, 2016 at 5:03 AM, Daniel Wagner <wagi@monom.org> wrote:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> There is only one waiter for the completion, therefore there
> is no need to use complete_all(). Let's make that clear by
> using complete() instead of complete_all().
>
> The usage pattern of the completion is:
>
> bcm_kona_send_i2c_cmd()
> reinit_completion()
> ...
> bcm_kona_i2c_send_cmd_to_ctrl()
> ...
> wait_for_completion_timeout()
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reviewed-by: Tim Kryger <tim.kryger@gmail.com>
> ---
> drivers/i2c/busses/i2c-bcm-kona.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-kona.c b/drivers/i2c/busses/i2c-bcm-kona.c
> index ac9f476..f987432 100644
> --- a/drivers/i2c/busses/i2c-bcm-kona.c
> +++ b/drivers/i2c/busses/i2c-bcm-kona.c
> @@ -229,7 +229,7 @@ static irqreturn_t bcm_kona_i2c_isr(int irq, void *devid)
> dev->base + TXFCR_OFFSET);
>
> writel(status & ~ISR_RESERVED_MASK, dev->base + ISR_OFFSET);
> - complete_all(&dev->done);
> + complete(&dev->done);
>
> return IRQ_HANDLED;
> }
> --
> 2.7.4
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v0 4/4] i2c: meson: Use complete() instead of complete_all()
2016-08-03 12:03 ` [PATCH v0 4/4] i2c: meson: " Daniel Wagner
@ 2016-08-16 13:12 ` Kevin Hilman
0 siblings, 0 replies; 10+ messages in thread
From: Kevin Hilman @ 2016-08-16 13:12 UTC (permalink / raw)
To: Daniel Wagner
Cc: Wolfram Sang, Kamal Dasu, Tim Kryger, Ray Jui, Daniel Wagner,
linux-kernel, Beniamino Galvani, bcm-kernel-feedback-list,
linux-i2c, linux-amlogic, linux-arm-kernel
Daniel Wagner <wagi@monom.org> writes:
> From: Daniel Wagner <daniel.wagner@bmw-carit.de>
>
> There is only one waiter for the completion, therefore there
> is no need to use complete_all(). Let's make that clear by
> using complete() instead of complete_all().
>
> The usage pattern of the completion is:
>
> meson_i2c_xfer_msg()
> reinit_completion()
> ...
> /* Start the transfer */
> ...
> wait_for_completion_timeout()
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Kevin Hilman <khilman@baylibre.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-08-16 13:12 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-03 12:03 [PATCH v0 0/4] Use complete() instead of complete_all() Daniel Wagner
2016-08-03 12:03 ` [PATCH v0 1/4] i2c: bcm-iproc: " Daniel Wagner
2016-08-03 16:36 ` Ray Jui
2016-08-03 12:03 ` [PATCH v0 2/4] i2c: bcm-kona: " Daniel Wagner
2016-08-03 16:38 ` Ray Jui
2016-08-05 4:16 ` Tim Kryger
2016-08-03 12:03 ` [PATCH v0 3/4] i2c: brcmstb: " Daniel Wagner
2016-08-03 16:00 ` Kamal Dasu
2016-08-03 12:03 ` [PATCH v0 4/4] i2c: meson: " Daniel Wagner
2016-08-16 13:12 ` Kevin Hilman
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).