Thread (119 messages) 119 messages, 7 authors, 2021-08-26

Re: [PATCH v8 20/34] mmc: sdhci-tegra: Add runtime PM and OPP support

From: Thierry Reding <hidden>
Date: 2021-08-20 11:35:36
Also in: dri-devel, linux-clk, linux-devicetree, linux-media, linux-mmc, linux-pwm, linux-spi, linux-staging, linux-tegra, linux-usb, lkml

On Fri, Aug 20, 2021 at 01:37:13AM +0300, Dmitry Osipenko wrote:
19.08.2021 20:03, Thierry Reding пишет:
quoted
On Tue, Aug 17, 2021 at 04:27:40AM +0300, Dmitry Osipenko wrote:
quoted
The SDHCI on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now SDHCI must be resumed using
runtime PM API in order to initialize the SDHCI power state. The SDHCI
clock rate must be changed using OPP API that will reconfigure the power
domain performance state in accordance to the rate. Add runtime PM and OPP
support to the SDHCI driver.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/mmc/host/sdhci-tegra.c | 146 ++++++++++++++++++++++++---------
 1 file changed, 105 insertions(+), 41 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 387ce9cdbd7c..a3583359c972 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -15,6 +15,8 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/pm_opp.h>
+#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/mmc/card.h>
@@ -24,6 +26,8 @@
 #include <linux/gpio/consumer.h>
 #include <linux/ktime.h>
 
+#include <soc/tegra/common.h>
+
 #include "sdhci-pltfm.h"
 #include "cqhci.h"
 
@@ -123,6 +127,12 @@
 					 SDHCI_TRNS_BLK_CNT_EN | \
 					 SDHCI_TRNS_DMA)
 
+enum {
+	TEGRA_CLK_BULK_SDHCI,
+	TEGRA_CLK_BULK_TMCLK,
+	TEGRA_CLK_BULK_NUM,
+};
+
 struct sdhci_tegra_soc_data {
 	const struct sdhci_pltfm_data *pdata;
 	u64 dma_mask;
@@ -171,6 +181,8 @@ struct sdhci_tegra {
 	bool enable_hwcq;
 	unsigned long curr_clk_rate;
 	u8 tuned_tap_delay;
+
+	struct clk_bulk_data clocks[TEGRA_CLK_BULK_NUM];
This doesn't seem worth it to me. There's a lot of churn in this driver
that's only needed to convert this to the clk_bulk API and it makes the
code a lot more difficult to read, in my opinion.

It looks like the only benefit that this gives us is that runtime
suspend and resume become a few lines shorter.
The driver probe code looks cleaner with that. You should be looking at
the final result and not at the patch to see it.
I did look at the final result and didn't find it cleaner at all. =)

Thierry

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help