[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFoNnrwoRbtvTHHnjarDTKEHnQMaMDERPKi_vnYym3n8tVpzOA@mail.gmail.com>
Date: Tue, 12 Aug 2025 23:38:08 -0700
From: Will Whang <will@...lwhang.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 1/4] dt-bindings: media: Add Sony IMX585 CMOS image sensor
On Tue, Aug 12, 2025 at 11:08 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 13/08/2025 06:30, Will Whang wrote:
> > On Tue, Aug 12, 2025 at 2:56 AM Laurent Pinchart
> > <laurent.pinchart@...asonboard.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@...nel.org> wrote:
> >>>>> On 12/08/2025 04:47, Will Whang wrote:
> >>>>>> On Mon, Aug 11, 2025 at 1:01 AM Krzysztof Kozlowski <krzk@...nel.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
Powered by blists - more mailing lists