From: Luca Ceresoli <luca@lucaceresoli.net>
To: jacopo mondi <jacopo@jmondi.org>
Cc: kieran.bingham+renesas@ideasonboard.com,
"Kieran Bingham" <kieran.bingham@ideasonboard.com>,
linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, sakari.ailus@iki.fi,
"Niklas Söderlund" <niklas.soderlund@ragnatech.se>,
"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
linux-kernel@vger.kernel.org,
"Jacopo Mondi" <jacopo+renesas@jmondi.org>,
"Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>,
"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
Subject: Re: [PATCH v4 3/4] media: i2c: Add MAX9286 driver
Date: Thu, 8 Nov 2018 12:24:23 +0100 [thread overview]
Message-ID: <71bc438b-321e-cecc-120c-a95ff12642f2@lucaceresoli.net> (raw)
In-Reply-To: <20181108101101.GE24024@w540>
Hi Jacopo,
On 08/11/18 11:11, jacopo mondi wrote:
> Hi Luca, Kieran,
> sorry to jump up, but I feel this should be clarified.
>
> On Wed, Nov 07, 2018 at 06:24:18PM +0100, Luca Ceresoli wrote:
>> Hi Kieran,
>>
>> thanks for the clarification. One additional note below.
>>
>> On 07/11/18 16:06, Kieran Bingham wrote:
>>> Hi Luca
>>>
>>> <Top posting for new topic>
>>>
>>>> <lucaceresoli> kbingham: hi, I'm looking at your GMSL v4 patches
>>>> <lucaceresoli> kbingham: jmondi helped me in understanding parts of it, but I still have a question
>>>> <lucaceresoli> kbingham: are the drivers waiting for the established link before the remote chips are accessed? where?
>>>
>>> I'm replying here rather than spam the IRC channel with a big paste.
>>> It's also a useful description to the probe sequence, so I've kept it
>>> with the driver posting.
>>>
>>> I hope the following helps illustrate the sequences which are involved:
>>>
>>> max9286_probe()
>>> - max9286_i2c_mux_close() # Disable all links
>>> - max9286_configure_i2c # Configure early communication settings
>>> - max9286_init():
>>> - regulator_enable() # Power up all cameras
>>> - max9286_setup() # Most link setup is done here.
>>> ... Set up v4l2/async/media-controller endpoints
>>> - max9286_i2c_mux_init() # Start configuring cameras:
>>> - i2c_mux_alloc() # Create our mux device
>>> - for_each_source(dev, source)
>>> i2c_mux_add_adapter() # This is where sensors get probed.
>>>
>>> So yes sensors are only communicated with once the link is brought up as
>>> much as possible.
>>
>> For the records, an additional bit of explanation I got from Kieran via IRC.
>>
>> The fact that link is already up when the sensors are probed is due to
>> the fact that the power regulator has a delay of *8 seconds*. This is
>> intended, because there's an MCU on the camera modules that talks on the
>> I2C bus during that time, and thus the drivers need to wait after it's done.
>
> The 8sec delay is due to the fact an integrated MCU on the remote
> camera module programs the local sensor and the serializer intgrated
> in the module in to some default configuration state. At power up, we
> just want to let it finish, with all reverse channels closed
> (camera module -> SoC direction) not to have the MCU transmitted
> messages repeated to the local side (our remote serializer does repeat
> messages not directed to it on it's remote side, as our local
> deserialier does).
>
> The "link up" thing is fairly more complicated for GMSL than just
> having a binary "on" or "off" mode. This technology defines two different
> "channels", a 'configuration-channel' for transmitting control messages
> on the serial link (i2c messages for the deserializer/serializer pair
> this patches support) and a 'video-channel' for transmission of
> high-speed data, such as, no surprise, video and images :)
>
> GMSL also defines two "link modes": a clock-less "configuration link"
> and an high-speed "video link". The "configuration link" is available a
> few msec after power up (roughly), while the "video link" needs a pixel
> clock to be supplied to the serializer for it to enter this mode and
> be able to lock the status between itself and the deserializer. Then it can
> begin serializing video data.
>
> The 'control channel' is available both when the link is in
> 'configuration' and 'video' mode, while the 'video' channel is
> available only when the link is in 'video' mode (or, to put it more
> simply: you can send i2c configuration messages while the link is
> serializing video).
>
> Our implementation uses the link in 'configuration mode' during the
> remote side programming phase, at 'max9286_i2c_mux_init()' time, with
> the 'max9286_i2c_mux_select()' function enabling selectively the
> 'configuration link' of each single remote end. It probes the remote device
> by instantiating a new i2c_adapter connected to the mux, one for each
> remote end, and performs the device configuration by initially using its
> default power up i2c address (it is safe to do so, all other links are
> closed), then changes the remote devices address to an unique one
> (as our devices allows us to do so, otherwise you should use the
> deserializer address translation feature to mask and translate the
> remote addresses).
>
> Now all remote devices have an unique i2c address, and we can operate
> with all 'configuration links' open with no risk of i2c addresses
> collisions.
>
> At this point when we want to start the video stream, we send a
> control message to the remote device, which enables the pixel clock
> output from the image sensor, and activate the 'video channel' on the
> remote serializer. The local deserializer makes sure all 'video links'
> are locked (see 'max9286_check_video_links()') and at this point we
> can begin serializing/deserializing video data.
>
> As you can see, the initial delay only plays a role in avoiding
> collision before we properly configure the channels and the i2c
> addresses. The link setup phase is instead an integral part of the
> system configuration, and there are no un-necessary delays used to
> work around it setup procedure.
>
> Does this help clarifying the system startup procedure?
Yes, that's very informative, thank you very much.
Given the complexity of the driver and the non-obviousness of some
workarounds to "unfortunate hardware design choices", I think [some of]
this explanation should be committed together with the driver, in order
to make it more understandable to other people. Even more since you've
already taken time to write it.
Thanks,
--
Luca
next prev parent reply other threads:[~2018-11-08 11:24 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-02 15:47 [PATCH v4 0/4] MAX9286 GMSL Support Kieran Bingham
2018-11-02 15:47 ` [PATCH v4 1/4] dt-bindings: media: i2c: Add bindings for Maxim Integrated MAX9286 Kieran Bingham
2018-11-05 20:02 ` Rob Herring
2018-11-13 22:42 ` Luca Ceresoli
2018-11-13 23:12 ` Kieran Bingham
2018-11-14 10:04 ` Luca Ceresoli
2018-11-27 22:47 ` Kieran Bingham
2018-12-01 18:41 ` Luca Ceresoli
2018-11-02 15:47 ` [PATCH v4 2/4] dt-bindings: media: i2c: Add bindings for IMI RDACM20 Kieran Bingham
2018-11-05 20:13 ` Rob Herring
2018-11-02 15:47 ` [PATCH v4 3/4] media: i2c: Add MAX9286 driver Kieran Bingham
2018-11-07 15:06 ` Kieran Bingham
2018-11-07 17:24 ` Luca Ceresoli
2018-11-08 10:11 ` jacopo mondi
2018-11-08 11:24 ` Luca Ceresoli [this message]
2018-11-13 22:49 ` Luca Ceresoli
2018-11-14 0:46 ` Kieran Bingham
2018-11-14 10:04 ` Luca Ceresoli
2018-11-20 0:32 ` Kieran Bingham
2018-11-20 10:51 ` Luca Ceresoli
2018-11-20 17:44 ` jacopo mondi
2018-11-21 10:05 ` Luca Ceresoli
2018-11-02 15:47 ` [PATCH v4 4/4] media: i2c: Add RDACM20 driver Kieran Bingham
2018-11-20 8:34 ` Sakari Ailus
2018-11-28 12:51 ` Kieran Bingham
2018-12-20 23:11 ` Sakari Ailus
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=71bc438b-321e-cecc-120c-a95ff12642f2@lucaceresoli.net \
--to=luca@lucaceresoli.net \
--cc=devicetree@vger.kernel.org \
--cc=jacopo+renesas@jmondi.org \
--cc=jacopo@jmondi.org \
--cc=kieran.bingham+renesas@ideasonboard.com \
--cc=kieran.bingham@ideasonboard.com \
--cc=laurent.pinchart+renesas@ideasonboard.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=niklas.soderlund@ragnatech.se \
--cc=sakari.ailus@iki.fi \
/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).