linux-kernel - Re: [PATCH v5 1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Oct 2020 12:54:09 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        linux-media@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Rob Herring <robh@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, linux-clk@...r.kernel.org
Subject: Re: [PATCH v5 1/4] dt-bindings: media: imx258: add bindings for
 IMX258 sensor

On Tue, 20 Oct 2020 at 12:38, Sakari Ailus <sakari.ailus@...ux.intel.com> wrote:
>
> Hi Krzysztof,
>
> On Mon, Oct 19, 2020 at 07:02:44PM +0200, Krzysztof Kozlowski wrote:
> > Add bindings for the IMX258 camera sensor.  The bindings, just like the
> > driver, are quite limited, e.g. do not support regulator supplies.
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@...nel.org>
> > Reviewed-by: Rob Herring <robh@...nel.org>
> >
> > ---
> >
> > Changes since v4:
> > 1. Add clock-lanes,
> > 2. Add Rob's review,
> > 3. Add one more example and extend existing one,
> > 4. Add common clock properties (assigned-*).
>
> Using the assigned-* clock properties may be workable for this driver at
> the moment. But using these properties does not guarantee the external
> clock frequency intended to be used on the hardware.

It guarantees it. The clock frequency will be as expected (except if
someone misconfigures the DTS).

> Using other
> frequencies *is not* expected to work. That applies to this driver as well.

This is the binding which is HW description. According to HW datasheet
other frequencies from described range are accepted and expected to
work.

> This, instead of the clock-frequency property, effectively removes the
> ability to set the correct frequency from the driver, at least with current
> set of the used APIs.

It seems you confuse DT bindings with some specific driver
implementation. Bindings do not describe the driver behavior but the
HW. The ability to set the correct frequency from the driver is not
removed. It was never part of the bindings and never should. It is
part of the driver.

>
> I suppose you could add a function to set the assigned clock frequency and
> keep it, just as clk_set_rate_exclusive does?
>
> Cc the common clock framework list + maintainers.

The bindings have Rob review which is the DT maintainer. His
ack/review is needed for the bindings to be accepted. What more do you
need? Shall I point to submitting-bindings document?

I am really tired of discussing this. You raise some concerns about
driver behavior in the wrong context - in the patch for device tree
bindings. You use the arguments about the driver while we talk about
bindings. This is clearly not correct. I am all the time repeating
myself - the bindings describe the hardware, not the driver.

Best regards,
Krzysztof