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;
Message-ID: <e446d0c0-7084-8b5a-132e-977b25913343@quicinc.com>
Date:   Tue, 20 Sep 2022 09:06:22 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH v2 2/3] clk: qcom: gcc-sc7180: Update the .pwrsts for usb
 gdsc
Content-Language: en-US
To:     Johan Hovold <johan@kernel.org>
CC:     <agross@kernel.org>, <andersson@kernel.org>,
        <konrad.dybcio@somainline.org>, <mturquette@baylibre.com>,
        <sboyd@kernel.org>, <mka@chromium.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <johan+linaro@kernel.org>, <quic_kriskura@quicinc.com>,
        <dianders@chromium.org>, <linux-clk@vger.kernel.org>
References: <20220916102417.24549-1-quic_rjendra@quicinc.com>
 <20220916102417.24549-2-quic_rjendra@quicinc.com>
 <YyiOiudZuMDXOvGr@hovoldconsulting.com>
From:   Rajendra Nayak <quic_rjendra@quicinc.com>
In-Reply-To: <YyiOiudZuMDXOvGr@hovoldconsulting.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 9/19/2022 9:15 PM, Johan Hovold wrote:
> On Fri, Sep 16, 2022 at 03:54:16PM +0530, Rajendra Nayak wrote:
>> The USB controller on sc7180 does not retain the state when
>> the system goes into low power state and the GDSC is
>> turned off. This results in the controller reinitializing and
>> re-enumerating all the connected devices (resulting in additional
>> delay while coming out of suspend)
>> Fix this by updating the .pwrsts for the USB GDSC so it only
>> transitions to retention state in low power.
>>
>> Signed-off-by: Rajendra Nayak <quic_rjendra@quicinc.com>
>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>> Tested-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>> v2:
>> Updated the changelog
>>
>>   drivers/clk/qcom/gcc-sc7180.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/qcom/gcc-sc7180.c b/drivers/clk/qcom/gcc-sc7180.c
>> index c2ea09945c47..2d3980251e78 100644
>> --- a/drivers/clk/qcom/gcc-sc7180.c
>> +++ b/drivers/clk/qcom/gcc-sc7180.c
>> @@ -2224,7 +2224,7 @@ static struct gdsc usb30_prim_gdsc = {
>>   	.pd = {
>>   		.name = "usb30_prim_gdsc",
>>   	},
>> -	.pwrsts = PWRSTS_OFF_ON,
>> +	.pwrsts = PWRSTS_RET_ON,
>>   };
>>   
>>   static struct gdsc hlos1_vote_mmnoc_mmu_tbu_hf0_gdsc = {
> 
> It seems like the above will not work unless you also provide the
> registers offsets that gdsc_force_mem_on() expects.

That's true, but its needed only on platforms that support complete
CX domain power collapse. sc7280 (and sc7180) does not support
that and hence we can achieve GDSC RET without any of the RETAIN_MEM/
RETAIN_PERIPH bits programmed.
I explained some of that in detail on another related thread a
while back [1]

[1] https://lore.kernel.org/all/5ff21b1e-3af9-36ef-e13e-fa33f526d0e3@quicinc.com/

> 
> Specifically, unless you set cxc_count for the GDSC, the above call is a
> no-op and the retention setting (i.e. the RETAIN_MEM and RETAIN_PERIPH
> bits) will not be updated when registering the GDSC.
> 
> Johan