Re: [PATCH] i2c: mv64xxx: Implement I2C_M_RECV_LEN and I2C_FUNC_SMBUS_READ_BLOCK_DATA

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]


> +    /* If we should retrieve the length from the buffer, make sure */
> +	/* to read enough bytes to avoid sending the */
> +	/* STOP bit after the read if the first byte */

Please stick to Kernel commenting style.

> +		/* length=0 should not be allowed, but is indeed possible.
> +		 * To avoid locking the chip, we keep reading at least 2 bytes
> +		 */

Ditto.

> +		drv_data->effective_length = data+1;
> +		drv_data->bytes_left = data+1;
> +		drv_data->msg->len = data+1;

Also, checkpatch complains about this if you run it with "--strict'

    CHECKPATCH
CHECK: spaces preferred around that '+' (ctx:VxV)
#81: FILE: drivers/i2c/busses/i2c-mv64xxx.c:384:
+		drv_data->effective_length = data+1;
 		                                 ^

CHECK: spaces preferred around that '+' (ctx:VxV)
#82: FILE: drivers/i2c/busses/i2c-mv64xxx.c:385:
+		drv_data->bytes_left = data+1;
 		                           ^

CHECK: spaces preferred around that '+' (ctx:VxV)
#83: FILE: drivers/i2c/busses/i2c-mv64xxx.c:386:
+		drv_data->msg->len = data+1;
 		                         ^
I think the complaints make sense.

> -	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR | I2C_FUNC_SMBUS_EMUL;
> +	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR |
> +		I2C_FUNC_SMBUS_READ_BLOCK_DATA | I2C_FUNC_SMBUS_EMUL;

Likely I2C_FUNC_SMBUS_BLOCK_PROC_CALL will work as well, but I assume
you can't test it?

Otherwise, looks good.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]