Skip to contentSkip to conversation

dma: edma: Provide granular residue accounting

PATCH New Thomas Gleixner 5 participants 14 messages Apr 28, 2014 – May 1, 2014

lore
Share & tools
lore
#0 lore raw

v2 cover dma: edma: Provide granular residue accounting

A simpler version to provide granular residue accounting and readout for EDMA.

Delta to V1:

- Removed the double read of the address in PaRAM

- Simplified the stats update in the interrupt callback for intermediate transfers

Thanks,

tglx

#6 lore raw

PATCH v2 5/6 edma: Make reading the position of active channels work

Quoted 10 lines · Thomas Gleixner

As Joel pointed out, edma_read_position() uses memcpy_fromio() to read the parameter ram. That's not synchronized with the internal update as it does a byte by byte copy. We need to do a 32bit read to get a consistent value.

Further reading destination and source is pointless. In DEV_TO_MEM transfers we are only interested in the destination, in MEM_TO_DEV we care about the source. In MEM_TO_MEM it really does not matter which one you read.

Quoted 5 lines

Simple solution: Remove the pointers, select dest/source via a bool and return the read value.

Remove the export of this function while at it. The only potential user is the dmaengine and that's always builtin.

While this is true today, there are other DMA drivers which are modules.

Quoted 2 lines

Irrespective of above,

Thanks,
Sekhar

#7 lore raw

PATCH v2 5/6 edma: Make reading the position of active channels work

In reply to Thomas Gleixner
Quoted 10 lines · Thomas Gleixner

As Joel pointed out, edma_read_position() uses memcpy_fromio() to read the parameter ram. That's not synchronized with the internal update as it does a byte by byte copy. We need to do a 32bit read to get a consistent value.

Further reading destination and source is pointless. In DEV_TO_MEM transfers we are only interested in the destination, in MEM_TO_DEV we care about the source. In MEM_TO_MEM it really does not matter which one you read.

Quoted 7 lines

Simple solution: Remove the pointers, select dest/source via a bool and return the read value.

Remove the export of this function while at it. The only potential user is the dmaengine and that's always builtin.

Thanks, this looks good.

Regards,
-Joel

#9 lore raw

v2 cover dma: edma: Provide granular residue accounting

In reply to Thomas Gleixner
Quoted 5 lines · Thomas Gleixner

A simpler version to provide granular residue accounting and readout for EDMA.

Delta to V1:

Quoted 9 lines

- Removed the double read of the address in PaRAM

- Simplified the stats update in the interrupt callback for intermediate transfers

Thanks,

tglx

Thanks for the series. I went over all the patches and it looks great.

The patches however didn't apply and had some conflicts with my dma memcpy series and peter's cyclic series so I resolved conflicts and created a single branch based on Vinod's slave-dma next branch (commit 406efb1a745c1dc512dc9c3c859e302e7b7f907e) that Vinod can pull.

I also renamed subject line of patches in Thomas's series to be "dmaengine: edma" and documented some of the variables used.

https://github.com/joelagnel/linux-kernel.git (for-vinod branch)

Vinod, could you pull if it looks OK?

regards,
-Joel

#10 lore raw

v2 cover dma: edma: Provide granular residue accounting

Quoted 24 lines · Joel Fernandes
Quoted 5 lines · Thomas Gleixner

A simpler version to provide granular residue accounting and readout for EDMA.

Delta to V1:

Quoted 9 lines

- Removed the double read of the address in PaRAM

- Simplified the stats update in the interrupt callback for intermediate transfers

Thanks,

tglx

Thanks for the series. I went over all the patches and it looks great.

The patches however didn't apply and had some conflicts with my dma memcpy series and peter's cyclic series so I resolved conflicts and created a single branch based on Vinod's slave-dma next branch (commit 406efb1a745c1dc512dc9c3c859e302e7b7f907e) that Vinod can pull.

Quoted 6 lines

I also renamed subject line of patches in Thomas's series to be "dmaengine: edma" and documented some of the variables used.

https://github.com/joelagnel/linux-kernel.git (for-vinod branch)

Vinod, could you pull if it looks OK?

The patches look good.

But,
commit 770f0f3a20188b7e17db2790803b9da925dc0b94

Author: Thomas Gleixner <[email protected]>
Date:   Mon Apr 28 10:49:43 2014 +0000

dmaengine: edma: Make reading the position of active channels work

As Joel pointed out, edma_read_position() uses memcpy_fromio() to read the parameter ram. That's not synchronized with the internal update as it does a byte by byte copy. We need to do a 32bit read to get a consistent value.

Further reading destination and source is pointless. In DEV_TO_MEM transfers we are only interested in the destination, in MEM_TO_DEV we care about the source. In MEM_TO_MEM it really does not matter which one you read.

