dmaengine: memset clarifications and fixes
cover dmaengine: memset clarifications and fixes
The following contains a clarification for the behavior of the 'value' parameter in the memset operation. It is intended to be a single byte pattern as laid out here:
https://lore.kernel.org/dmaengine/YejrA5ZWZ3lTRO%2F1@matsya/
Then I'm attempting to fix all places it is currently used. But note that I do not have access to this hardware and cannot test it. We'll really need a maintainer to take a look at each of these to verify that the changes are correct.
Ben Walker (4):
dmaengine: Document dmaengine_prep_dma_memset
dmaengine: at_hdmac: In atc_prep_dma_memset, treat value as a single
byte
dmaengine: at_xdmac: In at_xdmac_prep_dma_memset, treat value as a
single byte
dmaengine: hidma: In hidma_prep_dma_memset treat value as a single
byte
drivers/dma/at_hdmac.c | 10 +++++++++- drivers/dma/at_xdmac.c | 9 ++++++++- drivers/dma/qcom/hidma.c | 13 ++++++++++++- include/linux/dmaengine.h | 8 ++++++++ 4 files changed, 37 insertions(+), 3 deletions(-)
Signature · 1 line
2.33.1
RFC PATCH 1/4 dmaengine: Document dmaengine_prep_dma_memset
Document this function to make clear the expected behavior of the 'value' parameter. It was intended to match the behavior of POSIX memset as laid out here:
https://lore.kernel.org/dmaengine/YejrA5ZWZ3lTRO%2F1@matsya/
--- include/linux/dmaengine.h | 8 ++++++++ 1 file changed, 8 insertions(+)
include/linux/dmaengine.h +8 -0
@@ -1031,6 +1031,14 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(return chan->device->device_prep_interleaved_dma (chan, xt, flags); } +/** +* dmaengine_prep_dma_memset() - Prepare a DMA memset descriptor. +* @chan: The channel to be used for this descriptor +* @dest: Address of buffer to be set +* @value: Treated as a single byte value that fills the destination buffer +* @len: The total size of dest +* @flags: DMA engine flags +*/ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memset (struct dma_chan *chan,dma_addr_t dest,int value,size_t len,unsigned long flags)
Signature · 1 line
2.33.1
Vinod Koul
maintainerRe: [RFC PATCH 1/4] dmaengine: Document dmaengine_prep_dma_memset
Quoted 5 lines · Ben Walker
Document this function to make clear the expected behavior of the 'value' parameter. It was intended to match the behavior of POSIX memset as laid out here:
https://lore.kernel.org/dmaengine/YejrA5ZWZ3lTRO%2F1@matsya/
Can we add this to Documentation too? Documentation/driver-api/dmaengine/
Quoted 27 lines
Signed-off-by Ben Walker <[email protected]>--- include/linux/dmaengine.h | 8 ++++++++ 1 file changed, 8 insertions(+)include/linux/dmaengine.h +8
-0@@ -1031,6 +1031,14 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(return chan->device->device_prep_interleaved_dma (chan, xt, flags); } +/** +* dmaengine_prep_dma_memset() - Prepare a DMA memset descriptor. +* @chan: The channel to be used for this descriptor +* @dest: Address of buffer to be set +* @value: Treated as a single byte value that fills the destination buffer +* @len: The total size of dest +* @flags: DMA engine flags +*/ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memset (struct dma_chan *chan,dma_addr_t dest,int value,size_t len,unsigned long flags)Signature · 1 line
2.33.1
Signature · 1 line
~Vinod
Re: [RFC PATCH 1/4] dmaengine: Document dmaengine_prep_dma_memset
Quoted 8 lines · Vinod Koul
Quoted 5 lines · Ben Walker
Document this function to make clear the expected behavior of the 'value' parameter. It was intended to match the behavior of POSIX memset as laid out here:
https://lore.kernel.org/dmaengine/YejrA5ZWZ3lTRO%2F1@matsya/
Can we add this to Documentation too?
Documentation/driver-api/dmaengine/
Ack. Updated in my v2.
Quoted 29 lines
Quoted 28 lines
Signed-off-by Ben Walker <[email protected]>--- include/linux/dmaengine.h | 8 ++++++++ 1 file changed, 8 insertions(+)include/linux/dmaengine.h +8
-0@@ -1031,6 +1031,14 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma(return chan->device->device_prep_interleaved_dma (chan, xt, flags); } +/** +* dmaengine_prep_dma_memset() - Prepare a DMA memset descriptor. +* @chan: The channel to be used for this descriptor +* @dest: Address of buffer to be set +* @value: Treated as a single byte value that fills the destination buffer +* @len: The total size of dest +* @flags: DMA engine flags +*/ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memset (struct dma_chan *chan,dma_addr_t dest,int value,size_t len,unsigned long flags)Signature · 1 line
2.33.1
RFC PATCH 2/4 dmaengine: at_hdmac: In atc_prep_dma_memset, treat value as a single byte
In reply to Ben WalkerThe value passed in to .prep_dma_memset is to be treated as a single byte repeating pattern.
--- drivers/dma/at_hdmac.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/dma/at_hdmac.c +9 -1
@@ -942,6 +942,7 @@ atc_prep_dma_memset(struct dma_chan *chan, dma_addr_t dest, int value,struct at_desc *desc;void __iomem *vaddr;dma_addr_t paddr; +unsigned char fill_pattern;dev_vdbg (chan2dev (chan),"%s: d%pad v0x%x l0x%zx f0x%lx\n" , __func__, &dest, value, len, flags);
@@ -963,7 +964,14 @@ atc_prep_dma_memset(struct dma_chan *chan, dma_addr_t dest, int value, __func__);return NULL ; } - *(u32 *)vaddr = value; + +/* Only the first byte of value is to be used according to dmaengine */ + fill_pattern = (unsigned char )value; + + *(u32 *)vaddr = (fill_pattern <<24 ) | + (fill_pattern <<16 ) | + (fill_pattern <<8 ) | + fill_pattern; desc =atc_create_memset_desc (chan, paddr, dest, len);if (!desc) {
Signature · 1 line
2.33.1
Vinod Koul
maintainerRe: [RFC PATCH 2/4] dmaengine: at_hdmac: In atc_prep_dma_memset, treat value as a single byte
Quoted 23 lines · Ben Walker
The value passed in to .prep_dma_memset is to be treated as a single byte repeating pattern.
Signed-off-by Ben Walker <[email protected]>Cc Ludovic Desroches <[email protected]>Cc Tudor Ambarus <[email protected]>--- drivers/dma/at_hdmac.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)drivers/dma/at_hdmac.c +1
-0@@ -942,6 +942,7 @@ atc_prep_dma_memset(struct dma_chan *chan, dma_addr_t dest, int value,struct at_desc *desc;void __iomem *vaddr;dma_addr_t paddr; +unsigned char fill_pattern;dev_vdbg (chan2dev (chan),"%s: d%pad v0x%x l0x%zx f0x%lx\n" , __func__, &dest, value, len, flags);
drivers/dma/at_hdmac.c atc_prep_dma_memset(struct dma_chan *chan, dma_addr_t dest, int value, · line 969
Quoted 7 lines
drivers/dma/at_hdmac.c +3
-1@@ -963,7 +964,14 @@ atc_prep_dma_memset(struct dma_chan *chan, dma_addr_t dest, int value, __func__);return NULL ; } - *(u32 *)vaddr = value; + +/* Only the first byte of value is to be used according to dmaengine */ + fill_pattern = (unsigned char )value;
why cast as unsigned?
Quoted 10 lines
drivers/dma/at_hdmac.c +5
-0+ + *(u32 *)vaddr = (fill_pattern <<24 ) | + (fill_pattern <<16 ) | + (fill_pattern <<8 ) | + fill_pattern; desc =atc_create_memset_desc (chan, paddr, dest, len);if (!desc) {Signature · 1 line
2.33.1
Signature · 1 line
~Vinod
Re: [RFC PATCH 2/4] dmaengine: at_hdmac: In atc_prep_dma_memset, treat value as a single byte
Quoted 21 lines · Vinod Koul
Quoted 20 lines · Ben Walker
The value passed in to .prep_dma_memset is to be treated as a single byte repeating pattern.
Signed-off-by Ben Walker <[email protected]>Cc Ludovic Desroches <[email protected]>Cc Tudor Ambarus <[email protected]>--- drivers/dma/at_hdmac.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)drivers/dma/at_hdmac.c +1
-0@@ -942,6 +942,7 @@ atc_prep_dma_memset(struct dma_chan *chan, dma_addr_t dest, int value,struct at_desc *desc;void __iomem *vaddr;dma_addr_t paddr; +unsigned char fill_pattern;
Quoted 12 lines
drivers/dma/at_hdmac.c atc_prep_dma_memset(struct dma_chan *chan, dma_addr_t dest, int value, · line 969
Quoted 10 lines
drivers/dma/at_hdmac.c +3
-1dev_vdbg (chan2dev (chan),"%s: d%pad v0x%x l0x%zx f0x%lx\n" , __func__, &dest, value, len, flags);@@ -963,7 +964,14 @@ atc_prep_dma_memset(struct dma_chan *chan, dma_addr_t dest, int value, __func__);return NULL ; } - *(u32 *)vaddr = value; + +/* Only the first byte of value is to be used according to dmaengine */ + fill_pattern = (unsigned char )value;why cast as unsigned?
No good reason - I didn't think negatives would be used. I've updated this to be signed in the v2.
Quoted 12 lines
Quoted 11 lines
drivers/dma/at_hdmac.c +5
-0+ + *(u32 *)vaddr = (fill_pattern <<24 ) | + (fill_pattern <<16 ) | + (fill_pattern <<8 ) | + fill_pattern; desc =atc_create_memset_desc (chan, paddr, dest, len);if (!desc) {Signature · 1 line
2.33.1
RFC PATCH 3/4 dmaengine: at_xdmac: In at_xdmac_prep_dma_memset, treat value as a single byte
In reply to Ben WalkerThe value passed in to .prep_dma_memset is to be treated as a single byte repeating pattern.
--- drivers/dma/at_xdmac.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/dma/at_xdmac.c +8 -1
@@ -1202,6 +1202,7 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan,unsigned long flags;size_t ublen;u32 dwidth; +unsigned char pattern;/* * WARNING: The channel configuration is set here since there is no * dmaengine_slave_config call in this case. Moreover we don't know the
@@ -1244,10 +1245,16 @@ static struct at_xdmac_desc *at_xdmac_memset_create_desc(struct dma_chan *chan, chan_cc |=AT_XDMAC_CC_DWIDTH (dwidth); +/* Only the first byte of value is to be used according to dmaengine */ + pattern = (unsigned char )value; + ublen = len >> dwidth; desc->lld.mbr_da = dst_addr; - desc->lld.mbr_ds = value; + desc->lld.mbr_ds = (pattern <<24 ) | + (pattern <<16 ) | + (pattern <<8 ) | + pattern; desc->lld.mbr_ubc =AT_XDMAC_MBR_UBC_NDV3 |AT_XDMAC_MBR_UBC_NDEN |AT_XDMAC_MBR_UBC_NSEN
Signature · 1 line
2.33.1
RFC PATCH 4/4 dmaengine: hidma: In hidma_prep_dma_memset treat value as a single byte
In reply to Ben WalkerThe value parameter is a single byte, so duplicate it to the 8 byte range that is used as the pattern.
--- drivers/dma/qcom/hidma.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/dma/qcom/hidma.c +12 -1
@@ -431,6 +431,7 @@ hidma_prep_dma_memset(struct dma_chan *dmach, dma_addr_t dest, int value,struct hidma_desc *mdesc =NULL ;struct hidma_dev *mdma = mchan->dmadev;unsigned long irqflags; +u64 pattern;/* Get free descriptor */ spin_lock_irqsave (&mchan->lock, irqflags);
@@ -443,9 +444,19 @@ hidma_prep_dma_memset(struct dma_chan *dmach, dma_addr_t dest, int value,if (!mdesc)return NULL ; + pattern = (unsigned char )value; + pattern = (pattern <<56 ) | + (pattern <<48 ) | + (pattern <<40 ) | + (pattern <<32 ) | + (pattern <<24 ) | + (pattern <<16 ) | + (pattern <<8 ) | + pattern; + mdesc->desc.flags = flags;hidma_ll_set_transfer_params (mdma->lldev, mdesc->tre_ch, - value, dest, len, flags, + pattern, dest, len, flags,HIDMA_TRE_MEMSET );/* Place descriptor in prepared list */
Signature · 1 line
2.33.1