* Re: [RFC PATCH] media: i2c: add SCCB helpers
2018-04-26 16:13 [RFC PATCH] media: i2c: add SCCB helpers Akinobu Mita
@ 2018-04-29 19:55 ` Sakari Ailus
2018-05-05 14:51 ` Mauro Carvalho Chehab
2018-05-09 10:57 ` Sebastian Reichel
2 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2018-04-29 19:55 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, linux-i2c, Wolfram Sang, Jacopo Mondi,
Laurent Pinchart, Hans Verkuil, Mauro Carvalho Chehab
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC PATCH] media: i2c: add SCCB helpers
2018-04-26 16:13 [RFC PATCH] media: i2c: add SCCB helpers Akinobu Mita
2018-04-29 19:55 ` Sakari Ailus
@ 2018-05-05 14:51 ` Mauro Carvalho Chehab
2018-05-09 14:29 ` Akinobu Mita
2018-05-09 10:57 ` Sebastian Reichel
2 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2018-05-05 14:51 UTC (permalink / raw)
To: Akinobu Mita, Sakari Ailus
Cc: linux-media, linux-i2c, Wolfram Sang, Jacopo Mondi,
Laurent Pinchart, Hans Verkuil, 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC PATCH] media: i2c: add SCCB helpers
2018-05-05 14:51 ` Mauro Carvalho Chehab
@ 2018-05-09 14:29 ` Akinobu Mita
0 siblings, 0 replies; 6+ messages in thread
From: Akinobu Mita @ 2018-05-09 14:29 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, linux-i2c, Wolfram Sang, Jacopo Mondi,
Laurent Pinchart, Hans Verkuil, Mauro Carvalho Chehab
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] media: i2c: add SCCB helpers
2018-04-26 16:13 [RFC PATCH] media: i2c: add SCCB helpers Akinobu Mita
2018-04-29 19:55 ` Sakari Ailus
2018-05-05 14:51 ` Mauro Carvalho Chehab
@ 2018-05-09 10:57 ` Sebastian Reichel
2018-05-09 14:22 ` Sakari Ailus
2 siblings, 1 reply; 6+ messages in thread
From: Sebastian Reichel @ 2018-05-09 10:57 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-media, linux-i2c, Wolfram Sang, Jacopo Mondi,
Laurent Pinchart, Hans Verkuil, Sakari Ailus,
Mauro Carvalho Chehab
[-- 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 --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC PATCH] media: i2c: add SCCB helpers
2018-05-09 10:57 ` Sebastian Reichel
@ 2018-05-09 14:22 ` Sakari Ailus
0 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2018-05-09 14:22 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Akinobu Mita, linux-media, linux-i2c, Wolfram Sang, Jacopo Mondi,
Laurent Pinchart, Hans Verkuil, Sakari Ailus,
Mauro Carvalho Chehab
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
^ permalink raw reply [flat|nested] 6+ messages in thread