Patch for i2c driver to support sht21 sensors

[oliverju]

The bcm2708 default clock stretching timeout is too short to allow waiting for conversions of sht21. Without this patch accesses to the sensor would result in an I/O-Error.

--- a/i2c-bcm2708.c +++ b/i2c-bcm2708.c @@ -151,6 +151,8 @@ static inline void bcm2708_bsc_setup(struct bcm2708_i2c *bi) u32 cdiv; u32 c = BSC_C_I2CEN | BSC_C_INTD | BSC_C_ST | BSC_C_CLEAR_1;

• u32 clkt; + bus_hz = clk_get_rate(bi->clk); cdiv = bus_hz / baudrate;

@@ -163,6 +165,11 @@ static inline void bcm2708_bsc_setup(struct bcm2708_i2c *bi bcm2708_wr(bi, BSC_A, bi->msg->addr); bcm2708_wr(bi, BSC_DLEN, bi->msg->len); bcm2708_wr(bi, BSC_C, c); +

• /* longer clock stretching timeout (needed by sht21), */
• /* default timeout is 64 i2c clock cycles (0x40) */
• clkt = baudrate / 10; /* set to 100 ms */
• bcm2708_wr(bi, BSC_CLKT, clkt); }

Greetings Oliver

[popcornmix]

Your patch has been mangled by github. Please quote it or use a pastebin link.

[oliverju]

Sorry. I don't know how to comment here. I posted the patch on pastebin now.

http://pastebin.com/HiSRygt8

[P33M]

Holdonnaminute

The I2C spec defines no maximum value for a timeout during ACK clock stretching - the device should be given as long as needed to process the data it has received.

Hardcoding any value (64 SCLKs or otherwise) seems like a silly thing to do. Ideally the linux I2C core API would nicely document setting timeouts on a per-transaction basis, but at the moment this facility exists as an undocumented and largely unimplemented struct member.

In absence of any driver-determined timeouts, there are several de-facto bus recovery techniques such as clocking SCL until SDA goes high, or leaving SCL low for at least 35ms so that the attached devices reset themselves.

My point is that the driver implements a fixed timeout - question: how large should this fixed timeout be on a bus that the spec says should have no timeout?

[Ferroin]

@P33M The I2C spec doesn’t specify a time-out, but the SMBus spec does, and the driver is built to play nice with SMBus devices as well.

[P33M]

After looking at the datasheet for the SHT21, I can see that using an alternate conversion mode will prevent hitting the timeout - the chip supports a blocking and non-blocking conversion mode, whereby in non-blocking the command is issued and then the device can be polled for a complete conversion (taking up to 85ms).

http://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/Humidity/Sensirion_Humidity_SHT21_Datasheet_V3.pdf

Userspace can take care of the poll/timeout in this case. This obviates the need to modify the driver for this specific device, but there is still the somewhat sticky issue of what to set the SCL timeout to - SMBus mandates that a timeout should occur after 35ms, i2c doesn't care. The value in use is currently unchanged from the default 64 SCL clocks.

We can expect users to be attaching both potted SMBUS devices and perhaps their own i2c-based peripherals - each requiring different things.

Perhaps a pragmatic approach is needed - set the i2c timeout to 35ms except where the i2c bus speed is extremely slow - in this case set a minimum of 64 clocks. In addition, the BSC_DIV value is never validated at probe-time and so risks setting reserved bits in the CDIV register.

gist: https://gist.github.com/P33M/137a47974eca9a6ee990

[oliverju]

Generally I think, that SMbus-devices are designed to work on I2C busses too, although the spec mandates a special timeout procedure. So the 35ms timeout-detection should not be necessary.

Greetings Oliver

[MikeDK]

Hi there!

I can see that the part of @P33M 's patch in bcm2708_i2c_probe() where cdiv and baudrate are calculated correctly was included in the kernel, but the part in bcm2708_bsc_setup() where BSC_CLKT is written was not.

My Question: Why not?

I had a problem with a HTU21D sensor chip connected to a Pi, using the htu21 kernel module. I always got an I/O Error when trying to read sensor data.

When I patch i2c-bcm2708.c to write BSC_CLKT to a higher value, then everything is ok.

I think this problem might still occur on every I2C sensor chip which needs a long time to answer.

regards, Michael