Re: [PATCH v4 5/8] dmaengine: dw-edma: Fix programming the source & dest addresses for ep

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 22, 2022 at 08:55:49AM -0500, Zhi Li wrote:
> On Sun, Mar 20, 2022 at 6:16 PM Serge Semin <fancer.lancer@xxxxxxxxx> wrote:
> >
> > On Fri, Mar 18, 2022 at 09:19:13PM +0300, Serge Semin wrote:
> > > On Fri, Mar 18, 2022 at 11:36:05PM +0530, Manivannan Sadhasivam wrote:
> > > > On Mon, Mar 14, 2022 at 11:33:40AM +0300, Serge Semin wrote:
> > > > > On Sat, Mar 12, 2022 at 11:07:20AM +0530, Manivannan Sadhasivam wrote:
> > > > > > On Fri, Mar 11, 2022 at 10:01:47PM +0300, Serge Semin wrote:
> > > > > > > On Fri, Mar 11, 2022 at 11:11:34PM +0530, Manivannan Sadhasivam wrote:
> > > > >
> > > > > [nip]
> > > > >
> > > > > > >
> > > > > > > > As per my understanding, the eDMA is solely used in the PCIe endpoint. And the
> > > > > > > > access to it happens over PCIe bus or by the local CPU.
> > > > > > >
> > > > > > > Not fully correct. Root Ports can also have eDMA embedded. In that
> > > > > > > case the eDMA can be only accessible from the local CPU. At the same
> > > > > > > time the DW PCIe End-point case is the IP-core synthesize parameters
> > > > > > > specific. It's always possible to access the eDMA CSRs from local
> > > > > > > CPU, but a particular End-point BAR can be pre-synthesize to map
> > > > > > > either Port Logic, or eDMA or iATU CSRs. Thus a PCIe root port can
> > > > > > > perform a full End-point configuration. Anyway the case if the eDMA
> > > > > > > functionality being accessible over the PCIe wire doesn't really make
> > > > > > > much sense with no info regarding the application logic hidden behind
> > > > > > > the PCIe End-point interface since SAR/DAR LLP is supposed to be
> > > > > > > initialized with an address from the local (application) memory space.
> > > > > > >
> > > > > >
> > > > > > Thanks for the explanation, it clarifies my doubt. I got misleaded by the
> > > > > > earlier commits...
> > > > > >
> > > > > > > So AFAICS the main usecase of the controller is 1) - when eDMA is a
> > > > > > > part of the Root Port/End-point and only local CPU is supposed to have
> > > > > > > it accessed and configured.
> > > > > > >
> > > > > > > I can resend this patch with my fix to the problem. What do you think?
> > > > > > >
> > > > > >
> > > > >
> > > > > > Yes, please do.
> > > > >
> > > > > Ok. I'll be AFK today, but will send my patches tomorrow.  @Frank,
> > > > > Could you please hold on with respinning the series for a few days?
> > > > > I'll send out some of my patches then with a note which one of them
> > > > > could be picked up by you and merged into this series.
> > > > >
> > > >
> > >
> >
> > > > Any update on your patches?
> > >
> > > No worries. The patches are ready. But since Frank was on vacation I
> > > decided to rebase all of my work on top of his series. I'll finish it
> > > up shortly and send out my patchset till Monday for review. Then Frank
> > > will be able to pick up two patches from there so to close up his
> > > patchset (I'll give a note which one of them is of Frank' interes).
> > > My series will be able to be merged in after Frank's series is reviewed
> > > and accepted.
> >
> > Folks, couldn't make it this weekend. Too much sidework to do. Terribly
> > sorry about that. Will send out the series tomorrow or at most in a day
> > after tomorrow. Sorry for the inconvenience.
> 

> Any update on your patches?

The patches are ready. Writing cover-letters and sending them out this
night. Sorry for the delay.

-Sergey

> 
> best regards
> Frank Li
> 
> >
> > -Sergey
> >
> > >
> > > >
> > > > Btw, my colleage worked on merging the two dma devices used by the eDMA core
> > > > for read & write channels into one. Initially I thought that was not needed as
> > > > he did that for devicetree integration, but looking deeply I think that patch is
> > > > necessary irrespective of DT.
> > > >
> > > > One standout problem is, we can't register debugfs directory under "dmaengine"
> > > > properly because, both read and write dma devices share the same parent
> > > > chip->dev.
> > >
> > > Right, my series fixes that and some other problems too. So please be
> > > patient for a few more days.
> > >
> > > -Sergey
> > >
> > > >
> > > > Thanks,
> > > > Mani
> > > >
> > > > > -Sergey
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > > Mani
> > > > > >
> > > > > > > -Sergey
> > > > > > >
> > > > > > > >
> > > > > > > > The commit from Alan Mikhak is what I took as a reference since the patch was
> > > > > > > > already merged:
> > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/dma/dw-edma?id=bd96f1b2f43a39310cc576bb4faf2ea24317a4c9
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Mani
> > > > > > > >
> > > > > > > > > -Sergey
> > > > > > > > >
> > > > > > > > > > +                *
> > > > > > > > > > +                ****************************************************************/
> > > > > > > > > > +
> > > > > > > > >
> > > > > > > > > > +               if ((dir == DMA_DEV_TO_MEM && chan->dir == EDMA_DIR_READ) ||
> > > > > > > > > > +                   (dir == DMA_DEV_TO_MEM && chan->dir == EDMA_DIR_WRITE))
> > > > > > > > > > +                       read = true;
> > > > > > > > >
> > > > > > > > > Seeing the driver support only two directions DMA_DEV_TO_MEM/DMA_DEV_TO_MEM
> > > > > > > > > and EDMA_DIR_READ/EDMA_DIR_WRITE, this conditional statement seems
> > > > > > > > > redundant.
> > > > > > > > >
> > > > > > > > > > +
> > > > > > > > > > +               /* Program the source and destination addresses for DMA read/write */
> > > > > > > > > > +               if (read) {
> > > > > > > > > >                         burst->sar = src_addr;
> > > > > > > > > >                         if (xfer->type == EDMA_XFER_CYCLIC) {
> > > > > > > > > >                                 burst->dar = xfer->xfer.cyclic.paddr;
> > > > > > > > > > --
> > > > > > > > > > 2.24.0.rc1
> > > > > > > > > >



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux