linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jacopo Mondi <jacopo@jmondi.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Jacopo Mondi <jacopo+renesas@jmondi.org>,
	kieran.bingham+renesas@ideasonboard.com,
	laurent.pinchart+renesas@ideasonboard.com,
	niklas.soderlund+renesas@ragnatech.se, geert@linux-m68k.org,
	linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org, Hyun Kwon <hyunk@xilinx.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	sergei.shtylyov@gmail.com
Subject: Re: [PATCH v6 5/5] media: i2c: max9286: Configure reverse channel amplitude
Date: Thu, 14 Jan 2021 09:09:27 +0100	[thread overview]
Message-ID: <20210114080927.idz5v472ex25p5r4@uno.localdomain> (raw)
In-Reply-To: <X//cYHkyELaH4XHb@pendragon.ideasonboard.com>

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

      reply	other threads:[~2021-01-14  8:10 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-15 17:09 [PATCH v6 0/5] media: i2c: Add RDACM21 camera module Jacopo Mondi
2020-12-15 17:09 ` [PATCH v6 1/5] media: i2c: Add driver for " Jacopo Mondi
2020-12-16 17:00   ` Laurent Pinchart
2020-12-15 17:09 ` [PATCH v6 2/5] dt-bindings: media: max9286: Document 'maxim,reverse-channel-microvolt' Jacopo Mondi
2020-12-16 17:05   ` Laurent Pinchart
2020-12-16 17:17     ` Laurent Pinchart
2020-12-21 18:58       ` Rob Herring
2020-12-22  8:53         ` Jacopo Mondi
2020-12-15 17:09 ` [PATCH v6 3/5] media: i2c: max9286: Break-out reverse channel setup Jacopo Mondi
2020-12-16 17:06   ` Laurent Pinchart
2020-12-15 17:09 ` [PATCH v6 4/5] media: i2c: max9286: Make channel amplitude programmable Jacopo Mondi
2020-12-16 17:14   ` Laurent Pinchart
2020-12-16 17:14     ` Laurent Pinchart
2020-12-15 17:09 ` [PATCH v6 5/5] media: i2c: max9286: Configure reverse channel amplitude Jacopo Mondi
2020-12-16 17:22   ` Laurent Pinchart
2021-01-11 10:43     ` Jacopo Mondi
2021-01-11 10:58       ` Laurent Pinchart
2021-01-11 11:20         ` Jacopo Mondi
2021-01-12  5:03           ` Laurent Pinchart
2021-01-12  9:08             ` Jacopo Mondi
2021-01-12  9:10               ` Geert Uytterhoeven
2021-01-12 10:00                 ` Jacopo Mondi
2021-01-14  5:53               ` Laurent Pinchart
2021-01-14  8:09                 ` Jacopo Mondi [this message]

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=20210114080927.idz5v472ex25p5r4@uno.localdomain \
    --to=jacopo@jmondi.org \
    --cc=geert@linux-m68k.org \
    --cc=hyunk@xilinx.com \
    --cc=jacopo+renesas@jmondi.org \
    --cc=kieran.bingham+renesas@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=manivannan.sadhasivam@linaro.org \
    --cc=niklas.soderlund+renesas@ragnatech.se \
    --cc=sergei.shtylyov@gmail.com \
    /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).