[PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor

Will Whang posted 4 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Will Whang 1 month, 3 weeks ago
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@willwhang.com>
---
 .../bindings/media/i2c/sony,imx585.yaml       | 116 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 2 files changed, 122 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..020b7cfb9
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
@@ -0,0 +1,116 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/sony,imx585.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony IMX585 CMOS image sensor
+
+maintainers:
+  - Will Whang <will@willwhang.com>
+
+description:
+  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
+
+properties:
+  compatible:
+    enum:
+      - sony,imx585
+      - sony,imx585-mono
+
+  reg:
+    maxItems: 1
+
+  assigned-clocks: true
+  assigned-clock-parents: true
+  assigned-clock-rates: true
+
+  clocks:
+    description: Clock frequency 74.25MHz, 37.125MHz, 72MHz, 27MHz, 24MHz
+    maxItems: 1
+
+  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
+
+  sony,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 ]
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            oneOf:
+              - items:
+                  - const: 1
+                  - const: 2
+              - items:
+                  - const: 1
+                  - const: 2
+                  - const: 3
+                  - const: 4
+
+          link-frequencies: true
+
+        required:
+          - data-lanes
+          - link-frequencies
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - port
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        imx585@1a {
+            compatible = "sony,imx585";
+            reg = <0x1a>;
+            clocks = <&imx585_clk>;
+
+            assigned-clocks = <&imx585_clk>;
+            assigned-clock-rates = <24000000>;
+
+            vana-supply = <&camera_vadd_3v3>;
+            vdig-supply = <&camera_vdd1_1v8>;
+            vddl-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 24c557ee0..ef04ee4ec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23178,6 +23178,12 @@ 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@willwhang.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/media/i2c/sony,imx585.yaml
+
 SONY MEMORYSTICK SUBSYSTEM
 M:	Maxim Levitsky <maximlevitsky@gmail.com>
 M:	Alex Dubov <oakad@yahoo.com>
-- 
2.39.5

Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Krzysztof Kozlowski 1 month, 3 weeks ago
On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
> +description:
> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sony,imx585
> +      - sony,imx585-mono

I don't understand this second compatible. Is this different hardware?
Can you point me to "mono" datasheet?

Your description should explain this. Commit msg as well, instead of
speaking about driver (in fact drop all driver related comments).

> +
> +  reg:
> +    maxItems: 1
> +
> +  assigned-clocks: true
> +  assigned-clock-parents: true
> +  assigned-clock-rates: true

Drop all three.

> +
> +  clocks:
> +    description: Clock frequency 74.25MHz, 37.125MHz, 72MHz, 27MHz, 24MHz
> +    maxItems: 1
> +
> +  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
> +
> +  sony,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 ]

Previous comments not applied.

> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    additionalProperties: false
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            oneOf:
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +                  - const: 3
> +                  - const: 4
> +
> +          link-frequencies: true

Drop

> +
> +        required:
> +          - data-lanes
> +          - link-frequencies
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - port
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        imx585@1a {

Nothing improved.

You replied that you applied comment, but send the same.


Best regards,
Krzysztof
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Will Whang 1 month, 3 weeks ago
Hi Krzysztof,
Reply inline.
Thanks,
Will Whang

On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
> > +description:
> > +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - sony,imx585
> > +      - sony,imx585-mono
>
> I don't understand this second compatible. Is this different hardware?
> Can you point me to "mono" datasheet?
>
> Your description should explain this. Commit msg as well, instead of
> speaking about driver (in fact drop all driver related comments).
>
Mono version of this sensor is basically just removing the bayer
filter, so the sensor itself actually doesn't know if it is color or
mono and from my knowledge there are no registers programmed in the
factory that will show the variant and model number. (That is why when
the driver probing it only test blacklevel register because there are
no ID registers)
Originally in V1 patch I've made the switch between color and mono in
dtoverlay config but reviewer comments is to move it to compatible
string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)

In this case, what would you recommend?

