[PATCH v2] mmc: sdhi: fill in actual_clock

[PATCH v2] mmc: sdhi: fill in actual_clock

[Tamás Szűcs]

Save set clock in mmc_host actual_clock enabling exporting it via debugfs.
This will indicate the precise SD clock in I/O settings rather than only the
sometimes misleading requested clock.

Signed-off-by: Tamás Szűcs <tszucs@protonmail.ch>
---
 drivers/mmc/host/renesas_sdhi_core.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 64d3b5fb7fe5..4c9774dbcfc1 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -124,7 +124,7 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
 {
 	struct renesas_sdhi *priv = host_to_priv(host);
 	unsigned int freq, diff, best_freq = 0, diff_min = ~0;
-	int i, ret;
+	int i;
 
 	/* tested only on R-Car Gen2+ currently; may work for others */
 	if (!(host->pdata->flags & TMIO_MMC_MIN_RCAR2))
@@ -153,9 +153,9 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
 		}
 	}
 
-	ret = clk_set_rate(priv->clk, best_freq);
+	clk_set_rate(priv->clk, best_freq);
 
-	return ret == 0 ? best_freq : clk_get_rate(priv->clk);
+	return clk_get_rate(priv->clk);
 }
 
 static void renesas_sdhi_set_clock(struct tmio_mmc_host *host,
@@ -166,10 +166,13 @@ static void renesas_sdhi_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));
 
-	if (new_clock == 0)
+	if (new_clock == 0) {
+		host->mmc->actual_clock = 0;
 		goto out;
+	}
 
-	clock = renesas_sdhi_clk_update(host, new_clock) / 512;
+	host->mmc->actual_clock = renesas_sdhi_clk_update(host, new_clock);
+	clock = host->mmc->actual_clock / 512;
 
 	for (clk = 0x80000080; new_clock >= (clock << 1); clk >>= 1)
 		clock <<= 1;
-- 
2.11.0

Re: [PATCH v2] mmc: sdhi: fill in actual_clock

[Geert Uytterhoeven]

Hi Tamás,

On Thu, Aug 29, 2019 at 8:37 PM Tamás Szűcs <tszucs@protonmail.ch> wrote:
> Save set clock in mmc_host actual_clock enabling exporting it via debugfs.
> This will indicate the precise SD clock in I/O settings rather than only the
> sometimes misleading requested clock.
>
> Signed-off-by: Tamás Szűcs <tszucs@protonmail.ch>

Thanks for the update!

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

However, one question below.

> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -166,10 +166,13 @@ static void renesas_sdhi_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));
>
> -       if (new_clock == 0)
> +       if (new_clock == 0) {
> +               host->mmc->actual_clock = 0;

The actual clock is present in the debugfs output only when non-zero.
Hence userspace cannot distinguish between an old kernel where the
Renesas SDHI driver didn't fill in actual_clock, and a new kernel when
the SDHI controller is powered down.
Could that be an issue? Should the old value be retained?
Probably it's OK, as this is debugfs, not an official ABI.

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: [PATCH v2] mmc: sdhi: fill in actual_clock

[Tamás Szűcs]

Hi Geert,

You are correct, there is no way to distinguish between the old and new kernels just by mmc ios output when the bus is down. I don't think it's an issue. I find it more helpful to have this information available.
Yes, actual_clock should only display when non-zero and it should be zero when the bus is down. I fixed this in v2.

Kind regards,
Tamas


Tamás Szűcs
tszucs@protonmail.ch

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, August 30, 2019 9:21 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Tamás,
>
> On Thu, Aug 29, 2019 at 8:37 PM Tamás Szűcs tszucs@protonmail.ch wrote:
>
> > Save set clock in mmc_host actual_clock enabling exporting it via debugfs.
> > This will indicate the precise SD clock in I/O settings rather than only the
> > sometimes misleading requested clock.
> > Signed-off-by: Tamás Szűcs tszucs@protonmail.ch
>
> Thanks for the update!
>
> Reviewed-by: Geert Uytterhoeven geert+renesas@glider.be
>
> However, one question below.
>
> > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > @@ -166,10 +166,13 @@ static void renesas_sdhi_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));
> >
> > -         if (new_clock == 0)
> >
> >
> >
> > -         if (new_clock == 0) {
> >
> >
> > -                 host->mmc->actual_clock = 0;
> >
> >
>
> The actual clock is present in the debugfs output only when non-zero.
> Hence userspace cannot distinguish between an old kernel where the
> Renesas SDHI driver didn't fill in actual_clock, and a new kernel when
> the SDHI controller is powered down.
> Could that be an issue? Should the old value be retained?
> Probably it's OK, as this is debugfs, not an official ABI.
>
> 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: [PATCH v2] mmc: sdhi: fill in actual_clock

[Ulf Hansson]

On Thu, 29 Aug 2019 at 20:36, Tamás Szűcs <tszucs@protonmail.ch> wrote:
>
> Save set clock in mmc_host actual_clock enabling exporting it via debugfs.
> This will indicate the precise SD clock in I/O settings rather than only the
> sometimes misleading requested clock.
>
> Signed-off-by: Tamás Szűcs <tszucs@protonmail.ch>

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/renesas_sdhi_core.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 64d3b5fb7fe5..4c9774dbcfc1 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -124,7 +124,7 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
>  {
>         struct renesas_sdhi *priv = host_to_priv(host);
>         unsigned int freq, diff, best_freq = 0, diff_min = ~0;
> -       int i, ret;
> +       int i;
>
>         /* tested only on R-Car Gen2+ currently; may work for others */
>         if (!(host->pdata->flags & TMIO_MMC_MIN_RCAR2))
> @@ -153,9 +153,9 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
>                 }
>         }
>
> -       ret = clk_set_rate(priv->clk, best_freq);
> +       clk_set_rate(priv->clk, best_freq);
>
> -       return ret == 0 ? best_freq : clk_get_rate(priv->clk);
> +       return clk_get_rate(priv->clk);
>  }
>
>  static void renesas_sdhi_set_clock(struct tmio_mmc_host *host,
> @@ -166,10 +166,13 @@ static void renesas_sdhi_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));
>
> -       if (new_clock == 0)
> +       if (new_clock == 0) {
> +               host->mmc->actual_clock = 0;
>                 goto out;
> +       }
>
> -       clock = renesas_sdhi_clk_update(host, new_clock) / 512;
> +       host->mmc->actual_clock = renesas_sdhi_clk_update(host, new_clock);
> +       clock = host->mmc->actual_clock / 512;
>
>         for (clk = 0x80000080; new_clock >= (clock << 1); clk >>= 1)
>                 clock <<= 1;
> --
> 2.11.0
>

Re: [PATCH v2] mmc: sdhi: fill in actual_clock

[Wolfram Sang]

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

On Thu, Aug 29, 2019 at 08:36:34PM +0200, Tamás Szűcs wrote:
> Save set clock in mmc_host actual_clock enabling exporting it via debugfs.
> This will indicate the precise SD clock in I/O settings rather than only the
> sometimes misleading requested clock.
> 
> Signed-off-by: Tamás Szűcs <tszucs@protonmail.ch>

Oops, I thought I replied already. For the record:

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


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