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: <48c560ef-67fd-8903-a7c0-2fd7a9bd6b19@quicinc.com>
Date:   Tue, 20 Sep 2022 15:11:04 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v6 3/5] phy: core: Add support for phy power down & power
 up
Content-Language: en-US
To:     Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
CC:     <helgaas@kernel.org>, <linux-pci@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <mka@chromium.org>, <quic_vbadigan@quicinc.com>,
        <quic_hemantk@quicinc.com>, <quic_nitegupt@quicinc.com>,
        <quic_skananth@quicinc.com>, <quic_ramkri@quicinc.com>,
        <manivannan.sadhasivam@linaro.org>, <swboyd@chromium.org>,
        Kishon Vijay Abraham I <kishon@ti.com>,
        Vinod Koul <vkoul@kernel.org>,
        "open list:GENERIC PHY FRAMEWORK" <linux-phy@lists.infradead.org>
References: <1662713084-8106-1-git-send-email-quic_krichai@quicinc.com>
 <1662713084-8106-4-git-send-email-quic_krichai@quicinc.com>
 <CAA8EJppgaAuEDU44ePOt+ZWK0_rNsXHnE3WOEc9F-n=VE=3aVQ@mail.gmail.com>
 <87138bc9-2fc0-39fb-89c1-d3826e28594e@quicinc.com>
 <bd6fc826-94b9-f539-a37e-820ab49b9d14@linaro.org>
From:   Krishna Chaitanya Chundru <quic_krichai@quicinc.com>
In-Reply-To: <bd6fc826-94b9-f539-a37e-820ab49b9d14@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 9/19/2022 10:59 PM, Dmitry Baryshkov wrote:
> On 14/09/2022 17:50, Krishna Chaitanya Chundru wrote:
>>
>> On 9/9/2022 2:34 PM, Dmitry Baryshkov wrote:
>>> On Fri, 9 Sept 2022 at 11:45, Krishna chaitanya chundru
>>> <quic_krichai@quicinc.com> wrote:
>>>> Introducing phy power down/up callbacks for allowing to park the
>>>> link-state in L1ss without holding any PCIe resources during
>>>> system suspend.
>>>>
>>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>>> ---
>>>>   drivers/phy/phy-core.c  | 30 ++++++++++++++++++++++++++++++
>>>>   include/linux/phy/phy.h | 20 ++++++++++++++++++++
>>>>   2 files changed, 50 insertions(+)
>>>>
>>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>>> index d93ddf1..1b0b757 100644
>>>> --- a/drivers/phy/phy-core.c
>>>> +++ b/drivers/phy/phy-core.c
>>>> @@ -441,6 +441,36 @@ int phy_set_speed(struct phy *phy, int speed)
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(phy_set_speed);
>>>>
>>>> +int phy_power_down(struct phy *phy)
>>>> +{
>>>> +       int ret;
>>>> +
>>>> +       if (!phy || !phy->ops->power_down)
>>>> +               return 0;
>>>> +
>>>> +       mutex_lock(&phy->mutex);
>>>> +       ret = phy->ops->power_down(phy);
>>>> +       mutex_unlock(&phy->mutex);
>>>> +
>>>> +       return ret;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(phy_power_down);
>>>> +
>>>> +int phy_power_up(struct phy *phy)
>>>> +{
>>>> +       int ret;
>>>> +
>>>> +       if (!phy || !phy->ops->power_up)
>>>> +               return 0;
>>>> +
>>>> +       mutex_lock(&phy->mutex);
>>>> +       ret = phy->ops->power_up(phy);
>>>> +       mutex_unlock(&phy->mutex);
>>>> +
>>>> +       return ret;
>>>> +}
>>> As it can be seen from the phy_power_off(), the PHY can be a shared
>>> resource, with the power_count counting the number of users that
>>> requested the PHY to be powered up. By introducing suc calls you break
>>> directly into this by allowing a single user to power down the PHY, no
>>> matter how many other users have requested the PHY to stay alive.
>>
>> can we use same power_count in this function also here and restrict 
>> the single user to
>>
>> power down the PHY same like phy_power_off?.
>
> What is the difference between power_off() and power_down()?

In power_off  we are turning off PCIe PHY-specific clocks, and also 
resetting the PHY due to this PCIe link
also will go down. To retain, the PCIe link state in l1ss with PHY 
clocks turned off, we need
park PCIe PHY in the power-down state and skip the resets of the PHY so 
that it can maintain the link state in l1ss
with the help of the always-on power domain aka MX).

To support this PHY Power-down state PHY driver has been updated with 
new interface APIs.

Initially we added phy_suspend & phy_resume to phy but as this API's are 
already used by drivers/net/phy/phy_device.c we are getting compilation 
errors.

So we used these power_down & power up.

As power_off & power_down is confusing we will change new api's 
power_down & power_up to phy_pm_suspend & phy_pm_resume in

the next patch series.
>
>