From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753221AbeDSELM (ORCPT ); Thu, 19 Apr 2018 00:11:12 -0400 Received: from lelnx194.ext.ti.com ([198.47.27.80]:50519 "EHLO lelnx194.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750913AbeDSELL (ORCPT ); Thu, 19 Apr 2018 00:11:11 -0400 Subject: Re: [PATCH 01/14] memory: ti-emif-sram: Add resume function to recopy sram code To: "santosh.shilimkar@oracle.com" , , , CC: , , , , , , , References: <1523505239-16229-1-git-send-email-j-keerthy@ti.com> <1523505239-16229-2-git-send-email-j-keerthy@ti.com> <31688cf4-b6ca-e7ce-3407-46262006b38f@oracle.com> From: Keerthy Message-ID: <8e92db66-c2cb-8110-120d-fb6ff06ac643@ti.com> Date: Mon, 16 Apr 2018 15:50:32 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <31688cf4-b6ca-e7ce-3407-46262006b38f@oracle.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday 12 April 2018 10:14 PM, santosh.shilimkar@oracle.com wrote: > On 4/11/18 9:53 PM, Keerthy wrote: >> From: Dave Gerlach >> >> After an RTC+DDR cycle we lose sram context so emif pm functions present >> in sram are lost. We can check if the first byte of the original >> code in DDR contains the same first byte as the code in sram, and if >> they do not match we know we have lost context and must recopy the >> functions to the previous address to maintain PM functionality. >> >> Signed-off-by: Dave Gerlach >> Signed-off-by: Keerthy >> --- >>   drivers/memory/ti-emif-pm.c | 24 ++++++++++++++++++++++++ >>   1 file changed, 24 insertions(+) >> >> diff --git a/drivers/memory/ti-emif-pm.c b/drivers/memory/ti-emif-pm.c >> index 632651f..ec4a62c 100644 >> --- a/drivers/memory/ti-emif-pm.c >> +++ b/drivers/memory/ti-emif-pm.c >> @@ -249,6 +249,25 @@ int ti_emif_get_mem_type(void) >>   }; >>   MODULE_DEVICE_TABLE(of, ti_emif_of_match); >>   +#ifdef CONFIG_PM_SLEEP >> +static int ti_emif_resume(struct device *dev) >> +{ >> +    unsigned long tmp = >> +            __raw_readl((void *)emif_instance->ti_emif_sram_virt); >> + >> +    /* >> +     * Check to see if what we are copying is already present in the >> +     * first byte at the destination, only copy if it is not which >> +     * indicates we have lost context and sram no longer contains >> +     * the PM code >> +     */ > >> +    if (tmp != ti_emif_sram) >> +        ti_emif_push_sram(dev, emif_instance); >> + >> +    return 0; >> +} >> +#endif /* CONFIG_PM_SLEEP */ > Instead of this indirect method , why can't just check the previous > deep sleep mode and based on that do copy or not. EMIF power status > register should have something like that ? I will check if we have a register that tells previous state of sram, not sure of it. > > Another minor point is even though there is nothing to do in suspend, > might be good to have a callback with comment that nothing to do with > some explanation why not. Don't have strong preference but may for > better readability. > > Regards, > Santosh > >