diff mbox series

[V2,1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support

Message ID 1591269283-24084-2-git-send-email-ppvk@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Add SDHC interconnect bandwidth scaling | expand

Commit Message

Pradeep P V K June 4, 2020, 11:14 a.m. UTC
Interconnect bandwidth scaling support is now added as a
part of OPP [1]. So, make sure interconnect driver is ready
before handling interconnect scaling.

This change is based on
[1] [Patch v8] Introduce OPP bandwidth bindings
(https://lkml.org/lkml/2020/5/12/493)

[2] [Patch v3] mmc: sdhci-msm: Fix error handling
for dev_pm_opp_of_add_table()
(https://lkml.org/lkml/2020/5/5/491)

Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
---
 drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Matthias Kaehlcke June 4, 2020, 5:09 p.m. UTC | #1
On Thu, Jun 04, 2020 at 04:44:42PM +0530, Pradeep P V K wrote:
> Interconnect bandwidth scaling support is now added as a
> part of OPP [1]. So, make sure interconnect driver is ready
> before handling interconnect scaling.
> 
> This change is based on
> [1] [Patch v8] Introduce OPP bandwidth bindings
> (https://lkml.org/lkml/2020/5/12/493)
> 
> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
> for dev_pm_opp_of_add_table()
> (https://lkml.org/lkml/2020/5/5/491)
> 
> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index b277dd7..a13ff1b 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -14,6 +14,7 @@
>  #include <linux/slab.h>
>  #include <linux/iopoll.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/interconnect.h>
>  
>  #include "sdhci-pltfm.h"
>  #include "cqhci.h"
> @@ -2070,6 +2071,18 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	}
>  	msm_host->bulk_clks[0].clk = clk;
>  
> +	/* Make sure that ICC driver is ready for interconnect bandwdith
> +	 * scaling before registering the device for OPP.
> +	 */
> +	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
> +	if (ret) {
> +		if (ret == -EPROBE_DEFER)
> +			dev_info(&pdev->dev, "defer icc path: %d\n", ret);

I already commented on this on v1:

  This log seems to add little more than noise, or are there particular reasons
  why it is useful in this driver? Most drivers just return silently in case of
  deferred probing.

If you think the log is really needed please explain why.
Sibi Sankar June 4, 2020, 6:34 p.m. UTC | #2
On 2020-06-04 22:39, Matthias Kaehlcke wrote:
> On Thu, Jun 04, 2020 at 04:44:42PM +0530, Pradeep P V K wrote:
>> Interconnect bandwidth scaling support is now added as a
>> part of OPP [1]. So, make sure interconnect driver is ready
>> before handling interconnect scaling.
>> 
>> This change is based on
>> [1] [Patch v8] Introduce OPP bandwidth bindings
>> (https://lkml.org/lkml/2020/5/12/493)
>> 
>> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
>> for dev_pm_opp_of_add_table()
>> (https://lkml.org/lkml/2020/5/5/491)
>> 
>> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/drivers/mmc/host/sdhci-msm.c 
>> b/drivers/mmc/host/sdhci-msm.c
>> index b277dd7..a13ff1b 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/iopoll.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/interconnect.h>
>> 
>>  #include "sdhci-pltfm.h"
>>  #include "cqhci.h"
>> @@ -2070,6 +2071,18 @@ static int sdhci_msm_probe(struct 
>> platform_device *pdev)
>>  	}
>>  	msm_host->bulk_clks[0].clk = clk;
>> 
>> +	/* Make sure that ICC driver is ready for interconnect bandwdith
>> +	 * scaling before registering the device for OPP.
>> +	 */
>> +	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
>> +	if (ret) {
>> +		if (ret == -EPROBE_DEFER)
>> +			dev_info(&pdev->dev, "defer icc path: %d\n", ret);
> 
> I already commented on this on v1:
> 
>   This log seems to add little more than noise, or are there particular 
> reasons
>   why it is useful in this driver? Most drivers just return silently in 
> case of
>   deferred probing.
> 
> If you think the log is really needed please explain why.

Both the err logs seem redundant.
EPROBE_DEFERS are rather readily
noticeable through the return val.
dev_.._find_icc_paths already prints
err messages when we fail to get icc
paths.
Pradeep P V K June 5, 2020, 8:06 a.m. UTC | #3
On 2020-06-05 00:04, Sibi Sankar wrote:
> On 2020-06-04 22:39, Matthias Kaehlcke wrote:
>> On Thu, Jun 04, 2020 at 04:44:42PM +0530, Pradeep P V K wrote:
>>> Interconnect bandwidth scaling support is now added as a
>>> part of OPP [1]. So, make sure interconnect driver is ready
>>> before handling interconnect scaling.
>>> 
>>> This change is based on
>>> [1] [Patch v8] Introduce OPP bandwidth bindings
>>> (https://lkml.org/lkml/2020/5/12/493)
>>> 
>>> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
>>> for dev_pm_opp_of_add_table()
>>> (https://lkml.org/lkml/2020/5/5/491)
>>> 
>>> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
>>> ---
>>>  drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>> 
>>> diff --git a/drivers/mmc/host/sdhci-msm.c 
>>> b/drivers/mmc/host/sdhci-msm.c
>>> index b277dd7..a13ff1b 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -14,6 +14,7 @@
>>>  #include <linux/slab.h>
>>>  #include <linux/iopoll.h>
>>>  #include <linux/regulator/consumer.h>
>>> +#include <linux/interconnect.h>
>>> 
>>>  #include "sdhci-pltfm.h"
>>>  #include "cqhci.h"
>>> @@ -2070,6 +2071,18 @@ static int sdhci_msm_probe(struct 
>>> platform_device *pdev)
>>>  	}
>>>  	msm_host->bulk_clks[0].clk = clk;
>>> 
>>> +	/* Make sure that ICC driver is ready for interconnect bandwdith
>>> +	 * scaling before registering the device for OPP.
>>> +	 */
>>> +	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
>>> +	if (ret) {
>>> +		if (ret == -EPROBE_DEFER)
>>> +			dev_info(&pdev->dev, "defer icc path: %d\n", ret);
>> 
>> I already commented on this on v1:
>> 
>>   This log seems to add little more than noise, or are there 
>> particular reasons
>>   why it is useful in this driver? Most drivers just return silently 
>> in case of
>>   deferred probing.
>> 
>> If you think the log is really needed please explain why.
> 
Sorry. i forget to remove this print on V2. i will address this in my 
next patch set.

> Both the err logs seem redundant.
> EPROBE_DEFERS are rather readily
> noticeable through the return val.
> dev_.._find_icc_paths already prints
> err messages when we fail to get icc
> paths.

True. i will remove this too in my next patch set.
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index b277dd7..a13ff1b 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -14,6 +14,7 @@ 
 #include <linux/slab.h>
 #include <linux/iopoll.h>
 #include <linux/regulator/consumer.h>
+#include <linux/interconnect.h>
 
 #include "sdhci-pltfm.h"
 #include "cqhci.h"
@@ -2070,6 +2071,18 @@  static int sdhci_msm_probe(struct platform_device *pdev)
 	}
 	msm_host->bulk_clks[0].clk = clk;
 
+	/* Make sure that ICC driver is ready for interconnect bandwdith
+	 * scaling before registering the device for OPP.
+	 */
+	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
+	if (ret) {
+		if (ret == -EPROBE_DEFER)
+			dev_info(&pdev->dev, "defer icc path: %d\n", ret);
+		else
+			dev_err(&pdev->dev, "failed to get icc path:%d\n", ret);
+		goto bus_clk_disable;
+	}
+
 	msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
 	if (IS_ERR(msm_host->opp_table)) {
 		ret = PTR_ERR(msm_host->opp_table);