| 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 |
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 :)
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
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 --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
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(-)