[RFC 0/4] mmc: tmio/sdhi: prevent regressions when setting clock

[RFC 0/4] mmc: tmio/sdhi: prevent regressions when setting clock

[Wolfram Sang]

Here is a small refactoring series for setting the clock for SDHI cores. This
is to prevent regression on platforms where this feature has not been tested.
One such regression is known on r8a7740 (patches 2+3). Patch 4 fixes an
oversight and patch 1 is a small cleanup.

RFC for now since I can't test these patches on my hardware currently. Will do
so on Wednesday, probably. Posting now since because Geert may already want to
test on r8a7740. Branch is here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git sdhi-for-ulf

Thanks,

   Wolfram

Wolfram Sang (4):
  mmc: sh_mobile_sdhi: make clk_update function more compact
  mmc: tmio/sdhi: introduce flag for RCar 2+ specific features
  mmc: sh_mobile_sdhi: only change the clock on RCar Gen2+
  mmc: sh_mobile_sdhi: check return value when changing clk

 drivers/mmc/host/sh_mobile_sdhi.c | 17 +++++++++--------
 drivers/mmc/host/tmio_mmc_pio.c   |  6 +++---
 include/linux/mfd/tmio.h          |  4 ++--
 3 files changed, 14 insertions(+), 13 deletions(-)

-- 
2.6.2

[RFC 1/4] mmc: sh_mobile_sdhi: make clk_update function more compact

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Save a few lines, the codebase is large enough.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index f8ea3d1d6de382..6857e4ba724415 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -165,12 +165,9 @@ static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host,
 					      unsigned int new_clock)
 {
 	struct sh_mobile_sdhi *priv = host_to_priv(host);
-	unsigned int freq, best_freq, diff_min, diff;
+	unsigned int freq, diff, best_freq = 0, diff_min = ~0;
 	int i;
 
-	diff_min = ~0;
-	best_freq = 0;
-
 	/*
 	 * We want the bus clock to be as close as possible to, but no
 	 * greater than, new_clock.  As we can divide by 1 << i for
-- 
2.6.2

[RFC 2/4] mmc: tmio/sdhi: introduce flag for RCar 2+ specific features

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

RCar Gen2 and later implementations of TMIO/SDHI have their own set of
features and additions. FAST_CLK_CHG is just one of them and I see a few
others being added soon. Some may work on older chipsets but this needs
to be tested case by case. Instead of adding a bunch of flags for each
feature, add a global RCar2+ one for now. We can still break out
features if the need arises.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c | 4 ++--
 drivers/mmc/host/tmio_mmc_pio.c   | 6 +++---
 include/linux/mfd/tmio.h          | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 6857e4ba724415..19e4c29e4233b0 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -62,7 +62,7 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen1_compatible = {
 
 static const struct sh_mobile_sdhi_of_data of_rcar_gen2_compatible = {
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE |
-			  TMIO_MMC_CLK_ACTUAL | TMIO_MMC_FAST_CLK_CHG,
+			  TMIO_MMC_CLK_ACTUAL | TMIO_MMC_MIN_RCAR2,
 	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
 	.dma_buswidth	= DMA_SLAVE_BUSWIDTH_4_BYTES,
 	.dma_rx_offset	= 0x2000,
@@ -70,7 +70,7 @@ static const struct sh_mobile_sdhi_of_data of_rcar_gen2_compatible = {
 
 static const struct sh_mobile_sdhi_of_data of_rcar_gen3_compatible = {
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_WRPROTECT_DISABLE |
-			  TMIO_MMC_CLK_ACTUAL | TMIO_MMC_FAST_CLK_CHG,
+			  TMIO_MMC_CLK_ACTUAL | TMIO_MMC_MIN_RCAR2,
 	.capabilities	= MMC_CAP_SD_HIGHSPEED,
 	.bus_shift	= 2,
 };
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 95f22997f31a66..f44e2ab7aea2ba 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -157,7 +157,7 @@ static void tmio_mmc_clk_start(struct tmio_mmc_host *host)
 {
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, CLK_CTL_SCLKEN |
 		sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
-	msleep(host->pdata->flags & TMIO_MMC_FAST_CLK_CHG ? 1 : 10);
+	msleep(host->pdata->flags & TMIO_MMC_MIN_RCAR2 ? 1 : 10);
 
 	if (host->pdata->flags & TMIO_MMC_HAVE_HIGH_REG) {
 		sd_ctrl_write16(host, CTL_CLK_AND_WAIT_CTL, 0x0100);
@@ -174,7 +174,7 @@ static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
 
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
 		sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
-	msleep(host->pdata->flags & TMIO_MMC_FAST_CLK_CHG ? 5 : 10);
+	msleep(host->pdata->flags & TMIO_MMC_MIN_RCAR2 ? 5 : 10);
 }
 
 static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
@@ -205,7 +205,7 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
 			sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
 	sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, clk & CLK_CTL_DIV_MASK);
-	if (!(host->pdata->flags & TMIO_MMC_FAST_CLK_CHG))
+	if (!(host->pdata->flags & TMIO_MMC_MIN_RCAR2))
 		msleep(10);
 
 	tmio_mmc_clk_start(host);
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 05d58ee5e6a78f..7a26286db895c5 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -66,8 +66,8 @@
  */
 #define TMIO_MMC_SDIO_IRQ		(1 << 2)
 
-/* Some controllers don't need to wait 10ms for clock changes */
-#define TMIO_MMC_FAST_CLK_CHG		(1 << 3)
+/* Some features are only available or tested on RCar Gen2 or later */
+#define TMIO_MMC_MIN_RCAR2		(1 << 3)
 
 /*
  * Some controllers require waiting for the SD bus to become
-- 
2.6.2

[RFC 3/4] mmc: sh_mobile_sdhi: only change the clock on RCar Gen2+

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

We had a regression on r8a7740 where the SDHI clock was a generic
peripheral clock, so changing its rate was not desired. This should be
fixed in the clock driver. However, it also shows that the new clock
calculation should only be used on tested systems. Add a check for that.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 19e4c29e4233b0..ac9ba36da39b5b 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -168,6 +168,10 @@ static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host,
 	unsigned int freq, diff, best_freq = 0, diff_min = ~0;
 	int i;
 
+	/* tested only on RCar Gen2+ currently; may work for others */
+	if (!(host->pdata->flags & TMIO_MMC_MIN_RCAR2))
+		return clk_get_rate(priv->clk);
+
 	/*
 	 * We want the bus clock to be as close as possible to, but no
 	 * greater than, new_clock.  As we can divide by 1 << i for
-- 
2.6.2

[RFC 4/4] mmc: sh_mobile_sdhi: check return value when changing clk

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

And return the old clock rate if something went wrong.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/sh_mobile_sdhi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index ac9ba36da39b5b..5309c73be1f04f 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -166,7 +166,7 @@ static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host,
 {
 	struct sh_mobile_sdhi *priv = host_to_priv(host);
 	unsigned int freq, diff, best_freq = 0, diff_min = ~0;
-	int i;
+	int i, ret;
 
 	/* tested only on RCar Gen2+ currently; may work for others */
 	if (!(host->pdata->flags & TMIO_MMC_MIN_RCAR2))
@@ -195,9 +195,9 @@ static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host,
 		}
 	}
 
