Re: [RFC 7/7] [media] rc: add support for IR LEDs driven through SPI

[Andi Shyti <andi.shyti@samsung.com>]

Hi Sean,

> > +	int ret;
> > +	struct ir_spi_data *idata = (struct ir_spi_data *) dev->priv;
> 
> No cast needed.

yes, thanks.

> > +	ret = regulator_enable(idata->regulator);
> > +	if (ret)
> > +		return ret;
> > +
> > +	mutex_lock(&idata->mutex);
> > +	idata->xfer.len = n;
> > +	idata->xfer.tx_buf = buffer;
> > +	mutex_unlock(&idata->mutex);
> 
> I'm not convinced the locking works here. You want to guard against 
> someone modifying xfer while you are sending (so in spi_sync_transfer), 
> which this locking is not doing. You could declare a 
> local "struct spi_transfer xfer" and avoid the mutex altogether.

I cannot declare xfer locally because the spi framework needs
a statically allocated xfer, so that either I dynamically
allocate it in the function or I declare it global in idata.

With the mutex I would like to prevent different tasks to change
the value at the same time, it's an easy case, it shouldn't make
much difference.

There are checkpatch issues, in the next patchset I will fix
them.

Thanks a lot for your review,
Andi