Messages in this thread | | | Date | Wed, 26 Oct 2011 07:55:22 -0500 | Subject | Re: [PATCH] rtc-isl1208: Use SMBus functions if I2C isn't available | From | Ben Gardner <> |
| |
Hi Jonathan,
Thanks for the review.
On Wed, Oct 26, 2011 at 3:40 AM, Jonathan Cameron <jic23@cam.ac.uk> wrote: > On 10/26/11 03:30, Ben Gardner wrote: >> The rtc-isl1208 driver currently depends on raw I2C functionality. >> This patch adds a fall-back to SMBus functions so that the driver can be >> used on a SMBus-only platforms, such as i2c-isch. >> > Perhaps a summary of how bad things would be if smbus were all that is used? > Afterall it is emulated on i2c buses if they don't support it directly.
Read and writes to the RTC chip should be a very rare thing, so saving a few cycles by using the native I2C block read/write seems less important than being compatible. Perhaps I should just switch it to using SMBus functions and eliminate the I2C block ops altogether?
Anyone else have an opinion on this?
>> - ret = i2c_transfer(client->adapter, msgs, 2); >> - if (ret > 0) >> - ret = 0; > It's a bit early in the morning, but at least at first glance I think > this is an i2c_smbus_i2c_read_block_data reimplementation?
Do you mean i2c_smbus_read_i2c_block_data()? That function is a bit different - it uses i2c_transfer() to emulate the SMBus block read. I'm not aware of any SMBus emulation of a I2C block read.
>> + if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { >> + u8 reg_addr[1] = { reg }; >> + struct i2c_msg msgs[2] = { >> + {client->addr, 0, sizeof(reg_addr), reg_addr} >> + , > Odd spacing.
Sure is. It was that way in the original, but I guess I'll fix it. Or eliminate it, if I remove I2C ops in the next version.
I'll await further comments and then repost in a day or so.
Thanks, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |