Add and helper macro for iterating over all DMAC channels, taking into
account the channel mask. Use it where appropriate, to simplify code.
Restore "reverse Christmas tree" order of local variables while adding a
new variable.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
v2:
- Put the full loop control of for_each_rcar_dmac_chan() on a single
line, to improve readability.
---
drivers/dma/sh/rcar-dmac.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index a57705356e8bb796..537550b4121bbc22 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -209,6 +209,10 @@ struct rcar_dmac {
#define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine)
+#define for_each_rcar_dmac_chan(i, chan, dmac) \
+ for (i = 0, chan = &(dmac)->channels[0]; i < (dmac)->n_channels; i++, chan++) \
+ if (!((dmac)->channels_mask & BIT(i))) continue; else
+
/*
* struct rcar_dmac_of_data - This driver's OF data
* @chan_offset_base: DMAC channels base offset
@@ -817,15 +821,11 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
static void rcar_dmac_stop_all_chan(struct rcar_dmac *dmac)
{
+ struct rcar_dmac_chan *chan;
unsigned int i;
/* Stop all channels. */
- for (i = 0; i < dmac->n_channels; ++i) {
- struct rcar_dmac_chan *chan = &dmac->channels[i];
-
- if (!(dmac->channels_mask & BIT(i)))
- continue;
-
+ for_each_rcar_dmac_chan(i, chan, dmac) {
/* Stop and reinitialize the channel. */
spin_lock_irq(&chan->lock);
rcar_dmac_chan_halt(chan);
@@ -1828,9 +1828,10 @@ static int rcar_dmac_probe(struct platform_device *pdev)
DMA_SLAVE_BUSWIDTH_2_BYTES | DMA_SLAVE_BUSWIDTH_4_BYTES |
DMA_SLAVE_BUSWIDTH_8_BYTES | DMA_SLAVE_BUSWIDTH_16_BYTES |
DMA_SLAVE_BUSWIDTH_32_BYTES | DMA_SLAVE_BUSWIDTH_64_BYTES;
+ const struct rcar_dmac_of_data *data;
+ struct rcar_dmac_chan *chan;
struct dma_device *engine;
struct rcar_dmac *dmac;
- const struct rcar_dmac_of_data *data;
unsigned int i;
int ret;
@@ -1916,11 +1917,8 @@ static int rcar_dmac_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&engine->channels);
- for (i = 0; i < dmac->n_channels; ++i) {
- if (!(dmac->channels_mask & BIT(i)))
- continue;
-
- ret = rcar_dmac_chan_probe(dmac, &dmac->channels[i], data, i);
+ for_each_rcar_dmac_chan(i, chan, dmac) {
+ ret = rcar_dmac_chan_probe(dmac, chan, data, i);
if (ret < 0)
goto error;
}
--
2.25.1
On Mon, Jan 25, 2021 at 03:24:29PM +0100, Geert Uytterhoeven wrote:
> Add and helper macro for iterating over all DMAC channels, taking into
> account the channel mask. Use it where appropriate, to simplify code.
>
> Restore "reverse Christmas tree" order of local variables while adding a
> new variable.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
Looks good and works fine with I2C + DMA on V3U:
Reviewed-by: Wolfram Sang <[email protected]>
Tested-by: Wolfram Sang <[email protected]>
Hi Geert,
Thank you for the patch.
On Mon, Jan 25, 2021 at 03:24:29PM +0100, Geert Uytterhoeven wrote:
> Add and helper macro for iterating over all DMAC channels, taking into
s/and helper/a helper/
> account the channel mask. Use it where appropriate, to simplify code.
>
> Restore "reverse Christmas tree" order of local variables while adding a
> new variable.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> v2:
> - Put the full loop control of for_each_rcar_dmac_chan() on a single
> line, to improve readability.
> ---
> drivers/dma/sh/rcar-dmac.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index a57705356e8bb796..537550b4121bbc22 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -209,6 +209,10 @@ struct rcar_dmac {
>
> #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine)
>
> +#define for_each_rcar_dmac_chan(i, chan, dmac) \
I would have placed the iterator (chan) after the container being
iterated (dmac), but it seems there are some for_each_* macros doing it
the other way around (they may be older though).
Reviewed-by: Laurent Pinchart <[email protected]>
> + for (i = 0, chan = &(dmac)->channels[0]; i < (dmac)->n_channels; i++, chan++) \
> + if (!((dmac)->channels_mask & BIT(i))) continue; else
> +
> /*
> * struct rcar_dmac_of_data - This driver's OF data
> * @chan_offset_base: DMAC channels base offset
> @@ -817,15 +821,11 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
>
> static void rcar_dmac_stop_all_chan(struct rcar_dmac *dmac)
> {
> + struct rcar_dmac_chan *chan;
> unsigned int i;
>
> /* Stop all channels. */
> - for (i = 0; i < dmac->n_channels; ++i) {
> - struct rcar_dmac_chan *chan = &dmac->channels[i];
> -
> - if (!(dmac->channels_mask & BIT(i)))
> - continue;
> -
> + for_each_rcar_dmac_chan(i, chan, dmac) {
> /* Stop and reinitialize the channel. */
> spin_lock_irq(&chan->lock);
> rcar_dmac_chan_halt(chan);
> @@ -1828,9 +1828,10 @@ static int rcar_dmac_probe(struct platform_device *pdev)
> DMA_SLAVE_BUSWIDTH_2_BYTES | DMA_SLAVE_BUSWIDTH_4_BYTES |
> DMA_SLAVE_BUSWIDTH_8_BYTES | DMA_SLAVE_BUSWIDTH_16_BYTES |
> DMA_SLAVE_BUSWIDTH_32_BYTES | DMA_SLAVE_BUSWIDTH_64_BYTES;
> + const struct rcar_dmac_of_data *data;
> + struct rcar_dmac_chan *chan;
> struct dma_device *engine;
> struct rcar_dmac *dmac;
> - const struct rcar_dmac_of_data *data;
> unsigned int i;
> int ret;
>
> @@ -1916,11 +1917,8 @@ static int rcar_dmac_probe(struct platform_device *pdev)
>
> INIT_LIST_HEAD(&engine->channels);
>
> - for (i = 0; i < dmac->n_channels; ++i) {
> - if (!(dmac->channels_mask & BIT(i)))
> - continue;
> -
> - ret = rcar_dmac_chan_probe(dmac, &dmac->channels[i], data, i);
> + for_each_rcar_dmac_chan(i, chan, dmac) {
> + ret = rcar_dmac_chan_probe(dmac, chan, data, i);
> if (ret < 0)
> goto error;
> }
--
Regards,
Laurent Pinchart
Hi Laurent,
On Tue, Jan 26, 2021 at 10:55 PM Laurent Pinchart
<[email protected]> wrote:
> On Mon, Jan 25, 2021 at 03:24:29PM +0100, Geert Uytterhoeven wrote:
> > Add and helper macro for iterating over all DMAC channels, taking into
>
> s/and helper/a helper/
Oops.
> > account the channel mask. Use it where appropriate, to simplify code.
> >
> > Restore "reverse Christmas tree" order of local variables while adding a
> > new variable.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > --- a/drivers/dma/sh/rcar-dmac.c
> > +++ b/drivers/dma/sh/rcar-dmac.c
> > @@ -209,6 +209,10 @@ struct rcar_dmac {
> >
> > #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine)
> >
> > +#define for_each_rcar_dmac_chan(i, chan, dmac) \
>
> I would have placed the iterator (chan) after the container being
> iterated (dmac), but it seems there are some for_each_* macros doing it
> the other way around (they may be older though).
Makes sense.
> Reviewed-by: Laurent Pinchart <[email protected]>
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds