From: Luca Ceresoli <luca@lucaceresoli.net>
To: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>,
"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Wolfram Sang <wsa@the-dreams.de>,
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>,
Jacopo Mondi <jacopo@jmondi.org>,
Vladimir Zapolskiy <vz@mleia.com>, Peter Rosin <peda@axentia.se>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
"Mutanen, Mikko" <Mikko.Mutanen@fi.rohmeurope.com>
Subject: Re: [RFCv3 2/6] i2c: add I2C Address Translator (ATR) support
Date: Wed, 16 Mar 2022 15:25:11 +0100 [thread overview]
Message-ID: <02af807d-c35e-afc6-7a41-22eafd3c46f9@lucaceresoli.net> (raw)
In-Reply-To: <a8796cde-e97b-7157-33ac-1b6020053c5d@fi.rohmeurope.com>
Hi Matti,
On 16/03/22 15:11, Vaittinen, Matti wrote:
> Hi dee Ho peeps!
>
> On 2/6/22 13:59, Luca Ceresoli wrote:
>> An ATR is a device that looks similar to an i2c-mux: it has an I2C
>> slave "upstream" port and N master "downstream" ports, and forwards
>> transactions from upstream to the appropriate downstream port. But is
>> is different in that the forwarded transaction has a different slave
>> address. The address used on the upstream bus is called the "alias"
>> and is (potentially) different from the physical slave address of the
>> downstream chip.
>>
>> Add a helper file (just like i2c-mux.c for a mux or switch) to allow
>> implementing ATR features in a device driver. The helper takes care or
>> adapter creation/destruction and translates addresses at each transaction.
>>
>
> snip
>
>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
>> index 438905e2a1d0..c6d1a345ea6d 100644
>> --- a/drivers/i2c/Kconfig
>> +++ b/drivers/i2c/Kconfig
>> @@ -71,6 +71,15 @@ config I2C_MUX
>>
>> source "drivers/i2c/muxes/Kconfig"
>>
>> +config I2C_ATR
>> + tristate "I2C Address Translator (ATR) support"
>> + help
>> + Enable support for I2C Address Translator (ATR) chips.
>> +
>> + An ATR allows accessing multiple I2C busses from a single
>> + physical bus via address translation instead of bus selection as
>> + i2c-muxes do.
>> +
>
> I continued playing with the ROHM (de-)serializer and ended up having
> .config where the I2C_ATR was ='m', while my ATR driver was ='y' even
> though it selects the I2C_ATR.
>
> Yep, most probably my error somewhere.
>
> Anyways, this made me think that most of the I2C_ATR users are likely to
> just silently select the I2C_ATR, right? The I2C_ATR has no much reason
> to be compiled in w/o users, right? So perhaps the menu entry for
> selecting the I2C_ATR could be dropped(?) Do we really need this entry
> in already long list of configs to be manually picked?
Maybe we could make it a blind option, sure. The only reason it could be
useful that it's visible is that one might implement a user driver could
be written out of tree. I don't care very much about that, but it is
possible. Maybe it's the reason for I2C_MUX to be a visible option too.
Peter?
>> +struct i2c_atr *i2c_atr_new(struct i2c_adapter *parent, struct device *dev,
>> + const struct i2c_atr_ops *ops, int max_adapters)
>> +{
>> + struct i2c_atr *atr;
>> +
>> + if (!ops || !ops->attach_client || !ops->detach_client)
>> + return ERR_PTR(-EINVAL);
>> +
>
> I believe that most of the attach_client implementations will have
> similar approach of allocating and populating an address-pool and
> searching for first unused address. As a 'further dev' it'd be great to
> see a common helper implementation for attach/detach - perhaps so that
> the atr drivers would only need to specify the slave-address
> configuration register(s) / mask and the use a 'generic' attach/detach
> helpers. Well, just thinking how to reduce the code from actual IC
> drivers but this is really not something that is required during this
> initial series :)
>
> Also, devm-variants would be great - although that falls to the same
> category of things that do not need to be done immediately - but would
> perhaps be worth considering in the future.
Both of your proposals make sense, however I did deliberately not
generalize too much because I knew only one chipset. I don't like trying
to generalize for an unpredictable future use case, it generally leads
(me) to generalizing in the wrong direction. That means you'd be very
welcome to propose helpers and/or devm variants, possibly in the same
patchset as the first Rohm serdes driver. ;)
> Reviewed-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Thanks for your review!
--
Luca
next prev parent reply other threads:[~2022-03-16 14:25 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-06 11:59 [RFCv3 0/6] Hi, Luca Ceresoli
2022-02-06 11:59 ` [RFCv3 1/6] i2c: core: let adapters be notified of client attach/detach Luca Ceresoli
2022-02-06 11:59 ` [RFCv3 2/6] i2c: add I2C Address Translator (ATR) support Luca Ceresoli
2022-02-08 11:16 ` Andy Shevchenko
2022-02-16 8:40 ` Luca Ceresoli
2022-02-17 5:12 ` Vaittinen, Matti
2022-03-16 14:11 ` Vaittinen, Matti
2022-03-16 14:25 ` Luca Ceresoli [this message]
2022-02-06 11:59 ` [RFCv3 3/6] media: dt-bindings: add DS90UB953-Q1 video serializer Luca Ceresoli
2022-02-07 21:48 ` Rob Herring
2022-02-06 11:59 ` [RFCv3 4/6] media: dt-bindings: add DS90UB954-Q1 video deserializer Luca Ceresoli
2022-02-06 18:46 ` Rob Herring
2022-02-07 19:39 ` Rob Herring
2022-02-06 11:59 ` [RFCv3 5/6] media: ds90ub954: new driver for TI " Luca Ceresoli
2022-02-06 11:59 ` [RFCv3 6/6] media: ds90ub953: new driver for TI DS90UB953-Q1 video serializer Luca Ceresoli
2022-02-06 12:05 ` [RFCv3 0/6] TI camera serdes and I2C address translation Luca Ceresoli
2022-02-07 12:06 ` [RFCv3 0/6] TI camera serdes and I2C address translation (Was: [RFCv3 0/6] Hi,) Tomi Valkeinen
2022-02-07 13:21 ` Vaittinen, Matti
2022-02-07 14:07 ` Luca Ceresoli
2022-02-07 14:38 ` Vaittinen, Matti
2022-02-07 16:23 ` Tomi Valkeinen
2022-02-08 6:40 ` Vaittinen, Matti
2022-02-08 8:28 ` Tomi Valkeinen
2022-02-08 9:36 ` Vaittinen, Matti
Reply instructions:
You may reply publicly 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=02af807d-c35e-afc6-7a41-22eafd3c46f9@lucaceresoli.net \
--to=luca@lucaceresoli.net \
--cc=Matti.Vaittinen@fi.rohmeurope.com \
--cc=Mikko.Mutanen@fi.rohmeurope.com \
--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=tomi.valkeinen@ideasonboard.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).