Re: [PATCH 3/3] dt-bindings: iio: light: stk33xx: add compatible for stk3013

[Kaustabh Chakraborty <kauschluss@disroot.org>]

On 2024-07-16 16:43, Jonathan Cameron wrote:
> On Mon, 15 Jul 2024 20:02:57 +0000
> Kaustabh Chakraborty <kauschluss@disroot.org> wrote:
> 
>> On 2024-07-13 12:06, Jonathan Cameron wrote:
>> > On Fri, 12 Jul 2024 20:54:02 +0530
>> > Kaustabh Chakraborty <kauschluss@disroot.org> wrote:
>> >   
>> >> Add the compatible string of stk3013 to the existing list.  
>> > 
>> > Should include how this differs from existing devices such that it doesn't
>> > make sense to use a fallback compatible.  
>> 
>> STK3013 is a proximity sensor by Sensortek, bearing chipid of 0x31. Despite
>> being marketed as a proximity sensor, it also appears to have ambient
>> light sensing capabilities.
>> 
>> Add the compatible string of stk3013 to the existing list, as a part not
>> compatible with other devices.
> 
> That would be fine, but I'm not seeing any driver code changes, so when
> you say not compatible, in what way? If it's register changes in features
> we don't support yet or something like that, just add some examples.
> 
> A different whoami register value isn't sufficient as after the fix
> you have as patch 1 that will only result in a message print.

I understand that a whoami is not enough to justify not having a fallback
compatible. That's why I mentioned it's the most "convincing" argument I
could come up with, which is admittedly, isn't enough.

And there really isn't anything feature-wise which sets STK3013 apart from
other devices. All register addresses and functions are fully compatible
with the current driver.

> 
> Obviously doesn't help much for this addition as you are adding the
> bypass of the whoami and the new ID in the same series, but we want
> to set a precedence for future devices to use fallback compatibles
> now that path works.

I'll add stk3310 as a fallback compatible and change the commit message
appropriately. Conor did mention it in the last revision, but I totally
missed that. Apologies.

Ending the description with something along the lines of:

The part is fully compatible with the existing implementation of the
device driver. Add the compatible string of stk3013 to the existing list,
with a fallback of stk3310.

...would be alright?

> 
> Jonathan
> 
>> 
>> I hope this is good enough. I couldn't find anything more convincing.
>> 
>> >   
>> >> 
>> >> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> >> ---
>> >>  Documentation/devicetree/bindings/iio/light/stk33xx.yaml | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >> 
>> >> diff --git a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
>> >> index f6e22dc9814a..6003da66a7e6 100644
>> >> --- a/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
>> >> +++ b/Documentation/devicetree/bindings/iio/light/stk33xx.yaml
>> >> @@ -19,6 +19,7 @@ allOf:
>> >>  properties:
>> >>    compatible:
>> >>      enum:
>> >> +      - sensortek,stk3013
>> >>        - sensortek,stk3310
>> >>        - sensortek,stk3311
>> >>        - sensortek,stk3335