* [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
@ 2022-09-28 11:07 Biju Das
2022-09-30 9:09 ` Geert Uytterhoeven
2022-10-03 17:29 ` Wolfram Sang
0 siblings, 2 replies; 8+ messages in thread
From: Biju Das @ 2022-09-28 11:07 UTC (permalink / raw)
To: Ulf Hansson
Cc: Biju Das, Wolfram Sang, linux-mmc, linux-renesas-soc,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad
Due to clk rounding errors on RZ/G2L platforms, it selects a clock source
with a lower clock rate compared to a higher one.
For eg: The rounding error (533333333 Hz / 4 * 4 = 533333332 Hz < 5333333
33 Hz) selects a clk source of 400 MHz instead of 533.333333 MHz.
This patch fixes this issue by adding a margin of (1/1024) higher to
the clock rate.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v4->v5:
* Moved upper limit calculation inside the for loop as it caused
regression on R-Car M2-W board.
* Removed Rb tag from Wolfram as there is some new changes.
v3->v4:
* Added Tested-by tag from Wolfram.
* Updated commit description and code comment with real example.
v2->v3:
* Renamed the variable new_clock_margin->new_upper_limit in renesas_sdhi_clk_
update()
* Moved setting of new_upper_limit outside for loop.
* Updated the comment section to mention the rounding errors and merged with
existing comment out side the for loop.
* Updated commit description.
v1->v2:
* Add a comment explaining why margin is needed and set it to
that particular value.
---
drivers/mmc/host/renesas_sdhi_core.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 6edbf5c161ab..b970699743e0 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -128,6 +128,7 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
struct clk *ref_clk = priv->clk;
unsigned int freq, diff, best_freq = 0, diff_min = ~0;
unsigned int new_clock, clkh_shift = 0;
+ unsigned int new_upper_limit;
int i;
/*
@@ -153,13 +154,20 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
* greater than, new_clock. As we can divide by 1 << i for
* any i in [0, 9] we want the input clock to be as close as
* possible, but no greater than, new_clock << i.
+ *
+ * Add an upper limit of 1/1024 rate higher to the clock rate to fix
+ * clk rate jumping to lower rate due to rounding error (eg: RZ/G2L has
+ * 3 clk sources 533.333333 MHz, 400 MHz and 266.666666 MHz. The request
+ * for 533.333333 MHz will selects a slower 400 MHz due to rounding
+ * error (533333333 Hz / 4 * 4 = 533333332 Hz < 533333333 Hz)).
*/
for (i = min(9, ilog2(UINT_MAX / new_clock)); i >= 0; i--) {
freq = clk_round_rate(ref_clk, new_clock << i);
- if (freq > (new_clock << i)) {
+ new_upper_limit = (new_clock << i) + ((new_clock << i) >> 10);
+ if (freq > new_upper_limit) {
/* Too fast; look for a slightly slower option */
freq = clk_round_rate(ref_clk, (new_clock << i) / 4 * 3);
- if (freq > (new_clock << i))
+ if (freq > new_upper_limit)
continue;
}
@@ -181,6 +189,7 @@ static unsigned int renesas_sdhi_clk_update(struct tmio_mmc_host *host,
static void renesas_sdhi_set_clock(struct tmio_mmc_host *host,
unsigned int new_clock)
{
+ unsigned int clk_margin;
u32 clk = 0, clock;
sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN &
@@ -194,7 +203,13 @@ static void renesas_sdhi_set_clock(struct tmio_mmc_host *host,
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)
+ /*
+ * Add a margin of 1/1024 rate higher to the clock rate in order
+ * to avoid clk variable setting a value of 0 due to the margin
+ * provided for actual_clock in renesas_sdhi_clk_update().
+ */
+ clk_margin = new_clock >> 10;
+ for (clk = 0x80000080; new_clock + clk_margin >= (clock << 1); clk >>= 1)
clock <<= 1;
/* 1/1 clock is option */
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
2022-09-28 11:07 [PATCH v5] mmc: renesas_sdhi: Fix rounding errors Biju Das
@ 2022-09-30 9:09 ` Geert Uytterhoeven
2022-10-01 6:42 ` Wolfram Sang
2022-10-03 17:29 ` Wolfram Sang
1 sibling, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2022-09-30 9:09 UTC (permalink / raw)
To: Biju Das
Cc: Ulf Hansson, Wolfram Sang, linux-mmc, linux-renesas-soc,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad
On Wed, Sep 28, 2022 at 1:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Due to clk rounding errors on RZ/G2L platforms, it selects a clock source
> with a lower clock rate compared to a higher one.
> For eg: The rounding error (533333333 Hz / 4 * 4 = 533333332 Hz < 5333333
> 33 Hz) selects a clk source of 400 MHz instead of 533.333333 MHz.
>
> This patch fixes this issue by adding a margin of (1/1024) higher to
> the clock rate.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v4->v5:
> * Moved upper limit calculation inside the for loop as it caused
> regression on R-Car M2-W board.
> * Removed Rb tag from Wolfram as there is some new changes.
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Still works fine on R-Car Gen2/Gen3, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
2022-09-30 9:09 ` Geert Uytterhoeven
@ 2022-10-01 6:42 ` Wolfram Sang
0 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2022-10-01 6:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Biju Das, Ulf Hansson, linux-mmc, linux-renesas-soc,
Geert Uytterhoeven, Chris Paterson, Biju Das,
Prabhakar Mahadev Lad
[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]
On Fri, Sep 30, 2022 at 11:09:07AM +0200, Geert Uytterhoeven wrote:
> On Wed, Sep 28, 2022 at 1:08 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Due to clk rounding errors on RZ/G2L platforms, it selects a clock source
> > with a lower clock rate compared to a higher one.
> > For eg: The rounding error (533333333 Hz / 4 * 4 = 533333332 Hz < 5333333
> > 33 Hz) selects a clk source of 400 MHz instead of 533.333333 MHz.
> >
> > This patch fixes this issue by adding a margin of (1/1024) higher to
> > the clock rate.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v4->v5:
> > * Moved upper limit calculation inside the for loop as it caused
> > regression on R-Car M2-W board.
> > * Removed Rb tag from Wolfram as there is some new changes.
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Still works fine on R-Car Gen2/Gen3, so:
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
I'll test this patch on Monday.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
2022-09-28 11:07 [PATCH v5] mmc: renesas_sdhi: Fix rounding errors Biju Das
2022-09-30 9:09 ` Geert Uytterhoeven
@ 2022-10-03 17:29 ` Wolfram Sang
2022-10-06 11:49 ` Ulf Hansson
1 sibling, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2022-10-03 17:29 UTC (permalink / raw)
To: Biju Das
Cc: Ulf Hansson, linux-mmc, linux-renesas-soc, Geert Uytterhoeven,
Chris Paterson, Biju Das, Prabhakar Mahadev Lad
[-- Attachment #1: Type: text/plain, Size: 702 bytes --]
On Wed, Sep 28, 2022 at 12:07:55PM +0100, Biju Das wrote:
> Due to clk rounding errors on RZ/G2L platforms, it selects a clock source
> with a lower clock rate compared to a higher one.
> For eg: The rounding error (533333333 Hz / 4 * 4 = 533333332 Hz < 5333333
> 33 Hz) selects a clk source of 400 MHz instead of 533.333333 MHz.
>
> This patch fixes this issue by adding a margin of (1/1024) higher to
> the clock rate.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Can only test on Gen3 currently, but clock settings are the same there.
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
2022-10-03 17:29 ` Wolfram Sang
@ 2022-10-06 11:49 ` Ulf Hansson
2022-10-06 15:32 ` Wolfram Sang
0 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2022-10-06 11:49 UTC (permalink / raw)
To: Wolfram Sang, Biju Das, Biju Das
Cc: Geert Uytterhoeven, Chris Paterson, Prabhakar Mahadev Lad,
linux-mmc, linux-renesas-soc
On Mon, 3 Oct 2022 at 19:29, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Wed, Sep 28, 2022 at 12:07:55PM +0100, Biju Das wrote:
> > Due to clk rounding errors on RZ/G2L platforms, it selects a clock source
> > with a lower clock rate compared to a higher one.
> > For eg: The rounding error (533333333 Hz / 4 * 4 = 533333332 Hz < 5333333
> > 33 Hz) selects a clk source of 400 MHz instead of 533.333333 MHz.
> >
> > This patch fixes this issue by adding a margin of (1/1024) higher to
> > the clock rate.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Can only test on Gen3 currently, but clock settings are the same there.
>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Thanks!
>
Can someone tell me, if there is a corresponding fixes tag we could use here?
Or is this just a general bugfix that we should tag for stable?
Kind regards
Uffe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
2022-10-06 11:49 ` Ulf Hansson
@ 2022-10-06 15:32 ` Wolfram Sang
2022-10-06 16:07 ` Biju Das
0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2022-10-06 15:32 UTC (permalink / raw)
To: Ulf Hansson
Cc: Biju Das, Biju Das, Geert Uytterhoeven, Chris Paterson,
Prabhakar Mahadev Lad, linux-mmc, linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 186 bytes --]
> Or is this just a general bugfix that we should tag for stable?
I'd think this because I assume there is no commit causing the rounding
errors. But maybe Biju has something to add?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
2022-10-06 15:32 ` Wolfram Sang
@ 2022-10-06 16:07 ` Biju Das
2022-10-07 8:57 ` Ulf Hansson
0 siblings, 1 reply; 8+ messages in thread
From: Biju Das @ 2022-10-06 16:07 UTC (permalink / raw)
To: Wolfram Sang, Ulf Hansson
Cc: Biju Das, Geert Uytterhoeven, Chris Paterson,
Prabhakar Mahadev Lad, linux-mmc@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
Hi Wolfram Sang & Ulf Hansson,
> Subject: Re: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
>
>
> > Or is this just a general bugfix that we should tag for stable?
>
> I'd think this because I assume there is no commit causing the
> rounding errors. But maybe Biju has something to add?
There is rounding error present since commit [1], but no HW at that time to
introduce the error. Then we added RZ/G2L support and we started seeing this
issue after [2].
So may be treat this an enhancement patch or fixes to [1] or [2]
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/host/renesas_sdhi_core.c?h=v6.0&id=bb6d3fa98a418b071c5f735e75558604f5f4af66
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas/r9a07g044-cpg.c?h=v6.0&id=b289cdecc7c3e25e001cde260c882e4d9a8b0772
Cheers,
Biju
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
2022-10-06 16:07 ` Biju Das
@ 2022-10-07 8:57 ` Ulf Hansson
0 siblings, 0 replies; 8+ messages in thread
From: Ulf Hansson @ 2022-10-07 8:57 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, Biju Das, Geert Uytterhoeven, Chris Paterson,
Prabhakar Mahadev Lad, linux-mmc@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
On Thu, 6 Oct 2022 at 18:07, Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Wolfram Sang & Ulf Hansson,
>
> > Subject: Re: [PATCH v5] mmc: renesas_sdhi: Fix rounding errors
> >
> >
> > > Or is this just a general bugfix that we should tag for stable?
> >
> > I'd think this because I assume there is no commit causing the
> > rounding errors. But maybe Biju has something to add?
>
> There is rounding error present since commit [1], but no HW at that time to
> introduce the error. Then we added RZ/G2L support and we started seeing this
> issue after [2].
>
> So may be treat this an enhancement patch or fixes to [1] or [2]
Alright, I have added a fixes tag [1] and a stable tag - and applied
it for fixes, thanks!
Kind regards
Uffe
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/host/renesas_sdhi_core.c?h=v6.0&id=bb6d3fa98a418b071c5f735e75558604f5f4af66
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/clk/renesas/r9a07g044-cpg.c?h=v6.0&id=b289cdecc7c3e25e001cde260c882e4d9a8b0772
>
> Cheers,
> Biju
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-07 8:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-28 11:07 [PATCH v5] mmc: renesas_sdhi: Fix rounding errors Biju Das
2022-09-30 9:09 ` Geert Uytterhoeven
2022-10-01 6:42 ` Wolfram Sang
2022-10-03 17:29 ` Wolfram Sang
2022-10-06 11:49 ` Ulf Hansson
2022-10-06 15:32 ` Wolfram Sang
2022-10-06 16:07 ` Biju Das
2022-10-07 8:57 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox