Re: [PATCH v2 01/10] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING

[jacopo mondi <jacopo@jmondi.org>]

[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]

Hi Sakari,

On Wed, Apr 18, 2018 at 01:41:54PM +0300, Sakari Ailus wrote:
> On Wed, Apr 18, 2018 at 12:05:49PM +0200, jacopo mondi wrote:
> > Hi Akinobu,
> >
> > On Mon, Apr 16, 2018 at 11:51:42AM +0900, Akinobu Mita wrote:
> > > The ov772x driver only works when the i2c controller have
> > > I2C_FUNC_PROTOCOL_MANGLING.  However, many i2c controller drivers don't
> > > support it.
> > >
> > > The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that
> > > it doesn't support repeated starts.
> > >
> > > This changes the reading ov772x register method so that it doesn't
> > > require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages.
> > >
> > > Cc: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Cc: Hans Verkuil <hans.verkuil@cisco.com>
> > > Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > > ---
> > > * v2
> > > - Replace the implementation of ov772x_read() instead of adding an
> > >   alternative method
> >
> > I now wonder if my initial reply to this patch was wrong, and where
> > possible we should try to use smbus operations...
> >
> > From Documentation/i2c/smbus-protocol
> > "If you write a driver for some I2C device, please try to use the SMBus
> > commands if at all possible... " and that's because, according to
> > documentation, most I2c adapters support smbus protocol but may not
> > support the full i2c command set.
> >
> > The fact this driver then restricts the supported adapters to the ones
> > that support protocol mangling makes me think your change is fine,
> > but as it often happens, I would scale this to more knowledgable
> > people...
>
> Do you actually need to use this on SMBus adapters? A lot of sensor drivers
> just use I²C; if SMBus support is really needed it can be always added back
> later on...

That was actually my question, sorry for not being clear.

As the documentation says that SMBus has to be preferred when
possible, I was wondering if ditching it completely in favour of plain
I2c was wrong or not... I assume from your answer it is fine.

>
> --
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]