Re: [PATCH v5 5/6] i2c: npcm: use i2c frequency table

From: Andy Shevchenko
Date: Tue Oct 01 2024 - 09:23:33 EST


On Tue, Oct 01, 2024 at 02:28:54PM +0800, Tyrone Ting wrote:
> From: Tyrone Ting <kfting@xxxxxxxxxxx>
>
> Modify i2c frequency from table parameters
> for NPCM i2c modules.

This two lines have a too small wrapping limit.

> Supported frequencies are:
>
> 1. 100KHz
> 2. 400KHz
> 3. 1MHz
>
> 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.
>
> Meanwhile, 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.
>
> 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.
>
> 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 and get an even better
> optimization, but our users chose not to.
>
> 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.

Personally I consider table approach is not so flexible and it is definitely
does not scale (in the result — hard to maintain for all customers), but if
it's hard to calculate all necessary data and there are other pros of it,
I'm fine.

TL;DR: I don't like this patch, but I don't want to stop you, hence no tags
from me.

--
With Best Regards,
Andy Shevchenko