Hi Vinod,
On 21.08.2017 18:18, Vinod Koul wrote:
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.
Okay.
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..
Okay.
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...
ioread32/iowrite32() operates on "void __iomem *" and not on
pointer to struct members. So adding these offsetof constructs helps
to use the correct function parameter types here. The other solutions
would be to either add some casts (which I really mis-like)
or to move away from using a struct for the IP core registers
description to macros instead, like this:
#define MSGDMA_CSR_STATUS 0x00
#define MSGDMA_CSR_CONTROL 0x04
...
If you prefer the macros, I can change it this way as well - just
let me know.
BTW: I've received the linux-next build failure warning, resulting
from DMA_SG being removed. I plan to remove the DMA_SG support from
the driver and send a new patch version with all changes included
(sparse warnings fixed, W=1 warning removed and DMA_SG removed),
if this is okay for you.
When do you plan to pull the DMA_MEMCPY_SG patchset? For v4.14?
If yes, would it be okay to already integrate support for this
OP-type into the next patch version then? Or do you prefer that
I add support for this in some follow-up patch instead?
Thanks,
Stefan
--
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