compatible:
  enum:
    - sony,imx585
    - sony,imx585-mono
  description: IMX585 has two variants, color and mono which the
driver supports both.

Something like this?


> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  assigned-clocks: true
> > +  assigned-clock-parents: true
> > +  assigned-clock-rates: true
>
> Drop all three.
>
> > +
> > +  clocks:
> > +    description: Clock frequency 74.25MHz, 37.125MHz, 72MHz, 27MHz, 24MHz
> > +    maxItems: 1
> > +
> > +  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
> > +
> > +  sony,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 ]
>
> Previous comments not applied.
>
> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: /schemas/media/video-interfaces.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          data-lanes:
> > +            oneOf:
> > +              - items:
> > +                  - const: 1
> > +                  - const: 2
> > +              - items:
> > +                  - const: 1
> > +                  - const: 2
> > +                  - const: 3
> > +                  - const: 4
> > +
> > +          link-frequencies: true
>
> Drop
Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml and
sony,283.yaml doesn't seem to drop this?
Just to double check if my understanding is correct, you want me to
remove the line link-frequencies: true?

>
> > +
> > +        required:
> > +          - data-lanes
> > +          - link-frequencies
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - port
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        imx585@1a {
>
> Nothing improved.
>
> You replied that you applied comment, but send the same.
>
>
> Best regards,
> Krzysztof
>
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Krzysztof Kozlowski 1 month, 3 weeks ago
On 12/08/2025 04:47, Will Whang wrote:
> Hi Krzysztof,
> Reply inline.
> Thanks,
> Will Whang
> 
> On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
>>> +description:
>>> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - sony,imx585
>>> +      - sony,imx585-mono
>>
>> I don't understand this second compatible. Is this different hardware?
>> Can you point me to "mono" datasheet?
>>
>> Your description should explain this. Commit msg as well, instead of
>> speaking about driver (in fact drop all driver related comments).
>>
> Mono version of this sensor is basically just removing the bayer
> filter, so the sensor itself actually doesn't know if it is color or
> mono and from my knowledge there are no registers programmed in the
> factory that will show the variant and model number. (That is why when
> the driver probing it only test blacklevel register because there are
> no ID registers)
> Originally in V1 patch I've made the switch between color and mono in
> dtoverlay config but reviewer comments is to move it to compatible
> string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)

You only partially answer and judging by mentioning driver below:


> 
> In this case, what would you recommend?
> 
> compatible:
>   enum:
>     - sony,imx585
>     - sony,imx585-mono
>   description: IMX585 has two variants, color and mono which the
> driver supports both.

... I still have doubts that you really understand what I am asking. Is
this one device or two different devices?


Best regards,
Krzysztof
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Will Whang 1 month, 3 weeks ago
Hi Krzysztof,

Reply inline,
Thanks,
Will Whang

On Mon, Aug 11, 2025 at 11:23 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 12/08/2025 04:47, Will Whang wrote:
> > Hi Krzysztof,
> > Reply inline.
> > Thanks,
> > Will Whang
> >
> > On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
> >>> +description:
> >>> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - sony,imx585
> >>> +      - sony,imx585-mono
> >>
> >> I don't understand this second compatible. Is this different hardware?
> >> Can you point me to "mono" datasheet?
> >>
> >> Your description should explain this. Commit msg as well, instead of
> >> speaking about driver (in fact drop all driver related comments).
> >>
> > Mono version of this sensor is basically just removing the bayer
> > filter, so the sensor itself actually doesn't know if it is color or
> > mono and from my knowledge there are no registers programmed in the
> > factory that will show the variant and model number. (That is why when
> > the driver probing it only test blacklevel register because there are
> > no ID registers)
> > Originally in V1 patch I've made the switch between color and mono in
> > dtoverlay config but reviewer comments is to move it to compatible
> > string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)
>
> You only partially answer and judging by mentioning driver below:
>
>
> >
> > In this case, what would you recommend?
> >
> > compatible:
> >   enum:
> >     - sony,imx585
> >     - sony,imx585-mono
> >   description: IMX585 has two variants, color and mono which the
> > driver supports both.
>
> ... I still have doubts that you really understand what I am asking. Is
> this one device or two different devices?
One device that has two variants: IMX585-AAMJ1 (Mono) and IMX585-AAQJ1
(Color). Silicon-wise the difference is just with or without bayer
filter.
>
>
> Best regards,
> Krzysztof
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Krzysztof Kozlowski 1 month, 3 weeks ago
On 12/08/2025 08:31, Will Whang wrote:
> Hi Krzysztof,
> 
> Reply inline,
> Thanks,
> Will Whang
> 
> On Mon, Aug 11, 2025 at 11:23 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 12/08/2025 04:47, Will Whang wrote:
>>> Hi Krzysztof,
>>> Reply inline.
>>> Thanks,
>>> Will Whang
>>>
>>> On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
>>>>> +description:
>>>>> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - sony,imx585
>>>>> +      - sony,imx585-mono
>>>>
>>>> I don't understand this second compatible. Is this different hardware?
>>>> Can you point me to "mono" datasheet?
>>>>
>>>> Your description should explain this. Commit msg as well, instead of
>>>> speaking about driver (in fact drop all driver related comments).
>>>>
>>> Mono version of this sensor is basically just removing the bayer
>>> filter, so the sensor itself actually doesn't know if it is color or
>>> mono and from my knowledge there are no registers programmed in the
>>> factory that will show the variant and model number. (That is why when
>>> the driver probing it only test blacklevel register because there are
>>> no ID registers)
>>> Originally in V1 patch I've made the switch between color and mono in
>>> dtoverlay config but reviewer comments is to move it to compatible
>>> string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)
>>
>> You only partially answer and judging by mentioning driver below:
>>
>>
>>>
>>> In this case, what would you recommend?
>>>
>>> compatible:
>>>   enum:
>>>     - sony,imx585
>>>     - sony,imx585-mono
>>>   description: IMX585 has two variants, color and mono which the
>>> driver supports both.
>>
>> ... I still have doubts that you really understand what I am asking. Is
>> this one device or two different devices?
> One device that has two variants: IMX585-AAMJ1 (Mono) and IMX585-AAQJ1
> (Color). Silicon-wise the difference is just with or without bayer
> filter.
Then I would propose to use sony,imx585-aamj1 and -aaqj1 with short
explanation either in comment or description about difference in RGB
mosaic filter.

Best regards,
Krzysztof
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Laurent Pinchart 1 month, 3 weeks ago
On Tue, Aug 12, 2025 at 08:47:12AM +0200, Krzysztof Kozlowski wrote:
> On 12/08/2025 08:31, Will Whang wrote:
> > On Mon, Aug 11, 2025 at 11:23 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >> On 12/08/2025 04:47, Will Whang wrote:
> >>> On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
> >>>>> +description:
> >>>>> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    enum:
> >>>>> +      - sony,imx585
> >>>>> +      - sony,imx585-mono
> >>>>
> >>>> I don't understand this second compatible. Is this different hardware?
> >>>> Can you point me to "mono" datasheet?
> >>>>
> >>>> Your description should explain this. Commit msg as well, instead of
> >>>> speaking about driver (in fact drop all driver related comments).
> >>>>
> >>> Mono version of this sensor is basically just removing the bayer
> >>> filter, so the sensor itself actually doesn't know if it is color or
> >>> mono and from my knowledge there are no registers programmed in the
> >>> factory that will show the variant and model number. (That is why when
> >>> the driver probing it only test blacklevel register because there are
> >>> no ID registers)
> >>> Originally in V1 patch I've made the switch between color and mono in
> >>> dtoverlay config but reviewer comments is to move it to compatible
> >>> string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)
> >>
> >> You only partially answer and judging by mentioning driver below:
> >>
> >>> In this case, what would you recommend?
> >>>
> >>> compatible:
> >>>   enum:
> >>>     - sony,imx585
> >>>     - sony,imx585-mono
> >>>   description: IMX585 has two variants, color and mono which the
> >>> driver supports both.
> >>
> >> ... I still have doubts that you really understand what I am asking. Is
> >> this one device or two different devices?
> > 
> > One device that has two variants: IMX585-AAMJ1 (Mono) and IMX585-AAQJ1
> > (Color). Silicon-wise the difference is just with or without bayer
> > filter.
> 
> Then I would propose to use sony,imx585-aamj1 and -aaqj1 with short
> explanation either in comment or description about difference in RGB
> mosaic filter.