-	clk_set_rate(priv->clk, best_freq);
+	ret = clk_set_rate(priv->clk, best_freq);
 
-	return best_freq;
+	return ret == 0 ? best_freq : clk_get_rate(priv->clk);
 }
 
 static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
-- 
2.6.2

Re: [RFC 0/4] mmc: tmio/sdhi: prevent regressions when setting clock

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, May 2, 2016 at 10:25 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> Here is a small refactoring series for setting the clock for SDHI cores. This
> is to prevent regression on platforms where this feature has not been tested.
> One such regression is known on r8a7740 (patches 2+3). Patch 4 fixes an
> oversight and patch 1 is a small cleanup.
>
> RFC for now since I can't test these patches on my hardware currently. Will do
> so on Wednesday, probably. Posting now since because Geert may already want to
> test on r8a7740. Branch is here:

Thanks, I can confirm the HP clock is back to 99 MHz (from 12.375 MHz)
on r8a7740/armadillo, so

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC 0/4] mmc: tmio/sdhi: prevent regressions when setting clock

[Ulf Hansson]

On 2 May 2016 at 22:25, Wolfram Sang <wsa@the-dreams.de> wrote:
> Here is a small refactoring series for setting the clock for SDHI cores. This
> is to prevent regression on platforms where this feature has not been tested.
> One such regression is known on r8a7740 (patches 2+3). Patch 4 fixes an
> oversight and patch 1 is a small cleanup.
>
> RFC for now since I can't test these patches on my hardware currently. Will do
> so on Wednesday, probably. Posting now since because Geert may already want to
> test on r8a7740. Branch is here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git sdhi-for-ulf
>
> Thanks,
>
>    Wolfram
>
> Wolfram Sang (4):
>   mmc: sh_mobile_sdhi: make clk_update function more compact
>   mmc: tmio/sdhi: introduce flag for RCar 2+ specific features
>   mmc: sh_mobile_sdhi: only change the clock on RCar Gen2+
>   mmc: sh_mobile_sdhi: check return value when changing clk
>
>  drivers/mmc/host/sh_mobile_sdhi.c | 17 +++++++++--------
>  drivers/mmc/host/tmio_mmc_pio.c   |  6 +++---
>  include/linux/mfd/tmio.h          |  4 ++--
>  3 files changed, 14 insertions(+), 13 deletions(-)
>
> --
> 2.6.2
>

This looks good to me, so thanks, applied for next!

Kind regards
Uffe