Hi Sia Jee Heng, see my comments inlined: > From: Sia Jee Heng <jee.heng.sia@xxxxxxxxx> > Sent: Tuesday, December 29, 2020 07:47 > To: vkoul@xxxxxxxxxx; Eugeniy Paltsev; robh+dt@xxxxxxxxxx > Cc: andriy.shevchenko@xxxxxxxxxxxxxxx; dmaengine@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx > Subject: [PATCH v8 13/16] dmaengine: dw-axi-dmac: Add Intel KeemBay AxiDMA handshake > > Add support for Intel KeemBay AxiDMA device handshake programming. > Device handshake number passed in to the AxiDMA shall be written to > the Intel KeemBay AxiDMA hardware handshake registers before DMA > operations are started. > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Signed-off-by: Sia Jee Heng <jee.heng.sia@xxxxxxxxx> > --- > .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 52 +++++++++++++++++++ > 1 file changed, 52 insertions(+) > > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 062d27c61983..5e77eb3d040f 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c [snip] > + > + return 0; > +} > + > /* > * If DW_axi_dmac sees CHx_CTL.ShadowReg_Or_LLI_Last bit of the fetched LLI > * as 1, it understands that the current block is the final block in the > @@ -626,6 +668,9 @@ dw_axi_dma_chan_prep_cyclic(struct dma_chan *dchan, dma_addr_t dma_addr, > llp = hw_desc->llp; > } while (num_periods); > > + if (dw_axi_dma_set_hw_channel(chan->chip, chan->hw_handshake_num, true)) > + goto err_desc_get; > + In this implementation 'dw_axi_dma_chan_prep_cyclic' callback will fail if we don't have APB registers which are only specific for KeemBay. Looking for the code of 'dw_axi_dma_chan_prep_cyclic' I don't see the reason why it shouldn't work for vanila DW AXI DMAC without this extension. So, could you please change this so we wouldn't reject dw_axi_dma_chan_prep_cyclic in case of APB registers are missed. > return vchan_tx_prep(&chan->vc, &desc->vd, flags); > > err_desc_get: > @@ -684,6 +729,9 @@ dw_axi_dma_chan_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl, > llp = hw_desc->llp; > } while (sg_len); > > + if (dw_axi_dma_set_hw_channel(chan->chip, chan->hw_handshake_num, true)) > + goto err_desc_get; > + Same here. > return vchan_tx_prep(&chan->vc, &desc->vd, flags); > > err_desc_get: > @@ -959,6 +1007,10 @@ static int dma_chan_terminate_all(struct dma_chan *dchan) > dev_warn(dchan2dev(dchan), > "%s failed to stop\n", axi_chan_name(chan)); > > + if (chan->direction != DMA_MEM_TO_MEM) > + dw_axi_dma_set_hw_channel(chan->chip, > + chan->hw_handshake_num, false); > + > spin_lock_irqsave(&chan->vc.lock, flags); > > vchan_get_all_descriptors(&chan->vc, &head); > -- > 2.18.0 >