Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Aug 2023 21:27:10 +0530
From:   Dhruva Gole <d-gole@ti.com>
To:     Andrew Davis <afd@ti.com>
CC:     Nishanth Menon <nm@ti.com>, Tero Kristo <kristo@kernel.org>,
        "Santosh Shilimkar" <ssantosh@kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-pm@vger.kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Praneeth Bajjuri <praneeth@ti.com>,
        Tony Lindgren <tony@atomide.com>,
        Dave Gerlach <d-gerlach@ti.com>,
        Vibhore Vardhan <vibhore@ti.com>,
        Georgi Vlaev <g-vlaev@ti.com>,
        Roger Quadros <rogerq@kernel.org>
Subject: Re: [PATCH V6 3/4] firmware: ti_sci: Allocate memory for Low Power
 Modes
Message-ID: <20230803155710.fivxx7hufuravcdr@dhruva>
References: <20230803064247.503036-1-d-gole@ti.com>
 <20230803064247.503036-4-d-gole@ti.com>
 <7c12b109-d6b9-f0d4-cada-28140a33da21@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <7c12b109-d6b9-f0d4-cada-28140a33da21@ti.com>
Precedence: bulk

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@ti.com>
> > 
> > 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@ti.com>
> > Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> > Signed-off-by: Georgi Vlaev <g-vlaev@ti.com>
> > Tested-by: Roger Quadros <rogerq@kernel.org>
> > [d-gole@ti.com: Use dma_alloc_attrs instead of dma_alloc_coherent]
> > Signed-off-by: Dhruva Gole <d-gole@ti.com>
> > ---
> >   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@ti.com>