linux-phy.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops
@ 2025-10-27  6:24 Prashanth K
  2025-10-27  8:25 ` Konrad Dybcio
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Prashanth K @ 2025-10-27  6:24 UTC (permalink / raw)
  To: Vinod Koul, Kishon Vijay Abraham I
  Cc: linux-arm-msm, linux-phy, linux-kernel, Prashanth K

Add runtime power management operation callbacks for M31 EUSB2 PHY.
Enable/disable the clocks based on the runtime suspend/resume calls.

Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-m31-eusb2.c | 40 +++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c b/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
index 0a0d2d9fc846..1aeb5e3de07c 100644
--- a/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
@@ -240,6 +240,34 @@ static const struct phy_ops m31eusb2_phy_gen_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static int m31eusb2_phy_runtime_suspend(struct device *dev)
+{
+	struct m31eusb2_phy *phy = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "Suspending M31 eUSB2 Phy\n");
+	clk_disable_unprepare(phy->clk);
+
+	return 0;
+}
+
+static int m31eusb2_phy_runtime_resume(struct device *dev)
+{
+	struct m31eusb2_phy *phy = dev_get_drvdata(dev);
+	int ret = 0;
+
+	dev_dbg(dev, "Resuming M31 eUSB2 Phy\n");
+	ret = clk_prepare_enable(phy->clk);
+	if (ret)
+		dev_err(dev, "failed to enable ref clock, %d\n", ret);
+
+	return ret;
+}
+
+static const struct dev_pm_ops m31eusb2_phy_pm_ops = {
+	SET_RUNTIME_PM_OPS(m31eusb2_phy_runtime_suspend,
+			   m31eusb2_phy_runtime_resume, NULL)
+};
+
 static int m31eusb2_phy_probe(struct platform_device *pdev)
 {
 	struct phy_provider *phy_provider;
@@ -270,6 +298,17 @@ static int m31eusb2_phy_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(phy->clk),
 				     "failed to get clk\n");
 
+	dev_set_drvdata(dev, phy);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
+	/*
+	 * Prevent runtime pm from being ON by default. Users can enable
+	 * it using power/control in sysfs.
+	 */
+	pm_runtime_forbid(dev);
+
+
 	phy->phy = devm_phy_create(dev, NULL, &m31eusb2_phy_gen_ops);
 	if (IS_ERR(phy->phy))
 		return dev_err_probe(dev, PTR_ERR(phy->phy),
@@ -313,6 +352,7 @@ static struct platform_driver m31eusb2_phy_driver = {
 	.probe = m31eusb2_phy_probe,
 	.driver = {
 		.name = "qcom-m31eusb2-phy",
+		.pm = &m31eusb2_phy_pm_ops,
 		.of_match_table = m31eusb2_phy_id_table,
 	},
 };
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops
  2025-10-27  6:24 [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops Prashanth K
@ 2025-10-27  8:25 ` Konrad Dybcio
  2025-10-27 11:09   ` Prashanth K
  2025-10-27 22:17 ` kernel test robot
  2025-10-28 14:17 ` Bjorn Andersson
  2 siblings, 1 reply; 7+ messages in thread
From: Konrad Dybcio @ 2025-10-27  8:25 UTC (permalink / raw)
  To: Prashanth K, Vinod Koul, Kishon Vijay Abraham I
  Cc: linux-arm-msm, linux-phy, linux-kernel

On 10/27/25 7:24 AM, Prashanth K wrote:
> Add runtime power management operation callbacks for M31 EUSB2 PHY.
> Enable/disable the clocks based on the runtime suspend/resume calls.
> 
> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> ---

[...]

>  static int m31eusb2_phy_probe(struct platform_device *pdev)
>  {
>  	struct phy_provider *phy_provider;
> @@ -270,6 +298,17 @@ static int m31eusb2_phy_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(phy->clk),
>  				     "failed to get clk\n");
>  
> +	dev_set_drvdata(dev, phy);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
> +	/*
> +	 * Prevent runtime pm from being ON by default. Users can enable
> +	 * it using power/control in sysfs.
> +	 */
> +	pm_runtime_forbid(dev);

This screams "this patch is broken" or "there are bad issues" which
you did not describe at all.

Konrad

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops
  2025-10-27  8:25 ` Konrad Dybcio
