Re: [PATCH] dmaengine: altera-msgdma: Fix sparse warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 09, 2017 at 11:59:10AM +0200, Stefan Roese wrote:
> This patch fixes some sparse warnings by using "void __iomem *" for
> the register variables as this is required for the ioread32/iowrite32
> accessor functions.

A patch subject line should describe the change not the tool that found it,
so can you please revise the fixes. One possible way is to club similar type
of fixes in a file. It is also helpful to show the sparse warns to help
understand the fixes.

> 
> Please note that the following patch needs to be applied to quiet some
> incorrect sparse warnings when compiling this driver for ARM on 64bit
> platforms (GENMASK issue):
> 
> "arm: fix sparse flags for build on 64bit machines"
> https://patchwork.kernel.org/patch/9864431/

This info is useless for applying but useful for review and test, this
should not be kept part of the changelog and moved after S-o-b line..

> 
> Signed-off-by: Stefan Roese <sr@xxxxxxx>
> Cc: Vinod Koul <vinod.koul@xxxxxxxxx>
> ---
>  drivers/dma/altera-msgdma.c | 46 +++++++++++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c
> index 33b87b413793..c3cb92587001 100644
> --- a/drivers/dma/altera-msgdma.c
> +++ b/drivers/dma/altera-msgdma.c
> @@ -166,6 +166,10 @@ struct msgdma_response {
>  #define MSGDMA_RESP_EARLY_TERM	BIT(8)
>  #define MSGDMA_RESP_ERR_MASK	0xff
>  
> +#define respoffs(a)		(offsetof(struct msgdma_response, a))
> +#define csroffs(a)		(offsetof(struct msgdma_csr, a))
> +#define descoffs(a)		(offsetof(struct msgdma_extended_desc, a))
> +
>  /**
>   * struct msgdma_sw_desc - implements a sw descriptor
>   * @async_tx: support for the async_tx api
> @@ -204,13 +208,13 @@ struct msgdma_device {
>  	int irq;
>  
>  	/* mSGDMA controller */
> -	struct msgdma_csr *csr;
> +	void __iomem *csr;
>  
>  	/* mSGDMA descriptors */
> -	struct msgdma_extended_desc *desc;
> +	void __iomem *desc;
>  
>  	/* mSGDMA response */
> -	struct msgdma_response *resp;
> +	void __iomem *resp;
>  };
>  
>  #define to_mdev(chan)	container_of(chan, struct msgdma_device, dmachan)
> @@ -576,21 +580,21 @@ static void msgdma_reset(struct msgdma_device *mdev)
>  	int ret;
>  
>  	/* Reset mSGDMA */
> -	iowrite32(MSGDMA_CSR_STAT_MASK, &mdev->csr->status);
> -	iowrite32(MSGDMA_CSR_CTL_RESET, &mdev->csr->control);
> +	iowrite32(MSGDMA_CSR_STAT_MASK, mdev->csr + csroffs(status));
> +	iowrite32(MSGDMA_CSR_CTL_RESET, mdev->csr + csroffs(control));

why do you need to use offsetof here and other places?
something doesnt seem right here...

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux