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

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 17 Jul 2024 15:58:50 +0000
Kaustabh Chakraborty <kauschluss@disroot.org> wrote:

> 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?

Yes. Looks good.

Jonathan

> 
> > 
> > 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