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

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: Mon, 2 Nov 2020 17:05:47 +0200	[thread overview]
Message-ID: <20201102150547.GY26150@paasikivi.fi.intel.com> (raw)
In-Reply-To: <20201019170247.92002-1-krzk@kernel.org>

Hi Krysztof,

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-*).
> 
> Changes since v3:
> 1. Document also two lane setup.
> 
> Changes since v2:
> 1. Remove clock-frequency, add reset GPIOs, add supplies.
> 2. Use additionalProperties.
> 
> Changes since v1:
> 1. None
> ---
>  .../devicetree/bindings/media/i2c/imx258.yaml | 140 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 141 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx258.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> new file mode 100644
> index 000000000000..4a3471fb88a1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
> +
> +maintainers:
> +  - Krzysztof Kozlowski <krzk@kernel.org>
> +
> +description: |-
> +  IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> +  type stacked image sensor with a square pixel array of size 4208 x 3120. It
> +  is programmable through I2C interface.  Image data is sent through MIPI
> +  CSI-2.
> +
> +properties:
> +  compatible:
> +    const: sony,imx258
> +
> +  assigned-clocks: true
> +  assigned-clock-parents: true
> +  assigned-clock-rates: true

I discussed the matter of using assigned clocks with Rob some time ago and
the conclusion of that was that the sensor driver could use the default
frequency (set using assigned-clock-rates) instead of the explicit
frequency in DT. There are use cases (sharing the clock signal between two
sensors, but different frequencies) that would be affected by this but I
don't think we have any in mainline so I guess this approach works for now
without additional changes. If someone needs those use cases, it's likely
DT clock binding semantings and clock framework changes will be needed.
That'll be another discussion if it ever happens.

-- 
Regards,

Sakari Ailus