Re: [RFC PATCH v2] media: i2c: add SCCB helpers

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hello,

On Thursday, 14 June 2018 18:33:58 EEST Wolfram Sang wrote:
> On Wed, Jun 13, 2018 at 12:34:46AM +0900, Akinobu Mita wrote:
> > (This is 2nd version of SCCB helpers patch.  After 1st version was
> > submitted, I sent alternative patch titled "i2c: add I2C_M_FORCE_STOP".
> > But it wasn't accepted because it makes the I2C core code unreadable.
> > I couldn't find out a way to untangle it, so I returned to the original
> > approach.)
> > 
> > This adds Serial Camera Control Bus (SCCB) helper functions
> > (sccb_read_byte and sccb_write_byte) that are intended to be used by some
> > of Omnivision sensor drivers.
> > 
> > The ov772x driver is going to use these functions in order to make it work
> > with most i2c controllers.
> > 
> > As the ov772x device doesn't support repeated starts, this driver
> > currently requires I2C_FUNC_PROTOCOL_MANGLING that is not supported by
> > many i2c controller drivers.
> > 
> > With the sccb_read_byte() that issues two separated requests in order to
> > avoid repeated start, the driver doesn't require
> > I2C_FUNC_PROTOCOL_MANGLING.
> 
> From a first glance, this looks like my preferred solution so far.
> Thanks for doing it! Let me sleep a bit over it for a thorough review...
> 
> > --- /dev/null
> > +++ b/drivers/media/i2c/sccb.h
> 
> I'd prefer this file to be in the i2c realm. Maybe
> 'include/linux/i2c-sccb.h" or something. I will come back to this.

And while at it, I think we also need a .c file, the functions (and especially 
sccb_read_byte()) should not be static inline.

-- 
Regards,

Laurent Pinchart