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

Thierry Reding
2021-08-19T17:03:56Z

[-- Attachment #1: Type: text/plain, Size: 2154 bytes --]

On Tue, Aug 17, 2021 at 04:27:40AM +0300, Dmitry Osipenko wrote:
> 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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]