Re: [PATCH v2 1/1] dmaengine: fsl-qdma: add __iomem and struct in union to fix sparse warning

From: Frank Li
Date: Thu Feb 22 2024 - 10:25:25 EST

On Thu, Feb 22, 2024 at 07:30:59PM +0530, Vinod Koul wrote:
> On 19-02-24, 10:59, Frank Li wrote:
> > Fix below sparse warnings.
>
> This does not apply for me, can you rebase

Which branch do you try to apply?
This one is fix warnings in "fixes" branch, commit
https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git/commit/?h=fixes&id=9d739bccf261dd93ec1babf82f5c5d71dd4caa3e

Frank

>
> >
> > drivers/dma/fsl-qdma.c:645:50: sparse: warning: incorrect type in argument 2 (different address spaces)
> > drivers/dma/fsl-qdma.c:645:50: sparse: expected void [noderef] __iomem *addr
> > drivers/dma/fsl-qdma.c:645:50: sparse: got void
> >
> > drivers/dma/fsl-qdma.c:387:15: sparse: sparse: restricted __le32 degrades to integer
> > drivers/dma/fsl-qdma.c:390:19: sparse: expected restricted __le64 [usertype] data
> > drivers/dma/fsl-qdma.c:392:13: sparse: expected unsigned int [assigned] [usertype] cmd
> >
> > QDMA decriptor have below 3 kind formats. (little endian)
> >
> > Compound Command Descriptor Format
> > ┌──────┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┐
> > │Offset│3│3│2│2│2│2│2│2│2│2│2│2│1│1│1│1│1│1│1│1│1│1│ │ │ │ │ │ │ │ │ │ │
> > │ │1│0│9│8│7│6│5│4│3│2│1│0│9│8│7│6│5│4│3│2│1│0│9│8│7│6│5│4│3│2│1│0│
> > ├──────┼─┴─┼─┴─┴─┼─┴─┴─┼─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┼─┴─┴─┴─┴─┴─┴─┴─┤
> > │ 0x0C │DD │ - │QUEUE│ - │ ADDR │
> > ├──────┼───┴─────┴─────┴───────────────────────────────┴───────────────┤
> > │ 0x08 │ ADDR │
> > ├──────┼─────┬─────────────────┬───────────────────────────────────────┤
> > │ 0x04 │ FMT │ OFFSET │ - │
> > ├──────┼─┬─┬─┴─────────────────┴───────────────────────┬───────────────┤
> > │ │ │S│ │ │
> > │ 0x00 │-│E│ - │ STATUS │
> > │ │ │R│ │ │
> > └──────┴─┴─┴───────────────────────────────────────────┴───────────────┘
> >
> > Compound S/G Table Entry Format
> > ┌──────┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┐
> > │Offset│3│3│2│2│2│2│2│2│2│2│2│2│1│1│1│1│1│1│1│1│1│1│ │ │ │ │ │ │ │ │ │ │
> > │ │1│0│9│8│7│6│5│4│3│2│1│0│9│8│7│6│5│4│3│2│1│0│9│8│7│6│5│4│3│2│1│0│
> > ├──────┼─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┼─┴─┴─┴─┴─┴─┴─┴─┤
> > │ 0x0C │ - │ ADDR │
> > ├──────┼───────────────────────────────────────────────┴───────────────┤
> > │ 0x08 │ ADDR │
> > ├──────┼─┬─┬───────────────────────────────────────────────────────────┤
> > │ 0x04 │E│F│ LENGTH │
> > ├──────┼─┴─┴─────────────────────────────────┬─────────────────────────┤
> > │ 0x00 │ - │ OFFSET │
> > └──────┴─────────────────────────────────────┴─────────────────────────┘
> >
> > Source/Destination Descriptor Format
> > ┌──────┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┬─┐
> > │Offset│3│3│2│2│2│2│2│2│2│2│2│2│1│1│1│1│1│1│1│1│1│1│ │ │ │ │ │ │ │ │ │ │
> > │ │1│0│9│8│7│6│5│4│3│2│1│0│9│8│7│6│5│4│3│2│1│0│9│8│7│6│5│4│3│2│1│0│
> > ├──────┼─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┴─┤
> > │ 0x0C │ CMD │
> > ├──────┼───────────────────────────────────────────────────────────────┤
> > │ 0x08 │ - │
> > ├──────┼───────────────┬───────────────────────┬───────────────────────┤
> > │ 0x04 │ - │ S[D]SS │ S[D]SD │
> > ├──────┼───────────────┴───────────────────────┴───────────────────────┤
> > │ 0x00 │ - │
> > └──────┴───────────────────────────────────────────────────────────────┘
> >
> > Previous code use 64bit 'data' map to 0x8 and 0xC. In little endian system
> > CMD is high part of 64bit 'data'. It is correct by left shift 32. But in
> > big endian system, shift left 32 will write to 0x8 position. Sparse detect
> > this problem.
> >
> > Add below field ot match 'Source/Destination Descriptor Format'.
> > struct {
> > __le32 __reserved2;
> > __le32 cmd;
> > } __packed;
> >
> > Using ddf(sdf)->cmd save to correct posistion regardless endian.
> >
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202402081929.mggOTHaZ-lkp@xxxxxxxxx/
> > Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> > ---
> >
> > Notes:
> > Change from v1 to v2
> > - update commit message to show why add 'cmd'
> >
> > fsl-edma-common.c's build warning should not cause by this driver. which is
> > difference drivers. This driver will not use any code related with
> > fsl-edma-common.c.
> >
> > drivers/dma/fsl-qdma.c | 21 ++++++++++-----------
> > 1 file changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
> > index 1e3bf6f30f784..5005e138fc239 100644
> > --- a/drivers/dma/fsl-qdma.c
> > +++ b/drivers/dma/fsl-qdma.c
> > @@ -161,6 +161,10 @@ struct fsl_qdma_format {
> > u8 __reserved1[2];
> > u8 cfg8b_w1;
> > } __packed;
> > + struct {
> > + __le32 __reserved2;
> > + __le32 cmd;
> > + } __packed;
> > __le64 data;
> > };
> > } __packed;
> > @@ -355,7 +359,6 @@ static void fsl_qdma_free_chan_resources(struct dma_chan *chan)
> > static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
> > dma_addr_t dst, dma_addr_t src, u32 len)
> > {
> > - u32 cmd;
> > struct fsl_qdma_format *sdf, *ddf;
> > struct fsl_qdma_format *ccdf, *csgf_desc, *csgf_src, *csgf_dest;
> >
> > @@ -384,15 +387,11 @@ static void fsl_qdma_comp_fill_memcpy(struct fsl_qdma_comp *fsl_comp,
> > /* This entry is the last entry. */
> > qdma_csgf_set_f(csgf_dest, len);
> > /* Descriptor Buffer */
> > - cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
> > - FSL_QDMA_CMD_RWTTYPE_OFFSET) |
> > - FSL_QDMA_CMD_PF;
> > - sdf->data = QDMA_SDDF_CMD(cmd);
> > -
> > - cmd = cpu_to_le32(FSL_QDMA_CMD_RWTTYPE <<
> > - FSL_QDMA_CMD_RWTTYPE_OFFSET);
> > - cmd |= cpu_to_le32(FSL_QDMA_CMD_LWC << FSL_QDMA_CMD_LWC_OFFSET);
> > - ddf->data = QDMA_SDDF_CMD(cmd);
> > + sdf->cmd = cpu_to_le32((FSL_QDMA_CMD_RWTTYPE << FSL_QDMA_CMD_RWTTYPE_OFFSET) |
> > + FSL_QDMA_CMD_PF);
> > +
> > + ddf->cmd = cpu_to_le32((FSL_QDMA_CMD_RWTTYPE << FSL_QDMA_CMD_RWTTYPE_OFFSET) |
> > + (FSL_QDMA_CMD_LWC << FSL_QDMA_CMD_LWC_OFFSET));
> > }
> >
> > /*
> > @@ -626,7 +625,7 @@ static int fsl_qdma_halt(struct fsl_qdma_engine *fsl_qdma)
> >
> > static int
> > fsl_qdma_queue_transfer_complete(struct fsl_qdma_engine *fsl_qdma,
> > - void *block,
> > + __iomem void *block,
> > int id)
> > {
> > bool duplicate;
> > --
> > 2.34.1
>
> --
> ~Vinod