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: <cf4f7b9f-d870-a4f9-5056-666f820cf032@quicinc.com>
Date:   Tue, 20 Sep 2022 11:31:50 +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>
 <e446d0c0-7084-8b5a-132e-977b25913343@quicinc.com>
 <YylU9hx/9j0QKLsO@hovoldconsulting.com>
From:   Rajendra Nayak <quic_rjendra@quicinc.com>
In-Reply-To: <YylU9hx/9j0QKLsO@hovoldconsulting.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 9/20/2022 11:21 AM, Johan Hovold wrote:
> On Tue, Sep 20, 2022 at 09:06:22AM +0530, Rajendra Nayak wrote:
>>
>>
>> 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/
> 
> Thanks for the link, that was was very informative.
> 
> Could you please update the commit message of patch 1/3 so that it also
> covers these systems and explains why you don't set the RETAIN_MEM and
> RETAIN_PERIPH bits for them?
> 
> As that commit message stands now, it seems that those bits must be set
> for retention to work.

Agree, I will update the commit message and re-spin, thanks.

> 
>>> 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