Re: [PATCH v3] mmc: renesas_sdhi: Fix rounding errors

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Biju,

On Sun, Sep 25, 2022 at 7:06 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v3] mmc: renesas_sdhi: Fix rounding errors

> > > 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.
> >
> > I really like the new variable names.
> >
> > > +    * To fix rounding errors, eg:- (533333333 Hz / 4 * 4 = 533333332
> > Hz
> > > +<
> >
> > (What is the '-' after 'eg:' for?)
>
> Regarding 'eg:-', I got this habit from school days. On exams, I usually
> write for eg:- to provide some examples.
>
> OK, Will remove '-' after 'eg:'.
>
> >
> > > +    * 533333333 Hz) add an upper limit of 1/1024 rate higher to the
> > clock
> > > +    * rate.
> >
> > I know Geert suggesgted this. I think, however, this description is
> > too short. It should be mentioned IMHO that this rounding error can
> > lead to the selection of a clock which is way off (the 400MHz one). I
> > liked this example from v2. Geert, what do you say?
>
> Yes, I can put back the real example from v2, if that is useful.
> 533MHz->400 MHz is a big jump due to this rounding error and it has
> impacted the performance.
>
> Waiting for Geert's feedback.

I agree with Wolfram.
I'll give your patch a try on my collective tomorrow.

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