Works for me. We could possibly omit the "j1" suffix too.

-- 
Regards,

Laurent Pinchart
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Will Whang 1 month, 3 weeks ago
On Tue, Aug 12, 2025 at 2:56 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Tue, Aug 12, 2025 at 08:47:12AM +0200, Krzysztof Kozlowski wrote:
> > On 12/08/2025 08:31, Will Whang wrote:
> > > On Mon, Aug 11, 2025 at 11:23 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >> On 12/08/2025 04:47, Will Whang wrote:
> > >>> On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >>>> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
> > >>>>> +description:
> > >>>>> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> > >>>>> +
> > >>>>> +properties:
> > >>>>> +  compatible:
> > >>>>> +    enum:
> > >>>>> +      - sony,imx585
> > >>>>> +      - sony,imx585-mono
> > >>>>
> > >>>> I don't understand this second compatible. Is this different hardware?
> > >>>> Can you point me to "mono" datasheet?
> > >>>>
> > >>>> Your description should explain this. Commit msg as well, instead of
> > >>>> speaking about driver (in fact drop all driver related comments).
> > >>>>
> > >>> Mono version of this sensor is basically just removing the bayer
> > >>> filter, so the sensor itself actually doesn't know if it is color or
> > >>> mono and from my knowledge there are no registers programmed in the
> > >>> factory that will show the variant and model number. (That is why when
> > >>> the driver probing it only test blacklevel register because there are
> > >>> no ID registers)
> > >>> Originally in V1 patch I've made the switch between color and mono in
> > >>> dtoverlay config but reviewer comments is to move it to compatible
> > >>> string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)
> > >>
> > >> You only partially answer and judging by mentioning driver below:
> > >>
> > >>> In this case, what would you recommend?
> > >>>
> > >>> compatible:
> > >>>   enum:
> > >>>     - sony,imx585
> > >>>     - sony,imx585-mono
> > >>>   description: IMX585 has two variants, color and mono which the
> > >>> driver supports both.
> > >>
> > >> ... I still have doubts that you really understand what I am asking. Is
> > >> this one device or two different devices?
> > >
> > > One device that has two variants: IMX585-AAMJ1 (Mono) and IMX585-AAQJ1
> > > (Color). Silicon-wise the difference is just with or without bayer
> > > filter.
> >
> > Then I would propose to use sony,imx585-aamj1 and -aaqj1 with short
> > explanation either in comment or description about difference in RGB
> > mosaic filter.
>
> Works for me. We could possibly omit the "j1" suffix too.
>
My thinking is that imx585 and imx585-mono are easier to comprehend
than IMX585-AAM and IMX585-AAQ.
Because in dtoverlay for the users/me they will have to know what is
the exact name instead of easy to remember name.

dtoverlay=imx585-aam
is not as nice as
dtoverlay=imx585-mono

which is what it does, a mono variant of the sensor.

I really don't understand the standard for compatible string naming
here, is there something I missed? Is it required to use the full name
of the sensor parts number as a compatible string?

