Re: [PATCH -next v3 2/2] media: ov772x: use SCCB helpers
> > I think it would be even better to introduce a SSCB regmap layer > > and use that. > > I'm fine with introducing a SCCB regmap layer. I am fine with this approach, too. > But do we need to provide both a SCCB regmap and these SCCB helpers? I don't know much about the OV sensor drivers. I'd think they would benefit from regmap, so a-kind-of enforced conversion to regmap doesn't sound too bad to me. > If we have a regmap_init_sccb(), I feel these three SCCB helper functions > don't need to be exported anymore. Might be true. But other media guys are probably in a better position to comment on this? Note that I found SCCB devices with 16 bit regs: ov2659 & ov 2685. I think we can cover those with SMBus word accesses. regmap is probably also the cleanest way to hide this detail? signature.asc Description: PGP signature
Re: [PATCH -next v3 2/2] media: ov772x: use SCCB helpers
2018年7月10日(火) 6:23 Sebastian Reichel :
>
> Hi,
>
> On Mon, Jul 09, 2018 at 06:14:43PM +0200, Wolfram Sang wrote:
> > > static int ov772x_read(struct i2c_client *client, u8 addr)
> > > {
> > > - int ret;
> > > - u8 val;
> > > -
> > > - ret = i2c_master_send(client, , 1);
> > > - if (ret < 0)
> > > - return ret;
> > > - ret = i2c_master_recv(client, , 1);
> > > - if (ret < 0)
> > > - return ret;
> > > -
> > > - return val;
> > > + return sccb_read_byte(client, addr);
> > > }
> > >
> > > static inline int ov772x_write(struct i2c_client *client, u8 addr, u8
> > > value)
> > > {
> > > - return i2c_smbus_write_byte_data(client, addr, value);
> > > + return sccb_write_byte(client, addr, value);
> > > }
>
> Reviewed-by: Sebastian Reichel
>
> > Minor nit: I'd rather drop these two functions and use the
> > sccb-accessors directly.
> >
> > However, I really like how this looks here: It is totally clear we are
> > doing SCCB and hide away all the details.
>
> I think it would be even better to introduce a SSCB regmap layer
> and use that.
I'm fine with introducing a SCCB regmap layer.
But do we need to provide both a SCCB regmap and these SCCB helpers?
If we have a regmap_init_sccb(), I feel these three SCCB helper functions
don't need to be exported anymore.
Re: [PATCH -next v3 2/2] media: ov772x: use SCCB helpers
Hi,
On Mon, Jul 09, 2018 at 06:14:43PM +0200, Wolfram Sang wrote:
> > static int ov772x_read(struct i2c_client *client, u8 addr)
> > {
> > - int ret;
> > - u8 val;
> > -
> > - ret = i2c_master_send(client, , 1);
> > - if (ret < 0)
> > - return ret;
> > - ret = i2c_master_recv(client, , 1);
> > - if (ret < 0)
> > - return ret;
> > -
> > - return val;
> > + return sccb_read_byte(client, addr);
> > }
> >
> > static inline int ov772x_write(struct i2c_client *client, u8 addr, u8
> > value)
> > {
> > - return i2c_smbus_write_byte_data(client, addr, value);
> > + return sccb_write_byte(client, addr, value);
> > }
Reviewed-by: Sebastian Reichel
> Minor nit: I'd rather drop these two functions and use the
> sccb-accessors directly.
>
> However, I really like how this looks here: It is totally clear we are
> doing SCCB and hide away all the details.
I think it would be even better to introduce a SSCB regmap layer
and use that.
-- Sebastian
signature.asc
Description: PGP signature
Re: [PATCH -next v3 2/2] media: ov772x: use SCCB helpers
> static int ov772x_read(struct i2c_client *client, u8 addr)
> {
> - int ret;
> - u8 val;
> -
> - ret = i2c_master_send(client, , 1);
> - if (ret < 0)
> - return ret;
> - ret = i2c_master_recv(client, , 1);
> - if (ret < 0)
> - return ret;
> -
> - return val;
> + return sccb_read_byte(client, addr);
> }
>
> static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value)
> {
> - return i2c_smbus_write_byte_data(client, addr, value);
> + return sccb_write_byte(client, addr, value);
> }
Minor nit: I'd rather drop these two functions and use the
sccb-accessors directly.
However, I really like how this looks here: It is totally clear we are
doing SCCB and hide away all the details.
signature.asc
Description: PGP signature
[PATCH -next v3 2/2] media: ov772x: use SCCB helpers
Convert ov772x register access to use SCCB helpers.
Cc: Peter Rosin
Cc: Sebastian Reichel
Cc: Wolfram Sang
Cc: Jacopo Mondi
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/i2c/ov772x.c | 20 +---
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 7158c31..8a9a9ca 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -17,7 +17,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -551,22 +551,12 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev
*sd)
static int ov772x_read(struct i2c_client *client, u8 addr)
{
- int ret;
- u8 val;
-
- ret = i2c_master_send(client, , 1);
- if (ret < 0)
- return ret;
- ret = i2c_master_recv(client, , 1);
- if (ret < 0)
- return ret;
-
- return val;
+ return sccb_read_byte(client, addr);
}
static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value)
{
- return i2c_smbus_write_byte_data(client, addr, value);
+ return sccb_write_byte(client, addr, value);
}
static int ov772x_mask_set(struct i2c_client *client, u8 command, u8 mask,
@@ -1395,9 +1385,9 @@ static int ov772x_probe(struct i2c_client *client,
return -EINVAL;
}
- if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
+ if (!sccb_is_available(adapter)) {
dev_err(>dev,
- "I2C-Adapter doesn't support SMBUS_BYTE_DATA\n");
+ "I2C-Adapter doesn't support SCCB\n");
return -EIO;
}
--
2.7.4
