diff mbox series

[04/25] dmaengine: Fix dma_slave_config.dst_addr description

Message ID 20220324014836.19149-5-Sergey.Semin@baikalelectronics.ru (mailing list archive)
State Superseded
Headers show
Series dmaengine: dw-edma: Add RP/EP local DMA controllers support | expand

Commit Message

Serge Semin March 24, 2022, 1:48 a.m. UTC
Most likely due to a copy-paste mistake the dst_addr member of the
dma_slave_config structure has been marked as ignored if the !source!
address belong to the memory. That is relevant to the src_addr field of
the structure while the dst_addr field as containing a destination device
address is supposed to be ignored if the destination is the CPU memory.
Let's fix the field description accordingly.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 include/linux/dmaengine.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vinod Koul March 31, 2022, 5:38 a.m. UTC | #1
On 24-03-22, 19:38, Manivannan Sadhasivam wrote:
> On Thu, Mar 24, 2022 at 04:48:15AM +0300, Serge Semin wrote:
> > Most likely due to a copy-paste mistake the dst_addr member of the
> > dma_slave_config structure has been marked as ignored if the !source!
> > address belong to the memory. That is relevant to the src_addr field of
> > the structure while the dst_addr field as containing a destination device
> > address is supposed to be ignored if the destination is the CPU memory.
> > Let's fix the field description accordingly.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> One suggestion below.
> 
> > ---
> >  include/linux/dmaengine.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 842d4f7ca752..f204ea16ac1c 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -395,7 +395,7 @@ enum dma_slave_buswidth {
> >   * should be read (RX), if the source is memory this argument is
> >   * ignored.
> >   * @dst_addr: this is the physical address where DMA slave data
> > - * should be written (TX), if the source is memory this argument
> > + * should be written (TX), if the destination is memory this argument
> 
> Should we rename "memory" to "local memory" or something similar?

what do you mean by local memory :)
Serge Semin March 31, 2022, 7:13 a.m. UTC | #2
On Thu, Mar 31, 2022 at 11:08:06AM +0530, Vinod Koul wrote:
> On 24-03-22, 19:38, Manivannan Sadhasivam wrote:
> > On Thu, Mar 24, 2022 at 04:48:15AM +0300, Serge Semin wrote:
> > > Most likely due to a copy-paste mistake the dst_addr member of the
> > > dma_slave_config structure has been marked as ignored if the !source!
> > > address belong to the memory. That is relevant to the src_addr field of
> > > the structure while the dst_addr field as containing a destination device
> > > address is supposed to be ignored if the destination is the CPU memory.
> > > Let's fix the field description accordingly.
> > > 
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > 
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > 
> > One suggestion below.
> > 
> > > ---
> > >  include/linux/dmaengine.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > > index 842d4f7ca752..f204ea16ac1c 100644
> > > --- a/include/linux/dmaengine.h
> > > +++ b/include/linux/dmaengine.h
> > > @@ -395,7 +395,7 @@ enum dma_slave_buswidth {
> > >   * should be read (RX), if the source is memory this argument is
> > >   * ignored.
> > >   * @dst_addr: this is the physical address where DMA slave data
> > > - * should be written (TX), if the source is memory this argument
> > > + * should be written (TX), if the destination is memory this argument
> >
 
> > Should we rename "memory" to "local memory" or something similar?
> 
> what do you mean by local memory :)

Most likely Manivannan just confused the whole eDMA device specifics
with this patch purpose. This commit has nothing to do with "local"
and "remote" device memory. Such definitions are relevant to the DW
eDMA setups (whether device is integrated into the PCIe Host/End-point
controller then the CPU memory is a local memory for it, or it's a
remote PCI End-point, then the CPU memory is a remote memory for it).

Guys. Regarding the patchsets review procedure. I notice all the
comments. Just didn't have time to respond so far. Will do that till
the end of the week.

-Sergey

> 
> -- 
> ~Vinod
Manivannan Sadhasivam March 31, 2022, 10:50 a.m. UTC | #3
On Thu, Mar 31, 2022 at 10:13:43AM +0300, Serge Semin wrote:
> On Thu, Mar 31, 2022 at 11:08:06AM +0530, Vinod Koul wrote:
> > On 24-03-22, 19:38, Manivannan Sadhasivam wrote:
> > > On Thu, Mar 24, 2022 at 04:48:15AM +0300, Serge Semin wrote:
> > > > Most likely due to a copy-paste mistake the dst_addr member of the
> > > > dma_slave_config structure has been marked as ignored if the !source!
> > > > address belong to the memory. That is relevant to the src_addr field of
> > > > the structure while the dst_addr field as containing a destination device
> > > > address is supposed to be ignored if the destination is the CPU memory.
> > > > Let's fix the field description accordingly.
> > > > 
> > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > 
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > 
> > > One suggestion below.
> > > 
> > > > ---
> > > >  include/linux/dmaengine.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > > > index 842d4f7ca752..f204ea16ac1c 100644
> > > > --- a/include/linux/dmaengine.h
> > > > +++ b/include/linux/dmaengine.h
> > > > @@ -395,7 +395,7 @@ enum dma_slave_buswidth {
> > > >   * should be read (RX), if the source is memory this argument is
> > > >   * ignored.
> > > >   * @dst_addr: this is the physical address where DMA slave data
> > > > - * should be written (TX), if the source is memory this argument
> > > > + * should be written (TX), if the destination is memory this argument
> > >
>  
> > > Should we rename "memory" to "local memory" or something similar?
> > 
> > what do you mean by local memory :)
> 
> Most likely Manivannan just confused the whole eDMA device specifics
> with this patch purpose. This commit has nothing to do with "local"
> and "remote" device memory. Such definitions are relevant to the DW
> eDMA setups (whether device is integrated into the PCIe Host/End-point
> controller then the CPU memory is a local memory for it, or it's a
> remote PCI End-point, then the CPU memory is a remote memory for it).
> 

Ah, yes indeed. While I was reviewing the eDMA patches I just went with that
context. Sorry for the noise.

Thanks,
Mani

> Guys. Regarding the patchsets review procedure. I notice all the
> comments. Just didn't have time to respond so far. Will do that till
> the end of the week.
> 
> -Sergey
> 
> > 
> > -- 
> > ~Vinod
diff mbox series

Patch

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 842d4f7ca752..f204ea16ac1c 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -395,7 +395,7 @@  enum dma_slave_buswidth {
  * should be read (RX), if the source is memory this argument is
  * ignored.
  * @dst_addr: this is the physical address where DMA slave data
- * should be written (TX), if the source is memory this argument
+ * should be written (TX), if the destination is memory this argument
  * is ignored.
  * @src_addr_width: this is the width in bytes of the source (RX)
  * register where DMA data shall be read. If the source