> --
> Regards,
>
> Laurent Pinchart
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Krzysztof Kozlowski 1 month, 3 weeks ago
On 13/08/2025 06:30, Will Whang wrote:
> On Tue, Aug 12, 2025 at 2:56 AM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>>
>> On Tue, Aug 12, 2025 at 08:47:12AM +0200, Krzysztof Kozlowski wrote:
>>> On 12/08/2025 08:31, Will Whang wrote:
>>>> On Mon, Aug 11, 2025 at 11:23 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>> On 12/08/2025 04:47, Will Whang wrote:
>>>>>> On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>>> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
>>>>>>>> +description:
>>>>>>>> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
>>>>>>>> +
>>>>>>>> +properties:
>>>>>>>> +  compatible:
>>>>>>>> +    enum:
>>>>>>>> +      - sony,imx585
>>>>>>>> +      - sony,imx585-mono
>>>>>>>
>>>>>>> I don't understand this second compatible. Is this different hardware?
>>>>>>> Can you point me to "mono" datasheet?
>>>>>>>
>>>>>>> Your description should explain this. Commit msg as well, instead of
>>>>>>> speaking about driver (in fact drop all driver related comments).
>>>>>>>
>>>>>> Mono version of this sensor is basically just removing the bayer
>>>>>> filter, so the sensor itself actually doesn't know if it is color or
>>>>>> mono and from my knowledge there are no registers programmed in the
>>>>>> factory that will show the variant and model number. (That is why when
>>>>>> the driver probing it only test blacklevel register because there are
>>>>>> no ID registers)
>>>>>> Originally in V1 patch I've made the switch between color and mono in
>>>>>> dtoverlay config but reviewer comments is to move it to compatible
>>>>>> string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)
>>>>>
>>>>> You only partially answer and judging by mentioning driver below:
>>>>>
>>>>>> In this case, what would you recommend?
>>>>>>
>>>>>> compatible:
>>>>>>   enum:
>>>>>>     - sony,imx585
>>>>>>     - sony,imx585-mono
>>>>>>   description: IMX585 has two variants, color and mono which the
>>>>>> driver supports both.
>>>>>
>>>>> ... I still have doubts that you really understand what I am asking. Is
>>>>> this one device or two different devices?
>>>>
>>>> One device that has two variants: IMX585-AAMJ1 (Mono) and IMX585-AAQJ1
>>>> (Color). Silicon-wise the difference is just with or without bayer
>>>> filter.
>>>
>>> Then I would propose to use sony,imx585-aamj1 and -aaqj1 with short
>>> explanation either in comment or description about difference in RGB
>>> mosaic filter.
>>
>> Works for me. We could possibly omit the "j1" suffix too.
>>
> My thinking is that imx585 and imx585-mono are easier to comprehend
> than IMX585-AAM and IMX585-AAQ.
> Because in dtoverlay for the users/me they will have to know what is
> the exact name instead of easy to remember name.
> 
> dtoverlay=imx585-aam
> is not as nice as
> dtoverlay=imx585-mono

I have datasheet for AAQ, so how above is easier for me to figure out
which compatible I am using?

> 
> which is what it does, a mono variant of the sensor.
> 
> I really don't understand the standard for compatible string naming
> here, is there something I missed? Is it required to use the full name
> of the sensor parts number as a compatible string?

It's not part number. You have there different models. We don't add
prose to compatibles, but use device or model names.

