| Message ID | 20240712152417.97726-3-kauschluss@disroot.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] iio: light: stk3310: relax chipid check warning | expand |
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. > > 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
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. 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
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. 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. 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
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
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
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
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(+)