Hi everybody.
We do have a driver for a widely used I2C-controller from Designware in
src/drivers/i2c/designware.
It is used across multiple platforms and has a quite good configurability from
mainboard side. One can tweak everything needed and make it work with the
needed timing easily.
Though I stumbled lately across a thing I wouldn't expect the way it is.
It is possible to define the speed one needs to operate the I2C bus on in
devicetree by setting the .speed-value to one of the supported defines
(I2C_SPEED_STANDARD, I2C_SPEED_FAST, I2C_SPEED_FAST_PLUS, I2C_SPEED_HIGH or
I2C_SPEED_FAST_ULTRA), e.g.:
register "common_soc_config" = "{
.i2c[0] = {
.speed = I2C_SPEED_STANDARD
}
}"
If one just sets the .speed value, the driver will take care about the needed
SCL clock rate and set it up to a default value. Yes, these values might be not
the best for the current bus load but should at least provide a clock that
matches the requested speed.
This default values are defined in src/drivers/i2c/designware/dw_i2c.c as
follows:
/* High and low times in different speed modes (in ns) */
enum {
/* SDA Hold Time */
DEFAULT_SDA_HOLD_TIME = 300,
/* Standard Speed */
MIN_SS_SCL_HIGHTIME = 4000,
MIN_SS_SCL_LOWTIME = 4700,
/* Fast Speed */
MIN_FS_SCL_HIGHTIME = 600,
MIN_FS_SCL_LOWTIME = 1300,
/* Fast Plus Speed */
MIN_FP_SCL_HIGHTIME = 260,
MIN_FP_SCL_LOWTIME = 500,
/* High Speed */
MIN_HS_SCL_HIGHTIME = 60,
MIN_HS_SCL_LOWTIME = 160,
};
While these values do match the needed _minimum_ durations for the high and low
period of the SCL signal according to the I2C-specification [1], they are not
enough for the overall clock cycle to match the requested speed. Let's take
standard speed for example:
MIN_SS_SCL_HIGHTIME = 4,0 us, MIN_SS_SCL_LOWTIME = 4,7 µs
==> MIN_SS_SCL_HIGHTIME + MIN_SS_SCL_LOWTIME = 8,7 µs which equals to
114,943 kHz instead of 100 kHz
The same applies to the other speeds as well.
As the driver is aware of the clock the IP is clocked with inside the
Chipset/SoC (CONFIG_DRIVERS_I2C_DESIGNWARE_CLOCK_MHZ), it can be done right.
There is code already which computes the needed counter values for high and low
period from the provided IP-clock, *LOWTIME and *HIGHTIME in
dw_i2c_gen_speed_config():
...
if (speed >= I2C_SPEED_HIGH) {
/* High speed */
hcnt_min = MIN_HS_SCL_HIGHTIME;
lcnt_min = MIN_HS_SCL_LOWTIME;
} else if (speed >= I2C_SPEED_FAST_PLUS) {
/* Fast-Plus speed */
hcnt_min = MIN_FP_SCL_HIGHTIME;
lcnt_min = MIN_FP_SCL_LOWTIME;
} else if (speed >= I2C_SPEED_FAST) {
/* Fast speed */
hcnt_min = MIN_FS_SCL_HIGHTIME;
lcnt_min = MIN_FS_SCL_LOWTIME;
} else {
/* Standard speed */
hcnt_min = MIN_SS_SCL_HIGHTIME;
lcnt_min = MIN_SS_SCL_LOWTIME;
}
config->speed = speed;
config->scl_hcnt = ic_clk * hcnt_min / KHz;
config->scl_lcnt = ic_clk * lcnt_min / KHz;
config->sda_hold = ic_clk * DEFAULT_SDA_HOLD_TIME / KHz;
...
I tend to change the high and low values (MIN_{SS,FS,FPHS}_SCL_{HIGH,LOW}TIME)
to match the needed minimum constraints while still fulfilling the overall
clock cycle, e.g. by increasing the HIGHTIME.
But I would like to hear other opinions on that. Surely I am not aware of all
the use cases out there.
And again: These default values can easily be overridden in the devicetree with
something like this:
register "common_soc_config" = "{
.i2c[0] = {
.speed = I2C_SPEED_STANDARD,
.speed_config[0] = {
.speed = I2C_SPEED_STANDARD,
.scl_lcnt = 664,
.scl_hcnt = 657, /*This value is smaller due to the IP
adding a count of 8 internally, see [2]*/
.sda_hold = 39
}
},
}"
But I would expect the driver to do it right in the default configuration.
Any input is highly welcome.
Werner
[1] https://www.nxp.com/docs/en/user-guide/UM10204.pdf
[2]
https://dokumen.tips/documents/designware-dw-apb-i2c-databook-pudn-designware-dwapbi2c-databook-preface-preface.html
_______________________________________________
coreboot mailing list -- [email protected]
To unsubscribe send an email to [email protected]
