Hello, this v6 renames the 'maxim,initial-reverse-channel-mV' property
to 'maxim,reverse-channel-microvolt' to use standard suffix as suggested
by Rob.
v5: https://patchwork.linuxtv.org/project/linux-media/list/?series=3876
v4: https://patchwork.linuxtv.org/project/linux-media/list/?series=3847
v3: https://patchwork.linuxtv.org/project/linux-media/list/?series=3657
Background in v1 cover letter:
https://www.spinics.net/lists/linux-renesas-soc/msg52886.html
Jacopo Mondi (5):
media: i2c: Add driver for RDACM21 camera module
dt-bindings: media: max9286: Document
'maxim,reverse-channel-microvolt'
media: i2c: max9286: Break-out reverse channel setup
media: i2c: max9286: Make channel amplitude programmable
media: i2c: max9286: Configure reverse channel amplitude
.../bindings/media/i2c/maxim,max9286.yaml | 23 +
MAINTAINERS | 12 +
drivers/media/i2c/Kconfig | 13 +
drivers/media/i2c/Makefile | 2 +
drivers/media/i2c/max9286.c | 61 +-
drivers/media/i2c/rdacm21.c | 595 ++++++++++++++++++
6 files changed, 693 insertions(+), 13 deletions(-)
create mode 100644 drivers/media/i2c/rdacm21.c
--
2.29.2
Document the 'reverse-channel-microvolt' vendor property in the
bindings document of the max9286 driver.
The newly introduced property allows to specifying the initial
configuration of the GMSL reverse control channel to accommodate
remote serializers pre-programmed with the high threshold power
supply noise immunity enabled.
Reviewed-by: Kieran Bingham <[email protected]>
Signed-off-by: Jacopo Mondi <[email protected]>
---
v5->v6:
- Use standard unit suffix 'microvolt' for the custom property
- Drop '$ref' as according to 'example-schema.yaml':
"Vendor specific properties having a standard unit suffix don't need a type."
---
.../bindings/media/i2c/maxim,max9286.yaml | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
index 9ea827092fdd..b22ba3e0db4a 100644
--- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
@@ -51,6 +51,26 @@ properties:
'#gpio-cells':
const: 2
+ maxim,reverse-channel-microvolt:
+ minimum: 30000
+ maximum: 200000
+ default: 170000
+ description: |
+ Initial amplitude of the reverse control channel, in micro volts.
+
+ The initial amplitude shall be adjusted to a value compatible with the
+ configuration of the connected remote serializer.
+
+ Some camera modules (for example RDACM20) include an on-board MCU that
+ pre-programs the embedded serializer with power supply noise immunity
+ (high-threshold) enabled. A typical value of the deserializer's reverse
+ channel amplitude to communicate with pre-programmed serializers is
+ 170000 micro volts.
+
+ A typical value for the reverse channel amplitude to communicate with
+ a remote serializer whose high-threshold noise immunity is not enabled
+ is 100000 micro volts
+
ports:
type: object
description: |
@@ -221,6 +241,7 @@ required:
- ports
- i2c-mux
- gpio-controller
+ - maxim,reverse-channel-microvolt
additionalProperties: false
@@ -243,6 +264,8 @@ examples:
gpio-controller;
#gpio-cells = <2>;
+ maxim,reverse-channel-microvolt = <170000>;
+
ports {
#address-cells = <1>;
#size-cells = <0>;
--
2.29.2
Instrument the function that configures the reverse channel with a
programmable amplitude value.
This change serves to prepare to adjust the reverse channel amplitude
depending on the remote end high-threshold configuration.
Reviewed-by: Kieran Bingham <[email protected]>
Signed-off-by: Jacopo Mondi <[email protected]>
---
drivers/media/i2c/max9286.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 1cfc8801c0b2..021309c6dd6f 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -336,19 +336,29 @@ static void max9286_configure_i2c(struct max9286_priv *priv, bool localack)
usleep_range(3000, 5000);
}
-static void max9286_reverse_channel_setup(struct max9286_priv *priv)
+static void max9286_reverse_channel_setup(struct max9286_priv *priv,
+ unsigned int chan_amplitude)
{
+ /* Reverse channel transmission time: default to 1. */
+ u8 chan_config = MAX9286_REV_TRF(1);
+
/*
* Reverse channel setup.
*
* - Enable custom reverse channel configuration (through register 0x3f)
* and set the first pulse length to 35 clock cycles.
- * - Increase the reverse channel amplitude to 170mV to accommodate the
- * high threshold enabled by the serializer driver.
+ * - Adjust reverse channel amplitude: values > 130 are programmed
+ * using the additional +100mV REV_AMP_X boost flag
*/
max9286_write(priv, 0x3f, MAX9286_EN_REV_CFG | MAX9286_REV_FLEN(35));
- max9286_write(priv, 0x3b, MAX9286_REV_TRF(1) | MAX9286_REV_AMP(70) |
- MAX9286_REV_AMP_X);
+
+ if (chan_amplitude > 100) {
+ /* It is not possible to express values (100 < x < 130) */
+ chan_amplitude = chan_amplitude < 130
+ ? 30 : chan_amplitude - 100;
+ chan_config |= MAX9286_REV_AMP_X;
+ }
+ max9286_write(priv, 0x3b, chan_config | MAX9286_REV_AMP(chan_amplitude));
usleep_range(2000, 2500);
}
@@ -957,7 +967,7 @@ static int max9286_setup(struct max9286_priv *priv)
* only. This should be disabled after the mux is initialised.
*/
max9286_configure_i2c(priv, true);
- max9286_reverse_channel_setup(priv);
+ max9286_reverse_channel_setup(priv, 170);
/*
* Enable GMSL links, mask unused ones and autodetect link
--
2.29.2
Adjust the initial reverse channel amplitude parsing from
firmware interface the 'maxim,reverse-channel-microvolt'
property.
This change is required for both rdacm20 and rdacm21 camera
modules to be correctly probed when used in combination with
the max9286 deserializer.
Reviewed-by: Kieran Bingham <[email protected]>
Signed-off-by: Jacopo Mondi <[email protected]>
---
drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 021309c6dd6f..9b40a4890c4d 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -163,6 +163,8 @@ struct max9286_priv {
unsigned int mux_channel;
bool mux_open;
+ u32 reverse_channel_mv;
+
struct v4l2_ctrl_handler ctrls;
struct v4l2_ctrl *pixelrate;
@@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
* All enabled sources have probed and enabled their reverse control
* channels:
*
+ * - Increase the reverse channel amplitude to compensate for the
+ * remote ends high threshold, if not done already
* - Verify all configuration links are properly detected
* - Disable auto-ack as communication on the control channel are now
* stable.
*/
+ if (priv->reverse_channel_mv < 170)
+ max9286_reverse_channel_setup(priv, 170);
max9286_check_config_link(priv, priv->source_mask);
/*
@@ -967,7 +973,7 @@ static int max9286_setup(struct max9286_priv *priv)
* only. This should be disabled after the mux is initialised.
*/
max9286_configure_i2c(priv, true);
- max9286_reverse_channel_setup(priv, 170);
+ max9286_reverse_channel_setup(priv, priv->reverse_channel_mv);
/*
* Enable GMSL links, mask unused ones and autodetect link
@@ -1131,6 +1137,7 @@ static int max9286_parse_dt(struct max9286_priv *priv)
struct device_node *i2c_mux;
struct device_node *node = NULL;
unsigned int i2c_mux_mask = 0;
+ u32 reverse_channel_microvolt;
/* Balance the of_node_put() performed by of_find_node_by_name(). */
of_node_get(dev->of_node);
@@ -1221,6 +1228,20 @@ static int max9286_parse_dt(struct max9286_priv *priv)
}
of_node_put(node);
+ /*
+ * Parse the initial value of the reverse channel amplitude from
+ * the firmware interface and convert it to millivolts.
+ *
+ * Default it to 170mV for backward compatibility with DTBs that do not
+ * provide the property.
+ */
+ if (of_property_read_u32(dev->of_node,
+ "maxim,reverse-channel-microvolt",
+ &reverse_channel_microvolt))
+ priv->reverse_channel_mv = 170;
+ else
+ priv->reverse_channel_mv = reverse_channel_microvolt / 1000U;
+
priv->route_mask = priv->source_mask;
return 0;
--
2.29.2
Break out the reverse channel setup configuration procedure to its own
function.
This change prepares for configuring the reverse channel conditionally
to the remote side high threshold configuration.
No functional changes intended.
Reviewed-by: Kieran Bingham <[email protected]>
Signed-off-by: Jacopo Mondi <[email protected]>
---
drivers/media/i2c/max9286.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index c82c1493e099..1cfc8801c0b2 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -336,6 +336,22 @@ static void max9286_configure_i2c(struct max9286_priv *priv, bool localack)
usleep_range(3000, 5000);
}
+static void max9286_reverse_channel_setup(struct max9286_priv *priv)
+{
+ /*
+ * Reverse channel setup.
+ *
+ * - Enable custom reverse channel configuration (through register 0x3f)
+ * and set the first pulse length to 35 clock cycles.
+ * - Increase the reverse channel amplitude to 170mV to accommodate the
+ * high threshold enabled by the serializer driver.
+ */
+ max9286_write(priv, 0x3f, MAX9286_EN_REV_CFG | MAX9286_REV_FLEN(35));
+ max9286_write(priv, 0x3b, MAX9286_REV_TRF(1) | MAX9286_REV_AMP(70) |
+ MAX9286_REV_AMP_X);
+ usleep_range(2000, 2500);
+}
+
/*
* max9286_check_video_links() - Make sure video links are detected and locked
*
@@ -941,19 +957,7 @@ static int max9286_setup(struct max9286_priv *priv)
* only. This should be disabled after the mux is initialised.
*/
max9286_configure_i2c(priv, true);
-
- /*
- * Reverse channel setup.
- *
- * - Enable custom reverse channel configuration (through register 0x3f)
- * and set the first pulse length to 35 clock cycles.
- * - Increase the reverse channel amplitude to 170mV to accommodate the
- * high threshold enabled by the serializer driver.
- */
- max9286_write(priv, 0x3f, MAX9286_EN_REV_CFG | MAX9286_REV_FLEN(35));
- max9286_write(priv, 0x3b, MAX9286_REV_TRF(1) | MAX9286_REV_AMP(70) |
- MAX9286_REV_AMP_X);
- usleep_range(2000, 2500);
+ max9286_reverse_channel_setup(priv);
/*
* Enable GMSL links, mask unused ones and autodetect link
--
2.29.2
On Wed, Dec 16, 2020 at 07:17:05PM +0200, Laurent Pinchart wrote:
> On Wed, Dec 16, 2020 at 07:05:34PM +0200, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > On Tue, Dec 15, 2020 at 06:09:54PM +0100, Jacopo Mondi wrote:
> > > Document the 'reverse-channel-microvolt' vendor property in the
> > > bindings document of the max9286 driver.
> > >
> > > The newly introduced property allows to specifying the initial
> > > configuration of the GMSL reverse control channel to accommodate
> > > remote serializers pre-programmed with the high threshold power
> > > supply noise immunity enabled.
> > >
> > > Reviewed-by: Kieran Bingham <[email protected]>
> > > Signed-off-by: Jacopo Mondi <[email protected]>
> >
> > Reviewed-by: Laurent Pinchart <[email protected]>
> >
> > > ---
> > > v5->v6:
> > > - Use standard unit suffix 'microvolt' for the custom property
> > > - Drop '$ref' as according to 'example-schema.yaml':
> > > "Vendor specific properties having a standard unit suffix don't need a type."
> > > ---
> > > .../bindings/media/i2c/maxim,max9286.yaml | 23 +++++++++++++++++++
> > > 1 file changed, 23 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > > index 9ea827092fdd..b22ba3e0db4a 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > > @@ -51,6 +51,26 @@ properties:
> > > '#gpio-cells':
> > > const: 2
> > >
> > > + maxim,reverse-channel-microvolt:
> > > + minimum: 30000
> > > + maximum: 200000
> > > + default: 170000
> > > + description: |
> > > + Initial amplitude of the reverse control channel, in micro volts.
> > > +
> > > + The initial amplitude shall be adjusted to a value compatible with the
> > > + configuration of the connected remote serializer.
> > > +
> > > + Some camera modules (for example RDACM20) include an on-board MCU that
> > > + pre-programs the embedded serializer with power supply noise immunity
> > > + (high-threshold) enabled. A typical value of the deserializer's reverse
> > > + channel amplitude to communicate with pre-programmed serializers is
> > > + 170000 micro volts.
> > > +
> > > + A typical value for the reverse channel amplitude to communicate with
> > > + a remote serializer whose high-threshold noise immunity is not enabled
> > > + is 100000 micro volts
> > > +
> > > ports:
> > > type: object
> > > description: |
> > > @@ -221,6 +241,7 @@ required:
> > > - ports
> > > - i2c-mux
> > > - gpio-controller
> > > + - maxim,reverse-channel-microvolt
>
> One comment though: You specify a default value above, which isn't very
> useful when the property is required. Should we either drop the default
> value, or make the property optional ?
And generally added properties can't be required unless for some reason
DT's without the property are broken.
With required dropped,
Reviewed-by: Rob Herring <[email protected]>
Rob
Hi Rob, Laurent,
On Mon, Dec 21, 2020 at 11:58:27AM -0700, Rob Herring wrote:
> On Wed, Dec 16, 2020 at 07:17:05PM +0200, Laurent Pinchart wrote:
> > > > @@ -221,6 +241,7 @@ required:
> > > > - ports
> > > > - i2c-mux
> > > > - gpio-controller
> > > > + - maxim,reverse-channel-microvolt
> >
> > One comment though: You specify a default value above, which isn't very
> > useful when the property is required. Should we either drop the default
> > value, or make the property optional ?
>
> And generally added properties can't be required unless for some reason
> DT's without the property are broken.
My thinking was to make it required for new DTS and specify a default
for the old ones that do not have the property. I'll drop required and
keep the default value in next version.
Thanks
j
>
> With required dropped,
>
> Reviewed-by: Rob Herring <[email protected]>
>
> Rob
Hi Laurent,
On Wed, Dec 16, 2020 at 07:22:17PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Dec 15, 2020 at 06:09:57PM +0100, Jacopo Mondi wrote:
> > Adjust the initial reverse channel amplitude parsing from
> > firmware interface the 'maxim,reverse-channel-microvolt'
> > property.
> >
> > This change is required for both rdacm20 and rdacm21 camera
> > modules to be correctly probed when used in combination with
> > the max9286 deserializer.
> >
> > Reviewed-by: Kieran Bingham <[email protected]>
> > Signed-off-by: Jacopo Mondi <[email protected]>
> > ---
> > drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
> > 1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > index 021309c6dd6f..9b40a4890c4d 100644
> > --- a/drivers/media/i2c/max9286.c
> > +++ b/drivers/media/i2c/max9286.c
> > @@ -163,6 +163,8 @@ struct max9286_priv {
> > unsigned int mux_channel;
> > bool mux_open;
> >
> > + u32 reverse_channel_mv;
> > +
> > struct v4l2_ctrl_handler ctrls;
> > struct v4l2_ctrl *pixelrate;
> >
> > @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> > * All enabled sources have probed and enabled their reverse control
> > * channels:
> > *
> > + * - Increase the reverse channel amplitude to compensate for the
> > + * remote ends high threshold, if not done already
> > * - Verify all configuration links are properly detected
> > * - Disable auto-ack as communication on the control channel are now
> > * stable.
> > */
> > + if (priv->reverse_channel_mv < 170)
> > + max9286_reverse_channel_setup(priv, 170);
>
> I'm beginning to wonder if there will be a need in the future to not
> increase the reverse channel amplitude (keeping the threshold low on the
> remote side). An increased amplitude increases power consumption, and if
> the environment isn't noisy, a low amplitude would work. The device tree
> would then need to specify both the initial amplitude required by the
> remote side, and the desired amplitude after initialization. What do you
> think ? Is it overkill ? We don't have to implement this now, so
>
> Reviewed-by: Laurent Pinchart <[email protected]>
>
> but if this feature could be required later, we may want to take into
> account in the naming of the new DT property to reflect the fact that it
> is the initial value.
I had the same thought when I initially proposed
"maxim,initial-reverse-channel-mV"
Having to use the standard unit suffix that would have become
"maxim,initial-reverse-channel-microvolt"
which is extremely long.
I can't tell if there will be any need to adjust the amplitude later.
In any case, I would not rely on a DTS property to do so, as once we
have probed the remote we have a subdev where to call
'get_mbus_config()' on, and from there we can report the high threshold
status of the serializer and adjust the deser amplitude accordingly.
The property documentation clearly says the there specified amplitude
is 'initial' many times, so I don't think it is strictly necessary to
report it in the name too.
Would this work for you ?
>
> > max9286_check_config_link(priv, priv->source_mask);
> >
> > /*
> > @@ -967,7 +973,7 @@ static int max9286_setup(struct max9286_priv *priv)
> > * only. This should be disabled after the mux is initialised.
> > */
> > max9286_configure_i2c(priv, true);
> > - max9286_reverse_channel_setup(priv, 170);
> > + max9286_reverse_channel_setup(priv, priv->reverse_channel_mv);
> >
> > /*
> > * Enable GMSL links, mask unused ones and autodetect link
> > @@ -1131,6 +1137,7 @@ static int max9286_parse_dt(struct max9286_priv *priv)
> > struct device_node *i2c_mux;
> > struct device_node *node = NULL;
> > unsigned int i2c_mux_mask = 0;
> > + u32 reverse_channel_microvolt;
> >
> > /* Balance the of_node_put() performed by of_find_node_by_name(). */
> > of_node_get(dev->of_node);
> > @@ -1221,6 +1228,20 @@ static int max9286_parse_dt(struct max9286_priv *priv)
> > }
> > of_node_put(node);
> >
> > + /*
> > + * Parse the initial value of the reverse channel amplitude from
> > + * the firmware interface and convert it to millivolts.
> > + *
> > + * Default it to 170mV for backward compatibility with DTBs that do not
> > + * provide the property.
> > + */
> > + if (of_property_read_u32(dev->of_node,
> > + "maxim,reverse-channel-microvolt",
> > + &reverse_channel_microvolt))
> > + priv->reverse_channel_mv = 170;
> > + else
> > + priv->reverse_channel_mv = reverse_channel_microvolt / 1000U;
> > +
> > priv->route_mask = priv->source_mask;
> >
> > return 0;
>
> --
> Regards,
>
> Laurent Pinchart
Hi Laurent,
On Mon, Jan 11, 2021 at 12:58:59PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Jan 11, 2021 at 11:43:11AM +0100, Jacopo Mondi wrote:
> > On Wed, Dec 16, 2020 at 07:22:17PM +0200, Laurent Pinchart wrote:
> > > On Tue, Dec 15, 2020 at 06:09:57PM +0100, Jacopo Mondi wrote:
> > > > Adjust the initial reverse channel amplitude parsing from
> > > > firmware interface the 'maxim,reverse-channel-microvolt'
> > > > property.
> > > >
> > > > This change is required for both rdacm20 and rdacm21 camera
> > > > modules to be correctly probed when used in combination with
> > > > the max9286 deserializer.
> > > >
> > > > Reviewed-by: Kieran Bingham <[email protected]>
> > > > Signed-off-by: Jacopo Mondi <[email protected]>
> > > > ---
> > > > drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
> > > > 1 file changed, 22 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > > > index 021309c6dd6f..9b40a4890c4d 100644
> > > > --- a/drivers/media/i2c/max9286.c
> > > > +++ b/drivers/media/i2c/max9286.c
> > > > @@ -163,6 +163,8 @@ struct max9286_priv {
> > > > unsigned int mux_channel;
> > > > bool mux_open;
> > > >
> > > > + u32 reverse_channel_mv;
> > > > +
> > > > struct v4l2_ctrl_handler ctrls;
> > > > struct v4l2_ctrl *pixelrate;
> > > >
> > > > @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> > > > * All enabled sources have probed and enabled their reverse control
> > > > * channels:
> > > > *
> > > > + * - Increase the reverse channel amplitude to compensate for the
> > > > + * remote ends high threshold, if not done already
> > > > * - Verify all configuration links are properly detected
> > > > * - Disable auto-ack as communication on the control channel are now
> > > > * stable.
> > > > */
> > > > + if (priv->reverse_channel_mv < 170)
> > > > + max9286_reverse_channel_setup(priv, 170);
> > >
> > > I'm beginning to wonder if there will be a need in the future to not
> > > increase the reverse channel amplitude (keeping the threshold low on the
> > > remote side). An increased amplitude increases power consumption, and if
> > > the environment isn't noisy, a low amplitude would work. The device tree
> > > would then need to specify both the initial amplitude required by the
> > > remote side, and the desired amplitude after initialization. What do you
> > > think ? Is it overkill ? We don't have to implement this now, so
> > >
> > > Reviewed-by: Laurent Pinchart <[email protected]>
> > >
> > > but if this feature could be required later, we may want to take into
> > > account in the naming of the new DT property to reflect the fact that it
> > > is the initial value.
> >
> > I had the same thought when I initially proposed
> > "maxim,initial-reverse-channel-mV"
> >
> > Having to use the standard unit suffix that would have become
> > "maxim,initial-reverse-channel-microvolt"
> > which is extremely long.
> >
> > I can't tell if there will be any need to adjust the amplitude later.
> > In any case, I would not rely on a DTS property to do so, as once we
> > have probed the remote we have a subdev where to call
> > 'get_mbus_config()' on, and from there we can report the high threshold
> > status of the serializer and adjust the deser amplitude accordingly.
>
> I don't think that's the point. The threshold of the serializer is
> something we can configure at runtime. What voltage level to use after
How so ? I mean, we can add an API for this, but currently it's
configured at probe time and that's it. Its configuration might as
well come from a DT property like we do on the deserializer here but I
fail to see why it's different. Both settings depends on the required
noise immunity of th system.
> initialization time is a system property as it depends on noise
> immunity, so we'll have to specify it in DT.
I don't see it differently than what happens on the serializer. We can
add an API if we want to, but it's configured at probe time (initial
value) and later can be adjusted in reponse to the serializer
configuration setting.
I feel like we're on different pages :/
>
> > The property documentation clearly says the there specified amplitude
> > is 'initial' many times, so I don't think it is strictly necessary to
> > report it in the name too.
> >
> > Would this work for you ?
>
> I don't mind either way.
>
> > > > max9286_check_config_link(priv, priv->source_mask);
> > > >
> > > > /*
> > > > @@ -967,7 +973,7 @@ static int max9286_setup(struct max9286_priv *priv)
> > > > * only. This should be disabled after the mux is initialised.
> > > > */
> > > > max9286_configure_i2c(priv, true);
> > > > - max9286_reverse_channel_setup(priv, 170);
> > > > + max9286_reverse_channel_setup(priv, priv->reverse_channel_mv);
> > > >
> > > > /*
> > > > * Enable GMSL links, mask unused ones and autodetect link
> > > > @@ -1131,6 +1137,7 @@ static int max9286_parse_dt(struct max9286_priv *priv)
> > > > struct device_node *i2c_mux;
> > > > struct device_node *node = NULL;
> > > > unsigned int i2c_mux_mask = 0;
> > > > + u32 reverse_channel_microvolt;
> > > >
> > > > /* Balance the of_node_put() performed by of_find_node_by_name(). */
> > > > of_node_get(dev->of_node);
> > > > @@ -1221,6 +1228,20 @@ static int max9286_parse_dt(struct max9286_priv *priv)
> > > > }
> > > > of_node_put(node);
> > > >
> > > > + /*
> > > > + * Parse the initial value of the reverse channel amplitude from
> > > > + * the firmware interface and convert it to millivolts.
> > > > + *
> > > > + * Default it to 170mV for backward compatibility with DTBs that do not
> > > > + * provide the property.
> > > > + */
> > > > + if (of_property_read_u32(dev->of_node,
> > > > + "maxim,reverse-channel-microvolt",
> > > > + &reverse_channel_microvolt))
> > > > + priv->reverse_channel_mv = 170;
> > > > + else
> > > > + priv->reverse_channel_mv = reverse_channel_microvolt / 1000U;
> > > > +
> > > > priv->route_mask = priv->source_mask;
> > > >
> > > > return 0;
>
> --
> Regards,
>
> Laurent Pinchart
Hi Jacopo,
On Mon, Jan 11, 2021 at 11:43:11AM +0100, Jacopo Mondi wrote:
> On Wed, Dec 16, 2020 at 07:22:17PM +0200, Laurent Pinchart wrote:
> > On Tue, Dec 15, 2020 at 06:09:57PM +0100, Jacopo Mondi wrote:
> > > Adjust the initial reverse channel amplitude parsing from
> > > firmware interface the 'maxim,reverse-channel-microvolt'
> > > property.
> > >
> > > This change is required for both rdacm20 and rdacm21 camera
> > > modules to be correctly probed when used in combination with
> > > the max9286 deserializer.
> > >
> > > Reviewed-by: Kieran Bingham <[email protected]>
> > > Signed-off-by: Jacopo Mondi <[email protected]>
> > > ---
> > > drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
> > > 1 file changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > > index 021309c6dd6f..9b40a4890c4d 100644
> > > --- a/drivers/media/i2c/max9286.c
> > > +++ b/drivers/media/i2c/max9286.c
> > > @@ -163,6 +163,8 @@ struct max9286_priv {
> > > unsigned int mux_channel;
> > > bool mux_open;
> > >
> > > + u32 reverse_channel_mv;
> > > +
> > > struct v4l2_ctrl_handler ctrls;
> > > struct v4l2_ctrl *pixelrate;
> > >
> > > @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> > > * All enabled sources have probed and enabled their reverse control
> > > * channels:
> > > *
> > > + * - Increase the reverse channel amplitude to compensate for the
> > > + * remote ends high threshold, if not done already
> > > * - Verify all configuration links are properly detected
> > > * - Disable auto-ack as communication on the control channel are now
> > > * stable.
> > > */
> > > + if (priv->reverse_channel_mv < 170)
> > > + max9286_reverse_channel_setup(priv, 170);
> >
> > I'm beginning to wonder if there will be a need in the future to not
> > increase the reverse channel amplitude (keeping the threshold low on the
> > remote side). An increased amplitude increases power consumption, and if
> > the environment isn't noisy, a low amplitude would work. The device tree
> > would then need to specify both the initial amplitude required by the
> > remote side, and the desired amplitude after initialization. What do you
> > think ? Is it overkill ? We don't have to implement this now, so
> >
> > Reviewed-by: Laurent Pinchart <[email protected]>
> >
> > but if this feature could be required later, we may want to take into
> > account in the naming of the new DT property to reflect the fact that it
> > is the initial value.
>
> I had the same thought when I initially proposed
> "maxim,initial-reverse-channel-mV"
>
> Having to use the standard unit suffix that would have become
> "maxim,initial-reverse-channel-microvolt"
> which is extremely long.
>
> I can't tell if there will be any need to adjust the amplitude later.
> In any case, I would not rely on a DTS property to do so, as once we
> have probed the remote we have a subdev where to call
> 'get_mbus_config()' on, and from there we can report the high threshold
> status of the serializer and adjust the deser amplitude accordingly.
I don't think that's the point. The threshold of the serializer is
something we can configure at runtime. What voltage level to use after
initialization time is a system property as it depends on noise
immunity, so we'll have to specify it in DT.
> The property documentation clearly says the there specified amplitude
> is 'initial' many times, so I don't think it is strictly necessary to
> report it in the name too.
>
> Would this work for you ?
I don't mind either way.
> > > max9286_check_config_link(priv, priv->source_mask);
> > >
> > > /*
> > > @@ -967,7 +973,7 @@ static int max9286_setup(struct max9286_priv *priv)
> > > * only. This should be disabled after the mux is initialised.
> > > */
> > > max9286_configure_i2c(priv, true);
> > > - max9286_reverse_channel_setup(priv, 170);
> > > + max9286_reverse_channel_setup(priv, priv->reverse_channel_mv);
> > >
> > > /*
> > > * Enable GMSL links, mask unused ones and autodetect link
> > > @@ -1131,6 +1137,7 @@ static int max9286_parse_dt(struct max9286_priv *priv)
> > > struct device_node *i2c_mux;
> > > struct device_node *node = NULL;
> > > unsigned int i2c_mux_mask = 0;
> > > + u32 reverse_channel_microvolt;
> > >
> > > /* Balance the of_node_put() performed by of_find_node_by_name(). */
> > > of_node_get(dev->of_node);
> > > @@ -1221,6 +1228,20 @@ static int max9286_parse_dt(struct max9286_priv *priv)
> > > }
> > > of_node_put(node);
> > >
> > > + /*
> > > + * Parse the initial value of the reverse channel amplitude from
> > > + * the firmware interface and convert it to millivolts.
> > > + *
> > > + * Default it to 170mV for backward compatibility with DTBs that do not
> > > + * provide the property.
> > > + */
> > > + if (of_property_read_u32(dev->of_node,
> > > + "maxim,reverse-channel-microvolt",
> > > + &reverse_channel_microvolt))
> > > + priv->reverse_channel_mv = 170;
> > > + else
> > > + priv->reverse_channel_mv = reverse_channel_microvolt / 1000U;
> > > +
> > > priv->route_mask = priv->source_mask;
> > >
> > > return 0;
--
Regards,
Laurent Pinchart
Hi Jacopo,
On Mon, Jan 11, 2021 at 12:20:23PM +0100, Jacopo Mondi wrote:
> On Mon, Jan 11, 2021 at 12:58:59PM +0200, Laurent Pinchart wrote:
> > On Mon, Jan 11, 2021 at 11:43:11AM +0100, Jacopo Mondi wrote:
> >> On Wed, Dec 16, 2020 at 07:22:17PM +0200, Laurent Pinchart wrote:
> >>> On Tue, Dec 15, 2020 at 06:09:57PM +0100, Jacopo Mondi wrote:
> >>>> Adjust the initial reverse channel amplitude parsing from
> >>>> firmware interface the 'maxim,reverse-channel-microvolt'
> >>>> property.
> >>>>
> >>>> This change is required for both rdacm20 and rdacm21 camera
> >>>> modules to be correctly probed when used in combination with
> >>>> the max9286 deserializer.
> >>>>
> >>>> Reviewed-by: Kieran Bingham <[email protected]>
> >>>> Signed-off-by: Jacopo Mondi <[email protected]>
> >>>> ---
> >>>> drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
> >>>> 1 file changed, 22 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> >>>> index 021309c6dd6f..9b40a4890c4d 100644
> >>>> --- a/drivers/media/i2c/max9286.c
> >>>> +++ b/drivers/media/i2c/max9286.c
> >>>> @@ -163,6 +163,8 @@ struct max9286_priv {
> >>>> unsigned int mux_channel;
> >>>> bool mux_open;
> >>>>
> >>>> + u32 reverse_channel_mv;
> >>>> +
> >>>> struct v4l2_ctrl_handler ctrls;
> >>>> struct v4l2_ctrl *pixelrate;
> >>>>
> >>>> @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> >>>> * All enabled sources have probed and enabled their reverse control
> >>>> * channels:
> >>>> *
> >>>> + * - Increase the reverse channel amplitude to compensate for the
> >>>> + * remote ends high threshold, if not done already
> >>>> * - Verify all configuration links are properly detected
> >>>> * - Disable auto-ack as communication on the control channel are now
> >>>> * stable.
> >>>> */
> >>>> + if (priv->reverse_channel_mv < 170)
> >>>> + max9286_reverse_channel_setup(priv, 170);
> >>>
> >>> I'm beginning to wonder if there will be a need in the future to not
> >>> increase the reverse channel amplitude (keeping the threshold low on the
> >>> remote side). An increased amplitude increases power consumption, and if
> >>> the environment isn't noisy, a low amplitude would work. The device tree
> >>> would then need to specify both the initial amplitude required by the
> >>> remote side, and the desired amplitude after initialization. What do you
> >>> think ? Is it overkill ? We don't have to implement this now, so
> >>>
> >>> Reviewed-by: Laurent Pinchart <[email protected]>
> >>>
> >>> but if this feature could be required later, we may want to take into
> >>> account in the naming of the new DT property to reflect the fact that it
> >>> is the initial value.
> >>
> >> I had the same thought when I initially proposed
> >> "maxim,initial-reverse-channel-mV"
> >>
> >> Having to use the standard unit suffix that would have become
> >> "maxim,initial-reverse-channel-microvolt"
> >> which is extremely long.
> >>
> >> I can't tell if there will be any need to adjust the amplitude later.
> >> In any case, I would not rely on a DTS property to do so, as once we
> >> have probed the remote we have a subdev where to call
> >> 'get_mbus_config()' on, and from there we can report the high threshold
> >> status of the serializer and adjust the deser amplitude accordingly.
> >
> > I don't think that's the point. The threshold of the serializer is
> > something we can configure at runtime. What voltage level to use after
>
> How so ? I mean, we can add an API for this, but currently it's
> configured at probe time and that's it. Its configuration might as
> well come from a DT property like we do on the deserializer here but I
> fail to see why it's different. Both settings depends on the required
> noise immunity of th system.
The voltage level configuration need to match between the tserializer
(transmitter) and the deserializer (receiver). The serializer is
configured with a voltage level, and the deserializer needs to be
configured with a corresponding threshold.
The voltage level of the serializer is configurable on the camera side
when the system is powered up. The RDACM20 has a microcontroller which
can configure the serializer, and other cameras may have similar
mechanisms. As the deserializer can't query the information from the
serializer (communication is unreliable if the threshold has an
incorrect value), we need a DT property to tell the deserializer what
threshold is initially used by the camera when it gets powered up.
This only covers initialization. A camera could boot up with a low
voltage level, but we may want to increase the voltage level (and thus
the threshold on the deserializer side) to increase noise immunity. Or,
if the system environment isn't noisy, we may want to keep a low voltage
level, or even decrease it if the camera boots up with a high voltage
level. This runtime voltage level depends on the system design and its
susceptibility to noise, and is thus a system property. Should we want
to make it configurable, it should be specified in DT, and it's separate
from the initial voltage level that is used to establish communication.
> > initialization time is a system property as it depends on noise
> > immunity, so we'll have to specify it in DT.
>
> I don't see it differently than what happens on the serializer. We can
> add an API if we want to, but it's configured at probe time (initial
> value) and later can be adjusted in reponse to the serializer
> configuration setting.
>
> I feel like we're on different pages :/
>
> >> The property documentation clearly says the there specified amplitude
> >> is 'initial' many times, so I don't think it is strictly necessary to
> >> report it in the name too.
> >>
> >> Would this work for you ?
> >
> > I don't mind either way.
> >
> >>>> max9286_check_config_link(priv, priv->source_mask);
> >>>>
> >>>> /*
> >>>> @@ -967,7 +973,7 @@ static int max9286_setup(struct max9286_priv *priv)
> >>>> * only. This should be disabled after the mux is initialised.
> >>>> */
> >>>> max9286_configure_i2c(priv, true);
> >>>> - max9286_reverse_channel_setup(priv, 170);
> >>>> + max9286_reverse_channel_setup(priv, priv->reverse_channel_mv);
> >>>>
> >>>> /*
> >>>> * Enable GMSL links, mask unused ones and autodetect link
> >>>> @@ -1131,6 +1137,7 @@ static int max9286_parse_dt(struct max9286_priv *priv)
> >>>> struct device_node *i2c_mux;
> >>>> struct device_node *node = NULL;
> >>>> unsigned int i2c_mux_mask = 0;
> >>>> + u32 reverse_channel_microvolt;
> >>>>
> >>>> /* Balance the of_node_put() performed by of_find_node_by_name(). */
> >>>> of_node_get(dev->of_node);
> >>>> @@ -1221,6 +1228,20 @@ static int max9286_parse_dt(struct max9286_priv *priv)
> >>>> }
> >>>> of_node_put(node);
> >>>>
> >>>> + /*
> >>>> + * Parse the initial value of the reverse channel amplitude from
> >>>> + * the firmware interface and convert it to millivolts.
> >>>> + *
> >>>> + * Default it to 170mV for backward compatibility with DTBs that do not
> >>>> + * provide the property.
> >>>> + */
> >>>> + if (of_property_read_u32(dev->of_node,
> >>>> + "maxim,reverse-channel-microvolt",
> >>>> + &reverse_channel_microvolt))
> >>>> + priv->reverse_channel_mv = 170;
> >>>> + else
> >>>> + priv->reverse_channel_mv = reverse_channel_microvolt / 1000U;
> >>>> +
> >>>> priv->route_mask = priv->source_mask;
> >>>>
> >>>> return 0;
--
Regards,
Laurent Pinchart
Hi Laurent,
On Tue, Jan 12, 2021 at 07:03:42AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Jan 11, 2021 at 12:20:23PM +0100, Jacopo Mondi wrote:
> > On Mon, Jan 11, 2021 at 12:58:59PM +0200, Laurent Pinchart wrote:
> > > On Mon, Jan 11, 2021 at 11:43:11AM +0100, Jacopo Mondi wrote:
> > >> On Wed, Dec 16, 2020 at 07:22:17PM +0200, Laurent Pinchart wrote:
> > >>> On Tue, Dec 15, 2020 at 06:09:57PM +0100, Jacopo Mondi wrote:
> > >>>> Adjust the initial reverse channel amplitude parsing from
> > >>>> firmware interface the 'maxim,reverse-channel-microvolt'
> > >>>> property.
> > >>>>
> > >>>> This change is required for both rdacm20 and rdacm21 camera
> > >>>> modules to be correctly probed when used in combination with
> > >>>> the max9286 deserializer.
> > >>>>
> > >>>> Reviewed-by: Kieran Bingham <[email protected]>
> > >>>> Signed-off-by: Jacopo Mondi <[email protected]>
> > >>>> ---
> > >>>> drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
> > >>>> 1 file changed, 22 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > >>>> index 021309c6dd6f..9b40a4890c4d 100644
> > >>>> --- a/drivers/media/i2c/max9286.c
> > >>>> +++ b/drivers/media/i2c/max9286.c
> > >>>> @@ -163,6 +163,8 @@ struct max9286_priv {
> > >>>> unsigned int mux_channel;
> > >>>> bool mux_open;
> > >>>>
> > >>>> + u32 reverse_channel_mv;
> > >>>> +
> > >>>> struct v4l2_ctrl_handler ctrls;
> > >>>> struct v4l2_ctrl *pixelrate;
> > >>>>
> > >>>> @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> > >>>> * All enabled sources have probed and enabled their reverse control
> > >>>> * channels:
> > >>>> *
> > >>>> + * - Increase the reverse channel amplitude to compensate for the
> > >>>> + * remote ends high threshold, if not done already
> > >>>> * - Verify all configuration links are properly detected
> > >>>> * - Disable auto-ack as communication on the control channel are now
> > >>>> * stable.
> > >>>> */
> > >>>> + if (priv->reverse_channel_mv < 170)
> > >>>> + max9286_reverse_channel_setup(priv, 170);
> > >>>
> > >>> I'm beginning to wonder if there will be a need in the future to not
> > >>> increase the reverse channel amplitude (keeping the threshold low on the
> > >>> remote side). An increased amplitude increases power consumption, and if
> > >>> the environment isn't noisy, a low amplitude would work. The device tree
> > >>> would then need to specify both the initial amplitude required by the
> > >>> remote side, and the desired amplitude after initialization. What do you
> > >>> think ? Is it overkill ? We don't have to implement this now, so
> > >>>
> > >>> Reviewed-by: Laurent Pinchart <[email protected]>
> > >>>
> > >>> but if this feature could be required later, we may want to take into
> > >>> account in the naming of the new DT property to reflect the fact that it
> > >>> is the initial value.
> > >>
> > >> I had the same thought when I initially proposed
> > >> "maxim,initial-reverse-channel-mV"
> > >>
> > >> Having to use the standard unit suffix that would have become
> > >> "maxim,initial-reverse-channel-microvolt"
> > >> which is extremely long.
> > >>
> > >> I can't tell if there will be any need to adjust the amplitude later.
> > >> In any case, I would not rely on a DTS property to do so, as once we
> > >> have probed the remote we have a subdev where to call
> > >> 'get_mbus_config()' on, and from there we can report the high threshold
> > >> status of the serializer and adjust the deser amplitude accordingly.
> > >
> > > I don't think that's the point. The threshold of the serializer is
> > > something we can configure at runtime. What voltage level to use after
> >
> > How so ? I mean, we can add an API for this, but currently it's
> > configured at probe time and that's it. Its configuration might as
> > well come from a DT property like we do on the deserializer here but I
> > fail to see why it's different. Both settings depends on the required
> > noise immunity of th system.
>
> The voltage level configuration need to match between the tserializer
> (transmitter) and the deserializer (receiver). The serializer is
> configured with a voltage level, and the deserializer needs to be
> configured with a corresponding threshold.
>
If I'm not mistaken it's actually the other way around, at least for
the chips we're dealing with.
The serializer (MAX9271) has an "Reverse Channel Receiver High
Threshold Enable" bit (register 0x08[0]) undocumented in the chip
manual but described in the "MAX9286 Programming Guide 2 10.pdf"
document in the "Important Registers" section.
The deserializer (MAX9286) has instead a configurable setting for the reverse
channel signal amplitude, which is what we are controlling in this
series.
The deserializer reverse channel amplitude has to match the remote
side 'high threshold enable' setting. If it is enabled the amplitude
has to be increased to be able to probe the remote side. If it's not
a lower amplitude has to be used to make comunication reliable.
As you said, some models (RDACM20) might be pre-programmed with the
'high threshold enable' bit set, and so the deserializer reverse
channel amplitude has to be adjusted accordingly to be able to
comunicate on the reverse channel.
> The voltage level of the serializer is configurable on the camera side
> when the system is powered up. The RDACM20 has a microcontroller which
> can configure the serializer, and other cameras may have similar
> mechanisms. As the deserializer can't query the information from the
> serializer (communication is unreliable if the threshold has an
> incorrect value), we need a DT property to tell the deserializer what
> threshold is initially used by the camera when it gets powered up.
>
That's what this series does, yes.
> This only covers initialization. A camera could boot up with a low
> voltage level, but we may want to increase the voltage level (and thus
> the threshold on the deserializer side) to increase noise immunity. Or,
> if the system environment isn't noisy, we may want to keep a low voltage
> level, or even decrease it if the camera boots up with a high voltage
> level. This runtime voltage level depends on the system design and its
> susceptibility to noise, and is thus a system property. Should we want
> to make it configurable, it should be specified in DT, and it's separate
> from the initial voltage level that is used to establish communication.
>
And that's what I meant. Assuming we handle initialization correctly
with this series, the serializers 'high threshold' configuration
-after- initialization can be specified with a DT property on the
-serializer- side. Then, to adjust the deserializer reverse channel
amplitude, once we the remote has probed and we have a subdevice
registered for it, we can query the 'high threshold' configuration
using get_mbus_config() (or another API if we think it's better) and
adjust the deserializer accordingly.
All in all:
- yes, I think there might be a need to control the noise immunity
settings after initialization
- I think it should be done on the serializer side, possibly with a DT
property, possibly something like a boolean 'maxim,high-threshold-enable'
- the deserializer can query that information with a kAPI like
get_mbus_config() after the remote has probed
- Because of that there is no need for an additional deserializer property
Hope this makes sense
Hi Jacopo,
On Tue, Jan 12, 2021 at 10:07 AM Jacopo Mondi <[email protected]> wrote:
> On Tue, Jan 12, 2021 at 07:03:42AM +0200, Laurent Pinchart wrote:
> > On Mon, Jan 11, 2021 at 12:20:23PM +0100, Jacopo Mondi wrote:
> > > On Mon, Jan 11, 2021 at 12:58:59PM +0200, Laurent Pinchart wrote:
> > > > On Mon, Jan 11, 2021 at 11:43:11AM +0100, Jacopo Mondi wrote:
> > > >> On Wed, Dec 16, 2020 at 07:22:17PM +0200, Laurent Pinchart wrote:
> > > >>> On Tue, Dec 15, 2020 at 06:09:57PM +0100, Jacopo Mondi wrote:
> > > >>>> Adjust the initial reverse channel amplitude parsing from
> > > >>>> firmware interface the 'maxim,reverse-channel-microvolt'
> > > >>>> property.
> > > >>>>
> > > >>>> This change is required for both rdacm20 and rdacm21 camera
> > > >>>> modules to be correctly probed when used in combination with
> > > >>>> the max9286 deserializer.
> > > >>>>
> > > >>>> Reviewed-by: Kieran Bingham <[email protected]>
> > > >>>> Signed-off-by: Jacopo Mondi <[email protected]>
> > > >>>> ---
> > > >>>> drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
> > > >>>> 1 file changed, 22 insertions(+), 1 deletion(-)
> > > >>>>
> > > >>>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > > >>>> index 021309c6dd6f..9b40a4890c4d 100644
> > > >>>> --- a/drivers/media/i2c/max9286.c
> > > >>>> +++ b/drivers/media/i2c/max9286.c
> > > >>>> @@ -163,6 +163,8 @@ struct max9286_priv {
> > > >>>> unsigned int mux_channel;
> > > >>>> bool mux_open;
> > > >>>>
> > > >>>> + u32 reverse_channel_mv;
> > > >>>> +
> > > >>>> struct v4l2_ctrl_handler ctrls;
> > > >>>> struct v4l2_ctrl *pixelrate;
> > > >>>>
> > > >>>> @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> > > >>>> * All enabled sources have probed and enabled their reverse control
> > > >>>> * channels:
> > > >>>> *
> > > >>>> + * - Increase the reverse channel amplitude to compensate for the
> > > >>>> + * remote ends high threshold, if not done already
> > > >>>> * - Verify all configuration links are properly detected
> > > >>>> * - Disable auto-ack as communication on the control channel are now
> > > >>>> * stable.
> > > >>>> */
> > > >>>> + if (priv->reverse_channel_mv < 170)
> > > >>>> + max9286_reverse_channel_setup(priv, 170);
> > > >>>
> > > >>> I'm beginning to wonder if there will be a need in the future to not
> > > >>> increase the reverse channel amplitude (keeping the threshold low on the
> > > >>> remote side). An increased amplitude increases power consumption, and if
> > > >>> the environment isn't noisy, a low amplitude would work. The device tree
> > > >>> would then need to specify both the initial amplitude required by the
> > > >>> remote side, and the desired amplitude after initialization. What do you
> > > >>> think ? Is it overkill ? We don't have to implement this now, so
> > > >>>
> > > >>> Reviewed-by: Laurent Pinchart <[email protected]>
> > > >>>
> > > >>> but if this feature could be required later, we may want to take into
> > > >>> account in the naming of the new DT property to reflect the fact that it
> > > >>> is the initial value.
> > > >>
> > > >> I had the same thought when I initially proposed
> > > >> "maxim,initial-reverse-channel-mV"
> > > >>
> > > >> Having to use the standard unit suffix that would have become
> > > >> "maxim,initial-reverse-channel-microvolt"
> > > >> which is extremely long.
> > > >>
> > > >> I can't tell if there will be any need to adjust the amplitude later.
> > > >> In any case, I would not rely on a DTS property to do so, as once we
> > > >> have probed the remote we have a subdev where to call
> > > >> 'get_mbus_config()' on, and from there we can report the high threshold
> > > >> status of the serializer and adjust the deser amplitude accordingly.
> > > >
> > > > I don't think that's the point. The threshold of the serializer is
> > > > something we can configure at runtime. What voltage level to use after
> > >
> > > How so ? I mean, we can add an API for this, but currently it's
> > > configured at probe time and that's it. Its configuration might as
> > > well come from a DT property like we do on the deserializer here but I
> > > fail to see why it's different. Both settings depends on the required
> > > noise immunity of th system.
> >
> > The voltage level configuration need to match between the tserializer
> > (transmitter) and the deserializer (receiver). The serializer is
> > configured with a voltage level, and the deserializer needs to be
> > configured with a corresponding threshold.
> >
>
> If I'm not mistaken it's actually the other way around, at least for
> the chips we're dealing with.
>
> The serializer (MAX9271) has an "Reverse Channel Receiver High
> Threshold Enable" bit (register 0x08[0]) undocumented in the chip
> manual but described in the "MAX9286 Programming Guide 2 10.pdf"
> document in the "Important Registers" section.
>
> The deserializer (MAX9286) has instead a configurable setting for the reverse
> channel signal amplitude, which is what we are controlling in this
> series.
>
> The deserializer reverse channel amplitude has to match the remote
> side 'high threshold enable' setting. If it is enabled the amplitude
> has to be increased to be able to probe the remote side. If it's not
> a lower amplitude has to be used to make comunication reliable.
>
> As you said, some models (RDACM20) might be pre-programmed with the
> 'high threshold enable' bit set, and so the deserializer reverse
> channel amplitude has to be adjusted accordingly to be able to
> comunicate on the reverse channel.
>
> > The voltage level of the serializer is configurable on the camera side
> > when the system is powered up. The RDACM20 has a microcontroller which
> > can configure the serializer, and other cameras may have similar
> > mechanisms. As the deserializer can't query the information from the
> > serializer (communication is unreliable if the threshold has an
> > incorrect value), we need a DT property to tell the deserializer what
> > threshold is initially used by the camera when it gets powered up.
> >
>
> That's what this series does, yes.
>
> > This only covers initialization. A camera could boot up with a low
> > voltage level, but we may want to increase the voltage level (and thus
> > the threshold on the deserializer side) to increase noise immunity. Or,
> > if the system environment isn't noisy, we may want to keep a low voltage
> > level, or even decrease it if the camera boots up with a high voltage
> > level. This runtime voltage level depends on the system design and its
> > susceptibility to noise, and is thus a system property. Should we want
> > to make it configurable, it should be specified in DT, and it's separate
> > from the initial voltage level that is used to establish communication.
> >
>
> And that's what I meant. Assuming we handle initialization correctly
> with this series, the serializers 'high threshold' configuration
> -after- initialization can be specified with a DT property on the
> -serializer- side. Then, to adjust the deserializer reverse channel
> amplitude, once we the remote has probed and we have a subdevice
> registered for it, we can query the 'high threshold' configuration
> using get_mbus_config() (or another API if we think it's better) and
> adjust the deserializer accordingly.
>
> All in all:
> - yes, I think there might be a need to control the noise immunity
> settings after initialization
> - I think it should be done on the serializer side, possibly with a DT
> property, possibly something like a boolean 'maxim,high-threshold-enable'
Can the needed voltage level be calibrated at runtime, cfr. DDR
link training?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert
On Tue, Jan 12, 2021 at 10:10:39AM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Tue, Jan 12, 2021 at 10:07 AM Jacopo Mondi <[email protected]> wrote:
> > On Tue, Jan 12, 2021 at 07:03:42AM +0200, Laurent Pinchart wrote:
> > > On Mon, Jan 11, 2021 at 12:20:23PM +0100, Jacopo Mondi wrote:
> > > > On Mon, Jan 11, 2021 at 12:58:59PM +0200, Laurent Pinchart wrote:
> > > > > On Mon, Jan 11, 2021 at 11:43:11AM +0100, Jacopo Mondi wrote:
> > > > >> On Wed, Dec 16, 2020 at 07:22:17PM +0200, Laurent Pinchart wrote:
> > > > >>> On Tue, Dec 15, 2020 at 06:09:57PM +0100, Jacopo Mondi wrote:
> > > > >>>> Adjust the initial reverse channel amplitude parsing from
> > > > >>>> firmware interface the 'maxim,reverse-channel-microvolt'
> > > > >>>> property.
> > > > >>>>
> > > > >>>> This change is required for both rdacm20 and rdacm21 camera
> > > > >>>> modules to be correctly probed when used in combination with
> > > > >>>> the max9286 deserializer.
> > > > >>>>
> > > > >>>> Reviewed-by: Kieran Bingham <[email protected]>
> > > > >>>> Signed-off-by: Jacopo Mondi <[email protected]>
> > > > >>>> ---
> > > > >>>> drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
> > > > >>>> 1 file changed, 22 insertions(+), 1 deletion(-)
> > > > >>>>
> > > > >>>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > > > >>>> index 021309c6dd6f..9b40a4890c4d 100644
> > > > >>>> --- a/drivers/media/i2c/max9286.c
> > > > >>>> +++ b/drivers/media/i2c/max9286.c
> > > > >>>> @@ -163,6 +163,8 @@ struct max9286_priv {
> > > > >>>> unsigned int mux_channel;
> > > > >>>> bool mux_open;
> > > > >>>>
> > > > >>>> + u32 reverse_channel_mv;
> > > > >>>> +
> > > > >>>> struct v4l2_ctrl_handler ctrls;
> > > > >>>> struct v4l2_ctrl *pixelrate;
> > > > >>>>
> > > > >>>> @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> > > > >>>> * All enabled sources have probed and enabled their reverse control
> > > > >>>> * channels:
> > > > >>>> *
> > > > >>>> + * - Increase the reverse channel amplitude to compensate for the
> > > > >>>> + * remote ends high threshold, if not done already
> > > > >>>> * - Verify all configuration links are properly detected
> > > > >>>> * - Disable auto-ack as communication on the control channel are now
> > > > >>>> * stable.
> > > > >>>> */
> > > > >>>> + if (priv->reverse_channel_mv < 170)
> > > > >>>> + max9286_reverse_channel_setup(priv, 170);
> > > > >>>
> > > > >>> I'm beginning to wonder if there will be a need in the future to not
> > > > >>> increase the reverse channel amplitude (keeping the threshold low on the
> > > > >>> remote side). An increased amplitude increases power consumption, and if
> > > > >>> the environment isn't noisy, a low amplitude would work. The device tree
> > > > >>> would then need to specify both the initial amplitude required by the
> > > > >>> remote side, and the desired amplitude after initialization. What do you
> > > > >>> think ? Is it overkill ? We don't have to implement this now, so
> > > > >>>
> > > > >>> Reviewed-by: Laurent Pinchart <[email protected]>
> > > > >>>
> > > > >>> but if this feature could be required later, we may want to take into
> > > > >>> account in the naming of the new DT property to reflect the fact that it
> > > > >>> is the initial value.
> > > > >>
> > > > >> I had the same thought when I initially proposed
> > > > >> "maxim,initial-reverse-channel-mV"
> > > > >>
> > > > >> Having to use the standard unit suffix that would have become
> > > > >> "maxim,initial-reverse-channel-microvolt"
> > > > >> which is extremely long.
> > > > >>
> > > > >> I can't tell if there will be any need to adjust the amplitude later.
> > > > >> In any case, I would not rely on a DTS property to do so, as once we
> > > > >> have probed the remote we have a subdev where to call
> > > > >> 'get_mbus_config()' on, and from there we can report the high threshold
> > > > >> status of the serializer and adjust the deser amplitude accordingly.
> > > > >
> > > > > I don't think that's the point. The threshold of the serializer is
> > > > > something we can configure at runtime. What voltage level to use after
> > > >
> > > > How so ? I mean, we can add an API for this, but currently it's
> > > > configured at probe time and that's it. Its configuration might as
> > > > well come from a DT property like we do on the deserializer here but I
> > > > fail to see why it's different. Both settings depends on the required
> > > > noise immunity of th system.
> > >
> > > The voltage level configuration need to match between the tserializer
> > > (transmitter) and the deserializer (receiver). The serializer is
> > > configured with a voltage level, and the deserializer needs to be
> > > configured with a corresponding threshold.
> > >
> >
> > If I'm not mistaken it's actually the other way around, at least for
> > the chips we're dealing with.
> >
> > The serializer (MAX9271) has an "Reverse Channel Receiver High
> > Threshold Enable" bit (register 0x08[0]) undocumented in the chip
> > manual but described in the "MAX9286 Programming Guide 2 10.pdf"
> > document in the "Important Registers" section.
> >
> > The deserializer (MAX9286) has instead a configurable setting for the reverse
> > channel signal amplitude, which is what we are controlling in this
> > series.
> >
> > The deserializer reverse channel amplitude has to match the remote
> > side 'high threshold enable' setting. If it is enabled the amplitude
> > has to be increased to be able to probe the remote side. If it's not
> > a lower amplitude has to be used to make comunication reliable.
> >
> > As you said, some models (RDACM20) might be pre-programmed with the
> > 'high threshold enable' bit set, and so the deserializer reverse
> > channel amplitude has to be adjusted accordingly to be able to
> > comunicate on the reverse channel.
> >
> > > The voltage level of the serializer is configurable on the camera side
> > > when the system is powered up. The RDACM20 has a microcontroller which
> > > can configure the serializer, and other cameras may have similar
> > > mechanisms. As the deserializer can't query the information from the
> > > serializer (communication is unreliable if the threshold has an
> > > incorrect value), we need a DT property to tell the deserializer what
> > > threshold is initially used by the camera when it gets powered up.
> > >
> >
> > That's what this series does, yes.
> >
> > > This only covers initialization. A camera could boot up with a low
> > > voltage level, but we may want to increase the voltage level (and thus
> > > the threshold on the deserializer side) to increase noise immunity. Or,
> > > if the system environment isn't noisy, we may want to keep a low voltage
> > > level, or even decrease it if the camera boots up with a high voltage
> > > level. This runtime voltage level depends on the system design and its
> > > susceptibility to noise, and is thus a system property. Should we want
> > > to make it configurable, it should be specified in DT, and it's separate
> > > from the initial voltage level that is used to establish communication.
> > >
> >
> > And that's what I meant. Assuming we handle initialization correctly
> > with this series, the serializers 'high threshold' configuration
> > -after- initialization can be specified with a DT property on the
> > -serializer- side. Then, to adjust the deserializer reverse channel
> > amplitude, once we the remote has probed and we have a subdevice
> > registered for it, we can query the 'high threshold' configuration
> > using get_mbus_config() (or another API if we think it's better) and
> > adjust the deserializer accordingly.
> >
> > All in all:
> > - yes, I think there might be a need to control the noise immunity
> > settings after initialization
> > - I think it should be done on the serializer side, possibly with a DT
> > property, possibly something like a boolean 'maxim,high-threshold-enable'
>
> Can the needed voltage level be calibrated at runtime, cfr. DDR
> link training?
I'm not familiar with the mechanism you mention here, but being the
voltages levels a tuning parameters meant to maximize the reliability
of a communication channel which might show failure that cannot be
recovered easily [1] I fear it hardly can be run-time
calibrated but it's rather a decision that system integrators have to
make, depending on their usage environment and possibly a lot of
reliability testing.
Thanks
j
[1] To give a simple example, if the deserializer detects it cannot
communicate with the remote side (and that's also hard to guess, as
the start-up phase requires auto-ack of i2c messages transmitted on
the reverse channel) how could it 'suggest' to the remote side that it
should enable its noise immunity threshold ?
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Hi Jacopo,
On Tue, Jan 12, 2021 at 10:08:05AM +0100, Jacopo Mondi wrote:
> On Tue, Jan 12, 2021 at 07:03:42AM +0200, Laurent Pinchart wrote:
> > On Mon, Jan 11, 2021 at 12:20:23PM +0100, Jacopo Mondi wrote:
> > > On Mon, Jan 11, 2021 at 12:58:59PM +0200, Laurent Pinchart wrote:
> > > > On Mon, Jan 11, 2021 at 11:43:11AM +0100, Jacopo Mondi wrote:
> > > >> On Wed, Dec 16, 2020 at 07:22:17PM +0200, Laurent Pinchart wrote:
> > > >>> On Tue, Dec 15, 2020 at 06:09:57PM +0100, Jacopo Mondi wrote:
> > > >>>> Adjust the initial reverse channel amplitude parsing from
> > > >>>> firmware interface the 'maxim,reverse-channel-microvolt'
> > > >>>> property.
> > > >>>>
> > > >>>> This change is required for both rdacm20 and rdacm21 camera
> > > >>>> modules to be correctly probed when used in combination with
> > > >>>> the max9286 deserializer.
> > > >>>>
> > > >>>> Reviewed-by: Kieran Bingham <[email protected]>
> > > >>>> Signed-off-by: Jacopo Mondi <[email protected]>
> > > >>>> ---
> > > >>>> drivers/media/i2c/max9286.c | 23 ++++++++++++++++++++++-
> > > >>>> 1 file changed, 22 insertions(+), 1 deletion(-)
> > > >>>>
> > > >>>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > > >>>> index 021309c6dd6f..9b40a4890c4d 100644
> > > >>>> --- a/drivers/media/i2c/max9286.c
> > > >>>> +++ b/drivers/media/i2c/max9286.c
> > > >>>> @@ -163,6 +163,8 @@ struct max9286_priv {
> > > >>>> unsigned int mux_channel;
> > > >>>> bool mux_open;
> > > >>>>
> > > >>>> + u32 reverse_channel_mv;
> > > >>>> +
> > > >>>> struct v4l2_ctrl_handler ctrls;
> > > >>>> struct v4l2_ctrl *pixelrate;
> > > >>>>
> > > >>>> @@ -557,10 +559,14 @@ static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
> > > >>>> * All enabled sources have probed and enabled their reverse control
> > > >>>> * channels:
> > > >>>> *
> > > >>>> + * - Increase the reverse channel amplitude to compensate for the
> > > >>>> + * remote ends high threshold, if not done already
> > > >>>> * - Verify all configuration links are properly detected
> > > >>>> * - Disable auto-ack as communication on the control channel are now
> > > >>>> * stable.
> > > >>>> */
> > > >>>> + if (priv->reverse_channel_mv < 170)
> > > >>>> + max9286_reverse_channel_setup(priv, 170);
> > > >>>
> > > >>> I'm beginning to wonder if there will be a need in the future to not
> > > >>> increase the reverse channel amplitude (keeping the threshold low on the
> > > >>> remote side). An increased amplitude increases power consumption, and if
> > > >>> the environment isn't noisy, a low amplitude would work. The device tree
> > > >>> would then need to specify both the initial amplitude required by the
> > > >>> remote side, and the desired amplitude after initialization. What do you
> > > >>> think ? Is it overkill ? We don't have to implement this now, so
> > > >>>
> > > >>> Reviewed-by: Laurent Pinchart <[email protected]>
> > > >>>
> > > >>> but if this feature could be required later, we may want to take into
> > > >>> account in the naming of the new DT property to reflect the fact that it
> > > >>> is the initial value.
> > > >>
> > > >> I had the same thought when I initially proposed
> > > >> "maxim,initial-reverse-channel-mV"
> > > >>
> > > >> Having to use the standard unit suffix that would have become
> > > >> "maxim,initial-reverse-channel-microvolt"
> > > >> which is extremely long.
> > > >>
> > > >> I can't tell if there will be any need to adjust the amplitude later.
> > > >> In any case, I would not rely on a DTS property to do so, as once we
> > > >> have probed the remote we have a subdev where to call
> > > >> 'get_mbus_config()' on, and from there we can report the high threshold
> > > >> status of the serializer and adjust the deser amplitude accordingly.
> > > >
> > > > I don't think that's the point. The threshold of the serializer is
> > > > something we can configure at runtime. What voltage level to use after
> > >
> > > How so ? I mean, we can add an API for this, but currently it's
> > > configured at probe time and that's it. Its configuration might as
> > > well come from a DT property like we do on the deserializer here but I
> > > fail to see why it's different. Both settings depends on the required
> > > noise immunity of th system.
> >
> > The voltage level configuration need to match between the tserializer
> > (transmitter) and the deserializer (receiver). The serializer is
> > configured with a voltage level, and the deserializer needs to be
> > configured with a corresponding threshold.
>
> If I'm not mistaken it's actually the other way around, at least for
> the chips we're dealing with.
Yes, I mixed up the direction, sorry about that.
> The serializer (MAX9271) has an "Reverse Channel Receiver High
> Threshold Enable" bit (register 0x08[0]) undocumented in the chip
> manual but described in the "MAX9286 Programming Guide 2 10.pdf"
> document in the "Important Registers" section.
>
> The deserializer (MAX9286) has instead a configurable setting for the reverse
> channel signal amplitude, which is what we are controlling in this
> series.
>
> The deserializer reverse channel amplitude has to match the remote
> side 'high threshold enable' setting. If it is enabled the amplitude
> has to be increased to be able to probe the remote side. If it's not
> a lower amplitude has to be used to make comunication reliable.
>
> As you said, some models (RDACM20) might be pre-programmed with the
> 'high threshold enable' bit set, and so the deserializer reverse
> channel amplitude has to be adjusted accordingly to be able to
> comunicate on the reverse channel.
>
> > The voltage level of the serializer is configurable on the camera side
> > when the system is powered up. The RDACM20 has a microcontroller which
> > can configure the serializer, and other cameras may have similar
> > mechanisms. As the deserializer can't query the information from the
> > serializer (communication is unreliable if the threshold has an
> > incorrect value), we need a DT property to tell the deserializer what
> > threshold is initially used by the camera when it gets powered up.
>
> That's what this series does, yes.
>
> > This only covers initialization. A camera could boot up with a low
> > voltage level, but we may want to increase the voltage level (and thus
> > the threshold on the deserializer side) to increase noise immunity. Or,
> > if the system environment isn't noisy, we may want to keep a low voltage
> > level, or even decrease it if the camera boots up with a high voltage
> > level. This runtime voltage level depends on the system design and its
> > susceptibility to noise, and is thus a system property. Should we want
> > to make it configurable, it should be specified in DT, and it's separate
> > from the initial voltage level that is used to establish communication.
>
> And that's what I meant. Assuming we handle initialization correctly
> with this series, the serializers 'high threshold' configuration
> -after- initialization can be specified with a DT property on the
> -serializer- side. Then, to adjust the deserializer reverse channel
> amplitude, once we the remote has probed and we have a subdevice
> registered for it, we can query the 'high threshold' configuration
> using get_mbus_config() (or another API if we think it's better) and
> adjust the deserializer accordingly.
>
> All in all:
> - yes, I think there might be a need to control the noise immunity
> settings after initialization
> - I think it should be done on the serializer side, possibly with a DT
> property, possibly something like a boolean 'maxim,high-threshold-enable'
> - the deserializer can query that information with a kAPI like
> get_mbus_config() after the remote has probed
> - Because of that there is no need for an additional deserializer property
>
> Hope this makes sense
Now I get what you meant. Sorry for missing the point.
While it would be technically feasible to query the property from the
serializer at runtime, there's the additional issue that the
deserializer has a single reverse channel amplitude setting for all the
channels. We would need to ensure that the property is set to the same
value in all camera DT nodes. Wouldn't it be best to then set it once
only, in the deserializer node ?
--
Regards,
Laurent Pinchart
Hi Laurent,
On Thu, Jan 14, 2021 at 07:53:36AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> >
> > All in all:
> > - yes, I think there might be a need to control the noise immunity
> > settings after initialization
> > - I think it should be done on the serializer side, possibly with a DT
> > property, possibly something like a boolean 'maxim,high-threshold-enable'
> > - the deserializer can query that information with a kAPI like
> > get_mbus_config() after the remote has probed
> > - Because of that there is no need for an additional deserializer property
> >
> > Hope this makes sense
>
> Now I get what you meant. Sorry for missing the point.
>
> While it would be technically feasible to query the property from the
> serializer at runtime, there's the additional issue that the
> deserializer has a single reverse channel amplitude setting for all the
> channels. We would need to ensure that the property is set to the same
> value in all camera DT nodes. Wouldn't it be best to then set it once
> only, in the deserializer node ?
>
To be honest I wouldn't mind a run-time error, or a fallback like "the
first one to probe is the authoritative one, the rest have to follow".
And don't forget we would need a serializer property anyway to tell
the chip if it has to enable its noise immunity threshold or not.
But anyway, the here introduced new property already requires
knwoledge on the deserializer about which camera is connected on the
other side. It's not so bad, as if cameras are described in a .dtsi or
.dtbo the deserializer property can be overridden. We can do the same
for an additional property.
ie. a deserializer-serializer 'maxim,high-threshold-enable' property
RDACM20: pre-programmed high threshold enable
-------------- rdacm20.dtsi -------------------
&gmsl {
maxim,reverse-channel-microvolt = <170000>;
i2c-mux {
i2c@0 {
camera@51 {
....
}
}
}
};
-------------------------------------------------
RDACM21: no pre-programmed high-threshold, high threshold enabled
after camera probe
-------------- rdacm21.dtsi -------------------
&gmsl {
maxim,reverse-channel-microvolt = <100000>;
maxim,high-threshold-enable;
i2c-mux {
i2c@0 {
camera@51 {
maxim,high-threshold-enable;
....
}
}
}
};
-------------------------------------------------
RDACM21: no high-threshold enabled at all
-------------- rdacm21.dtsi -------------------
&gmsl {
maxim,reverse-channel-microvolt = <100000>;
i2c-mux {
i2c@0 {
camera@51 {
....
}
}
}
};
-------------------------------------------------
For the serializer it's a boolean, for the deser we might need to
specify a voltage, so it might become an uint32
'maxim,high-threshold-microvolt' there.
-------------- rdacm21.dtsi -------------------
&gmsl {
maxim,reverse-channel-microvolt = <100000>;
maxim,high-threshold-microvolt = <170000>;
i2c-mux {
i2c@0 {
camera@51 {
maxim,high-threshold-enable;
....
}
}
}
};
-------------------------------------------------
> --
> Regards,
>
> Laurent Pinchart