Hi Hamza,
On Monday 19 October 2015 22:54:53 Hamza Farooq wrote:
> On Thu, Oct 15, 2015 at 10:42 PM, Laurent Pinchart wrote:
> > On Thursday 15 October 2015 18:08:56 Hamza Farooq wrote:
> >> Hi Laurent,
> >>
> >> Thank you for the review. Please see my comments below. Also please let
> >> me know if this code needs to be fixed on somewhat urgent basis since I
> >> am a little occupied with few other tasks. I'd try to prioritize it then
> >
> > I don't think this is urgent, so no worries.
> >
> >> On Thu, Oct 15, 2015 at 5:58 PM, Laurent Pinchart wrote:
> >> > On Tuesday 29 September 2015 22:44:43 hamzahfrq.sub@xxxxxxxxx wrote:
> >> >> From: Muhammad Hamza Farooq <mfarooq@xxxxxxxxxxx>
> >> >>
> >> >> DMA engine does not stop instantaneously when transaction is going on
> >> >> (see datasheet). Wait has been added
> >> >>
> >> >> Signed-off-by: Muhammad Hamza Farooq <mfarooq@xxxxxxxxxxx>
> >> >> ---
> >> >>
> >> >> drivers/dma/sh/rcar-dmac.c | 28 ++++++++++++++++++++++++++--
> >> >> 1 file changed, 26 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> >> >> index 7820d07..2b28291 100644
> >> >> --- a/drivers/dma/sh/rcar-dmac.c
> >> >> +++ b/drivers/dma/sh/rcar-dmac.c
> >> >> @@ -716,14 +716,38 @@ static int rcar_dmac_fill_hwdesc(struct
> >> >> rcar_dmac_chan *chan,
> >> >> /* -------------------------------------------------------------------
> >> >> * Stop and reset
> >> >> */
> >> >>
> >> >> +#define NR_READS_TO_WAIT 5 /* number of times to check if DE = 0 */
> >> >> +static inline int rcar_dmac_wait_stop(struct rcar_dmac_chan *chan)
> >> >> +{
> >> >> + unsigned int i = 0;
> >> >> +
> >> >> + do {
> >> >> + u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
> >> >> +
> >> >> + if (!(chcr & RCAR_DMACHCR_DE))
> >> >> + return 0;
> >> >> + cpu_relax();
> >> >> + dev_dbg(chan->chan.device->dev, "DMA xfer couldn't be
> >> >> stopped");
> >> >
> >> > Would it make sense to move the message out of the loop and use
> >> > dev_err() ? It seems like a pretty serious error.
> >>
> >> It happens quite often so dev_err might be too noisy. After couple
> >> attempts (1-3) it stops so I think it is better to not make it dev_err
> >
> > That's why I mentioned moving it outside of the loop and only printing it
> > when the DMA channel doesn't stop.
>
> I don't think it is a good idea to notify the user again and again
> about it, specially when it is handled by this patch. It occurs quite
> a lot (for me, a lot means around 10 times for a data transfer of 7 MB
> over sh-sci at 150 kbps)
I think there's some miscommunication going on. What I meant is
#define NR_READS_TO_WAIT 5 /* number of times to check if DE = 0 */
static inline int rcar_dmac_wait_stop(struct rcar_dmac_chan *chan)
{
unsigned int i;
for (i = 0; i < NR_READS_TO_WAIT; ++i) {
u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
if (!(chcr & RCAR_DMACHCR_DE))
return 0;
cpu_relax();
}
dev_err(chan->chan.device->dev, "DMA xfer couldn't be stopped");
return -EBUSY;
}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
