linux-kernel - Re: [PATCH 18/23] dt-bindings: media: imx258: Add alternate compatible strings

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <76f999a7-55e0-4676-aa75-8fcd466e046b@luigi311.com>
Date: Thu, 28 Mar 2024 13:16:22 -0600
From: Luigi311 <git@...gi311.com>
To: Rob Herring <robh@...nel.org>
Cc: linux-media@...r.kernel.org, dave.stevenson@...pberrypi.com,
 jacopo.mondi@...asonboard.com, mchehab@...nel.org,
 krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, shawnguo@...nel.org,
 s.hauer@...gutronix.de, kernel@...gutronix.de, festevam@...il.com,
 sakari.ailus@...ux.intel.com, devicetree@...r.kernel.org,
 imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 18/23] dt-bindings: media: imx258: Add alternate
 compatible strings

On 3/28/24 12:55, Rob Herring wrote:
> On Wed, Mar 27, 2024 at 05:17:04PM -0600, git@...gi311.com wrote:
>> From: Dave Stevenson <dave.stevenson@...pberrypi.com>
>>
>> There are a number of variants of the imx258 modules that can not
>> be differentiated at runtime, so add compatible strings for them.
>> But you are only adding 1 variant.

I can not speak for Dave but as to why this was added here but looking
at the imx296 yaml that has something similar where there are multiple
variants that may not be detectable at run time but does not include
similar verbiage in the main description. Should I drop this from the
description so it matches the imx296?

> 
>>
>> Signed-off-by: Dave Stevenson <dave.stevenson@...pberrypi.com>
>> Signed-off-by: Luigi311 <git@...gi311.com>
>> ---
>>  .../devicetree/bindings/media/i2c/sony,imx258.yaml          | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> index bee61a443b23..c7856de15ba3 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
>> @@ -14,10 +14,14 @@ description: |-
>>    type stacked image sensor with a square pixel array of size 4208 x 3120. It
>>    is programmable through I2C interface.  Image data is sent through MIPI
>>    CSI-2.
>> +  There are a number of variants of the sensor which cannot be detected at
>> +  runtime, so multiple compatible strings are required to differentiate these.
> 
> That's more reasoning/why for the patch than description of the h/w.
> 
>>  properties:
>>    compatible:
>> -    const: sony,imx258
>> +    - enum:
>> +        - sony,imx258
>> +        - sony,imx258-pdaf
> 
> How do I know which one to use? Please define what PDAF means somewhere 
> as well as perhaps what the original/default variant is or isn't.

Would it make sense to change the properties to include a description like so

properties:
  compatible:
    enum:
      - sony,imx258
      - sony,imx258-pdaf
    description:
      The IMX258 sensor exists in two different models, a standard variant
      (IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
      The camera module does not expose the model through registers, so the
      exact model needs to be specified.

> 
>>  
>>    assigned-clocks: true
>>    assigned-clock-parents: true
>> -- 
>> 2.42.0
>>