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

[Kaustabh Chakraborty <kauschluss@disroot.org>]

On 2024-07-03 19:30, Conor Dooley wrote:
> On Wed, Jul 03, 2024 at 06:31:13PM +0000, Kaustabh Chakraborty wrote:
>> On 2024-06-26 16:06, Conor Dooley wrote:
>> > On Tue, Jun 25, 2024 at 10:21:06PM +0530, Kaustabh Chakraborty wrote:
>> >> Add the compatible string of stk3013 to the existing list.
>> >> 
>> >> 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
>> > 
>> > The driver change suggests that this device is compatible with the
>> > existing sensors.
>> > Jonathan, could we relax the warning during init
>> 
>> What does 'relax' mean here? Earlier there used to be a probing error,
>> and now it's just a warning. Is that not relaxed enough?
> 
> If it is something intentionally, I don't think a warning is suitable.
> It makes the user thing something is wrong.

So, something like:

  dev_info(&client->dev, "chip id: 0x%x\n", chipid);

is suitable in this context?

And doesn't it make stk3310_check_chip_id() obsolete? In all cases chipid
should be printed as it's not an error/warning message.

> 
>> 
>> > 	ret = stk3310_check_chip_id(chipid);
>> > 	if (ret < 0)
>> > 		dev_warn(&client->dev, "unknown chip id: 0x%x\n", chipid);
>> > and allow fallback compatibles here please?
>> 
>> So, you mean something like this in devicetree?
>> 
>>   compatible = "sensortek,stk3013", "sensortek,stk3310";
>> 
>> I mean that's fine, but we also need to change devicetree sources for
>> other devices. If that's what we're doing, please let me know how do
>> I frame the commits.
> 
> Why would you need to change the dts for other devices to add a fallback
> for this new compatible that is being added?

Okay gotcha, so it's just for stk3013.

> 
>> >>        - sensortek,stk3310
>> >>        - sensortek,stk3311
>> >>        - sensortek,stk3335
>> >> -- 
>> >> 2.45.2
>> >>
>> 
>> Thank you.