[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <be967357-6ecb-5ecf-c860-5265e1527b2a@gmail.com>
Date: Sat, 5 Nov 2022 20:49:49 +0200
From: Péter Ujfalusi <peter.ujfalusi@...il.com>
To: Georgi Vlaev <g-vlaev@...com>, Vinod Koul <vkoul@...nel.org>
Cc: Nishanth Menon <nm@...com>, dmaengine@...r.kernel.org,
linux-kernel@...r.kernel.org, Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH v2] dmaengine: k3-udma: Add system suspend/resume support
On 03/11/2022 22:30, Georgi Vlaev wrote:
> From: Vignesh Raghavendra <vigneshr@...com>
>
> The K3 platforms configure the DMA resources with the
> help of the TI's System Firmware's Device Manager(DM)
> over TISCI. The group of DMA related Resource Manager[1]
> TISCI messages includes: INTA, RINGACC, UDMAP, and PSI-L.
> This configuration however, does not persist in the DM
> after leaving from Suspend-to-RAM state. We have to restore
> the DMA channel configuration over TISCI for all configured
> channels when returning from suspend.
>
> The TISCI resource management calls for each DMA type (UDMA,
> PKTDMA, BCDMA) happen in device_free_chan_resources() and
> device_alloc_chan_resources(). In pm_suspend() we store
> the current udma_chan_config for channels that still have
> attached clients and call device_free_chan_resources().
> In pm_resume() restore the udma_channel_config from backup
> and call device_alloc_chan_resources() for those channels.
> Drivers like CPSW can do their own DMA resource management,
> so use the late system suspend/resume hooks.
It is wrong to push the DMA context store/restore task to a client
driver (cpsw or icss), it has to be done by the glue layer.
With this patch the DMAengine side of the UDMA/BCDMA/PKTDMA will support
suspend/resume while the networking will remain broken, right?
I can not test this atm since my setup relies solely on NFS rootfs via
cpsw, I might be able to check with a USB-ethernet dongle..
Please do a followup for the glue layer support.
Acked-by: Peter Ujfalusi <peter.ujfalusi@...il.com>
> [1] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/index.html#resource-management-rm
>
> Signed-off-by: Vignesh Raghavendra <vigneshr@...com>
> [g-vlaev@...com: Add patch description and config backup]
> [g-vlaev@...com: Supend only channels with clients]
> Signed-off-by: Georgi Vlaev <g-vlaev@...com>
> ---
> Changes:
>
> v2:
> * Update the commit message
> * Use list_for_each_entry() to iterate over the channel list.
>
> drivers/dma/ti/k3-udma.c | 54 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
> index ce8b80bb34d7..29844044132c 100644
> --- a/drivers/dma/ti/k3-udma.c
> +++ b/drivers/dma/ti/k3-udma.c
> @@ -304,6 +304,8 @@ struct udma_chan {
>
> /* Channel configuration parameters */
> struct udma_chan_config config;
> + /* Channel configuration parameters (backup) */
> + struct udma_chan_config backup_config;
>
> /* dmapool for packet mode descriptors */
> bool use_dma_pool;
> @@ -5491,11 +5493,63 @@ static int udma_probe(struct platform_device *pdev)
> return ret;
> }
>
> +static int udma_pm_suspend(struct device *dev)
> +{
> + struct udma_dev *ud = dev_get_drvdata(dev);
> + struct dma_device *dma_dev = &ud->ddev;
> + struct dma_chan *chan;
> + struct udma_chan *uc;
> +
> + list_for_each_entry(chan, &dma_dev->channels, device_node) {
> + if (chan->client_count) {
> + uc = to_udma_chan(chan);
> + /* backup the channel configuration */
> + memcpy(&uc->backup_config, &uc->config,
> + sizeof(struct udma_chan_config));
> + dev_dbg(dev, "Suspending channel %s\n",
> + dma_chan_name(chan));
> + ud->ddev.device_free_chan_resources(chan);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int udma_pm_resume(struct device *dev)
> +{
> + struct udma_dev *ud = dev_get_drvdata(dev);
> + struct dma_device *dma_dev = &ud->ddev;
> + struct dma_chan *chan;
> + struct udma_chan *uc;
> + int ret;
> +
> + list_for_each_entry(chan, &dma_dev->channels, device_node) {
> + if (chan->client_count) {
> + uc = to_udma_chan(chan);
> + /* restore the channel configuration */
> + memcpy(&uc->config, &uc->backup_config,
> + sizeof(struct udma_chan_config));
> + dev_dbg(dev, "Resuming channel %s\n",
> + dma_chan_name(chan));
> + ret = ud->ddev.device_alloc_chan_resources(chan);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops udma_pm_ops = {
> + SET_LATE_SYSTEM_SLEEP_PM_OPS(udma_pm_suspend, udma_pm_resume)
> +};
> +
> static struct platform_driver udma_driver = {
> .driver = {
> .name = "ti-udma",
> .of_match_table = udma_of_match,
> .suppress_bind_attrs = true,
> + .pm = &udma_pm_ops,
> },
> .probe = udma_probe,
> };
>
> base-commit: 81214a573d19ae2fa5b528286ba23cd1cb17feec
--
Péter
Powered by blists - more mailing lists