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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v3] mmc: renesas_sdhi: Fix rounding errors
> 
> Hi Biju,
> 
> On Sun, Sep 25, 2022 at 7:06 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 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.

OK, Will add example along with Tested-by tag from Wolfram.

> I'll give your patch a try on my collective tomorrow.

Thank you.

Cheers,
Biju




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux