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

Serge Semin
2022-03-31T07:15:49Z

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