Re: [PATCH V6 3/4] firmware: ti_sci: Allocate memory for Low Power Modes

From: Dhruva Gole
Date: Thu Aug 03 2023 - 11:57:23 EST


On Aug 03, 2023 at 10:23:47 -0500, Andrew Davis wrote:
> On 8/3/23 1:42 AM, Dhruva Gole wrote:
> > From: Dave Gerlach <d-gerlach@xxxxxx>
> >
> > A region of memory in DDR must be used during Deep Sleep for saving
> > of some system context when using the ti_sci firmware. From DM's point
> > of view, this can be any contiguous region in the DDR, so can allocate
> > 512KB of DMA reserved memory in probe(), instead of another carveout.
> >
> > Also send a TISCI_MSG_QUERY_FW_CAPS message to the firmware during
> > probe to determine if any low power modes are supported and if
> > ti_sci_init_suspend should be called based on the response received.
> >
> > Signed-off-by: Dave Gerlach <d-gerlach@xxxxxx>
> > Signed-off-by: Vibhore Vardhan <vibhore@xxxxxx>
> > Signed-off-by: Georgi Vlaev <g-vlaev@xxxxxx>
> > Tested-by: Roger Quadros <rogerq@xxxxxxxxxx>
> > [d-gole@xxxxxx: Use dma_alloc_attrs instead of dma_alloc_coherent]
> > Signed-off-by: Dhruva Gole <d-gole@xxxxxx>
> > ---
> > drivers/firmware/ti_sci.c | 42 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
[..snip..]
> > static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
> > .default_host_id = 2,
> > @@ -3661,6 +3690,15 @@ static int ti_sci_probe(struct platform_device *pdev)
> > }
> > }
> > + /*
> > + * Check if the firmware supports any optional low power modes
> > + * and initialize them if present. Old revisions of TIFS (< 08.04)
> > + * will NACK the request.
> > + */
> > + ret = ti_sci_msg_cmd_query_fw_caps(&info->handle, &info->fw_caps);
> > + if (!ret && (info->fw_caps & MSG_MASK_CAPS_LPM))
> > + ti_sci_init_suspend(pdev, info);
> > +
> > dev_info(dev, "ABI: %d.%d (firmware rev 0x%04x '%s')\n",
> > info->handle.version.abi_major, info->handle.version.abi_minor,
> > info->handle.version.firmware_revision,
> > @@ -3708,6 +3746,10 @@ static int ti_sci_remove(struct platform_device *pdev)
> > mbox_free_channel(info->chan_rx);
> > }
> > + if (info->ctx_mem_buf)
> > + dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE,
>
> You allocated with dma_alloc_attrs() you should free with dma_free_attrs().

Good catch, will fix this in next revision

>
> Andrew
>
> > + info->ctx_mem_buf,
> > + info->ctx_mem_addr);
> > return ret;
> > }

--
Best regards,
Dhruva Gole <d-gole@xxxxxx>