Hi Andy,
On Mon, 19 Jul 2010 21:11:46 -0400, Andy Walls wrote:
> There was a small window between writing the cx25840 register
> address over the i2c bus and reading the register contents back from the
> cx25840 device that the i2c adapter lock was released. This change ensures the
> adapter lock is not released until the register read is done.
>
> Signed-off-by: Andy Walls <awalls@xxxxxxxxxxxxxxxx>
Good catch.
Acked-by: Jean Delvare <khali@xxxxxxxxxxxx>
Note that cx25840_and_or() still has a (smaller and less dangerous)
race window. If several calls to cx25840_and_or() happen in parallel on
the same register, some of the changes may be lost. I don't know if
this can be a problem in practice though. If it is, then additional
locking around cx25840_and_or() would be needed.
> ---
> drivers/media/video/cx25840/cx25840-core.c | 58 +++++++++++++++++++---------
> 1 files changed, 39 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/video/cx25840/cx25840-core.c b/drivers/media/video/cx25840/cx25840-core.c
> index bb4872b..4f908fa 100644
> --- a/drivers/media/video/cx25840/cx25840-core.c
> +++ b/drivers/media/video/cx25840/cx25840-core.c
> @@ -80,33 +80,53 @@ int cx25840_write4(struct i2c_client *client, u16 addr, u32 value)
>
> u8 cx25840_read(struct i2c_client * client, u16 addr)
> {
> - u8 buffer[2];
> - buffer[0] = addr >> 8;
> - buffer[1] = addr & 0xff;
> -
> - if (i2c_master_send(client, buffer, 2) < 2)
> - return 0;
> -
> - if (i2c_master_recv(client, buffer, 1) < 1)
> + struct i2c_msg msgs[2];
> + u8 tx_buf[2], rx_buf[1];
> +
> + /* Write register address */
> + tx_buf[0] = addr >> 8;
> + tx_buf[1] = addr & 0xff;
> + msgs[0].addr = client->addr;
> + msgs[0].flags = 0;
> + msgs[0].len = 2;
> + msgs[0].buf = (char *) tx_buf;
> +
> + /* Read data from register */
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> + msgs[1].len = 1;
> + msgs[1].buf = (char *) rx_buf;
> +
> + if (i2c_transfer(client->adapter, msgs, 2) < 2)
> return 0;
>
> - return buffer[0];
> + return rx_buf[0];
> }
>
> u32 cx25840_read4(struct i2c_client * client, u16 addr)
> {
> - u8 buffer[4];
> - buffer[0] = addr >> 8;
> - buffer[1] = addr & 0xff;
> -
> - if (i2c_master_send(client, buffer, 2) < 2)
> - return 0;
> -
> - if (i2c_master_recv(client, buffer, 4) < 4)
> + struct i2c_msg msgs[2];
> + u8 tx_buf[2], rx_buf[4];
> +
> + /* Write register address */
> + tx_buf[0] = addr >> 8;
> + tx_buf[1] = addr & 0xff;
> + msgs[0].addr = client->addr;
> + msgs[0].flags = 0;
> + msgs[0].len = 2;
> + msgs[0].buf = (char *) tx_buf;
> +
> + /* Read data from registers */
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> + msgs[1].len = 4;
> + msgs[1].buf = (char *) rx_buf;
> +
> + if (i2c_transfer(client->adapter, msgs, 2) < 2)
> return 0;
>
> - return (buffer[3] << 24) | (buffer[2] << 16) |
> - (buffer[1] << 8) | buffer[0];
> + return (rx_buf[3] << 24) | (rx_buf[2] << 16) | (rx_buf[1] << 8) |
> + rx_buf[0];
> }
>
> int cx25840_and_or(struct i2c_client *client, u16 addr, unsigned and_mask,
--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
