Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Luca Ceresoli <luca@lucaceresoli.net>
To: Vladimir Zapolskiy <vz@mleia.com>,
	jacopo mondi <jacopo@jmondi.org>,
	Wolfram Sang <wsa@the-dreams.de>, Peter Rosin <peda@axentia.se>
Cc: linux-media@vger.kernel.org, linux-i2c@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Kieran Bingham <kieran.bingham@ideasonboard.com>
Subject: Re: [RFC,v2 2/6] i2c: add I2C Address Translator (ATR) support
Date: Tue, 10 Sep 2019 18:40:26 +0100
Message-ID: <2f883643-6af3-4e34-cf2b-f65fed9392a1@lucaceresoli.net> (raw)
In-Reply-To: <1fb71437-eaa2-99a7-885f-63ee769969aa@mleia.com>

Hi Vladimir,

On 09/09/19 05:56, Vladimir Zapolskiy wrote:
> Hi Luca, Jacopo, Wolfram, Peter,
> 
> On 09/08/2019 11:45 PM, Vladimir Zapolskiy wrote:
>> Hi Luca, Jacopo, Wolfram, Peter,
>>
>> On 09/01/2019 05:31 PM, jacopo mondi wrote:
>>> Hi Luca,
>>>    thanks for keep pushing this series! I hope we can use part of this
>>> for the (long time) on-going GMSL work...
>>>
>>> I hope you will be patient enough to provide (another :) overview
>>> of this work during the BoF Wolfram has organized at LPC for the next
>>> week.
>>>
>>> In the meantime I would have some comments after having a read at the
>>> series and trying to apply its concept to GMSL
>>>
>>
>> I won't attend the LPC, however I would appreciate if you book some
>> time to review my original / alternative implementation of the TI
>> DS90Ux9xx I2C bridge device driver.
>>
>> For your convenience the links to the driver are given below:
>> * dt bindings: https://lore.kernel.org/lkml/20181012060314.GU4939@dell/T/#mead5ea226550b
>> * driver code: https://lore.kernel.org/lkml/20181012060314.GU4939@dell/T/#m2fe3664c5f884
>> * usage example: https://lore.kernel.org/lkml/20181012060314.GU4939@dell/T/#m56c146f5decdc
>>
>> The reasons why my driver is better/more flexible/more functional are
>> discussed earlier, please let me know, if you expect anything else
>> from me to add, also I would be happy to get a summary of your offline
>> discussion.
> 
> I forgot to repeat my main objection against Luca's approach, the TI
> DS90Ux9xx I2C bridge driver does not require to call i2c_add_adapter()
> or register a new mux/bus and then do run select/deselect in runtime to
> overcome the created handicap.

Whether the ser/deser drivers should or not instantiate an adapter
depends on whether the child I2C bus is a separate bus or it's "the same
bus" as the parent bus. Which in turn is not obvious, and depends on how
you look at it.

On one hand, the child bus looks like it is the same as the parent bus
because transactions on the child bus also happen on the parent bus (bus
not the other way around).

On the other hand the child bus also looks like a separate bus because
it is electrically separated from the parent bus, it can have a
different speed, and devices on one child bus cannot talk with devices
on another child bus under the same ser/deser.

The way you modeled it has the advantage of not requiring a runtime
rewriting of slave addresses. Thas's what I implemented in
i2c_atr_map_msgs(), I don't love the fact that it happens at every
iteration, but its runtime cost is negligible compared to I2C speeds. On
the other hand I'm not sure how your approach would work if you have an
i2c bridge behind another i2c bridge (see the "ATR behind another ATR"
case mentioned by Peter).

By contrast, adding an adapter for each child bus has its advantages. I
didn't have to write code to instantiate the devices, letting the i2c
core do it. Also, as child busses show up as real busses it's possible
to instantiate devices from userspace just like any standard i2c bus with:

  echo eeprom 0x50 > /sys/bus/i2c/devices/i2c-4/new_device

and devices will be assigned an alias and be usable normally.

Finally, there is no need to call select()/deselect() in runtime. Those
callbacks are optional, and I'm probably removing them in a future
iteration as I'm more and more convinced they are not needed at all.

-- 
Luca

  reply index

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-23 20:37 [RFC,v2 0/6] TI camera serdes and I2C address translation Luca Ceresoli
2019-07-23 20:37 ` [RFC,v2 1/6] i2c: core: let adapters be notified of client attach/detach Luca Ceresoli
2019-07-23 20:37 ` [RFC,v2 2/6] i2c: add I2C Address Translator (ATR) support Luca Ceresoli
2019-09-01 14:31   ` jacopo mondi
2019-09-03  7:31     ` Luca Ceresoli
2019-09-03  7:37       ` Wolfram Sang
2019-09-04  8:09       ` Peter Rosin
2019-09-08 19:40         ` Luca Ceresoli
2019-09-10 18:46           ` Wolfram Sang
2019-09-08 20:45     ` Vladimir Zapolskiy
2019-09-09  4:56       ` Vladimir Zapolskiy
2019-09-10 17:40         ` Luca Ceresoli [this message]
2019-09-09  7:22       ` Wolfram Sang
2019-09-09 15:10         ` Vladimir Zapolskiy
2019-09-09 17:48           ` Luca Ceresoli
2019-09-10 17:16             ` Wolfram Sang
2019-09-02 20:42   ` Wolfram Sang
2019-09-03  8:48     ` Luca Ceresoli
2019-09-03  9:06       ` Wolfram Sang
2019-07-23 20:37 ` [RFC,v2 3/6] media: dt-bindings: add DS90UB954-Q1 video deserializer Luca Ceresoli
2019-08-13 15:44   ` Rob Herring
2019-08-19 22:41     ` Luca Ceresoli
2019-08-20 15:44       ` Rob Herring
2019-08-21 21:50         ` Luca Ceresoli
2019-09-02 20:48   ` Wolfram Sang
2019-09-03  9:09     ` Luca Ceresoli
2019-09-03  9:34       ` Wolfram Sang
2019-09-03 11:03         ` Luca Ceresoli
2019-09-03 14:16           ` Wolfram Sang
2019-09-10  9:43   ` Sakari Ailus
2019-09-10 15:02     ` Luca Ceresoli
2019-07-23 20:37 ` [RFC,v2 4/6] media: dt-bindings: add DS90UB953-Q1 video serializer Luca Ceresoli
2019-07-23 20:37 ` [RFC,v2 5/6] media: ds90ub954: new driver for TI DS90UB954-Q1 video deserializer Luca Ceresoli
2019-07-23 20:37 ` [RFC,v2 6/6] media: ds90ub953: new driver for TI DS90UB953-Q1 video serializer Luca Ceresoli

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2f883643-6af3-4e34-cf2b-f65fed9392a1@lucaceresoli.net \
    --to=luca@lucaceresoli.net \
    --cc=devicetree@vger.kernel.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jacopo@jmondi.org \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mchehab@kernel.org \
    --cc=peda@axentia.se \
    --cc=robh+dt@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=vz@mleia.com \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git