linux-kernel - Re: [PATCH -next v4 1/3] regmap: add SCCB support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 21 Jul 2018 02:41:37 +0900
From:   Akinobu Mita <akinobu.mita@...il.com>
To:     Wolfram Sang <wsa@...-dreams.de>
Cc:     linux-media@...r.kernel.org, linux-i2c@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        Mark Brown <broonie@...nel.org>, Peter Rosin <peda@...ntia.se>,
        Sebastian Reichel <sebastian.reichel@...labora.co.uk>,
        Sylwester Nawrocki <s.nawrocki@...sung.com>,
        Jacopo Mondi <jacopo+renesas@...ndi.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Mauro Carvalho Chehab <mchehab@...pensource.com>
Subject: Re: [PATCH -next v4 1/3] regmap: add SCCB support

2018年7月19日(木) 0:28 Wolfram Sang <wsa@...-dreams.de>:
>
> On Tue, Jul 17, 2018 at 12:47:48AM +0900, Akinobu Mita wrote:
> > This adds Serial Camera Control Bus (SCCB) support for regmap API that
> > is intended to be used by some of Omnivision sensor drivers.
> >
> > The ov772x and ov9650 drivers are going to use this SCCB regmap API.
> >
> > The ov772x driver was previously only worked with the i2c controller
> > drivers that support I2C_FUNC_PROTOCOL_MANGLING, because the ov772x
> > device doesn't support repeated starts.  After commit 0b964d183cbf
> > ("media: ov772x: allow i2c controllers without
> > I2C_FUNC_PROTOCOL_MANGLING"), reading ov772x register is replaced with
> > issuing two separated i2c messages in order to avoid repeated start.
> > Using this SCCB regmap hides the implementation detail.
> >
> > The ov9650 driver also issues two separated i2c messages to read the
> > registers as the device doesn't support repeated start.  So it can
> > make use of this SCCB regmap.
>
> Cool, this series really gets better and better each time. Thank you for
> keeping at it! And thanks to everyone for their suggestions. I really
> like how we do not introduce a couple of i2c_sccb_* functions with a
> need to export them. And given the nature of regmap, I'd think it should
> be fairly easy to add support for ov2659 somewhen which has 16 bit
> registers? Only minor nits:

I have an ov2659 sensor.  The ov2659 driver works with i2c adapter that
doesn't know I2C_FUNC_PROTOCOL_MANGLING, so it actually supports repeated
start.

But ov5645, ov5647, ov7251 drivers may want to use SCCB regmap API for
16-bit register.  Because they use i2c_master_send + i2c_master_recv
for reading register value.

> > +#include <linux/regmap.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
>
> Sort these?
>
> > +/**
> > + * regmap_sccb_read - Read data from SCCB slave device
> > + * @context: Device that will be interacted wit
>
> "with"
>
> > +     ret = __i2c_smbus_xfer(i2c->adapter, i2c->addr, i2c->flags,
> > +                            I2C_SMBUS_WRITE, reg, I2C_SMBUS_BYTE, NULL);
>
> Mark: __i2c_smbus_xfer is a dependency on i2c/for-next. Do you want an
> immutable branch for that?
>
> > +/**
> > + * regmap_sccb_write - Write data to SCCB slave device
> > + * @context: Device that will be interacted wit
>
> "with"
>
> But in general (especially for the I2C parts):
>
> Reviewed-by: Wolfram Sang <wsa+renesas@...g-engineering.com>

Thank you for your reviewing.