Best regards,
Krzysztof
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Will Whang 1 month, 3 weeks ago
On Tue, Aug 12, 2025 at 11:08 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 13/08/2025 06:30, Will Whang wrote:
> > On Tue, Aug 12, 2025 at 2:56 AM Laurent Pinchart
> > <laurent.pinchart@ideasonboard.com> wrote:
> >>
> >> On Tue, Aug 12, 2025 at 08:47:12AM +0200, Krzysztof Kozlowski wrote:
> >>> On 12/08/2025 08:31, Will Whang wrote:
> >>>> On Mon, Aug 11, 2025 at 11:23 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>>> On 12/08/2025 04:47, Will Whang wrote:
> >>>>>> On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>>>>> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
> >>>>>>>> +description:
> >>>>>>>> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
> >>>>>>>> +
> >>>>>>>> +properties:
> >>>>>>>> +  compatible:
> >>>>>>>> +    enum:
> >>>>>>>> +      - sony,imx585
> >>>>>>>> +      - sony,imx585-mono
> >>>>>>>
> >>>>>>> I don't understand this second compatible. Is this different hardware?
> >>>>>>> Can you point me to "mono" datasheet?
> >>>>>>>
> >>>>>>> Your description should explain this. Commit msg as well, instead of
> >>>>>>> speaking about driver (in fact drop all driver related comments).
> >>>>>>>
> >>>>>> Mono version of this sensor is basically just removing the bayer
> >>>>>> filter, so the sensor itself actually doesn't know if it is color or
> >>>>>> mono and from my knowledge there are no registers programmed in the
> >>>>>> factory that will show the variant and model number. (That is why when
> >>>>>> the driver probing it only test blacklevel register because there are
> >>>>>> no ID registers)
> >>>>>> Originally in V1 patch I've made the switch between color and mono in
> >>>>>> dtoverlay config but reviewer comments is to move it to compatible
> >>>>>> string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)
> >>>>>
> >>>>> You only partially answer and judging by mentioning driver below:
> >>>>>
> >>>>>> In this case, what would you recommend?
> >>>>>>
> >>>>>> compatible:
> >>>>>>   enum:
> >>>>>>     - sony,imx585
> >>>>>>     - sony,imx585-mono
> >>>>>>   description: IMX585 has two variants, color and mono which the
> >>>>>> driver supports both.
> >>>>>
> >>>>> ... I still have doubts that you really understand what I am asking. Is
> >>>>> this one device or two different devices?
> >>>>
> >>>> One device that has two variants: IMX585-AAMJ1 (Mono) and IMX585-AAQJ1
> >>>> (Color). Silicon-wise the difference is just with or without bayer
> >>>> filter.
> >>>
> >>> Then I would propose to use sony,imx585-aamj1 and -aaqj1 with short
> >>> explanation either in comment or description about difference in RGB
> >>> mosaic filter.
> >>
> >> Works for me. We could possibly omit the "j1" suffix too.
> >>
> > My thinking is that imx585 and imx585-mono are easier to comprehend
> > than IMX585-AAM and IMX585-AAQ.
> > Because in dtoverlay for the users/me they will have to know what is
> > the exact name instead of easy to remember name.
> >
> > dtoverlay=imx585-aam
> > is not as nice as
> > dtoverlay=imx585-mono
>
> I have datasheet for AAQ, so how above is easier for me to figure out
> which compatible I am using?
>
I propose this:

compatible:
  enum:
    - sony,imx585
    - sony,imx585-mono
    - sony,imx585-AAQJ1
    - sony,imx585-AAMJ1

  description: IMX585 has two variants, color (IMX585-AAQ) and mono
(IMX585-AAM) which
the driver supports both.

Description is there for a reason, dtoverlay has description also. See
sony,imx296.yaml as an example.
If you are looking at AAQ you know it is a color sensor and all the
color sensors from sony can be used with imx+three numbers in the
current list.
This is following the established convention.

