Re: [PATCH v6 3/5] phy: core: Add support for phy power down & power up

[Krishna Chaitanya Chundru <quic_krichai@quicinc.com>]

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy