Skip to contentSkip to conversation

Add SDHC interconnect bandwidth scaling

PATCH In review Pradeep P V K 3 participants 11 messages Jun 4, 2020 – Jun 9, 2020

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v2 cover Add SDHC interconnect bandwidth scaling

Add interconnect bandwidths for SDHC driver using OPP framework that is required by SDHC driver based on the clock frequency and bus width of the card. Otherwise, the system clocks may run at minimum clock speed and thus affecting the performance.

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

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

[3] [RFC v6 2/2] dt-bindings: mmc: sdhci-msm: Add interconnect BW scaling strings
(https://lkml.org/lkml/2020/3/23/409)

as there were no extra changes made on [3], retaining the Acked-by and Reviewed-by sign-off from [3].

Pradeep P V K (2):
mmc: sdhci-msm: Add interconnect bandwidth scaling support
dt-bindings: mmc: sdhci-msm: Add interconnect BW scaling strings

 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 18 ++++++++++++++++++
 drivers/mmc/host/sdhci-msm.c                        | 13 +++++++++++++
 2 files changed, 31 insertions(+)
Signature · 2 lines
changes since V1:
- Added dev_pm_opp_find_icc_paths() to check for icc paths.
#1 lore raw

PATCH v2 1/2 mmc: sdhci-msm: Add interconnect bandwidth scaling support

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)

---
 drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
drivers/mmc/host/sdhci-msm.c +13 -0
index b277dd7..a13ff1b 100644
@@ -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);
Signature · 1 line
1.9.1
#2 lore raw

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

Quoted 30 lines · Pradeep P V K

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)

---
 drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
drivers/mmc/host/sdhci-msm.c +1 -0
index b277dd7..a13ff1b 100644
@@ -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"

drivers/mmc/host/sdhci-msm.c static int sdhci_msm_probe(struct platform_device *pdev) · line 2080

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +7 -0
@@ -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.

#3 lore raw

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

Quoted 45 lines · Matthias Kaehlcke
Quoted 31 lines · Pradeep P V K

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)

---
 drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
drivers/mmc/host/sdhci-msm.c +1 -0
index b277dd7..a13ff1b 100644
@@ -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)

drivers/mmc/host/sdhci-msm.c static int sdhci_msm_probe(struct · line 2081

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +7 -0
 	}
 	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:

Quoted 7 lines

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.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
#4 lore raw

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

Quoted 46 lines · Sibi Sankar
Quoted 45 lines · Matthias Kaehlcke
Quoted 31 lines · Pradeep P V K

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)

---
 drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
drivers/mmc/host/sdhci-msm.c +1 -0
index b277dd7..a13ff1b 100644
@@ -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)

drivers/mmc/host/sdhci-msm.c static int sdhci_msm_probe(struct · line 2081

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +7 -0
 	}
 	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:

Quoted 8 lines
Quoted 7 lines

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.

Quoted 6 lines

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.

#5 lore raw

PATCH v2 2/2 dt-bindings: mmc: sdhci-msm: Add interconnect BW scaling strings

In reply to Pradeep P V K

Add interconnect bandwidth scaling supported strings for qcom-sdhci controller.

---
 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +18 -0
index b8e1d2b..3b602fd 100644
@@ -54,6 +54,21 @@ Required properties:
 - qcom,dll-config: Chipset and Platform specific value. Use this field to
 	specify the DLL_CONFIG register value as per Hardware Programming Guide.
 
