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

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

[Krishna chaitanya chundru]

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;
+}
+EXPORT_SYMBOL_GPL(phy_power_up);
+
 int phy_reset(struct phy *phy)
 {
 	int ret;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index b141375..3a45f4d 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -76,6 +76,8 @@ union phy_configure_opts {
  * @set_mode: set the mode of the phy
  * @set_media: set the media type of the phy (optional)
  * @set_speed: set the speed of the phy (optional)
+ * @power_down: parking the phy in power down state
+ * @power_up: pulling back the phy from power down
  * @reset: resetting the phy
  * @calibrate: calibrate the phy
  * @release: ops to be performed while the consumer relinquishes the PHY
@@ -89,6 +91,8 @@ struct phy_ops {
 	int	(*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
 	int	(*set_media)(struct phy *phy, enum phy_media media);
 	int	(*set_speed)(struct phy *phy, int speed);
+	int	(*power_down)(struct phy *phy);
+	int	(*power_up)(struct phy *phy);
 
 	/**
 	 * @configure:
@@ -226,6 +230,8 @@ int phy_init(struct phy *phy);
 int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
+int phy_power_down(struct phy *phy);
+int phy_power_up(struct phy *phy);
 int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
 #define phy_set_mode(phy, mode) \
 	phy_set_mode_ext(phy, mode, 0)
@@ -349,6 +355,20 @@ static inline int phy_power_off(struct phy *phy)
 	return -ENOSYS;
 }
 
+static inline int phy_power_down(struct phy *phy)
+{
+	if (!phy)
+		return 0;
+	return -ENOSYS;
+}
+
+static inline int phy_power_up(struct phy *phy)
+{
+	if (!phy)
+		return 0;
+	return -ENOSYS;
+}
+
 static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode,
 				   int submode)
 {
-- 
2.7.4


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

[PATCH v6 4/5] phy: qcom: Add power down/up callbacks to pcie phy

[Krishna chaitanya chundru]

Add phy power down/up callbacks to pcie phy. Using these callbacks
we can release phy resources like phy specific clocks but continue
maintain pcie link in l1ss state.

This can help in parking pcie link in l1ss state during system
suspend (S3).

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c   |  6 ++--
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 15c2067..1d4b1b0 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1326,7 +1326,8 @@ static int qcom_pcie_resume_2_7_0(struct qcom_pcie *pcie)
 
 	ret = clk_bulk_prepare_enable(res->num_clks, res->clks);
 
-	phy_power_on(pcie->phy);
+	/* Bring back PHY from power down */
+	phy_power_up(pcie->phy);
 
 	return ret;
 }
@@ -1335,7 +1336,8 @@ static int qcom_pcie_suspend_2_7_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
 
-	phy_power_off(pcie->phy);
+	/* Power down PHY to park the link state in L1ss */
+	phy_power_down(pcie->phy);
 
 	clk_bulk_disable_unprepare(res->num_clks, res->clks);
 	return 0;
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 3ddbb8e..c6b3b82 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -2145,6 +2145,54 @@ static int qcom_qmp_phy_pcie_exit(struct phy *phy)
 	return 0;
 }
 
+static int qcom_qmp_phy_pcie_power_up(struct phy *phy)
+{
+	struct qmp_phy *qphy = phy_get_drvdata(phy);
+	struct qcom_qmp *qmp = qphy->qmp;
+	const struct qmp_phy_cfg *cfg = qphy->cfg;
+	int ret;
+
+	ret = clk_bulk_prepare_enable(cfg->num_clks, qmp->clks);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(qphy->pipe_clk);
+	if (ret)
+		return ret;
+
+	/* Pull out PHY from POWER DOWN state */
+	if (cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL]) {
+		qphy_setbits(qphy->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
+			     cfg->pwrdn_ctrl);
+	} else {
+		qphy_setbits(qphy->pcs, QPHY_V2_PCS_POWER_DOWN_CONTROL,
+				cfg->pwrdn_ctrl);
+	}
+
+	return 0;
+}
+
+static int qcom_qmp_phy_pcie_power_down(struct phy *phy)
+{
+	struct qmp_phy *qphy = phy_get_drvdata(phy);
+	struct qcom_qmp *qmp = qphy->qmp;
+	const struct qmp_phy_cfg *cfg = qphy->cfg;
+
+	clk_disable_unprepare(qphy->pipe_clk);
+	clk_bulk_disable_unprepare(cfg->num_clks, qmp->clks);
+
+	/* Put PHY into POWER DOWN state: active low */
+	if (cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL]) {
+		qphy_clrbits(qphy->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
+			     cfg->pwrdn_ctrl);
+	} else {
+		qphy_clrbits(qphy->pcs, QPHY_V2_PCS_POWER_DOWN_CONTROL,
+				cfg->pwrdn_ctrl);
+	}
+
+	return 0;
+}
+
 static int qcom_qmp_phy_pcie_enable(struct phy *phy)
 {
 	int ret;
@@ -2304,6 +2352,8 @@ static const struct phy_ops qcom_qmp_phy_pcie_ops = {
 	.power_on	= qcom_qmp_phy_pcie_enable,
 	.power_off	= qcom_qmp_phy_pcie_disable,
 	.set_mode	= qcom_qmp_phy_pcie_set_mode,
+	.power_down	= qcom_qmp_phy_pcie_power_down,
+	.power_up	= qcom_qmp_phy_pcie_power_up,
 	.owner		= THIS_MODULE,
 };
 
-- 
2.7.4


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

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

[Dmitry Baryshkov]

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.

> +EXPORT_SYMBOL_GPL(phy_power_up);
> +
>  int phy_reset(struct phy *phy)
>  {
>         int ret;
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index b141375..3a45f4d 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -76,6 +76,8 @@ union phy_configure_opts {
>   * @set_mode: set the mode of the phy
>   * @set_media: set the media type of the phy (optional)
>   * @set_speed: set the speed of the phy (optional)
> + * @power_down: parking the phy in power down state
> + * @power_up: pulling back the phy from power down
>   * @reset: resetting the phy
>   * @calibrate: calibrate the phy
>   * @release: ops to be performed while the consumer relinquishes the PHY
> @@ -89,6 +91,8 @@ struct phy_ops {
>         int     (*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
>         int     (*set_media)(struct phy *phy, enum phy_media media);
>         int     (*set_speed)(struct phy *phy, int speed);
> +       int     (*power_down)(struct phy *phy);
> +       int     (*power_up)(struct phy *phy);
>
>         /**
>          * @configure:
> @@ -226,6 +230,8 @@ int phy_init(struct phy *phy);
>  int phy_exit(struct phy *phy);
>  int phy_power_on(struct phy *phy);
>  int phy_power_off(struct phy *phy);
> +int phy_power_down(struct phy *phy);
> +int phy_power_up(struct phy *phy);
>  int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
>  #define phy_set_mode(phy, mode) \
>         phy_set_mode_ext(phy, mode, 0)
> @@ -349,6 +355,20 @@ static inline int phy_power_off(struct phy *phy)
>         return -ENOSYS;
>  }
>
> +static inline int phy_power_down(struct phy *phy)
> +{
> +       if (!phy)
> +               return 0;
> +       return -ENOSYS;
> +}
> +
> +static inline int phy_power_up(struct phy *phy)
> +{
> +       if (!phy)
> +               return 0;
> +       return -ENOSYS;
> +}
> +
>  static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode,
>                                    int submode)
>  {
> --
> 2.7.4
>


-- 
With best wishes
Dmitry

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

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

[Vinod Koul]

On 09-09-22, 14:14, Krishna chaitanya chundru wrote:
> Introducing phy power down/up callbacks for allowing to park the
> link-state in L1ss without holding any PCIe resources during
> system suspend.

where is the rest of the series, pls cc relevant folks on cover at
least!

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

I dont think this is a good idea as Dmitry already updated...

> +
> +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;
> +}
> +EXPORT_SYMBOL_GPL(phy_power_up);
> +
>  int phy_reset(struct phy *phy)
>  {
>  	int ret;
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index b141375..3a45f4d 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -76,6 +76,8 @@ union phy_configure_opts {
>   * @set_mode: set the mode of the phy
>   * @set_media: set the media type of the phy (optional)
>   * @set_speed: set the speed of the phy (optional)
> + * @power_down: parking the phy in power down state
> + * @power_up: pulling back the phy from power down
>   * @reset: resetting the phy
>   * @calibrate: calibrate the phy
>   * @release: ops to be performed while the consumer relinquishes the PHY
> @@ -89,6 +91,8 @@ struct phy_ops {
>  	int	(*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
>  	int	(*set_media)(struct phy *phy, enum phy_media media);
>  	int	(*set_speed)(struct phy *phy, int speed);
> +	int	(*power_down)(struct phy *phy);
> +	int	(*power_up)(struct phy *phy);
>  
>  	/**
>  	 * @configure:
> @@ -226,6 +230,8 @@ int phy_init(struct phy *phy);
>  int phy_exit(struct phy *phy);
>  int phy_power_on(struct phy *phy);
>  int phy_power_off(struct phy *phy);
> +int phy_power_down(struct phy *phy);
> +int phy_power_up(struct phy *phy);
>  int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
>  #define phy_set_mode(phy, mode) \
>  	phy_set_mode_ext(phy, mode, 0)
> @@ -349,6 +355,20 @@ static inline int phy_power_off(struct phy *phy)
>  	return -ENOSYS;
>  }
>  
> +static inline int phy_power_down(struct phy *phy)
> +{
> +	if (!phy)
> +		return 0;
> +	return -ENOSYS;
> +}
> +
> +static inline int phy_power_up(struct phy *phy)
> +{
> +	if (!phy)
> +		return 0;
> +	return -ENOSYS;
> +}
> +
>  static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode,
>  				   int submode)
>  {
> -- 
> 2.7.4

-- 
~Vinod

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

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

[Bjorn Helgaas]

On Tue, Sep 13, 2022 at 08:28:15PM +0530, Vinod Koul wrote:
> On 09-09-22, 14:14, Krishna chaitanya chundru wrote:
> > Introducing phy power down/up callbacks for allowing to park the
> > link-state in L1ss without holding any PCIe resources during
> > system suspend.
> 
> where is the rest of the series, pls cc relevant folks on cover at
> least!

Would be best to cc relevant folks, but in the meantime, it's easy to
find via lore, e.g., 3/5 has:

  Message-Id: <1662713084-8106-4-git-send-email-quic_krichai@quicinc.com>

so the lore URL is:

  https://lore.kernel.org/r/1662713084-8106-4-git-send-email-quic_krichai@quicinc.com

and the thread overview is at the bottom.

I use this incredibly handy mutt hook that adds lore URLs to emails
directly when viewing them: https://github.com/danrue/lorifier

Bjorn

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

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

[Krishna Chaitanya Chundru]

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

>> +EXPORT_SYMBOL_GPL(phy_power_up);
>> +
>>   int phy_reset(struct phy *phy)
>>   {
>>          int ret;
>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>> index b141375..3a45f4d 100644
>> --- a/include/linux/phy/phy.h
>> +++ b/include/linux/phy/phy.h
>> @@ -76,6 +76,8 @@ union phy_configure_opts {
>>    * @set_mode: set the mode of the phy
>>    * @set_media: set the media type of the phy (optional)
>>    * @set_speed: set the speed of the phy (optional)
>> + * @power_down: parking the phy in power down state
>> + * @power_up: pulling back the phy from power down
>>    * @reset: resetting the phy
>>    * @calibrate: calibrate the phy
>>    * @release: ops to be performed while the consumer relinquishes the PHY
>> @@ -89,6 +91,8 @@ struct phy_ops {
>>          int     (*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
>>          int     (*set_media)(struct phy *phy, enum phy_media media);
>>          int     (*set_speed)(struct phy *phy, int speed);
>> +       int     (*power_down)(struct phy *phy);
>> +       int     (*power_up)(struct phy *phy);
>>
>>          /**
>>           * @configure:
>> @@ -226,6 +230,8 @@ int phy_init(struct phy *phy);
>>   int phy_exit(struct phy *phy);
>>   int phy_power_on(struct phy *phy);
>>   int phy_power_off(struct phy *phy);
>> +int phy_power_down(struct phy *phy);
>> +int phy_power_up(struct phy *phy);
>>   int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode);
>>   #define phy_set_mode(phy, mode) \
>>          phy_set_mode_ext(phy, mode, 0)
>> @@ -349,6 +355,20 @@ static inline int phy_power_off(struct phy *phy)
>>          return -ENOSYS;
>>   }
>>
>> +static inline int phy_power_down(struct phy *phy)
>> +{
>> +       if (!phy)
>> +               return 0;
>> +       return -ENOSYS;
>> +}
>> +
>> +static inline int phy_power_up(struct phy *phy)
>> +{
>> +       if (!phy)
>> +               return 0;
>> +       return -ENOSYS;
>> +}
>> +
>>   static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode,
>>                                     int submode)
>>   {
>> --
>> 2.7.4
>>
>

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

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

[Dmitry Baryshkov]

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()?


-- 
With best wishes
Dmitry


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

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

[Krishna Chaitanya Chundru]

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