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: <20171009093425.ftxgckycj2nuumle@valkosipuli.retiisi.org.uk>
Date:   Mon, 9 Oct 2017 12:34:26 +0300
From:   Sakari Ailus <sakari.ailus@....fi>
To:     Todor Tomov <todor.tomov@...aro.org>
Cc:     Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        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

Hi Todor,

On Mon, Oct 09, 2017 at 11:36:01AM +0300, Todor Tomov wrote:
> Hi Sakari,
> 
> On  4.10.2017 13:47, Laurent Pinchart wrote:
> > 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?
> 
> Thank you for this suggestion.
> 
> The way I have done it is to put the new I2C address in the DT and the driver
> programs the change using the original I2C address. The original I2C address
> is hardcoded in the driver. So maybe we can extend the DT binding and the I2C
> framework so that both addresses come from the DT and avoid hiding the
> original I2C address in the driver. This sounds good to me.

Agreed.

In this case the address is known but in general that's not the case it's
not that simple. There are register compatible devices that have different
addresses even if they're the same devices.

It might be a good idea to make this explicit.

> 
> Then changing the address could be device specific and also this must be done
> right after power on so that there are no address conflicts. So I don't think
> that we can support this through the I2C framework only, the drivers that we
> want to do that will have to be expanded with this functionality. Or do you
> have any other idea?

Yes, how the address is changed is always hardware specific. This would be
most conveniently done in driver's probe or PM runtime_resume functions.

It could be as simple as providing an adapter specific mutex to serialise
address changes on the bus so that no two address changes are taking place
at the same time. Which is essentially the impliementation you had, only
the mutex would be for the I²C adapter, not the driver. An helper functions
for acquiring and releasing the mutex.

I wonder what others think.

-- 
Regards,

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