On Sun, Sep 08, 2024 at 11:54:50AM +0300, Tali Perry wrote: > On Mon, Sep 2, 2024 at 3:00 PM Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Sun, Sep 01, 2024 at 06:53:38PM +0300, Tali Perry wrote: > > > On Fri, Aug 30, 2024 at 10:19 PM Andy Shevchenko > > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: ... > > > The original equations were tested on a variety of chips and base clocks. > > > Since we added devices that use higher frequencies of the module we > > > saw that there is a mismatch between the equation and the actual > > > results on the bus itself, measured on scope. > > > So instead of using the equations we did an optimization per module > > > frequency, verified on a device. > > > Most of the work was focused on the rise time of the SCL and SDA, > > > which depends on external load of the bus and PU. > > > We needed to make sure that in all valid range of load the rise time > > > is compliant of the SMB spec timing requirements. > > > > > > This patch include the final values after extensive testing both at > > > Nuvoton as well as at customer sites. > > > > But: > > 1) why is it better than do calculations? > > 2) can it be problematic on theoretically different PCB design in the future? > > > > P.S. If there is a good explanations to these and more, elaborate this in the > > commit message. > > Thanks for your comments, > > 1) The equations were not accurate to begin with. > They are an approximation of the ideal value. > The ideal value is calculated per frequency of the core module. This is crucial detail. > 2) As you wrote , different PCB designs, or specifically to this case > : the number and type of targets on the bus, > impact the required values for the timing registers. > Users can recalculate the numbers for each bus ( out of 24) and get > an even better optimization, > but our users chose not to. > Instead - we manually picked values per frequency that match the > entire valid range of targets (from 1 to max number). > Then we check against the AMR described in SMB spec and make sure > that none of the values is exceeding. > this process was led by the chip architect and included a lot of testing. > > Do we need to add this entire description to the commit message? It > sounds a bit too detailed to me. Yes, please. -- With Best Regards, Andy Shevchenko
