Re: [PATCH v2 1/1] dmaengine: fsl-qdma: add __iomem and struct in union to fix sparse warning
From: Vinod Koul
Date: Thu Feb 22 2024 - 09:01:11 EST
On 19-02-24, 10:59, Frank Li wrote:
> Fix below sparse warnings.
This does not apply for me, can you rebase
>
> 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