[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <faf398eb-2be4-43cd-a5bf-2c688dd7bc18@kernel.org>
Date: Wed, 13 Aug 2025 09:34:20 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Will Whang <will@...lwhang.com>
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 13/08/2025 08:38, Will Whang wrote:
> 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.
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
Powered by blists - more mailing lists