Skip to contentSkip to conversation

Add SDHC interconnect bandwidth scaling

RFC Superseded Pradeep P V K 4 participants 7 messages Feb 18, 2020 – Mar 8, 2020

Subsystems (4)
Device tree bindings Arm Qualcomm mailing list MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Pradeep P V K

author
#0 lore raw

v4 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
[RFC] mmc: host: sdhci-msm: Use the interconnect API
(https://lkml.org/lkml/2018/10/11/499) and

[PATCH v6] Introduce Bandwidth OPPs for interconnects
(https://lkml.org/lkml/2019/12/6/740)

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

 .../devicetree/bindings/mmc/sdhci-msm.txt          |  18 ++
 drivers/mmc/host/sdhci-msm.c                       | 204 ++++++++++++++++++++-
 2 files changed, 218 insertions(+), 4 deletions(-)

--
changes from RFC v3 -> v4:
- Addressed review comments from Bjorn and Matthias 1.9.1

author
#1 lore raw

RFC v4 1/2 mmc: sdhci-msm: Add interconnect bus bandwidth scaling support

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
[RFC] mmc: host: sdhci-msm: Use the interconnect API
(https://lkml.org/lkml/2018/10/11/499) and

[PATCH v6] Introduce Bandwidth OPPs for interconnects
(https://lkml.org/lkml/2019/12/6/740)

Co-developed-by Sahitya Tummala <[email protected]>
Signed-off-by Sahitya Tummala <[email protected]>
Co-developed-by Subhash Jadavani <[email protected]>
Signed-off-by Subhash Jadavani <[email protected]>
Co-developed-by Veerabhadrarao Badiganti <[email protected]>
Signed-off-by Veerabhadrarao Badiganti <[email protected]>
Co-developed-by Pradeep P V K <[email protected]>
Signed-off-by Pradeep P V K <[email protected]>

---

changes from RFC v3 -> v4:

- Addressed review comments from Bjorn and Matthias

 drivers/mmc/host/sdhci-msm.c | 204 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 200 insertions(+), 4 deletions(-)
drivers/mmc/host/sdhci-msm.c +200 -4
index 71f29ba..5af1c58 100644
@@ -11,8 +11,10 @@
 #include <linux/mmc/mmc.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
+#include <linux/interconnect.h>
 #include <linux/iopoll.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_opp.h>
 
 #include "sdhci-pltfm.h"
 

@@ -229,6 +231,12 @@ struct sdhci_msm_variant_info {
 	const struct sdhci_msm_offset *offset;
 };
 
+struct sdhci_msm_bus_vote_data {
+	struct icc_path *sdhc_to_ddr;
+	struct icc_path *cpu_to_sdhc;
+	u32 curr_freq;
+};
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */

@@ -255,8 +263,11 @@ struct sdhci_msm_host {
 	bool use_cdr;
 	u32 transfer_mode;
 	bool updated_ddr_cfg;
+	struct sdhci_msm_bus_vote_data *bus_vote_data;
 };
 
+static void sdhci_msm_bus_voting(struct sdhci_host *host, bool enable);
+
 static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);

@@ -1564,6 +1575,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 
 	msm_set_clock_rate_for_bus_mode(host, clock);
 out:
+	sdhci_msm_bus_voting(host, !!clock);
 	__sdhci_msm_set_clock(host, clock);
 }
 

@@ -1685,6 +1697,174 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
 	pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
 }
 
+/*
+ * Returns required bandwidth in Bytes per Sec
+ */
+static unsigned long sdhci_get_bw_required(struct sdhci_host *host,
+					struct mmc_ios *ios)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+	switch (ios->bus_width) {
+	case MMC_BUS_WIDTH_1:
+		return msm_host->clk_rate/8;
+	case MMC_BUS_WIDTH_4:
+		return msm_host->clk_rate/2;
+	case MMC_BUS_WIDTH_8:
+		break;
+	}
+	return msm_host->clk_rate;
+}
+
+/*
+ * Helper function to parse the exact OPP node
+ * Returns OPP pointer on success else NULL on error
+ */
+static struct dev_pm_opp
+		*sdhci_msm_find_opp_for_freq(struct sdhci_msm_host *msm_host,
+							unsigned long bw)
+{
+	struct dev_pm_opp *opp;
+	struct sdhci_host *host = mmc_priv(msm_host->mmc);
+	unsigned int freq = bw;
+	struct device *dev = &msm_host->pdev->dev;
+
+
+	if (!freq)
+		opp = dev_pm_opp_find_peak_bw_floor(dev, &freq);
+	else
+		opp = dev_pm_opp_find_peak_bw_exact(dev, freq, true);
+
+	/* Max bandwidth vote */
+	if (PTR_ERR(opp) == -ERANGE && freq > sdhci_msm_get_max_clock(host))
+		opp = dev_pm_opp_find_peak_bw_ceil(dev, &bw);
+
+	if (IS_ERR(opp)) {
+		dev_err(dev, "Failed to find OPP for freq:%u err:%ld\n",
+				freq, PTR_ERR(opp));
+		return NULL;
+	}
+	return opp;
+}
+
+/*
+ * This function sets the interconnect bus bandwidth
+ * vote based on bw (bandwidth) argument.
+ */
+#define BUS_INTERCONNECT_PATHS 2 /* 1. sdhc -> ddr 2. cpu -> sdhc */
+static void sdhci_msm_bus_set_vote(struct sdhci_host *host,
+						unsigned int bw)
+{
+	int i, ddr_rc, cpu_rc;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct sdhci_msm_bus_vote_data *vote_data = msm_host->bus_vote_data;
+	struct device *dev = &msm_host->pdev->dev;
+	struct dev_pm_opp *opp;
+	unsigned long freq = bw;
+	unsigned long peak_bw[BUS_INTERCONNECT_PATHS] = {0};
+	unsigned long avg_bw[BUS_INTERCONNECT_PATHS] = {0};
+
+	if (bw == vote_data->curr_freq)
+		return;
+
+	for (i = 0; i < BUS_INTERCONNECT_PATHS; i++) {
+		opp = sdhci_msm_find_opp_for_freq(msm_host, freq);
+		if (opp) {
+			avg_bw[i] = dev_pm_opp_get_bw(opp, &peak_bw[i]);
+			freq += 1; /* Next bandwidth vote */
+			dev_pm_opp_put(opp);
+		}
+	}
+	pr_debug("%s: freq:%d sdhc_to_ddr avg_bw:%lu peak_bw:%lu cpu_to_sdhc avg_bw:%lu peak_bw:%lu\n",
+			mmc_hostname(host->mmc), bw, avg_bw[0], peak_bw[0],
+				avg_bw[1], peak_bw[1]);
+	ddr_rc = icc_set_bw(vote_data->sdhc_to_ddr, 0, peak_bw[0]);
+	cpu_rc = icc_set_bw(vote_data->cpu_to_sdhc, 0, peak_bw[1]);
+	if (ddr_rc || cpu_rc) {
+		dev_err(dev, "icc_set() failed ddr_rc:%d cpu_rc:%d\n",
+							ddr_rc, cpu_rc);
+		return;
+	}
+	vote_data->curr_freq = bw;
+}
+
+/*
+ * Helper function to register for OPP and interconnect
+ * frameworks.
+ */
+static struct sdhci_msm_bus_vote_data
+		*sdhci_msm_bus_register(struct sdhci_msm_host *host,
+				struct platform_device *pdev)
+{
+	struct sdhci_msm_bus_vote_data *vote_data;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	vote_data = devm_kzalloc(dev, sizeof(*vote_data), GFP_KERNEL);
+	if (!vote_data)
+		return NULL;
+
+	vote_data->sdhc_to_ddr = of_icc_get(&pdev->dev, "sdhc-ddr");
+	vote_data->cpu_to_sdhc = of_icc_get(&pdev->dev, "cpu-sdhc");
+	if (!vote_data->sdhc_to_ddr || !vote_data->cpu_to_sdhc) {
+		dev_info(&pdev->dev, "ICC DT property is missing. skip vote !!\n");
+		return NULL;
+	} else if (IS_ERR(vote_data->sdhc_to_ddr) ||
+			IS_ERR(vote_data->cpu_to_sdhc)) {
+		dev_err(&pdev->dev, "(%ld): failed getting %s path\n",
+				PTR_ERR(vote_data->sdhc_to_ddr), "sdhc-ddr");
+		dev_err(&pdev->dev, "(%ld): failed getting %s path\n",
+				PTR_ERR(vote_data->sdhc_to_ddr), "cpu-sdhc");
+
+		return IS_ERR(vote_data->sdhc_to_ddr) ?
+			ERR_CAST(vote_data->sdhc_to_ddr) :
+			ERR_CAST(vote_data->cpu_to_sdhc);
+	}
+	ret = dev_pm_opp_of_add_table(dev);
+	if (ret) {
+		if (ret == -ENODEV || ret == -ENODATA)
+			dev_err(dev, "OPP dt properties missing:%d\n", ret);
+		else
+			dev_err(dev, "OPP registration failed:%d\n", ret);
+		return ERR_PTR(ret);
+	}
+	return vote_data;
+}
+
+static void sdhci_msm_bus_unregister(struct device *dev,
+				struct sdhci_msm_host *host)
+{
+	struct sdhci_msm_bus_vote_data *vote_data = host->bus_vote_data;
+
+	if (!vote_data ||
+		IS_ERR(vote_data->sdhc_to_ddr) ||
+		IS_ERR(vote_data->cpu_to_sdhc))
+		return;
+
+	icc_put(vote_data->sdhc_to_ddr);
+	icc_put(vote_data->cpu_to_sdhc);
+}
+
+static void sdhci_msm_bus_voting(struct sdhci_host *host, bool enable)
+{
+	struct mmc_ios *ios = &host->mmc->ios;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	unsigned int bw;
+
+	if (!msm_host->bus_vote_data ||
+		IS_ERR(msm_host->bus_vote_data))
+		return;
+
+	if (enable) {
+		bw = sdhci_get_bw_required(host, ios);
+		sdhci_msm_bus_set_vote(host, bw);
+	} else
+		sdhci_msm_bus_set_vote(host, 0);
+}
+
 static const struct sdhci_msm_variant_ops mci_var_ops = {
 	.msm_readl_relaxed = sdhci_msm_mci_variant_readl_relaxed,
 	.msm_writel_relaxed = sdhci_msm_mci_variant_writel_relaxed,

@@ -1845,11 +2025,20 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "TCXO clk not present (%d)\n", ret);
 	}
 
+	msm_host->bus_vote_data = sdhci_msm_bus_register(msm_host, pdev);
+	if (IS_ERR(msm_host->bus_vote_data)) {
+		ret = PTR_ERR(msm_host->bus_vote_data);
+		dev_err(&pdev->dev, "Bus registration failed (%d)\n", ret);
+		goto clk_disable;
+	}
+
+	sdhci_msm_bus_voting(host, true);
+
 	if (!msm_host->mci_removed) {
 		msm_host->core_mem = devm_platform_ioremap_resource(pdev, 1);
 		if (IS_ERR(msm_host->core_mem)) {
 			ret = PTR_ERR(msm_host->core_mem);
-			goto clk_disable;
+			goto bus_unregister;
 		}
 	}
 

@@ -1924,7 +2113,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	msm_host->pwr_irq = platform_get_irq_byname(pdev, "pwr_irq");
 	if (msm_host->pwr_irq < 0) {
 		ret = msm_host->pwr_irq;
-		goto clk_disable;
+		goto bus_unregister;
 	}
 
 	sdhci_msm_init_pwr_irq_wait(msm_host);

@@ -1937,7 +2126,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 					dev_name(&pdev->dev), host);
 	if (ret) {
 		dev_err(&pdev->dev, "Request IRQ failed (%d)\n", ret);
-		goto clk_disable;
+		goto bus_unregister;
 	}
 
 	pm_runtime_get_noresume(&pdev->dev);

@@ -1962,6 +2151,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
+bus_unregister:
+	sdhci_msm_bus_voting(host, false);
+	sdhci_msm_bus_unregister(&pdev->dev, msm_host);
 clk_disable:
 	clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks),
 				   msm_host->bulk_clks);

@@ -1991,6 +2183,9 @@ static int sdhci_msm_remove(struct platform_device *pdev)
 				   msm_host->bulk_clks);
 	if (!IS_ERR(msm_host->bus_clk))
 		clk_disable_unprepare(msm_host->bus_clk);
+
+	sdhci_msm_bus_voting(host, false);
+	sdhci_msm_bus_unregister(&pdev->dev, msm_host);
 	sdhci_pltfm_free(pdev);
 	return 0;
 }

@@ -2003,7 +2198,7 @@ static __maybe_unused int sdhci_msm_runtime_suspend(struct device *dev)
 
 	clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks),
 				   msm_host->bulk_clks);
-
+	sdhci_msm_bus_voting(host, false);
 	return 0;
 }
 

@@ -2025,6 +2220,7 @@ static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev)
 	if (msm_host->restore_dll_config && msm_host->clk_rate)
 		return sdhci_msm_restore_sdr_dll_config(host);
 
+	sdhci_msm_bus_voting(host, true);
 	return 0;
 }
Signature · 1 line
1.9.1
#2 lore raw

Re: [RFC v4 1/2] mmc: sdhci-msm: Add interconnect bus bandwidth scaling support

Hi Pradeep,

Quoted 20 lines · 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
[RFC] mmc: host: sdhci-msm: Use the interconnect API
(https://lkml.org/lkml/2018/10/11/499) and

[PATCH v6] Introduce Bandwidth OPPs for interconnects
(https://lkml.org/lkml/2019/12/6/740)

Co-developed-by Sahitya Tummala <[email protected]>
Signed-off-by Sahitya Tummala <[email protected]>
Co-developed-by Subhash Jadavani <[email protected]>
Signed-off-by Subhash Jadavani <[email protected]>
Co-developed-by Veerabhadrarao Badiganti <[email protected]>
Signed-off-by Veerabhadrarao Badiganti <[email protected]>
Co-developed-by Pradeep P V K <[email protected]>
Signed-off-by Pradeep P V K <[email protected]>
Quoted 5 lines

---

changes from RFC v3 -> v4:

- Addressed review comments from Bjorn and Matthias

This is not helpful, please describe in future versions what those changes are.

Quoted 58 lines
 drivers/mmc/host/sdhci-msm.c | 204 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 200 insertions(+), 4 deletions(-)
drivers/mmc/host/sdhci-msm.c +14 -0
index 71f29ba..5af1c58 100644
@@ -11,8 +11,10 @@
 #include <linux/mmc/mmc.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
+#include <linux/interconnect.h>
 #include <linux/iopoll.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_opp.h>
 
 #include "sdhci-pltfm.h"
 

@@ -229,6 +231,12 @@ struct sdhci_msm_variant_info {
 	const struct sdhci_msm_offset *offset;
 };
 
+struct sdhci_msm_bus_vote_data {
+	struct icc_path *sdhc_to_ddr;
+	struct icc_path *cpu_to_sdhc;
+	u32 curr_freq;
+};
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */

@@ -255,8 +263,11 @@ struct sdhci_msm_host {
 	bool use_cdr;
 	u32 transfer_mode;
 	bool updated_ddr_cfg;
+	struct sdhci_msm_bus_vote_data *bus_vote_data;
 };
 
+static void sdhci_msm_bus_voting(struct sdhci_host *host, bool enable);
+
 static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);

@@ -1564,6 +1575,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
 
 	msm_set_clock_rate_for_bus_mode(host, clock);
 out:
+	sdhci_msm_bus_voting(host, !!clock);
 	__sdhci_msm_set_clock(host, clock);
 }
 

@@ -1685,6 +1697,174 @@ static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
 	pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
 }
 
+/*
+ * Returns required bandwidth in Bytes per Sec

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1711

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+ */
+static unsigned long sdhci_get_bw_required(struct sdhci_host *host,
+					struct mmc_ios *ios)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+	switch (ios->bus_width) {
+	case MMC_BUS_WIDTH_1:
+		return msm_host->clk_rate/8;

nit: it would be more readable with spaces around '/'.

Quoted 68 lines
drivers/mmc/host/sdhci-msm.c +68 -0
+	case MMC_BUS_WIDTH_4:
+		return msm_host->clk_rate/2;
+	case MMC_BUS_WIDTH_8:
+		break;
+	}
+	return msm_host->clk_rate;
+}
+
+/*
+ * Helper function to parse the exact OPP node
+ * Returns OPP pointer on success else NULL on error
+ */
+static struct dev_pm_opp
+		*sdhci_msm_find_opp_for_freq(struct sdhci_msm_host *msm_host,
+							unsigned long bw)
+{
+	struct dev_pm_opp *opp;
+	struct sdhci_host *host = mmc_priv(msm_host->mmc);
+	unsigned int freq = bw;
+	struct device *dev = &msm_host->pdev->dev;
+
+
+	if (!freq)
+		opp = dev_pm_opp_find_peak_bw_floor(dev, &freq);
+	else
+		opp = dev_pm_opp_find_peak_bw_exact(dev, freq, true);
+
+	/* Max bandwidth vote */
+	if (PTR_ERR(opp) == -ERANGE && freq > sdhci_msm_get_max_clock(host))
+		opp = dev_pm_opp_find_peak_bw_ceil(dev, &bw);
+
+	if (IS_ERR(opp)) {
+		dev_err(dev, "Failed to find OPP for freq:%u err:%ld\n",
+				freq, PTR_ERR(opp));
+		return NULL;
+	}
+	return opp;
+}
+
+/*
+ * This function sets the interconnect bus bandwidth
+ * vote based on bw (bandwidth) argument.
+ */
+#define BUS_INTERCONNECT_PATHS 2 /* 1. sdhc -> ddr 2. cpu -> sdhc */
+static void sdhci_msm_bus_set_vote(struct sdhci_host *host,
+						unsigned int bw)
+{
+	int i, ddr_rc, cpu_rc;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct sdhci_msm_bus_vote_data *vote_data = msm_host->bus_vote_data;
+	struct device *dev = &msm_host->pdev->dev;
+	struct dev_pm_opp *opp;
+	unsigned long freq = bw;
+	unsigned long peak_bw[BUS_INTERCONNECT_PATHS] = {0};
+	unsigned long avg_bw[BUS_INTERCONNECT_PATHS] = {0};
+
+	if (bw == vote_data->curr_freq)
+		return;
+
+	for (i = 0; i < BUS_INTERCONNECT_PATHS; i++) {
+		opp = sdhci_msm_find_opp_for_freq(msm_host, freq);
+		if (opp) {
+			avg_bw[i] = dev_pm_opp_get_bw(opp, &peak_bw[i]);
+			freq += 1; /* Next bandwidth vote */
+			dev_pm_opp_put(opp);
+		}
+	}

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1789

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+	pr_debug("%s: freq:%d sdhc_to_ddr avg_bw:%lu peak_bw:%lu cpu_to_sdhc avg_bw:%lu peak_bw:%lu\n",
+			mmc_hostname(host->mmc), bw, avg_bw[0], peak_bw[0],
+				avg_bw[1], peak_bw[1]);
+	ddr_rc = icc_set_bw(vote_data->sdhc_to_ddr, 0, peak_bw[0]);
+	cpu_rc = icc_set_bw(vote_data->cpu_to_sdhc, 0, peak_bw[1]);
+	if (ddr_rc || cpu_rc) {
+		dev_err(dev, "icc_set() failed ddr_rc:%d cpu_rc:%d\n",
+							ddr_rc, cpu_rc);
+		return;
+	}

Do you necessarily want to set the bandwidth of the CPU to SDHC path, if setting the bandwidth for the SDHC to DDR path failed? If not I'd suggest to get rid of this double error handling with 'ddr_rc' and 'cpu_rc' and handle each error individually.

Quoted 8 lines
drivers/mmc/host/sdhci-msm.c +8 -0
+	vote_data->curr_freq = bw;
+}
+
+/*
+ * Helper function to register for OPP and interconnect
+ * frameworks.
+ */
+static struct sdhci_msm_bus_vote_data

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1807

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+		*sdhci_msm_bus_register(struct sdhci_msm_host *host,
+				struct platform_device *pdev)
+{
+	struct sdhci_msm_bus_vote_data *vote_data;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	vote_data = devm_kzalloc(dev, sizeof(*vote_data), GFP_KERNEL);
+	if (!vote_data)
+		return NULL;

Even though the interconnects are optional I think we want to propagate an out of memory error, i.e. you probably want to return -ENOMEM here.

Also the allocated 'vote_data' will not be used if the ICC configuration does not exist or is invalid. I think Bjorn suggested to do the allocation after getting the ICC paths and error handling, when you know the struct is actually used.

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1813

Quoted 6 lines
drivers/mmc/host/sdhci-msm.c +6 -0
+
+	vote_data->sdhc_to_ddr = of_icc_get(&pdev->dev, "sdhc-ddr");
+	vote_data->cpu_to_sdhc = of_icc_get(&pdev->dev, "cpu-sdhc");
+	if (!vote_data->sdhc_to_ddr || !vote_data->cpu_to_sdhc) {
+		dev_info(&pdev->dev, "ICC DT property is missing. skip vote !!\n");
+		return NULL;

This combined handling masks possible misconfigurations/errors, where one ICC path is specified (correctly or not), and the other not. Also the log message would be confusing in this case, since the ICC property exists.

Preferably NULL would only be returned if neither of the ICC paths is specified. The message could be something like "no interconnect configuration". This is still not entirely correct, since there could be a configuration, just not with the expected interconnect names.

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1817

Quoted 4 lines
drivers/mmc/host/sdhci-msm.c +4 -0
+	} else if (IS_ERR(vote_data->sdhc_to_ddr) ||
+			IS_ERR(vote_data->cpu_to_sdhc)) {
+		dev_err(&pdev->dev, "(%ld): failed getting %s path\n",
+				PTR_ERR(vote_data->sdhc_to_ddr), "sdhc-ddr");

What is the point of using '%s' here?

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1819

Quoted 2 lines
drivers/mmc/host/sdhci-msm.c +2 -0
+		dev_err(&pdev->dev, "(%ld): failed getting %s path\n",
+				PTR_ERR(vote_data->sdhc_to_ddr), "cpu-sdhc");

ditto

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1824

Quoted 5 lines
drivers/mmc/host/sdhci-msm.c +5 -0
+
+		return IS_ERR(vote_data->sdhc_to_ddr) ?
+			ERR_CAST(vote_data->sdhc_to_ddr) :
+			ERR_CAST(vote_data->cpu_to_sdhc);
+	}

The above could print an error message for an ICC path that doesn't have a problem. Also we don't want to yell in case of deferred probing.

Something like this could be a possible alternative:

     	struct icc_path *icc_paths[BUS_INTERCONNECT_PATHS];
	const char *path_names[] = {
		"sdhc-ddr",
		"cpu-sdhc"
	};

	// use loop?
	icc_paths[0] = of_icc_get(&pdev->dev, "sdhc-ddr");
	icc_paths[1] = of_icc_get(&pdev->dev, "cpu-sdhc");

	if (!icc_paths[0] && !icc_paths[1]) {
		dev_info(...);
		return NULL;
	}

	for (i = 0; i <  BUS_INTERCONNECT_PATHS; i++) {
	       	int err = 0;

		if (!icc_paths[i]) {
			dev_err(pdev->dev. "interconnect path '%s' is not configured\n", path_names[i]);
			err = -EINVAL;
			goto handle_err;
		}

		if (IS_ERR(icc_paths[i]) {
			err = PTR_ERR(icc_paths[i]);

			if (err != -EPROBE_DEFER)
				dev_err(pdev->dev. "interconnect path '%s' is invalid: %d\n", path_names[i], err);
		}

handle_err:

		if (err) {
			int other = (i == 0)? 1 : 0;

			if (!IS_ERR_OR_NULL(icc_paths[other]))
				icc_put(icc_paths[other]);

			return err;
		}
	}

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1832

Quoted 8 lines
drivers/mmc/host/sdhci-msm.c +8 -0
+	ret = dev_pm_opp_of_add_table(dev);
+	if (ret) {
+		if (ret == -ENODEV || ret == -ENODATA)
+			dev_err(dev, "OPP dt properties missing:%d\n", ret);
+		else
+			dev_err(dev, "OPP registration failed:%d\n", ret);
+		return ERR_PTR(ret);
+	}

Now that we know that there are no errors we can allocate 'vote_data' and initialize it.

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1843

Quoted 11 lines
drivers/mmc/host/sdhci-msm.c +11 -0
+	return vote_data;
+}
+
+static void sdhci_msm_bus_unregister(struct device *dev,
+				struct sdhci_msm_host *host)
+{
+	struct sdhci_msm_bus_vote_data *vote_data = host->bus_vote_data;
+
+	if (!vote_data ||
+		IS_ERR(vote_data->sdhc_to_ddr) ||
+		IS_ERR(vote_data->cpu_to_sdhc))

The check for errors in the ICC paths is not necessary. When an error is encountered sdhci_msm_bus_register() returns an error, not a struct.

Quoted 5 lines
drivers/mmc/host/sdhci-msm.c +5 -0
+		return;
+
+	icc_put(vote_data->sdhc_to_ddr);
+	icc_put(vote_data->cpu_to_sdhc);
+}

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1858

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+
+static void sdhci_msm_bus_voting(struct sdhci_host *host, bool enable)
+{
+	struct mmc_ios *ios = &host->mmc->ios;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	unsigned int bw;
+
+	if (!msm_host->bus_vote_data ||
+		IS_ERR(msm_host->bus_vote_data))

no need to check for errors, _probe() is aborted in case of errors.

Thanks

Matthias

author
#3 lore raw

Re: [RFC v4 1/2] mmc: sdhci-msm: Add interconnect bus bandwidth scaling support

Thanks Matthias for review and suggestions.

Quoted 25 lines · Matthias Kaehlcke

Hi Pradeep,

Quoted 22 lines · 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
[RFC] mmc: host: sdhci-msm: Use the interconnect API
(https://lkml.org/lkml/2018/10/11/499) and

[PATCH v6] Introduce Bandwidth OPPs for interconnects
(https://lkml.org/lkml/2019/12/6/740)

Co-developed-by Sahitya Tummala <[email protected]>
Signed-off-by Sahitya Tummala <[email protected]>
Co-developed-by Subhash Jadavani <[email protected]>
Signed-off-by Subhash Jadavani <[email protected]>
Co-developed-by Veerabhadrarao Badiganti <[email protected]>
Signed-off-by Veerabhadrarao Badiganti <[email protected]>
Co-developed-by Pradeep P V K <[email protected]>
Signed-off-by Pradeep P V K <[email protected]>

---

Quoted 7 lines
Quoted 3 lines

changes from RFC v3 -> v4:

- Addressed review comments from Bjorn and Matthias

This is not helpful, please describe in future versions what those changes are.

sure will do this from next patch series.

Quoted 61 lines
Quoted 61 lines
 drivers/mmc/host/sdhci-msm.c | 204 
++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 200 insertions(+), 4 deletions(-)
drivers/mmc/host/sdhci-msm.c +11 -0
index 71f29ba..5af1c58 100644
@@ -11,8 +11,10 @@
 #include <linux/mmc/mmc.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
+#include <linux/interconnect.h>
 #include <linux/iopoll.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_opp.h>
 
 #include "sdhci-pltfm.h"
 

@@ -229,6 +231,12 @@ struct sdhci_msm_variant_info {
 	const struct sdhci_msm_offset *offset;
 };
 
+struct sdhci_msm_bus_vote_data {
+	struct icc_path *sdhc_to_ddr;
+	struct icc_path *cpu_to_sdhc;
+	u32 curr_freq;
+};
+
 struct sdhci_msm_host {
 	struct platform_device *pdev;
 	void __iomem *core_mem;	/* MSM SDCC mapped address */

@@ -255,8 +263,11 @@ struct sdhci_msm_host {
 	bool use_cdr;
 	u32 transfer_mode;
 	bool updated_ddr_cfg;
+	struct sdhci_msm_bus_vote_data *bus_vote_data;
 };
 
+static void sdhci_msm_bus_voting(struct sdhci_host *host, bool 
 enable);
+
 static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct 
 sdhci_host *host)

{

drivers/mmc/host/sdhci-msm.c +1 -0
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);

@@ -1564,6 +1575,7 @@ static void sdhci_msm_set_clock(struct 
 sdhci_host *host, unsigned int clock)
 
 	msm_set_clock_rate_for_bus_mode(host, clock);
 out:
+	sdhci_msm_bus_voting(host, !!clock);
 	__sdhci_msm_set_clock(host, clock);
 }

@@ -1685,6 +1697,174 @@ static void 
 sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host)
 	pr_debug("%s: supported caps: 0x%08x\n", mmc_hostname(mmc), caps);
 }
Quoted 15 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1711

Quoted 12 lines
drivers/mmc/host/sdhci-msm.c +12 -0
+/*
+ * Returns required bandwidth in Bytes per Sec
+ */
+static unsigned long sdhci_get_bw_required(struct sdhci_host *host,
+					struct mmc_ios *ios)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+	switch (ios->bus_width) {
+	case MMC_BUS_WIDTH_1:
+		return msm_host->clk_rate/8;

nit: it would be more readable with spaces around '/'.

ok. i will address this in my next patchset.

Quoted 76 lines
Quoted 76 lines
drivers/mmc/host/sdhci-msm.c +75 -0
+	case MMC_BUS_WIDTH_4:
+		return msm_host->clk_rate/2;
+	case MMC_BUS_WIDTH_8:
+		break;
+	}
+	return msm_host->clk_rate;
+}
+
+/*
+ * Helper function to parse the exact OPP node
+ * Returns OPP pointer on success else NULL on error
+ */
+static struct dev_pm_opp
+		*sdhci_msm_find_opp_for_freq(struct sdhci_msm_host *msm_host,
+							unsigned long bw)
+{
+	struct dev_pm_opp *opp;
+	struct sdhci_host *host = mmc_priv(msm_host->mmc);
+	unsigned int freq = bw;
+	struct device *dev = &msm_host->pdev->dev;
+
+
+	if (!freq)
+		opp = dev_pm_opp_find_peak_bw_floor(dev, &freq);
+	else
+		opp = dev_pm_opp_find_peak_bw_exact(dev, freq, true);
+
+	/* Max bandwidth vote */
+	if (PTR_ERR(opp) == -ERANGE && freq > sdhci_msm_get_max_clock(host))
+		opp = dev_pm_opp_find_peak_bw_ceil(dev, &bw);
+
+	if (IS_ERR(opp)) {
+		dev_err(dev, "Failed to find OPP for freq:%u err:%ld\n",
+				freq, PTR_ERR(opp));
+		return NULL;
+	}
+	return opp;
+}
+
+/*
+ * This function sets the interconnect bus bandwidth
+ * vote based on bw (bandwidth) argument.
+ */
+#define BUS_INTERCONNECT_PATHS 2 /* 1. sdhc -> ddr 2. cpu -> sdhc */
+static void sdhci_msm_bus_set_vote(struct sdhci_host *host,
+						unsigned int bw)
+{
+	int i, ddr_rc, cpu_rc;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	struct sdhci_msm_bus_vote_data *vote_data = msm_host->bus_vote_data;
+	struct device *dev = &msm_host->pdev->dev;
+	struct dev_pm_opp *opp;
+	unsigned long freq = bw;
+	unsigned long peak_bw[BUS_INTERCONNECT_PATHS] = {0};
+	unsigned long avg_bw[BUS_INTERCONNECT_PATHS] = {0};
+
+	if (bw == vote_data->curr_freq)
+		return;
+
+	for (i = 0; i < BUS_INTERCONNECT_PATHS; i++) {
+		opp = sdhci_msm_find_opp_for_freq(msm_host, freq);
+		if (opp) {
+			avg_bw[i] = dev_pm_opp_get_bw(opp, &peak_bw[i]);
+			freq += 1; /* Next bandwidth vote */
+			dev_pm_opp_put(opp);
+		}
+	}
+	pr_debug("%s: freq:%d sdhc_to_ddr avg_bw:%lu peak_bw:%lu cpu_to_sdhc 
 avg_bw:%lu peak_bw:%lu\n",
+			mmc_hostname(host->mmc), bw, avg_bw[0], peak_bw[0],
+				avg_bw[1], peak_bw[1]);
+	ddr_rc = icc_set_bw(vote_data->sdhc_to_ddr, 0, peak_bw[0]);
+	cpu_rc = icc_set_bw(vote_data->cpu_to_sdhc, 0, peak_bw[1]);
+	if (ddr_rc || cpu_rc) {
+		dev_err(dev, "icc_set() failed ddr_rc:%d cpu_rc:%d\n",
Quoted 9 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1789

Quoted 3 lines
drivers/mmc/host/sdhci-msm.c +3 -0
+							ddr_rc, cpu_rc);
+		return;
+	}

Do you necessarily want to set the bandwidth of the CPU to SDHC path, if setting the bandwidth for the SDHC to DDR path failed? If not I'd suggest to get rid of this double error handling with 'ddr_rc' and 'cpu_rc' and handle each error individually.

It is not required to set bw for CPU to SDHC path, if setting to SDHC to DDR path failed. i will handle this individually in my next patch series.

Quoted 22 lines
Quoted 8 lines
drivers/mmc/host/sdhci-msm.c +8 -0
+	vote_data->curr_freq = bw;
+}
+
+/*
+ * Helper function to register for OPP and interconnect
+ * frameworks.
+ */
+static struct sdhci_msm_bus_vote_data

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1807

Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10 -0
+		*sdhci_msm_bus_register(struct sdhci_msm_host *host,
+				struct platform_device *pdev)
+{
+	struct sdhci_msm_bus_vote_data *vote_data;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	vote_data = devm_kzalloc(dev, sizeof(*vote_data), GFP_KERNEL);
+	if (!vote_data)
+		return NULL;

Even though the interconnects are optional I think we want to propagate an out of memory error, i.e. you probably want to return -ENOMEM here.

Quoted 5 lines

Also the allocated 'vote_data' will not be used if the ICC configuration does not exist or is invalid. I think Bjorn suggested to do the allocation after getting the ICC paths and error handling, when you know the struct is actually used.

ok i will do this in my next patch set.

Quoted 11 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1813

Quoted 6 lines
drivers/mmc/host/sdhci-msm.c +6 -0
+
+	vote_data->sdhc_to_ddr = of_icc_get(&pdev->dev, "sdhc-ddr");
+	vote_data->cpu_to_sdhc = of_icc_get(&pdev->dev, "cpu-sdhc");
+	if (!vote_data->sdhc_to_ddr || !vote_data->cpu_to_sdhc) {
+		dev_info(&pdev->dev, "ICC DT property is missing. skip vote !!\n");
+		return NULL;

This combined handling masks possible misconfigurations/errors, where one ICC path is specified (correctly or not), and the other not. Also the log message would be confusing in this case, since the ICC property exists.

Quoted 5 lines

Preferably NULL would only be returned if neither of the ICC paths is specified. The message could be something like "no interconnect configuration". This is still not entirely correct, since there could be a configuration, just not with the expected interconnect names.

i will take care this in my next patch series.

Quoted 7 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1817

Quoted 4 lines
drivers/mmc/host/sdhci-msm.c +4 -0
+	} else if (IS_ERR(vote_data->sdhc_to_ddr) ||
+			IS_ERR(vote_data->cpu_to_sdhc)) {
+		dev_err(&pdev->dev, "(%ld): failed getting %s path\n",
+				PTR_ERR(vote_data->sdhc_to_ddr), "sdhc-ddr");

What is the point of using '%s' here?

Not required specifically. i will take care this too in my next patch set.

Quoted 5 lines

drivers/mmc/host/sdhci-msm.c static void · line 1821

Quoted 2 lines
drivers/mmc/host/sdhci-msm.c +2 -0
+		dev_err(&pdev->dev, "(%ld): failed getting %s path\n",
+				PTR_ERR(vote_data->sdhc_to_ddr), "cpu-sdhc");

ditto

ok.

Quoted 45 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1824

Quoted 5 lines
drivers/mmc/host/sdhci-msm.c +5 -0
+
+		return IS_ERR(vote_data->sdhc_to_ddr) ?
+			ERR_CAST(vote_data->sdhc_to_ddr) :
+			ERR_CAST(vote_data->cpu_to_sdhc);
+	}

The above could print an error message for an ICC path that doesn't have a problem. Also we don't want to yell in case of deferred probing.

Something like this could be a possible alternative:

     	struct icc_path *icc_paths[BUS_INTERCONNECT_PATHS];
	const char *path_names[] = {
		"sdhc-ddr",
		"cpu-sdhc"
	};

	// use loop?
	icc_paths[0] = of_icc_get(&pdev->dev, "sdhc-ddr");
	icc_paths[1] = of_icc_get(&pdev->dev, "cpu-sdhc");

	if (!icc_paths[0] && !icc_paths[1]) {
		dev_info(...);
		return NULL;
	}

	for (i = 0; i <  BUS_INTERCONNECT_PATHS; i++) {
	       	int err = 0;

		if (!icc_paths[i]) {
			dev_err(pdev->dev. "interconnect path '%s' is not configured\n",
path_names[i]);
			err = -EINVAL;
			goto handle_err;
		}

		if (IS_ERR(icc_paths[i]) {
			err = PTR_ERR(icc_paths[i]);

			if (err != -EPROBE_DEFER)
				dev_err(pdev->dev. "interconnect path '%s' is invalid: %d\n",
path_names[i], err);
		}

handle_err:

Quoted 10 lines
		if (err) {
			int other = (i == 0)? 1 : 0;

			if (!IS_ERR_OR_NULL(icc_paths[other]))
				icc_put(icc_paths[other]);

			return err;
		}
	}

Thanks for the alternate code. i will address this in my next patch set.

Quoted 12 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1832

Quoted 8 lines
drivers/mmc/host/sdhci-msm.c +8 -0
+	ret = dev_pm_opp_of_add_table(dev);
+	if (ret) {
+		if (ret == -ENODEV || ret == -ENODATA)
+			dev_err(dev, "OPP dt properties missing:%d\n", ret);
+		else
+			dev_err(dev, "OPP registration failed:%d\n", ret);
+		return ERR_PTR(ret);
+	}

Now that we know that there are no errors we can allocate 'vote_data' and initialize it.

ok.

Quoted 6 lines
Quoted 6 lines
drivers/mmc/host/sdhci-msm.c +6 -0
+	return vote_data;
+}
+
+static void sdhci_msm_bus_unregister(struct device *dev,
+				struct sdhci_msm_host *host)
+{
Quoted 9 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1843

Quoted 5 lines
drivers/mmc/host/sdhci-msm.c +5 -0
+	struct sdhci_msm_bus_vote_data *vote_data = host->bus_vote_data;
+
+	if (!vote_data ||
+		IS_ERR(vote_data->sdhc_to_ddr) ||
+		IS_ERR(vote_data->cpu_to_sdhc))

The check for errors in the ICC paths is not necessary. When an error is encountered sdhci_msm_bus_register() returns an error, not a struct.

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

Quoted 10 lines
Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +9 -0
+		return;
+
+	icc_put(vote_data->sdhc_to_ddr);
+	icc_put(vote_data->cpu_to_sdhc);
+}
+
+static void sdhci_msm_bus_voting(struct sdhci_host *host, bool 
 enable)
+{
+	struct mmc_ios *ios = &host->mmc->ios;
Quoted 9 lines

drivers/mmc/host/sdhci-msm.c static void sdhci_msm_set_regulator_caps(struct sdhci_msm_host *msm_host) · line 1858

Quoted 6 lines
drivers/mmc/host/sdhci-msm.c +6 -0
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+	unsigned int bw;
+
+	if (!msm_host->bus_vote_data ||
+		IS_ERR(msm_host->bus_vote_data))

no need to check for errors, _probe() is aborted in case of errors.

ok.

Quoted 3 lines

Thanks

Matthias

author
#4 lore raw

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

Signed-off-by Pradeep P V K <[email protected]>

---

changes from RFC v3 -> v4:
- No changes.

 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +18 -0
index 7ee639b..cbe97b8 100644
@@ -40,6 +40,21 @@ Required properties:
 	"cal"	- reference clock for RCLK delay calibration (optional)
 	"sleep"	- sleep clock for RCLK delay calibration (optional)
 
+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 {

@@ -57,6 +72,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";
 	};
 
 	sdhc_2: sdhci@f98a4900 {
Signature · 1 line
1.9.1
maintainer Acked-by on v4 2/2
#5 lore raw

Re: [RFC v4 2/2] dt-bindings: mmc: sdhci-msm: Add interconnect BW scaling strings

Quoted 12 lines · Pradeep P V K

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

Signed-off-by Pradeep P V K <[email protected]>

---

changes from RFC v3 -> v4:
- No changes.

 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Acked-by Rob Herring <[email protected]>
Reviewed-by on v4 2/2
#6 lore raw

Re: [RFC v4 2/2] dt-bindings: mmc: sdhci-msm: Add interconnect BW scaling strings

In reply to Pradeep P V K
Quoted 4 lines · Pradeep P V K

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

Signed-off-by Pradeep P V K <[email protected]>
Reviewed-by Bjorn Andersson <[email protected]>
Quoted 47 lines

---

changes from RFC v3 -> v4:
- No changes.

 Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +18 -0
index 7ee639b..cbe97b8 100644
@@ -40,6 +40,21 @@ Required properties:
 	"cal"	- reference clock for RCLK delay calibration (optional)
 	"sleep"	- sleep clock for RCLK delay calibration (optional)
 
+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 {

@@ -57,6 +72,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";
 	};
 
 	sdhc_2: sdhci@f98a4900 {
Signature · 1 line
1.9.1