> >
> > which is what it does, a mono variant of the sensor.
> >
> > I really don't understand the standard for compatible string naming
> > here, is there something I missed? Is it required to use the full name
> > of the sensor parts number as a compatible string?
>
> It's not part number. You have there different models. We don't add
> prose to compatibles, but use device or model names.
>
> Best regards,
> Krzysztof
Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
Posted by Krzysztof Kozlowski 1 month, 3 weeks ago
On 13/08/2025 08:38, Will Whang wrote:
> On Tue, Aug 12, 2025 at 11:08 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 13/08/2025 06:30, Will Whang wrote:
>>> On Tue, Aug 12, 2025 at 2:56 AM Laurent Pinchart
>>> <laurent.pinchart@ideasonboard.com> wrote:
>>>>
>>>> On Tue, Aug 12, 2025 at 08:47:12AM +0200, Krzysztof Kozlowski wrote:
>>>>> On 12/08/2025 08:31, Will Whang wrote:
>>>>>> On Mon, Aug 11, 2025 at 11:23 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>>> On 12/08/2025 04:47, Will Whang wrote:
>>>>>>>> On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>>>>> On Sun, Aug 10, 2025 at 11:09:18PM +0100, Will Whang wrote:
>>>>>>>>>> +description:
>>>>>>>>>> +  IMX585 sensor is a Sony CMOS sensor with 4K and FHD outputs.
>>>>>>>>>> +
>>>>>>>>>> +properties:
>>>>>>>>>> +  compatible:
>>>>>>>>>> +    enum:
>>>>>>>>>> +      - sony,imx585
>>>>>>>>>> +      - sony,imx585-mono
>>>>>>>>>
>>>>>>>>> I don't understand this second compatible. Is this different hardware?
>>>>>>>>> Can you point me to "mono" datasheet?
>>>>>>>>>
>>>>>>>>> Your description should explain this. Commit msg as well, instead of
>>>>>>>>> speaking about driver (in fact drop all driver related comments).
>>>>>>>>>
>>>>>>>> Mono version of this sensor is basically just removing the bayer
>>>>>>>> filter, so the sensor itself actually doesn't know if it is color or
>>>>>>>> mono and from my knowledge there are no registers programmed in the
>>>>>>>> factory that will show the variant and model number. (That is why when
>>>>>>>> the driver probing it only test blacklevel register because there are
>>>>>>>> no ID registers)
>>>>>>>> Originally in V1 patch I've made the switch between color and mono in
>>>>>>>> dtoverlay config but reviewer comments is to move it to compatible
>>>>>>>> string and not property.(https://lore.kernel.org/linux-media/20250703175121.GA17709@pendragon.ideasonboard.com/)
>>>>>>>
>>>>>>> You only partially answer and judging by mentioning driver below:
>>>>>>>
>>>>>>>> In this case, what would you recommend?
>>>>>>>>
>>>>>>>> compatible:
>>>>>>>>   enum:
>>>>>>>>     - sony,imx585
>>>>>>>>     - sony,imx585-mono
>>>>>>>>   description: IMX585 has two variants, color and mono which the
>>>>>>>> driver supports both.
>>>>>>>
>>>>>>> ... I still have doubts that you really understand what I am asking. Is
>>>>>>> this one device or two different devices?
>>>>>>
>>>>>> One device that has two variants: IMX585-AAMJ1 (Mono) and IMX585-AAQJ1
>>>>>> (Color). Silicon-wise the difference is just with or without bayer
>>>>>> filter.
>>>>>
>>>>> Then I would propose to use sony,imx585-aamj1 and -aaqj1 with short
>>>>> explanation either in comment or description about difference in RGB
>>>>> mosaic filter.
>>>>
>>>> Works for me. We could possibly omit the "j1" suffix too.
>>>>
>>> My thinking is that imx585 and imx585-mono are easier to comprehend
>>> than IMX585-AAM and IMX585-AAQ.
>>> Because in dtoverlay for the users/me they will have to know what is
>>> the exact name instead of easy to remember name.
>>>
>>> dtoverlay=imx585-aam
>>> is not as nice as
>>> dtoverlay=imx585-mono
>>
>> I have datasheet for AAQ, so how above is easier for me to figure out
>> which compatible I am using?
>>
> I propose this:
> 
> compatible:
>   enum:
>     - sony,imx585
>     - sony,imx585-mono
>     - sony,imx585-AAQJ1
>     - sony,imx585-AAMJ1
> 
>   description: IMX585 has two variants, color (IMX585-AAQ) and mono
> (IMX585-AAM) which
> the driver supports both.

So why four compatibles? BTW, driver does not matter.

> 
> Description is there for a reason, dtoverlay has description also. See
> sony,imx296.yaml as an example.
> If you are looking at AAQ you know it is a color sensor and all the
> color sensors from sony can be used with imx+three numbers in the
> current list.
> This is following the established convention.
Which? Sorry, point me anywhere to convention that we do not use proper
full model names but shortened version for some variant of a device?
Such approach lead to many issues in the past, for example current
Risc-v reset mess where contributor wants to remove completely
incomplete compatible :/

Best regards,
Krzysztof