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

[Vinod Koul <vkoul@kernel.org>]

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@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202402081929.mggOTHaZ-lkp@intel.com/
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 
> 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