From: Krzysztof Kozlowski <krzk@kernel.org> To: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh+dt@kernel.org>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix Kernel Team <kernel@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, NXP Linux Team <linux-imx@nxp.com>, linux-media@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Rob Herring <robh@kernel.org>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, linux-clk@vger.kernel.org Subject: Re: [PATCH v5 1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor Date: Tue, 20 Oct 2020 12:54:09 +0200 [thread overview] Message-ID: <CAJKOXPdKHCQikYDLKDS2Y3NVyCjMYYH1pcxgaMoYgOK19vQCrQ@mail.gmail.com> (raw) In-Reply-To: <20201020103833.GT13341@paasikivi.fi.intel.com> On Tue, 20 Oct 2020 at 12:38, Sakari Ailus <sakari.ailus@linux.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@kernel.org> > > Reviewed-by: Rob Herring <robh@kernel.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
WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzk@kernel.org> To: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: devicetree@vger.kernel.org, Rob Herring <robh@kernel.org>, Stephen Boyd <sboyd@kernel.org>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-clk@vger.kernel.org, Rob Herring <robh+dt@kernel.org>, NXP Linux Team <linux-imx@nxp.com>, Pengutronix Kernel Team <kernel@pengutronix.de>, Mauro Carvalho Chehab <mchehab@kernel.org>, Fabio Estevam <festevam@gmail.com>, Michael Turquette <mturquette@baylibre.com>, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Subject: Re: [PATCH v5 1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor Date: Tue, 20 Oct 2020 12:54:09 +0200 [thread overview] Message-ID: <CAJKOXPdKHCQikYDLKDS2Y3NVyCjMYYH1pcxgaMoYgOK19vQCrQ@mail.gmail.com> (raw) In-Reply-To: <20201020103833.GT13341@paasikivi.fi.intel.com> On Tue, 20 Oct 2020 at 12:38, Sakari Ailus <sakari.ailus@linux.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@kernel.org> > > Reviewed-by: Rob Herring <robh@kernel.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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-10-20 10:54 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-10-19 17:02 [PATCH v5 1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor Krzysztof Kozlowski 2020-10-19 17:02 ` Krzysztof Kozlowski 2020-10-19 17:26 ` [PATCH v5 2/4] media: i2c: imx258: add support for binding via device tree Krzysztof Kozlowski 2020-10-19 17:26 ` Krzysztof Kozlowski 2020-10-19 17:26 ` [PATCH v5 3/4] media: i2c: imx258: simplify getting state container Krzysztof Kozlowski 2020-10-19 17:26 ` Krzysztof Kozlowski 2020-10-19 17:26 ` [PATCH v5 4/4] media: i2c: imx258: get clock from device properties and enable it via runtime PM Krzysztof Kozlowski 2020-10-19 17:26 ` Krzysztof Kozlowski 2020-11-02 15:08 ` Sakari Ailus 2020-11-02 15:08 ` Sakari Ailus 2020-11-18 20:27 ` Krzysztof Kozlowski 2020-11-18 20:27 ` Krzysztof Kozlowski 2020-10-20 10:38 ` [PATCH v5 1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor Sakari Ailus 2020-10-20 10:38 ` Sakari Ailus 2020-10-20 10:54 ` Krzysztof Kozlowski [this message] 2020-10-20 10:54 ` Krzysztof Kozlowski 2020-10-20 12:00 ` Sakari Ailus 2020-10-20 12:00 ` Sakari Ailus 2020-10-20 12:26 ` Krzysztof Kozlowski 2020-10-20 12:26 ` Krzysztof Kozlowski 2020-10-20 12:46 ` Sakari Ailus 2020-10-20 12:46 ` Sakari Ailus 2020-10-20 12:58 ` Krzysztof Kozlowski 2020-10-20 12:58 ` Krzysztof Kozlowski 2020-10-28 8:38 ` Krzysztof Kozlowski 2020-10-28 8:38 ` Krzysztof Kozlowski 2020-11-02 15:05 ` Sakari Ailus 2020-11-02 15:05 ` 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=CAJKOXPdKHCQikYDLKDS2Y3NVyCjMYYH1pcxgaMoYgOK19vQCrQ@mail.gmail.com \ --to=krzk@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=festevam@gmail.com \ --cc=kernel@pengutronix.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-imx@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=mturquette@baylibre.com \ --cc=robh+dt@kernel.org \ --cc=robh@kernel.org \ --cc=s.hauer@pengutronix.de \ --cc=sakari.ailus@linux.intel.com \ --cc=sboyd@kernel.org \ --cc=shawnguo@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.