Hi Vinod,
On 25.08.2017 08:20, Vinod Koul wrote:
On Tue, Aug 22, 2017 at 09:29:02AM +0200, Stefan Roese wrote:
Hi Vinod,
On 21.08.2017 18:18, Vinod Koul wrote:
@@ -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.
I think moving away from structures is better and ore intutive. We can have
a void__iomeme * as hw base address and you can use offsets as described..
Okay, I'll remove the structs from the driver in a follow-up patch.
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.
I have already fixed that up (i think you were cced on my reply) with the
patch to remove, do check if that was fine...
Looks good. Thanks for working on this.
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?
I think we might be bit late, MERGE window opens in a week ish...
I see. I'll send a patch adding DMA_MEMCPY_SG support to this
mSGDMA driver, once the DMA_MEMCPY_SG infrastructure is in place.
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
