On Tuesday, February 10, 2015 07:29:33 AM Werner Zeh wrote:
Hi coreboot community.
Hi back at you.
- Store the offset we need (in this case 20) into a buffer
- Call i2c_write_raw(bus, chip_adr, &buffer, 1);
- Call i2c_read_raw(bus, chip_adr, &buffer,10);
That's actually the way I2C is designed to work, and this sort of API models it more accurately. Linux uses it. libeopencm uses it. spidev uses it.
Here are my personal problems with this kind of interface:
Then provide utility functions to do i2c_reg_(read|write) which wrap i2c_transfer() directly. Construct the complete I2c transaction as an array of i2c_seg and pass that to i2c_transfer().
- Where we earlier had one simple call to i2c_read(), we now have to
fill
one variable with the length we need and call two different functions (i2c_write_raw and i2c_read_raw). 2. Though the read transfer is one logical access to the slave, we have split it into two (without any benefit I can see now) 3. We have added two wrapper layers where we earlier had none!
Not quite so. The i2c read/write of the past actually squashed together several steps, which ended up being more code as you had to fully model every sort of transaction you could think of doing. There's no extra wrapping. The code was just reorganized.
Alex
Am 10.02.2015 um 08:56 schrieb Alexandru Gagniuc:
That's actually the way I2C is designed to work, and this sort of API models it more accurately. Linux uses it. libeopencm uses it. spidev uses it.
But hardware doesn't. The API is fine when you bitbang everything, but not when there's a controller that takes over some of the work, like programming the address.
On such a controller, you have to pick apart struct i2c_seg* to figure out which elements are addresses and which are data, and split things into different registers.
I really don't see how it's useful to keep the data structures as low level as possible and then require drivers for more capable devices to uplift them to a higher abstraction level.
Patrick
-
Alexandru Gagniuc -
Patrick Georgi