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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
