[RFC PATCH] media: i2c: add SCCB helpers

[RFC PATCH] media: i2c: add SCCB helpers

[Akinobu Mita]

(This patch is in prototype stage)

This adds 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.

Cc: Wolfram Sang <wsa@the-dreams.de>
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>
---
 drivers/media/i2c/Kconfig  |  4 ++++
 drivers/media/i2c/Makefile |  1 +
 drivers/media/i2c/sccb.c   | 35 +++++++++++++++++++++++++++++++++++
 drivers/media/i2c/sccb.h   | 14 ++++++++++++++
 4 files changed, 54 insertions(+)
 create mode 100644 drivers/media/i2c/sccb.c
 create mode 100644 drivers/media/i2c/sccb.h

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 541f0d28..19e5601 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -569,6 +569,9 @@ config VIDEO_THS8200
 
 comment "Camera sensor devices"
 
+config SCCB
+	bool
+
 config VIDEO_APTINA_PLL
 	tristate
 
@@ -692,6 +695,7 @@ config VIDEO_OV772X
 	tristate "OmniVision OV772x sensor support"
 	depends on I2C && VIDEO_V4L2
 	depends on MEDIA_CAMERA_SUPPORT
+	select SCCB
 	---help---
 	  This is a Video4Linux2 sensor-level driver for the OmniVision
 	  OV772x camera.
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index ea34aee..82fbd78 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
 obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
+obj-$(CONFIG_SCCB) += sccb.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
 obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
diff --git a/drivers/media/i2c/sccb.c b/drivers/media/i2c/sccb.c
new file mode 100644
index 0000000..80a3fb7
--- /dev/null
+++ b/drivers/media/i2c/sccb.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/i2c.h>
+
+int sccb_read_byte(struct i2c_client *client, u8 addr)
+{
+	int ret;
+	u8 val;
+
+	/* Issue two separated requests in order to avoid repeated start */
+
+	ret = i2c_master_send(client, &addr, 1);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_master_recv(client, &val, 1);
+	if (ret < 0)
+		return ret;
+
+	return val;
+}
+EXPORT_SYMBOL_GPL(sccb_read_byte);
+
+int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data)
+{
+	int ret;
+	unsigned char msgbuf[] = { addr, data };
+
+	ret = i2c_master_send(client, msgbuf, 2);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sccb_write_byte);
diff --git a/drivers/media/i2c/sccb.h b/drivers/media/i2c/sccb.h
new file mode 100644
index 0000000..68da0e9
--- /dev/null
+++ b/drivers/media/i2c/sccb.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * SCCB helper functions
+ */
+
+#ifndef __SCCB_H__
+#define __SCCB_H__
+
+#include <linux/i2c.h>
+
+int sccb_read_byte(struct i2c_client *client, u8 addr);
+int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data);
+
+#endif /* __SCCB_H__ */
-- 
2.7.4

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

[Sakari Ailus]

Hi Mita-san,

On Fri, Apr 27, 2018 at 01:13:32AM +0900, Akinobu Mita wrote:
> (This patch is in prototype stage)
> 
> This adds 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.
> 
> Cc: Wolfram Sang <wsa@the-dreams.de>
> 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>
> ---
>  drivers/media/i2c/Kconfig  |  4 ++++
>  drivers/media/i2c/Makefile |  1 +
>  drivers/media/i2c/sccb.c   | 35 +++++++++++++++++++++++++++++++++++
>  drivers/media/i2c/sccb.h   | 14 ++++++++++++++
>  4 files changed, 54 insertions(+)
>  create mode 100644 drivers/media/i2c/sccb.c
>  create mode 100644 drivers/media/i2c/sccb.h
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 541f0d28..19e5601 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -569,6 +569,9 @@ config VIDEO_THS8200
>  
>  comment "Camera sensor devices"
>  
> +config SCCB
> +	bool

Tristate? This effectively depends on I²C but I²C is tristate...

