Re: [PATCH 01/14] dmaengine: dma-jz4780: Avoid hardcoding number of channels

[Paul Cercueil]

Le sam. 7 juil. 2018 à 9:34, PrasannaKumar Muralidharan 
<prasannatsmkumar@xxxxxxxxx> a écrit :
> On 5 July 2018 at 23:56, Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
> >  Hi PrasannaKumar,
> >
> >
> > >  Hi Paul,
> > >
> > >  On 3 July 2018 at 18:02, Paul Cercueil <paul@xxxxxxxxxxxxxxx> 
> > > wrote:
> > > >   As part of the work to support various other Ingenic JZ47xx SoC
> > > >  versions,
> > > >   which don't feature the same number of DMA channels per core, we 
> > > > now
> > > >   deduce the number of DMA channels available from the devicetree
> > > >   compatible string.
> > > >
> > > >   Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> > > >   ---
> > > >    drivers/dma/dma-jz4780.c | 53 
> > > > +++++++++++++++++++++++++++++-----------
> > > >    1 file changed, 39 insertions(+), 14 deletions(-)
> > > >
> > > >   diff --git a/drivers/dma/dma-jz4780.c b/drivers/dma/dma-jz4780.c
> > > >   index 85820a2d69d4..b40f491f0367 100644
> > > >   --- a/drivers/dma/dma-jz4780.c
> > > >   +++ b/drivers/dma/dma-jz4780.c
> > > >   @@ -16,6 +16,7 @@
> > > >    #include <linux/interrupt.h>
> > > >    #include <linux/module.h>
> > > >    #include <linux/of.h>
> > > >   +#include <linux/of_device.h>
> > > >    #include <linux/of_dma.h>
> > > >    #include <linux/platform_device.h>
> > > >    #include <linux/slab.h>
> > > >   @@ -23,8 +24,6 @@
> > > >    #include "dmaengine.h"
> > > >    #include "virt-dma.h"
> > > >
> > > >   -#define JZ_DMA_NR_CHANNELS     32
> > > >   -
> > > >    /* Global registers. */
> > > >    #define JZ_DMA_REG_DMAC                0x1000
> > > >    #define JZ_DMA_REG_DIRQP       0x1004
> > > >   @@ -135,14 +134,20 @@ struct jz4780_dma_chan {
> > > >           unsigned int curr_hwdesc;
> > > >    };
> > > >
> > > >   +enum jz_version {
> > > >   +       ID_JZ4780,
> > > >   +};
> > > >   +
> > > >    struct jz4780_dma_dev {
> > > >           struct dma_device dma_device;
> > > >           void __iomem *base;
> > > >           struct clk *clk;
> > > >           unsigned int irq;
> > > >   +       unsigned int nb_channels;
> > > >   +       enum jz_version version;
> > > >
> > > >           uint32_t chan_reserved;
> > > >   -       struct jz4780_dma_chan chan[JZ_DMA_NR_CHANNELS];
> > > >   +       struct jz4780_dma_chan chan[];
> > >
> > >
> > >  Looks like a variable length array in struct. I think there is some
> > >  effort to remove the usage of VLA. Can you revisit this? I may be
> > >  wrong, please feel free to correct.
> >
> >
> >  Are you sure? It's the first time I hear about it.
> >  Could anybody confirm?
>
> Please see [1] for info.
>
> Variable Length Arrays in struct is expressly forbidden in C99, C11.
> Clang does not support it. To make kernel compile with Clang few
> people are trying to remove/reduce VLAIS usage.
>
> 1. 
> https://blog.linuxplumbersconf.org/2013/ocw/system/presentations/1221/original/VLAIS.pdf

I read it, and my structure is not a VLAIS; my "chan" array is a 
flexible
array, its sizeof() is 0, so the sizeof() of the structure is constant.

See page 6 of the PDF, about alternatives to VLAIS:
"If possible use a flexible array member and move the array to the end 
of
the struct"
Which is what I am doing here.

-Paul