All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
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, 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 13:38:33 +0300	[thread overview]
Message-ID: <20201020103833.GT13341@paasikivi.fi.intel.com> (raw)
In-Reply-To: <20201019170247.92002-1-krzk@kernel.org>

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. Using other
frequencies *is not* expected to work. That applies to this driver as well.

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.

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.

-- 
Regards,

Sakari Ailus

WARNING: multiple messages have this Message-ID (diff)
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
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-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 13:38:33 +0300	[thread overview]
Message-ID: <20201020103833.GT13341@paasikivi.fi.intel.com> (raw)
In-Reply-To: <20201019170247.92002-1-krzk@kernel.org>

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. Using other
frequencies *is not* expected to work. That applies to this driver as well.

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.

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.

-- 
Regards,

Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2020-10-20 10:38 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 ` Sakari Ailus [this message]
2020-10-20 10:38   ` [PATCH v5 1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor Sakari Ailus
2020-10-20 10:54   ` Krzysztof Kozlowski
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=20201020103833.GT13341@paasikivi.fi.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=krzk@kernel.org \
    --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=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: link
Be 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.