| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJivOr7ZGwm8Bp1oGcYQHkao2zr0GsMQrcdawMHukmeA8wYVnQ@mail.gmail.com>
Date: Thu, 7 Nov 2019 10:11:33 +0800
From: Green Wan <green.wan@...ive.com>
To: Vinod Koul <vkoul@...nel.org>
Cc: kbuild test robot <lkp@...el.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Palmer Dabbelt <palmer@...ive.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Albert Ou <aou@...s.berkeley.edu>,
Dan Williams <dan.j.williams@...el.com>,
Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
"Paul E. McKenney" <paulmck@...ux.ibm.com>,
Yash Shah <yash.shah@...ive.com>,
Bin Meng <bmeng.cn@...il.com>,
Sagar Kadam <sagar.kadam@...ive.com>,
dmaengine@...r.kernel.org, devicetree@...r.kernel.org,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 3/4] dmaengine: sf-pdma: add platform DMA support for
HiFive Unleashed A00
On Wed, Nov 6, 2019 at 1:48 AM Vinod Koul <vkoul@...nel.org> wrote:
>
> On 28-10-19, 15:56, Green Wan wrote:
> > Add PDMA driver, sf-pdma, to enable DMA engine on HiFive Unleashed
> > Rev A00 board.
> >
> > - Implement dmaengine APIs, support MEM_TO_MEM async copy.
> > - Tested by DMA Test client
> > - Supports 4 channels DMA, each channel has 1 done and 1 err
> > interrupt connected to platform-level interrupt controller (PLIC).
> > - Depends on DMA_ENGINE and DMA_VIRTUAL_CHANNELS
> >
> > The datasheet is here:
> >
> > https://static.dev.sifive.com/FU540-C000-v1.0.pdf
> >
> > Follow the DMAengine controller doc,
> > "./Documentation/driver-api/dmaengine/provider.rst" to implement DMA
> > engine. And use the dma test client in doc,
> > "./Documentation/driver-api/dmaengine/dmatest.rst", to test.
> >
> > Each DMA channel has separate HW regs and support done and error ISRs.
> > 4 channels share 1 done and 1 err ISRs. There's no expander/arbitrator
> > in DMA HW.
> >
> > ------ ------
> > | |--< done 23 >--|ch 0|
> > | |--< err 24 >--| | (dma0chan0)
> > | | ------
> > | | ------
> > | |--< done 25 >--|ch 1|
> > | |--< err 26 >--| | (dma0chan1)
> > |PLIC| ------
> > | | ------
> > | |--< done 27 >--|ch 2|
> > | |--< err 28 >--| | (dma0chan2)
> > | | ------
> > | | ------
> > | |--< done 29 >--|ch 3|
> > | |--< err 30 >--| | (dma0chan3)
> > ------ ------
> >
> > Reviewed-by: Vinod Koul <vkoul@...nel.org>
>
> when did i provide this?
>
> > Signed-off-by: Green Wan <green.wan@...ive.com>
> > Reported-by: kbuild test robot <lkp@...el.com>
> > Fixes: 31c3b98b5a01 ("dmaengine: sf-pdma: add platform DMA support for HiFive Unleashed A00")
>
> Fixes what... this is not a upstream commit?
>
Since I received a RFC patch from kbuild with the fix commit number I
thought it was about merged. RFC requested to add the "Reported-by"
and "Fixes" but looks miss the contributor so I added them as well.
I'll remove them in next submit. Sorry for causing confusion.
> > Signed-off-by: kbuild test robot <lkp@...el.com>
> > ---
>
> Please list the changes done from prev version, here or in cover letter
>
will add change log.
> > +static struct sf_pdma_desc *sf_pdma_alloc_desc(struct sf_pdma_chan *chan)
> > +{
> > + struct sf_pdma_desc *desc;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&chan->lock, flags);
> > +
> > + if (chan->desc && !chan->desc->in_use) {
> > + spin_unlock_irqrestore(&chan->lock, flags);
> > + return chan->desc;
> > + }
> > +
> > + spin_unlock_irqrestore(&chan->lock, flags);
> > +
> > + desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> > +
>
> this empty line in not required
>
> > +static struct dma_async_tx_descriptor *
> > + sf_pdma_prep_dma_memcpy(struct dma_chan *dchan,
> > + dma_addr_t dest,
>
> please make it left justified
will fix.
>
> > +static int sf_pdma_slave_config(struct dma_chan *dchan,
> > + struct dma_slave_config *cfg)
> > +{
> > + struct sf_pdma_chan *chan = to_sf_pdma_chan(dchan);
> > +
> > + memcpy(&chan->cfg, cfg, sizeof(*cfg));
> > + chan->dma_dir = DMA_MEM_TO_MEM;
>
> ?? looking at changelog we have only memcpy support, so this should not
> be here, pls remove this.
>
> > +static enum dma_status
> > +sf_pdma_tx_status(struct dma_chan *dchan,
> > + dma_cookie_t cookie,
> > + struct dma_tx_state *txstate)
> > +{
> > + struct sf_pdma_chan *chan = to_sf_pdma_chan(dchan);
> > + enum dma_status status;
> > +
> > + status = dma_cookie_status(dchan, cookie, txstate);
> > +
> > + if (txstate && status != DMA_ERROR)
> > + dma_set_residue(txstate, sf_pdma_desc_residue(chan));
>
> which residue? the query can be for a cookie which is still in pending
> list! you need to check the cookie and only read register for cookie if
> submitted
>
WIll fix this.
> > +static int sf_pdma_remove(struct platform_device *pdev)
> > +{
> > + struct sf_pdma *pdma = platform_get_drvdata(pdev);
> > + struct sf_pdma_chan *ch;
> > + int i;
> > +
> > + for (i = 0; i < PDMA_NR_CH; i++) {
> > + ch = &pdma->chans[i];
> > +
> > + list_del(&ch->vchan.chan.device_node);
> > + tasklet_kill(&ch->vchan.task);
> > + tasklet_kill(&ch->done_tasklet);
> > + tasklet_kill(&ch->err_tasklet);
>
> you have an isr registered which can fire and schedule tasklets..
will fix it by free irq first. Thanks for reviewing.
> --
> ~Vinod
Powered by blists - more mailing lists