Re: [Patch V2 6/9] I2C: Add smbus word/block process call helper function

[Lan Tianyu]

On 05/17/2014 06:21 PM, Wolfram Sang wrote:
> On Mon, Apr 28, 2014 at 10:27:45PM +0800, Lan Tianyu wrote:
> > Add i2c_smbus_word/block_proc_call() helper function. These will be used
> > in the implementation of i2c ACPI address space handler.
> >
> > Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> > Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> > ---
> >   drivers/i2c/i2c-core.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >   include/linux/i2c.h    |  4 ++++
> >   2 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 3bf0048..638befd 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -2306,6 +2306,30 @@ s32 i2c_smbus_write_word_data(const struct i2c_client *client, u8 command,
> >   EXPORT_SYMBOL(i2c_smbus_write_word_data);
> >
> >   /**
> > + * i2c_smbus_word_proc_call - SMBus "word proc call" protocol
> > + * @client: Handle to slave device
> > + * @command: Byte interpreted by slave
> > + * @value: 16-bit "word" being written
> > + *
> > + * This executes the SMBus "word proc all" protocol, returning negative errno
> > + * else a 16-bit unsigned "word" received from the device.
> > + */
> > +s32 i2c_smbus_word_proc_call(const struct i2c_client *client, u8 command,
> > +			      u16 value)
> > +{
> > +	union i2c_smbus_data data;
> > +	int status;
> > +
> > +	data.word = value;
> > +	status = i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> > +			      I2C_SMBUS_READ, command,
> > +			      I2C_SMBUS_PROC_CALL, &data);
> > +
> > +	return (status < 0) ? status : data.word;
> > +}
> > +EXPORT_SYMBOL(i2c_smbus_word_proc_call);
> > +
> > +/**
>
> Same as smbus_quick function before, documentation updates are missing.
> Also similar, we had such a function before which was removed due to no
> users? It looks very much like the function here, so we should stick to
> the old naming IMO. Is it the old one? Such information would be nice
> since it affects reviewing.

Yes, there was an old function and commit c8110933 removed it. Sorry, I didn't 
that before. I will bring the old one back.

> >    * i2c_smbus_read_block_data - SMBus "block read" protocol
> >    * @client: Handle to slave device
> >    * @command: Byte interpreted by slave
> > @@ -2362,6 +2386,38 @@ s32 i2c_smbus_write_block_data(const struct i2c_client *client, u8 command,
> >   }
> >   EXPORT_SYMBOL(i2c_smbus_write_block_data);
> >
> > +/**
> > + * i2c_smbus_block_proc_call - SMBus "block write" protocol
> > + * @client: Handle to slave device
> > + * @command: Byte interpreted by slave
> > + * @length: Size of data block; SMBus allows at most 32 bytes
> > + * @values: Byte array which will be written.
> > + *
> > + * This executes the SMBus "block proc call" protocol, returning negative errno
> > + * else the number of read bytes.
> > + */
> > +s32 i2c_smbus_block_proc_call(const struct i2c_client *client, u8 command,
> > +			       u8 length, u8 *values)
>
> Please rename to "i2c_smbus_block_process_call" as the old function was
> named. Needs documentation updates.
>
> > +	if (length > I2C_SMBUS_BLOCK_MAX)
> > +		length = I2C_SMBUS_BLOCK_MAX;
>
> I think we should return an error here.

Ok.

> BTW what were your test scenarios for these functions and smbus quick?

From ACPI spec, Read/Write quick and Word/Block process call protocol will be 
used by Bios ACPI code. But so far, I don't find such machines. So these 
functions has not been tested.




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/