linux-kernel - Re: [PATCH] [media] ov5645: I2C address change

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]

Message-ID: <3073637.dhNDna4gKQ@avalon>
Date:   Wed, 04 Oct 2017 13:47:22 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Sakari Ailus <sakari.ailus@....fi>
Cc:     Todor Tomov <todor.tomov@...aro.org>, mchehab@...nel.org,
        hansverk@...co.com, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
        Wolfram Sang <wsa@...-dreams.de>
Subject: Re: [PATCH] [media] ov5645: I2C address change

CC'ing the I2C mainling list and the I2C maintainer.

On Wednesday, 4 October 2017 13:30:08 EEST Sakari Ailus wrote:
> Hi Todor,
> 
> On Mon, Oct 02, 2017 at 04:28:45PM +0300, Todor Tomov wrote:
> > As soon as the sensor is powered on, change the I2C address to the one
> > specified in DT. This allows to use multiple physical sensors connected
> > to the same I2C bus.
> > 
> > Signed-off-by: Todor Tomov <todor.tomov@...aro.org>
> 
> The smiapp driver does something similar and I understand Laurent might be
> interested in such functionality as well.
> 
> It'd be nice to handle this through the I²C framework instead and to define
> how the information is specified through DT. That way it could be made
> generic, to work with more devices than just this one.
> 
> What do you think?
> 
> Cc Laurent.
> 
> > ---
> > 
> >  drivers/media/i2c/ov5645.c | 42 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> > index d28845f..8541109 100644
> > --- a/drivers/media/i2c/ov5645.c
> > +++ b/drivers/media/i2c/ov5645.c
> > @@ -33,6 +33,7 @@
> >  #include <linux/i2c.h>
> >  #include <linux/init.h>
> >  #include <linux/module.h>
> > +#include <linux/mutex.h>
> >  #include <linux/of.h>
> >  #include <linux/of_graph.h>
> >  #include <linux/regulator/consumer.h>
> > @@ -42,6 +43,8 @@
> >  #include <media/v4l2-fwnode.h>
> >  #include <media/v4l2-subdev.h>
> > 
> > +static DEFINE_MUTEX(ov5645_lock);
> > +
> >  #define OV5645_VOLTAGE_ANALOG               2800000
> >  #define OV5645_VOLTAGE_DIGITAL_CORE         1500000
> >  #define OV5645_VOLTAGE_DIGITAL_IO           1800000
> > @@ -590,6 +593,31 @@ static void ov5645_regulators_disable(struct ov5645
> > *ov5645)
> >  		dev_err(ov5645->dev, "io regulator disable failed\n");
> >  }
> > 
> > +static int ov5645_write_reg_to(struct ov5645 *ov5645, u16 reg, u8 val,
> > +			       u16 i2c_addr)
> > +{
> > +	u8 regbuf[3] = {
> > +		reg >> 8,
> > +		reg & 0xff,
> > +		val
> > +	};
> > +	struct i2c_msg msgs = {
> > +		.addr = i2c_addr,
> > +		.flags = 0,
> > +		.len = 3,
> > +		.buf = regbuf
> > +	};
> > +	int ret;
> > +
> > +	ret = i2c_transfer(ov5645->i2c_client->adapter, &msgs, 1);
> > +	if (ret < 0)
> > +		dev_err(ov5645->dev,
> > +			"%s: write reg error %d on addr 0x%x: reg=0x%x, val=0x%x\n",
> > +			__func__, ret, i2c_addr, reg, val);
> > +
> > +	return ret;
> > +}
> > +
> >  static int ov5645_write_reg(struct ov5645 *ov5645, u16 reg, u8 val)
> >  {
> >  	u8 regbuf[3];
> > @@ -729,10 +757,24 @@ static int ov5645_s_power(struct v4l2_subdev *sd,
> > int on)
> >  	 */
> >  	if (ov5645->power_count == !on) {
> >  		if (on) {
> > +			mutex_lock(&ov5645_lock);
> > +
> >  			ret = ov5645_set_power_on(ov5645);
> >  			if (ret < 0)
> >  				goto exit;
> > 
> > +			ret = ov5645_write_reg_to(ov5645, 0x3100,
> > +						ov5645->i2c_client->addr, 0x78);
> > +			if (ret < 0) {
> > +				dev_err(ov5645->dev,
> > +					"could not change i2c address\n");
> > +				ov5645_set_power_off(ov5645);
> > +				mutex_unlock(&ov5645_lock);
> > +				goto exit;
> > +			}
> > +
> > +			mutex_unlock(&ov5645_lock);
> > +
> >  			ret = ov5645_set_register_array(ov5645,
> >  					ov5645_global_init_setting,
> >  					ARRAY_SIZE(ov5645_global_init_setting));

-- 
Regards,

Laurent Pinchart