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

[Will Whang <will@willwhang.com>]

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