Hi Morimoto-san, Thank you for the patch.
On Monday, 2 July 2018 04:07:17 EEST Kuninori Morimoto wrote: > From: Kuninori Morimoto <[email protected]> > > We need to clear channel register in error case as recovery. > The channel is already stopped in such case, thus we don't need to call > rcar_dmac_chan_halt() before clearing. > > rcar_dmac_chan_halt() will clear register and confirm DE bit. > But it will be failed because channel is already stopped in error case. > In other words, we shouldn't call it then. > > // This patch started to use C++ style comment out > // because it is recent Linus request I fear this will generate lots of frustration :-/ While I strongly prefer the traditional C style, I'm fine leaving the choice to driver authors. However, I think that mixing different styles in the same file only hinders readability. Would you like to convert the whole file ? ;-) > Reported-by: Hiroki Negishi <[email protected]> > Signed-off-by: Kuninori Morimoto <[email protected]> > Reviewed-by: Hiroki Negishi <[email protected]> > --- > drivers/dma/sh/rcar-dmac.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c > index 279c930..35d7a16 100644 > --- a/drivers/dma/sh/rcar-dmac.c > +++ b/drivers/dma/sh/rcar-dmac.c > @@ -1525,7 +1525,13 @@ static irqreturn_t rcar_dmac_isr_channel(int irq, > void *dev) > > chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR); > if (chcr & RCAR_DMACHCR_CAE) { > - rcar_dmac_chan_halt(chan); > + struct rcar_dmac *dmac = > dev_get_drvdata(chan->chan.device->dev); This could be simplified with struct rcar_dmac *dmac = to_rcar_dmac(chan->chan.device); > + > + // We don't need to call rcar_dmac_chan_halt() > + // because channel is already stopped in error case. > + // We need to clear register and check DE bit as recovery. Is it also a request from Linus to wrap text much before the 80 characters limit ? :-) > + rcar_dmac_write(dmac, RCAR_DMACHCLR, 1 << chan->index); > + rcar_dmac_chcr_de_barrier(chan); > reinit = true; > goto spin_lock_end; > } -- Regards, Laurent Pinchart