Simple solution: Remove the pointers, select dest/source via a bool and return the read value.

Remove the export of this function while at it. The only potential user is the dmaengine and that's always builtin.

You s-o-b missing in this one, also ack from Sekhar missing. Do you want to redo this or prefer me to cherry-pick patches adding acks and your s-o-b, since I already fetched your branch

Either way is fine with me...

Signature · 1 line
~Vinod
#11 lore raw

v2 cover dma: edma: Provide granular residue accounting

On 04/29/2014 03:46 AM, Vinod Koul wrote:
[..]

Quoted 22 lines

commit 770f0f3a20188b7e17db2790803b9da925dc0b94

Author: Thomas Gleixner <[email protected]>
Date:   Mon Apr 28 10:49:43 2014 +0000

dmaengine: edma: Make reading the position of active channels work

As Joel pointed out, edma_read_position() uses memcpy_fromio() to read the parameter ram. That's not synchronized with the internal update as it does a byte by byte copy. We need to do a 32bit read to get a consistent value.

Further reading destination and source is pointless. In DEV_TO_MEM transfers we are only interested in the destination, in MEM_TO_DEV we care about the source. In MEM_TO_MEM it really does not matter which one you read.

Simple solution: Remove the pointers, select dest/source via a bool and return the read value.

Remove the export of this function while at it. The only potential user is the dmaengine and that's always builtin.

Quoted 8 lines

You s-o-b missing in this one, also ack from Sekhar missing. Do you want to redo this or prefer me to cherry-pick patches adding acks and your s-o-b, since I already fetched your branch

Either way is fine with me...

If its Ok with you, it would great if you could add my Ack and Sob. Thanks a lot. Let me know if you'd want me to do anything else here.

Regards,
-Joel

#12 lore raw

v2 cover dma: edma: Provide granular residue accounting

Quoted 30 lines · Joel Fernandes

On 04/29/2014 03:46 AM, Vinod Koul wrote:
[..]

Quoted 28 lines

commit 770f0f3a20188b7e17db2790803b9da925dc0b94

Author: Thomas Gleixner <[email protected]>
Date:   Mon Apr 28 10:49:43 2014 +0000

dmaengine: edma: Make reading the position of active channels work

As Joel pointed out, edma_read_position() uses memcpy_fromio() to read the parameter ram. That's not synchronized with the internal update as it does a byte by byte copy. We need to do a 32bit read to get a consistent value.

Further reading destination and source is pointless. In DEV_TO_MEM transfers we are only interested in the destination, in MEM_TO_DEV we care about the source. In MEM_TO_MEM it really does not matter which one you read.

Simple solution: Remove the pointers, select dest/source via a bool and return the read value.

Remove the export of this function while at it. The only potential user is the dmaengine and that's always builtin.

You s-o-b missing in this one, also ack from Sekhar missing. Do you want to redo this or prefer me to cherry-pick patches adding acks and your s-o-b, since I already fetched your branch

Quoted 5 lines
Quoted 2 lines

Either way is fine with me...

If its Ok with you, it would great if you could add my Ack and Sob. Thanks a lot. Let me know if you'd want me to do anything else here.

Done, The changes are getting pushed, pls do verify

Signature · 1 line
~Vinod
#13 lore raw

v2 cover dma: edma: Provide granular residue accounting

Quoted 31 lines · Vinod Koul
Quoted 30 lines · Joel Fernandes

On 04/29/2014 03:46 AM, Vinod Koul wrote:
[..]

Quoted 28 lines

commit 770f0f3a20188b7e17db2790803b9da925dc0b94

Author: Thomas Gleixner <[email protected]>
Date:   Mon Apr 28 10:49:43 2014 +0000

dmaengine: edma: Make reading the position of active channels work

As Joel pointed out, edma_read_position() uses memcpy_fromio() to read the parameter ram. That's not synchronized with the internal update as it does a byte by byte copy. We need to do a 32bit read to get a consistent value.

Further reading destination and source is pointless. In DEV_TO_MEM transfers we are only interested in the destination, in MEM_TO_DEV we care about the source. In MEM_TO_MEM it really does not matter which one you read.

Simple solution: Remove the pointers, select dest/source via a bool and return the read value.

Remove the export of this function while at it. The only potential user is the dmaengine and that's always builtin.

You s-o-b missing in this one, also ack from Sekhar missing. Do you want to redo this or prefer me to cherry-pick patches adding acks and your s-o-b, since I already fetched your branch

Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

Either way is fine with me...

If its Ok with you, it would great if you could add my Ack and Sob. Thanks a lot. Let me know if you'd want me to do anything else here.

Done, The changes are getting pushed, pls do verify

Sorry for the late reply as I'm at a conference.

Thanks, I verified and it looks OK.

Regards,
-Joel