@ 2025-10-27 11:09   ` Prashanth K
  2025-10-27 11:11     ` Konrad Dybcio
  0 siblings, 1 reply; 7+ messages in thread
From: Prashanth K @ 2025-10-27 11:09 UTC (permalink / raw)
  To: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I
  Cc: linux-arm-msm, linux-phy, linux-kernel



On 10/27/2025 1:55 PM, Konrad Dybcio wrote:
> On 10/27/25 7:24 AM, Prashanth K wrote:
>> Add runtime power management operation callbacks for M31 EUSB2 PHY.
>> Enable/disable the clocks based on the runtime suspend/resume calls.
>>
>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>>  static int m31eusb2_phy_probe(struct platform_device *pdev)
>>  {
>>  	struct phy_provider *phy_provider;
>> @@ -270,6 +298,17 @@ static int m31eusb2_phy_probe(struct platform_device *pdev)
>>  		return dev_err_probe(dev, PTR_ERR(phy->clk),
>>  				     "failed to get clk\n");
>>  
>> +	dev_set_drvdata(dev, phy);
>> +	pm_runtime_set_active(dev);
>> +	pm_runtime_enable(dev);
>> +
>> +	/*
>> +	 * Prevent runtime pm from being ON by default. Users can enable
>> +	 * it using power/control in sysfs.
>> +	 */
>> +	pm_runtime_forbid(dev);
> 
> This screams "this patch is broken" or "there are bad issues" which
> you did not describe at all.
> 
> Konrad

Hi Konrad, I was followed the same sequence from other phy drivers
containing pm_ops. I assume the runtime_forbid is added to control
runtime pm from userspace.

Regards,
Prashanth K

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops
  2025-10-27 11:09   ` Prashanth K
@ 2025-10-27 11:11     ` Konrad Dybcio
  2025-10-28 11:11       ` Prashanth K
  0 siblings, 1 reply; 7+ messages in thread
From: Konrad Dybcio @ 2025-10-27 11:11 UTC (permalink / raw)
  To: Prashanth K, Vinod Koul, Kishon Vijay Abraham I
  Cc: linux-arm-msm, linux-phy, linux-kernel

On 10/27/25 12:09 PM, Prashanth K wrote:
> 
> 
> On 10/27/2025 1:55 PM, Konrad Dybcio wrote:
>> On 10/27/25 7:24 AM, Prashanth K wrote:
>>> Add runtime power management operation callbacks for M31 EUSB2 PHY.
>>> Enable/disable the clocks based on the runtime suspend/resume calls.
>>>
>>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>>> ---
>>
>> [...]
>>
>>>  static int m31eusb2_phy_probe(struct platform_device *pdev)
>>>  {
>>>  	struct phy_provider *phy_provider;
>>> @@ -270,6 +298,17 @@ static int m31eusb2_phy_probe(struct platform_device *pdev)
>>>  		return dev_err_probe(dev, PTR_ERR(phy->clk),
>>>  				     "failed to get clk\n");
>>>  
>>> +	dev_set_drvdata(dev, phy);
>>> +	pm_runtime_set_active(dev);
>>> +	pm_runtime_enable(dev);
>>> +
>>> +	/*
>>> +	 * Prevent runtime pm from being ON by default. Users can enable
>>> +	 * it using power/control in sysfs.
>>> +	 */
>>> +	pm_runtime_forbid(dev);
>>
>> This screams "this patch is broken" or "there are bad issues" which
>> you did not describe at all.
>>
>> Konrad
> 
> Hi Konrad, I was followed the same sequence from other phy drivers
> containing pm_ops. I assume the runtime_forbid is added to control
> runtime pm from userspace.

Other drivers call runtime_forbid() because the implementation doesn't
work.. Or at least historically the usb3 part hasn't been able to sus/
res properly

Konrad

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops
  2025-10-27  6:24 [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops Prashanth K
  2025-10-27  8:25 ` Konrad Dybcio
@ 2025-10-27 22:17 ` kernel test robot
  2025-10-28 14:17 ` Bjorn Andersson
  2 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-10-27 22:17 UTC (permalink / raw)
  To: Prashanth K, Vinod Koul, Kishon Vijay Abraham I
  Cc: oe-kbuild-all, linux-arm-msm, linux-phy, linux-kernel,
	Prashanth K

Hi Prashanth,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.18-rc3 next-20251027]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Prashanth-K/phy-qualcomm-m31-eusb2-Add-runtime-pm-ops/20251027-142647
base:   linus/master
patch link:    https://lore.kernel.org/r/20251027062458.1411096-1-prashanth.k%40oss.qualcomm.com
patch subject: [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops
config: sh-randconfig-002-20251028 (https://download.01.org/0day-ci/archive/20251028/202510280637.c18uAF98-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251028/202510280637.c18uAF98-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510280637.c18uAF98-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/phy/qualcomm/phy-qcom-m31-eusb2.c:253:12: warning: 'm31eusb2_phy_runtime_resume' defined but not used [-Wunused-function]
     253 | static int m31eusb2_phy_runtime_resume(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/phy/qualcomm/phy-qcom-m31-eusb2.c:243:12: warning: 'm31eusb2_phy_runtime_suspend' defined but not used [-Wunused-function]
     243 | static int m31eusb2_phy_runtime_suspend(struct device *dev)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/m31eusb2_phy_runtime_resume +253 drivers/phy/qualcomm/phy-qcom-m31-eusb2.c

   242	
 > 243	static int m31eusb2_phy_runtime_suspend(struct device *dev)
   244	{
   245		struct m31eusb2_phy *phy = dev_get_drvdata(dev);
   246	
   247		dev_dbg(dev, "Suspending M31 eUSB2 Phy\n");
   248		clk_disable_unprepare(phy->clk);
   249	
   250		return 0;
   251	}
   252	
 > 253	static int m31eusb2_phy_runtime_resume(struct device *dev)
   254	{
   255		struct m31eusb2_phy *phy = dev_get_drvdata(dev);
   256		int ret = 0;
   257	
   258		dev_dbg(dev, "Resuming M31 eUSB2 Phy\n");
   259		ret = clk_prepare_enable(phy->clk);
   260		if (ret)
   261			dev_err(dev, "failed to enable ref clock, %d\n", ret);
   262	
   263		return ret;
   264	}
   265	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops
  2025-10-27 11:11     ` Konrad Dybcio
@ 2025-10-28 11:11       ` Prashanth K
  0 siblings, 0 replies; 7+ messages in thread
From: Prashanth K @ 2025-10-28 11:11 UTC (permalink / raw)
  To: Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I
  Cc: linux-arm-msm, linux-phy, linux-kernel



On 10/27/2025 4:41 PM, Konrad Dybcio wrote:
> On 10/27/25 12:09 PM, Prashanth K wrote:
>>
>>
>> On 10/27/2025 1:55 PM, Konrad Dybcio wrote:
>>> On 10/27/25 7:24 AM, Prashanth K wrote:
>>>> Add runtime power management operation callbacks for M31 EUSB2 PHY.
>>>> Enable/disable the clocks based on the runtime suspend/resume calls.
>>>>
>>>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>>>> ---
>>>
>>> [...]
>>>
>>>>  static int m31eusb2_phy_probe(struct platform_device *pdev)
>>>>  {
>>>>  	struct phy_provider *phy_provider;
>>>> @@ -270,6 +298,17 @@ static int m31eusb2_phy_probe(struct platform_device *pdev)
>>>>  		return dev_err_probe(dev, PTR_ERR(phy->clk),
>>>>  				     "failed to get clk\n");
>>>>  
>>>> +	dev_set_drvdata(dev, phy);
>>>> +	pm_runtime_set_active(dev);
>>>> +	pm_runtime_enable(dev);
>>>> +
>>>> +	/*
>>>> +	 * Prevent runtime pm from being ON by default. Users can enable
>>>> +	 * it using power/control in sysfs.
>>>> +	 */
>>>> +	pm_runtime_forbid(dev);
>>>
>>> This screams "this patch is broken" or "there are bad issues" which
>>> you did not describe at all.
>>>
>>> Konrad
>>
>> Hi Konrad, I was followed the same sequence from other phy drivers
>> containing pm_ops. I assume the runtime_forbid is added to control
>> runtime pm from userspace.
> 
> Other drivers call runtime_forbid() because the implementation doesn't
> work.. Or at least historically the usb3 part hasn't been able to sus/
> res properly
> 
> Konrad

Sure, I'll remove the runtime_forbid call and send v2. Let me know if
there's anything else.

Thanks in advance,
Prashanth K

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops
  2025-10-27  6:24 [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops Prashanth K
  2025-10-27  8:25 ` Konrad Dybcio
  2025-10-27 22:17 ` kernel test robot
@ 2025-10-28 14:17 ` Bjorn Andersson
  2 siblings, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2025-10-28 14:17 UTC (permalink / raw)
  To: Prashanth K
  Cc: Vinod Koul, Kishon Vijay Abraham I, linux-arm-msm, linux-phy,
	linux-kernel

On Mon, Oct 27, 2025 at 11:54:58AM +0530, Prashanth K wrote:
> Add runtime power management operation callbacks for M31 EUSB2 PHY.
> Enable/disable the clocks based on the runtime suspend/resume calls.

Please start with describing which problem this solves.

Regards,
Bjorn

> 
> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-m31-eusb2.c | 40 +++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c b/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
> index 0a0d2d9fc846..1aeb5e3de07c 100644
> --- a/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
> @@ -240,6 +240,34 @@ static const struct phy_ops m31eusb2_phy_gen_ops = {
>  	.owner		= THIS_MODULE,
>  };
>  
> +static int m31eusb2_phy_runtime_suspend(struct device *dev)
> +{
> +	struct m31eusb2_phy *phy = dev_get_drvdata(dev);
> +
> +	dev_dbg(dev, "Suspending M31 eUSB2 Phy\n");
> +	clk_disable_unprepare(phy->clk);
> +
> +	return 0;
> +}
> +
> +static int m31eusb2_phy_runtime_resume(struct device *dev)
> +{
> +	struct m31eusb2_phy *phy = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	dev_dbg(dev, "Resuming M31 eUSB2 Phy\n");
> +	ret = clk_prepare_enable(phy->clk);
> +	if (ret)
> +		dev_err(dev, "failed to enable ref clock, %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops m31eusb2_phy_pm_ops = {
> +	SET_RUNTIME_PM_OPS(m31eusb2_phy_runtime_suspend,
> +			   m31eusb2_phy_runtime_resume, NULL)
> +};
> +
>  static int m31eusb2_phy_probe(struct platform_device *pdev)
>  {
>  	struct phy_provider *phy_provider;
> @@ -270,6 +298,17 @@ static int m31eusb2_phy_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(phy->clk),
>  				     "failed to get clk\n");
>  
> +	dev_set_drvdata(dev, phy);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_enable(dev);
> +
> +	/*
> +	 * Prevent runtime pm from being ON by default. Users can enable
> +	 * it using power/control in sysfs.
> +	 */
> +	pm_runtime_forbid(dev);
> +
> +
>  	phy->phy = devm_phy_create(dev, NULL, &m31eusb2_phy_gen_ops);
>  	if (IS_ERR(phy->phy))
>  		return dev_err_probe(dev, PTR_ERR(phy->phy),
> @@ -313,6 +352,7 @@ static struct platform_driver m31eusb2_phy_driver = {
>  	.probe = m31eusb2_phy_probe,
>  	.driver = {
>  		.name = "qcom-m31eusb2-phy",
> +		.pm = &m31eusb2_phy_pm_ops,
>  		.of_match_table = m31eusb2_phy_id_table,
>  	},
>  };
> -- 
> 2.34.1
> 
> 

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-10-28 14:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-27  6:24 [PATCH] phy: qualcomm: m31-eusb2: Add runtime pm ops Prashanth K
2025-10-27  8:25 ` Konrad Dybcio
2025-10-27 11:09   ` Prashanth K
2025-10-27 11:11     ` Konrad Dybcio
2025-10-28 11:11       ` Prashanth K
2025-10-27 22:17 ` kernel test robot
2025-10-28 14:17 ` Bjorn Andersson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).