[PATCH v2 1/3] drm: Add SCDC helpers

[Thierry Reding]

On Mon, Dec 05, 2016 at 10:12:39AM +0000, Jose Abreu wrote:
> On 02-12-2016 19:24, Thierry Reding wrote:
[...]
> > +/**
> > + * drm_scdc_write - write a block of data to SCDC
> > + * @adapter: I2C controller
> > + * @offset: start offset of block to write
> > + * @buffer: block of data to write
> > + * @size: size of the block to write
> > + *
> > + * Writes a block of data to SCDC, starting at a given offset.
> > + *
> > + * Returns:
> > + * The number of bytes written to SCDC or a negative error code on failure.
> > + */
> > +ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
> > +		       const void *buffer, size_t size)
> > +{
> > +	struct i2c_msg msg = {
> > +		.addr = SCDC_I2C_SLAVE_ADDRESS,
> > +		.flags = 0,
> > +		.len = 1 + size,
> > +		.buf = NULL,
> > +	};
> > +	void *data;
> > +	int err;
> > +
> > +	data = kmalloc(1 + size, GFP_TEMPORARY);
> > +	if (!data)
> > +		return -ENOMEM;
> > +
> > +	msg.buf = data;
> > +
> > +	memcpy(data, &offset, sizeof(offset));
> > +	memcpy(data + 1, buffer, size);
> 
> Don't you agree it would be better if you use the same scheme as
> drm_scdc_read()? Something like:
> 
> struct i2c_msg msgs[] = {
>     {
>         .addr = SCDC_I2C_SLAVE_ADDRESS,
>         .flags = 0,
>         .len = 1,
>         .buf = &offset,
>     }, {
>         .addr = SCDC_I2C_SLAVE_ADDRESS,
>         .flags = 0,
>         .len = size,
>         .buf = buffer,
>     },
> };

Ville had a similar comment on a prior iteration. It looks as if the
above should work, but it's probably best to test it a little more
widely to make sure we're not running into cases where it breaks.

Have you by any chance verified that it works on your hardware?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161205/25f08c7f/attachment.sig>