Mark M. Hoffman wrote:
> Comments are inline...
>
Thanks for review, fixing most issues was straightforward
for all except:
>> +static inline u16 f75375_read16(struct i2c_client *client, u8 reg)
>> +{
>> + return ((i2c_smbus_read_byte_data(client, reg) << 8)
>> + | i2c_smbus_read_byte_data(client, reg + 1));
>> +}
>> +
>>
>
> This is horrible hardware design. A single _read_word_data() would be much
> better here, but the datasheet apparently doesn't allow for it. Neither does
> the datasheet give advice for how to read the word-sized values coherently.
>
> E.g. it's common in such situations for the hardware to lock the second byte
> from updating for some time after the first byte is read. That's what I was
> looking for in the datasheet to make sure you did the accesses in the right
> order. AFAICT, nothing - so it's possible this thing is inherently racy.
>
>
I could use the update_mutex to avoid reading words when they are being
updated, but that does not protect from the chip modifying the values
itself?