> +
>  config VIDEO_APTINA_PLL
>  	tristate
>  
> @@ -692,6 +695,7 @@ config VIDEO_OV772X
>  	tristate "OmniVision OV772x sensor support"
>  	depends on I2C && VIDEO_V4L2
>  	depends on MEDIA_CAMERA_SUPPORT
> +	select SCCB
>  	---help---
>  	  This is a Video4Linux2 sensor-level driver for the OmniVision
>  	  OV772x camera.
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index ea34aee..82fbd78 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
>  obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
> +obj-$(CONFIG_SCCB) += sccb.o
>  obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
>  obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
>  obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
> diff --git a/drivers/media/i2c/sccb.c b/drivers/media/i2c/sccb.c
> new file mode 100644
> index 0000000..80a3fb7
> --- /dev/null
> +++ b/drivers/media/i2c/sccb.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/i2c.h>
> +
> +int sccb_read_byte(struct i2c_client *client, u8 addr)
> +{
> +	int ret;
> +	u8 val;
> +
> +	/* Issue two separated requests in order to avoid repeated start */
> +
> +	ret = i2c_master_send(client, &addr, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_master_recv(client, &val, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(sccb_read_byte);
> +
> +int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data)
> +{
> +	int ret;
> +	unsigned char msgbuf[] = { addr, data };
> +
> +	ret = i2c_master_send(client, msgbuf, 2);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(sccb_write_byte);

Please add MODULE_* macros... follows from tristate option.

> diff --git a/drivers/media/i2c/sccb.h b/drivers/media/i2c/sccb.h
> new file mode 100644
> index 0000000..68da0e9
> --- /dev/null
> +++ b/drivers/media/i2c/sccb.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * SCCB helper functions
> + */
> +
> +#ifndef __SCCB_H__
> +#define __SCCB_H__
> +
> +#include <linux/i2c.h>

struct i2c_client;

is enough; no need to include the whole header here.

> +
> +int sccb_read_byte(struct i2c_client *client, u8 addr);
> +int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data);
> +
> +#endif /* __SCCB_H__ */

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

[Mauro Carvalho Chehab]

Em Fri, 27 Apr 2018 01:13:32 +0900
Akinobu Mita <akinobu.mita@gmail.com> escreveu:

> (This patch is in prototype stage)
> 
> This adds SCCB helper functions (sccb_read_byte and sccb_write_byte) that
> are intended to be used by some of Omnivision sensor drivers.

What do you mean by "SCCB"? 

> 
> 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.
> 
> Cc: Wolfram Sang <wsa@the-dreams.de>
> 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>
> ---
>  drivers/media/i2c/Kconfig  |  4 ++++
>  drivers/media/i2c/Makefile |  1 +
>  drivers/media/i2c/sccb.c   | 35 +++++++++++++++++++++++++++++++++++
>  drivers/media/i2c/sccb.h   | 14 ++++++++++++++
>  4 files changed, 54 insertions(+)
>  create mode 100644 drivers/media/i2c/sccb.c
>  create mode 100644 drivers/media/i2c/sccb.h
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 541f0d28..19e5601 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -569,6 +569,9 @@ config VIDEO_THS8200
>  
>  comment "Camera sensor devices"
>  
> +config SCCB
> +	bool
> +
>  config VIDEO_APTINA_PLL
>  	tristate
>  
> @@ -692,6 +695,7 @@ config VIDEO_OV772X
>  	tristate "OmniVision OV772x sensor support"
>  	depends on I2C && VIDEO_V4L2
>  	depends on MEDIA_CAMERA_SUPPORT
> +	select SCCB
>  	---help---
>  	  This is a Video4Linux2 sensor-level driver for the OmniVision
>  	  OV772x camera.
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index ea34aee..82fbd78 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
>  obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
> +obj-$(CONFIG_SCCB) += sccb.o
>  obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
>  obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
>  obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
> diff --git a/drivers/media/i2c/sccb.c b/drivers/media/i2c/sccb.c
> new file mode 100644
> index 0000000..80a3fb7
> --- /dev/null
> +++ b/drivers/media/i2c/sccb.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/i2c.h>
> +
> +int sccb_read_byte(struct i2c_client *client, u8 addr)
> +{
> +	int ret;
> +	u8 val;
> +
> +	/* Issue two separated requests in order to avoid repeated start */
> +
> +	ret = i2c_master_send(client, &addr, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_master_recv(client, &val, 1);
> +	if (ret < 0)
> +		return ret;

Handling it this way is a very bad idea, as you may have an operation
between those two, as you're locking/unlocking for each i2c_master
call, e. g. the code should be, instead:

	i2c_lock_adapter();
	__i2c_transfer(); /* Send */
	__i2c_transfer(); /* Receive */
	i2c_unlock_adapter();

Also, if the problem is just due to I2C not supporting REPEAT, IMHO,
the best would be to add some IRC flag to indicate that.

Btw, this is not the first device that doesn't support repeats.
A good hint of drivers with similar issues is:

$ git grep i2c_lock_adapter drivers/media/
drivers/media/dvb-frontends/af9013.c:           i2c_lock_adapter(client->adapter);
drivers/media/dvb-frontends/af9013.c:           i2c_lock_adapter(client->adapter);
drivers/media/dvb-frontends/drxk_hard.c:        i2c_lock_adapter(state->i2c);
drivers/media/dvb-frontends/rtl2830.c:  i2c_lock_adapter(client->adapter);
drivers/media/dvb-frontends/rtl2830.c:  i2c_lock_adapter(client->adapter);
drivers/media/dvb-frontends/rtl2830.c:  i2c_lock_adapter(client->adapter);
drivers/media/dvb-frontends/tda1004x.c: i2c_lock_adapter(state->i2c);
drivers/media/tuners/tda18271-common.c:         i2c_lock_adapter(priv->i2c_props.adap);
drivers/media/tuners/tda18271-common.c: i2c_lock_adapter(priv->i2c_props.adap);

Regards,
Mauro

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

[Sebastian Reichel]

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

Hi,

On Fri, Apr 27, 2018 at 01:13:32AM +0900, Akinobu Mita wrote:
> diff --git a/drivers/media/i2c/sccb.c b/drivers/media/i2c/sccb.c
> new file mode 100644
> index 0000000..80a3fb7
> --- /dev/null
> +++ b/drivers/media/i2c/sccb.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/i2c.h>
> +
> +int sccb_read_byte(struct i2c_client *client, u8 addr)
> +{
> +	int ret;
> +	u8 val;
> +
> +	/* Issue two separated requests in order to avoid repeated start */
> +
> +	ret = i2c_master_send(client, &addr, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_master_recv(client, &val, 1);
> +	if (ret < 0)
> +		return ret;
> +
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(sccb_read_byte);
> +
> +int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data)
> +{
> +	int ret;
> +	unsigned char msgbuf[] = { addr, data };
> +
> +	ret = i2c_master_send(client, msgbuf, 2);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(sccb_write_byte);
> diff --git a/drivers/media/i2c/sccb.h b/drivers/media/i2c/sccb.h
> new file mode 100644
> index 0000000..68da0e9
> --- /dev/null
> +++ b/drivers/media/i2c/sccb.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * SCCB helper functions
> + */
> +
> +#ifndef __SCCB_H__
> +#define __SCCB_H__
> +
> +#include <linux/i2c.h>
> +
> +int sccb_read_byte(struct i2c_client *client, u8 addr);
> +int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data);
> +
> +#endif /* __SCCB_H__ */

The functions look very simple. Have you considered moving them into
sccb.h as static inline?

-- Sebastian

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

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

[Sakari Ailus]

On Wed, May 09, 2018 at 12:57:19PM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Apr 27, 2018 at 01:13:32AM +0900, Akinobu Mita wrote:
> > diff --git a/drivers/media/i2c/sccb.c b/drivers/media/i2c/sccb.c
> > new file mode 100644
> > index 0000000..80a3fb7
> > --- /dev/null
> > +++ b/drivers/media/i2c/sccb.c
> > @@ -0,0 +1,35 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/i2c.h>
> > +
> > +int sccb_read_byte(struct i2c_client *client, u8 addr)
> > +{
> > +	int ret;
> > +	u8 val;
> > +
> > +	/* Issue two separated requests in order to avoid repeated start */
> > +
> > +	ret = i2c_master_send(client, &addr, 1);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = i2c_master_recv(client, &val, 1);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return val;
> > +}
> > +EXPORT_SYMBOL_GPL(sccb_read_byte);
> > +
> > +int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data)
> > +{
> > +	int ret;
> > +	unsigned char msgbuf[] = { addr, data };
> > +
> > +	ret = i2c_master_send(client, msgbuf, 2);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(sccb_write_byte);
> > diff --git a/drivers/media/i2c/sccb.h b/drivers/media/i2c/sccb.h
> > new file mode 100644
> > index 0000000..68da0e9
> > --- /dev/null
> > +++ b/drivers/media/i2c/sccb.h
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * SCCB helper functions
> > + */
> > +
> > +#ifndef __SCCB_H__
> > +#define __SCCB_H__
> > +
> > +#include <linux/i2c.h>
> > +
> > +int sccb_read_byte(struct i2c_client *client, u8 addr);
> > +int sccb_write_byte(struct i2c_client *client, u8 addr, u8 data);
> > +
> > +#endif /* __SCCB_H__ */
> 
> The functions look very simple. Have you considered moving them into
> sccb.h as static inline?

I agree. (Considering my previous comment on the dependencies, this is a
better idea. No need for a module this small.)

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

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

[Akinobu Mita]

2018-05-05 23:51 GMT+09:00 Mauro Carvalho Chehab <mchehab+samsung@kernel.org>:
> Em Fri, 27 Apr 2018 01:13:32 +0900
> Akinobu Mita <akinobu.mita@gmail.com> escreveu:
>
>> (This patch is in prototype stage)
>>
>> This adds SCCB helper functions (sccb_read_byte and sccb_write_byte) that
>> are intended to be used by some of Omnivision sensor drivers.
>
> What do you mean by "SCCB"?

Serial Camera Control Bus (SCCB).  I'll write SCCB and the non
abbreviation together in the comment block and commit log.

>>
>> 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.
>>
>> Cc: Wolfram Sang <wsa@the-dreams.de>
>> 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>
>> ---
>>  drivers/media/i2c/Kconfig  |  4 ++++
>>  drivers/media/i2c/Makefile |  1 +
>>  drivers/media/i2c/sccb.c   | 35 +++++++++++++++++++++++++++++++++++
>>  drivers/media/i2c/sccb.h   | 14 ++++++++++++++
>>  4 files changed, 54 insertions(+)
>>  create mode 100644 drivers/media/i2c/sccb.c
>>  create mode 100644 drivers/media/i2c/sccb.h
>>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 541f0d28..19e5601 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -569,6 +569,9 @@ config VIDEO_THS8200
>>
>>  comment "Camera sensor devices"
>>
>> +config SCCB
>> +     bool
>> +
>>  config VIDEO_APTINA_PLL
>>       tristate
>>
>> @@ -692,6 +695,7 @@ config VIDEO_OV772X
>>       tristate "OmniVision OV772x sensor support"
>>       depends on I2C && VIDEO_V4L2
>>       depends on MEDIA_CAMERA_SUPPORT
>> +     select SCCB
>>       ---help---
>>         This is a Video4Linux2 sensor-level driver for the OmniVision
>>         OV772x camera.
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index ea34aee..82fbd78 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
>>  obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>>  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>>  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
>> +obj-$(CONFIG_SCCB) += sccb.o
>>  obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
>>  obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
>>  obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
>> diff --git a/drivers/media/i2c/sccb.c b/drivers/media/i2c/sccb.c
>> new file mode 100644
>> index 0000000..80a3fb7
>> --- /dev/null
>> +++ b/drivers/media/i2c/sccb.c
>> @@ -0,0 +1,35 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/i2c.h>
>> +
>> +int sccb_read_byte(struct i2c_client *client, u8 addr)
>> +{
>> +     int ret;
>> +     u8 val;
>> +
>> +     /* Issue two separated requests in order to avoid repeated start */
>> +
>> +     ret = i2c_master_send(client, &addr, 1);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     ret = i2c_master_recv(client, &val, 1);
>> +     if (ret < 0)
>> +             return ret;
>
> Handling it this way is a very bad idea, as you may have an operation
> between those two, as you're locking/unlocking for each i2c_master
> call, e. g. the code should be, instead:
>
>         i2c_lock_adapter();
>         __i2c_transfer(); /* Send */
>         __i2c_transfer(); /* Receive */
>         i2c_unlock_adapter();
>
> Also, if the problem is just due to I2C not supporting REPEAT, IMHO,
> the best would be to add some IRC flag to indicate that.

I sent a patch using this approach.

> Btw, this is not the first device that doesn't support repeats.
> A good hint of drivers with similar issues is:
>
> $ git grep i2c_lock_adapter drivers/media/
> drivers/media/dvb-frontends/af9013.c:           i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/af9013.c:           i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/drxk_hard.c:        i2c_lock_adapter(state->i2c);
> drivers/media/dvb-frontends/rtl2830.c:  i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/rtl2830.c:  i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/rtl2830.c:  i2c_lock_adapter(client->adapter);
> drivers/media/dvb-frontends/tda1004x.c: i2c_lock_adapter(state->i2c);
> drivers/media/tuners/tda18271-common.c:         i2c_lock_adapter(priv->i2c_props.adap);
> drivers/media/tuners/tda18271-common.c: i2c_lock_adapter(priv->i2c_props.adap);
>
> Regards,
> Mauro