| 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
| ||
|
Message-ID: <20250702092858.GA15241@pendragon.ideasonboard.com>
Date: Wed, 2 Jul 2025 12:28:58 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Will Whang <will@...lwhang.com>
Cc: Sakari Ailus <sakari.ailus@...ux.intel.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
"open list:SONY IMX585 SENSOR DRIVER" <linux-media@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v1 1/4] dt-bindings: media: Add Sony IMX585 CMOS image
sensor
Hi Will,
Thank you for the patch.
On Wed, Jul 02, 2025 at 07:38:33AM +0100, Will Whang wrote:
> Document the devicetree binding for the Sony IMX585. The schema
> covers the CSI-2 data-lanes, the optional 'mono-mode' flag,
> and the internal-sync properties used by the driver.
>
> Signed-off-by: Will Whang <will@...lwhang.com>
> ---
> .../bindings/media/i2c/sony,imx585.yaml | 120 ++++++++++++++++++
> MAINTAINERS | 8 ++
> 2 files changed, 128 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> new file mode 100644
> index 000000000..d050d1642
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> @@ -0,0 +1,120 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2024 Ideas on Board Oy
Unless there's something I'm not aware of, I don't think Ideas on Board
wrote this. You can use your own copyright.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/sony,imx585.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony IMX585 Sensor
> +
> +maintainers:
> + - Will Whang <will@...lwhang.com>
> +
> +description:
> + IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> +
You should add
allOf:
- $ref: /schemas/media/video-interface-devices.yaml#
here to support generic sensor properties. You will need to replace
additionalProperties: false
with
unevaluatedProperties: false
below.
> +properties:
> + compatible:
> + const: sony,imx585
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: xclk
When there's a single clock you can drop clock-names.
> +
> + clock-frequency:
> + enum: [ 74250000, 37125000, 72000000, 27000000, 24000000 ]
The clock-frequency property is frowned upon for sensors in DT. If the
aim is to set the frequency of the clock, it should be done through
assigned-clocks and assigned-clock-rates. If the aim is to convey the
clock frequency to the driver, it should be done by calling
clk_get_rate() in the driver.
> +
> + reg:
> + maxItems: 1
> + description: I2C Address for IMX585
You can drop the description, it's always the same for I2C devices.
> +
> + VANA-supply:
> + description: Analog power supply (3.3V)
> +
> + VDDL-supply:
> + description: Interface power supply (1.8V)
> +
> + VDIG-supply:
> + description: Digital power supply (1.1V)
> +
> + reset-gpios:
> + description: Sensor reset (XCLR) GPIO
> + maxItems: 1
> +
> + port:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + additionalProperties: false
> +
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + data-lanes:
> + anyOf:
> + - items:
> + - const: 1
> + - const: 2
> + - const: 3
> + - const: 4
Does that mean that the sensor supports data lane remapping ? I don't
see it implemented by the driver. If it's not supported by the hardware,
you should use
properties:
data-lanes:
minItems: 1
items:
- const: 1
- const: 2
- const: 3
- const: 4
To guarantee the order.
> +
> + sync-mode:
> + description: |
> + Select the synchronisation mode of the sensor
> + 0 – internal sync, leader (default)
> + 1 – internal sync, follower
> + 2 – external sync
> + $ref: /schemas/types.yaml#/definitions/uint8
> + enum: [ 0, 1, 2 ]
This seems to be a sensor-level property, not an endpoint property. As
it's not standard, it should also have a vendor prefix, i.e.
sony,sync-mode. I'm wondering, though, if we shouldn't try to
standardize it in video-interface-devices.yaml.
> +
> + link-frequencies:
> + description: Select the MIPI-CSI2 link speed in Mhz
You can drop the description, it's already described in
video-interfaces.yaml.
> + items:
> + enum: [ 297000000, 360000000, 445500000, 594000000,
> + 720000000, 891000000, 1039500000 ]
Are those frequencies the only ones the hardware can support, or do they
come from the driver only supporting a fixed set of sensor PLL
configurations ? In the latter case I would drop the enumeration.
> +
> + required:
> + - data-lanes
> + - link-frequencies
> +
> + required:
> + - endpoint
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-frequency
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + imx585@1a {
> + compatible = "sony,imx585";
> + reg = <0x1a>;
> + clocks = <&imx585_clk>;
> + clock-frequency = <24000000>;
> +
> + VANA-supply = <&camera_vadd_3v3>;
> + VDDL-supply = <&camera_vdd1_1v8>;
> + VDIG-supply = <&camera_vdd2_1v1>;
> +
> + port {
> + imx585: endpoint {
> + remote-endpoint = <&cam>;
> + data-lanes = <1 2 3 4>;
> + link-frequencies = /bits/ 64 <720000000>;
> + };
> + };
> + };
> + };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index da34c7227..9cc404790 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23150,6 +23150,14 @@ T: git git://linuxtv.org/media.git
> F: Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> F: drivers/media/i2c/imx415.c
>
> +SONY IMX585 SENSOR DRIVER
> +M: Will Whang <will@...lwhang.com>
> +L: linux-media@...r.kernel.org
> +S: Maintained
> +T: git git://linuxtv.org/media.git
> +F: Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
> +F: drivers/media/i2c/imx585.c
> +
> SONY MEMORYSTICK SUBSYSTEM
> M: Maxim Levitsky <maximlevitsky@...il.com>
> M: Alex Dubov <oakad@...oo.com>
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists