| Message ID | 20210107181524.1947173-3-geert+renesas@glider.be |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dmaengine: rcar-dmac: Add support for R-Car V3U | expand |
On 07-01-21, 19:15, 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 <geert+renesas@glider.be> > --- > drivers/dma/sh/rcar-dmac.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > index a57705356e8bb796..71cdaf446fcaeba5 100644 > --- a/drivers/dma/sh/rcar-dmac.c > +++ b/drivers/dma/sh/rcar-dmac.c > @@ -209,6 +209,11 @@ 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++) \ single line to make it more readable? we have limit of 100 now :) > + 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 +822,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 +1829,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 +1918,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 -- ~Vinod
Hi Vinod, On Tue, Jan 12, 2021 at 11:19 AM Vinod Koul <vkoul@kernel.org> wrote: > On 07-01-21, 19:15, 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 <geert+renesas@glider.be> > > --- a/drivers/dma/sh/rcar-dmac.c > > +++ b/drivers/dma/sh/rcar-dmac.c > > @@ -209,6 +209,11 @@ 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++) \ > > single line to make it more readable? we have limit of 100 now :) Do we have to push the limits? BTW, the new punched cards are 96-column wide, not 100-column ;-) https://en.wikipedia.org/wiki/Punched_card#IBM_96-column_format Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
On 12-01-21, 11:26, Geert Uytterhoeven wrote: > Hi Vinod, > > On Tue, Jan 12, 2021 at 11:19 AM Vinod Koul <vkoul@kernel.org> wrote: > > On 07-01-21, 19:15, 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 <geert+renesas@glider.be> > > > > --- a/drivers/dma/sh/rcar-dmac.c > > > +++ b/drivers/dma/sh/rcar-dmac.c > > > @@ -209,6 +209,11 @@ 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++) \ > > > > single line to make it more readable? we have limit of 100 now :) > > Do we have to push the limits? In cases where it helps, I certainly recommend.. I feel in this case it makes a better read to have it in a single line.. > BTW, the new punched cards are 96-column wide, not 100-column ;-) > https://en.wikipedia.org/wiki/Punched_card#IBM_96-column_format Did we err in choosing 100 :D -- ~Vinod
Hi Vinod, On Tue, Jan 12, 2021 at 11:38 AM Vinod Koul <vkoul@kernel.org> wrote: > On 12-01-21, 11:26, Geert Uytterhoeven wrote: > > On Tue, Jan 12, 2021 at 11:19 AM Vinod Koul <vkoul@kernel.org> wrote: > > > On 07-01-21, 19:15, 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 <geert+renesas@glider.be> > > > > > > --- a/drivers/dma/sh/rcar-dmac.c > > > > +++ b/drivers/dma/sh/rcar-dmac.c > > > > @@ -209,6 +209,11 @@ 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++) \ > > > > > > single line to make it more readable? we have limit of 100 now :) > > > > Do we have to push the limits? > > In cases where it helps, I certainly recommend.. I feel in this case it > makes a better read to have it in a single line.. OK, will fix. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c index a57705356e8bb796..71cdaf446fcaeba5 100644 --- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c @@ -209,6 +209,11 @@ 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 +822,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 +1829,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 +1918,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; }
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 <geert+renesas@glider.be> --- drivers/dma/sh/rcar-dmac.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)