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 <[email protected]>
---
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
Hi Tamás,
On Thu, Aug 29, 2019 at 8:37 PM Tamás Szűcs <[email protected]> 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 <[email protected]>
Thanks for the update!
Reviewed-by: Geert Uytterhoeven <[email protected]>
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 -- [email protected]
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
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
[email protected]
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, August 30, 2019 9:21 AM, Geert Uytterhoeven <[email protected]> wrote:
> Hi Tamás,
>
> On Thu, Aug 29, 2019 at 8:37 PM Tamás Szűcs [email protected] 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 [email protected]
>
> Thanks for the update!
>
> Reviewed-by: Geert Uytterhoeven [email protected]
>
> 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 -- [email protected]
>
> 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
On Thu, 29 Aug 2019 at 20:36, Tamás Szűcs <[email protected]> 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 <[email protected]>
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
>
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 <[email protected]>
Oops, I thought I replied already. For the record:
Acked-by: Wolfram Sang <[email protected]>