Re: [PATCH v3] dt-bindings: media: adv748x: Document re-mappable addresses

From: Rob Herring
Date: Tue Aug 14 2018 - 09:26:55 EST

On Mon, Aug 13, 2018 at 1:17 PM Kieran Bingham
<kieran.bingham@xxxxxxxxxxxxxxxx> wrote:
> Hi Rob,
> On 13/08/18 18:45, Rob Herring wrote:
> > On Thu, Aug 09, 2018 at 08:29:44PM +0100, Kieran Bingham wrote:
> >> The ADV748x supports configurable slave addresses for its I2C pages.
> >> Document the page names, and provide an example for setting each of the
> >> pages explicitly.
> >
> > It would be good to say why you need this.
> In fact - I should probably have added a fixes tag here, which would
> have added more context:
> Fixes: 67537fe960e5 ("media: i2c: adv748x: Add support for
> i2c_new_secondary_device")

That doesn't really explain things from a DT perspective.

> Should I repost with this fixes tag?
> Or can it be collected with the RB tag?

I'll leave that to Hans.

> > The only use I can think of
> > is if there are other devices on the bus and you need to make sure the
> > addresses don't conflict.
> Yes, precisely. This driver has 'slave pages' which are created and
> mapped by the driver. The device has default addresses which are used by
> the driver - but it's very easy for these to conflict with other devices
> on the same I2C bus.
> Because the mappings are simply a software construct, we have a means to
> specify the desired mappings through DT at the board level - which
> allows the boards to ensure that conflicts do not appear.
> > Arguably, that information could be figured out without this in DT.
> How so ?
> Scanning the bus is error prone, and dependant upon driver state (and
> presence), and we have no means currently of requesting 'free/unused'
> addresses from the I2C core framework.

True. But assuming all devices are in DT, then you just need to scan
the child nodes of the bus and get a map of the used addresses. Though
if you had 2 or more devices like this, then you'd need to maintain
s/w allocated addresses too. It could all be maintained with a bitmap
which you initialize with addresses in DT.