+Optional Properties:
+* Following bus parameters are required for interconnect bandwidth scaling:
+- interconnects: Pairs of phandles and interconnect provider specifier
+		 to denote the edge source and destination ports of
+		 the interconnect path.
+
+- interconnect-names: For sdhc, we have two main paths.
+		1. Data path : sdhc to ddr
+		2. Config path : cpu to sdhc
+		For Data interconnect path the name supposed to be
+		is "sdhc-ddr" and for config interconnect path it is
+		"cpu-sdhc".
+		Please refer to Documentation/devicetree/bindings/
+		interconnect/ for more details.
+
 Example:
 
 	sdhc_1: sdhci@f9824900 {
@@ -71,6 +86,9 @@ Example:
 
 		clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>;
 		clock-names = "core", "iface";
+		interconnects = <&qnoc MASTER_SDCC_ID &qnoc SLAVE_DDR_ID>,
+				<&qnoc MASTER_CPU_ID &qnoc SLAVE_SDCC_ID>;
+		interconnect-names = "sdhc-ddr","cpu-sdhc";
 
 		qcom,dll-config = <0x000f642c>;
 		qcom,ddr-config = <0x80040868>;
Signature · 1 line
1.9.1
#6 lore raw

=?y?q?=5BPATCH=C2=A0V3=200/2=5D=20Add=20SDHC=20interconnect=20bandwidth=20scaling=20?=

In reply to Pradeep P V K

Add interconnect bandwidths for SDHC driver using OPP framework that is required by SDHC driver based on the clock frequency and bus width of the card. Otherwise, the system clocks may run at minimum clock speed and thus affecting the performance.

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

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

[3] [RFC v6 2/2] dt-bindings: mmc: sdhci-msm: Add interconnect BW scaling strings
(https://lkml.org/lkml/2020/3/23/409)

as there were no extra changes made on [3], retaining the Acked-by and Reviewed-by sign-off from [3].

Changes since V2:
- Removed debug error prints on icc path ready check.

changes since V1:
- Added dev_pm_opp_find_icc_paths() to check for icc paths.

Pradeep P V K (2):
mmc: sdhci-msm: Add interconnect bandwidth scaling support
dt-bindings: mmc: sdhci-msm: Add interconnect BW scaling strings

 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 18 ++++++++++++++++++
 drivers/mmc/host/sdhci-msm.c                        |  8 ++++++++
 2 files changed, 26 insertions(+)
Signature · 1 line
1.9.1
#7 lore raw

=?y?q?=5BPATCH=C2=A0V3=202/2=5D=20dt-bindings=3A=20mmc=3A=20sdhci-msm=3A=20Add=20interconnect=20BW=20scaling=20strings?=

Add interconnect bandwidth scaling supported strings for qcom-sdhci controller.

---
 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +18 -0
index b8e1d2b..3b602fd 100644
@@ -54,6 +54,21 @@ Required properties:
 - qcom,dll-config: Chipset and Platform specific value. Use this field to
 	specify the DLL_CONFIG register value as per Hardware Programming Guide.
 
+Optional Properties:
+* Following bus parameters are required for interconnect bandwidth scaling:
+- interconnects: Pairs of phandles and interconnect provider specifier
+		 to denote the edge source and destination ports of
+		 the interconnect path.
+
+- interconnect-names: For sdhc, we have two main paths.
+		1. Data path : sdhc to ddr
+		2. Config path : cpu to sdhc
+		For Data interconnect path the name supposed to be
+		is "sdhc-ddr" and for config interconnect path it is
+		"cpu-sdhc".
+		Please refer to Documentation/devicetree/bindings/
+		interconnect/ for more details.
+
 Example:
 
 	sdhc_1: sdhci@f9824900 {
@@ -71,6 +86,9 @@ Example:
 
 		clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>;
 		clock-names = "core", "iface";
+		interconnects = <&qnoc MASTER_SDCC_ID &qnoc SLAVE_DDR_ID>,
+				<&qnoc MASTER_CPU_ID &qnoc SLAVE_SDCC_ID>;
+		interconnect-names = "sdhc-ddr","cpu-sdhc";
 
 		qcom,dll-config = <0x000f642c>;
 		qcom,ddr-config = <0x80040868>;
Signature · 1 line
1.9.1
#8 lore raw

=?y?q?=5BPATCH=C2=A0V3=201/2=5D=20mmc=3A=20sdhci-msm=3A=20Add=20interconnect=20bandwidth=20scaling=20support?=

In reply to Pradeep P V K

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)

---
 drivers/mmc/host/sdhci-msm.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mmc/host/sdhci-msm.c +8 -0
index b277dd7..a945e84 100644
@@ -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,13 @@ 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)
+		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);
Signature · 1 line
1.9.1
Reviewed-by
#9 lore raw

Re: [PATCH V3 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support

Hey Pradeep,
Thanks for the patch.

Quoted 2 lines · Pradeep P V K

Interconnect bandwidth scaling support is now added as a part of OPP [1]. So, make sure interconnect driver is ready

can you please replace driver with paths
instead?

Quoted 9 lines

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)

sry didn't notice ^^ earlier
you might want to place these
comments and dependencies similar
to the following patch.
https://patchwork.kernel.org/patch/11573903/

Quoted 19 lines
---
 drivers/mmc/host/sdhci-msm.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mmc/host/sdhci-msm.c +1 -0
index b277dd7..a945e84 100644
@@ -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"

drivers/mmc/host/sdhci-msm.c static int sdhci_msm_probe(struct · line 2075

Quoted 5 lines
drivers/mmc/host/sdhci-msm.c +1 -0
@@ -2070,6 +2071,13 @@ 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

typo /s/bandwdith/bandwidth

drivers/mmc/host/sdhci-msm.c static int sdhci_msm_probe(struct platform_device *pdev) · line 2076

Quoted 2 lines
drivers/mmc/host/sdhci-msm.c +2 -0
+	 * scaling before registering the device for OPP.
+	 */

/* Check for optional interconnect paths */
Maybe using ^^ would suffice since
that's what we are actually doing

Quoted 7 lines
drivers/mmc/host/sdhci-msm.c +4 -0
+	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
+	if (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);

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
#10 lore raw

Re: [PATCH V3 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support

Hi Sibi,

Thanks for the review.

Quoted 10 lines · Sibi Sankar

Hey Pradeep,
Thanks for the patch.

Quoted 2 lines · Pradeep P V K

Interconnect bandwidth scaling support is now added as a part of OPP [1]. So, make sure interconnect driver is ready

can you please replace driver with paths instead?

ok. I will address this in my next patch set.

Quoted 10 lines
Quoted 9 lines

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)

Quoted 6 lines

sry didn't notice ^^ earlier you might want to place these comments and dependencies similar to the following patch.
https://patchwork.kernel.org/patch/11573903/

ok. will modify in my next patch.

Quoted 27 lines
Quoted 19 lines
---
 drivers/mmc/host/sdhci-msm.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mmc/host/sdhci-msm.c +1 -0
index b277dd7..a945e84 100644
@@ -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"

drivers/mmc/host/sdhci-msm.c static int sdhci_msm_probe(struct · line 2075

Quoted 5 lines
drivers/mmc/host/sdhci-msm.c +1 -0
@@ -2070,6 +2071,13 @@ 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

typo /s/bandwdith/bandwidth

Quoted 7 lines

drivers/mmc/host/sdhci-msm.c static int sdhci_msm_probe(struct platform_device *pdev) · line 2076

Quoted 2 lines
drivers/mmc/host/sdhci-msm.c +2 -0
+	 * scaling before registering the device for OPP.
+	 */

/* Check for optional interconnect paths */ Maybe using ^^ would suffice since that's what we are actually doing

sure. i will re-modify the comments as suggested ^^ in my next patch.

Quoted 9 lines
Quoted 7 lines
drivers/mmc/host/sdhci-msm.c +4 -0
+	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
+	if (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);