RE: [Patch v2 2/4] dmaengine: tegra: Add Tegra GPC DMA driver

[Rajesh Gumasta]

> -----Original Message-----
> From: Dmitry Osipenko <digetx@xxxxxxxxx>
> Sent: Thursday, August 6, 2020 7:16 PM
> To: Rajesh Gumasta <rgumasta@xxxxxxxxxx>; Laxman Dewangan
> <ldewangan@xxxxxxxxxx>; Jonathan Hunter <jonathanh@xxxxxxxxxx>;
> vkoul@xxxxxxxxxx; dan.j.williams@xxxxxxxxx; thierry.reding@xxxxxxxxx;
> p.zabel@xxxxxxxxxxxxxx; dmaengine@xxxxxxxxxxxxxxx; linux-
> tegra@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: Krishna Yarlagadda <kyarlagadda@xxxxxxxxxx>; Pavan Kunapuli
> <pkunapuli@xxxxxxxxxx>
> Subject: Re: [Patch v2 2/4] dmaengine: tegra: Add Tegra GPC DMA driver
> 
> External email: Use caution opening links or attachments
> 
> 
> 06.08.2020 10:30, Rajesh Gumasta пишет:
> ...
> > +/*
> > + * Save and restore csr and channel register on pm_suspend
> > + * and pm_resume respectively
> > + */
> > +static int __maybe_unused tegra_dma_pm_suspend(struct device *dev)
> {
> > +     struct tegra_dma *tdma = dev_get_drvdata(dev);
> > +     int i;
> > +
> > +     for (i = 0; i < tdma->chip_data->nr_channels; i++) {
> > +             struct tegra_dma_channel *tdc = &tdma->channels[i];
> > +             struct tegra_dma_channel_regs *ch_reg =
> > + &tdc->channel_reg;
> > +
> > +             ch_reg->csr = tdc_read(tdc, TEGRA_GPCDMA_CHAN_CSR);
> > +             ch_reg->src_ptr = tdc_read(tdc,
> TEGRA_GPCDMA_CHAN_SRC_PTR);
> > +             ch_reg->dst_ptr = tdc_read(tdc,
> TEGRA_GPCDMA_CHAN_DST_PTR);
> > +             ch_reg->high_addr_ptr = tdc_read(tdc,
> > +                                              TEGRA_GPCDMA_CHAN_HIGH_ADDR_PTR);
> > +             ch_reg->mc_seq = tdc_read(tdc,
> TEGRA_GPCDMA_CHAN_MCSEQ);
> > +             ch_reg->mmio_seq = tdc_read(tdc,
> TEGRA_GPCDMA_CHAN_MMIOSEQ);
> > +             ch_reg->wcount = tdc_read(tdc,
> TEGRA_GPCDMA_CHAN_WCOUNT);
> > +     }
> > +     return 0;
> > +}
> > +
> > +static int __maybe_unused tegra_dma_pm_resume(struct device *dev) {
> > +     struct tegra_dma *tdma = dev_get_drvdata(dev);
> > +     int i;
> > +
> > +     for (i = 0; i < tdma->chip_data->nr_channels; i++) {
> > +             struct tegra_dma_channel *tdc = &tdma->channels[i];
> > +             struct tegra_dma_channel_regs *ch_reg =
> > + &tdc->channel_reg;
> > +
> > +             tdc_write(tdc, TEGRA_GPCDMA_CHAN_WCOUNT, ch_reg-
> >wcount);
> > +             tdc_write(tdc, TEGRA_GPCDMA_CHAN_DST_PTR, ch_reg-
> >dst_ptr);
> > +             tdc_write(tdc, TEGRA_GPCDMA_CHAN_SRC_PTR, ch_reg-
> >src_ptr);
> > +             tdc_write(tdc, TEGRA_GPCDMA_CHAN_HIGH_ADDR_PTR,
> > +                       ch_reg->high_addr_ptr);
> > +             tdc_write(tdc, TEGRA_GPCDMA_CHAN_MMIOSEQ, ch_reg-
> >mmio_seq);
> > +             tdc_write(tdc, TEGRA_GPCDMA_CHAN_MCSEQ, ch_reg->mc_seq);
> > +             tdc_write(tdc, TEGRA_GPCDMA_CHAN_CSR,
> > +                       (ch_reg->csr & ~TEGRA_GPCDMA_CSR_ENB));
> > +     }
> > +     return 0;
> > +}
> > +
> > +static const struct __maybe_unused dev_pm_ops
> tegra_dma_dev_pm_ops = {
> > +     SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(tegra_dma_pm_suspend,
> > +tegra_dma_pm_resume) };
> 
> Please explain why this is needed. All DMA should be stopped (not
> paused) on system's suspend, shouldn't it?
I have rechecked with HW verification team and they confirmed that after suspend, csr and channel registers will get reset hence on resume we need to restore back.
Also GPCDMA does not support power gate as a unit.

Thanks
Rajesh