Re: [PATCH v4 2/2] dt-bindings: iio: st-sensors: add LIS2DS12 accelerometer

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2447 bytes --]

On Thu, Jun 20, 2024 at 04:42:39PM +0000, Kaustabh Chakraborty wrote:
> On 2024-06-18 17:38, Conor Dooley wrote:
> > On Tue, Jun 18, 2024 at 12:21:17PM +0000, Kaustabh Chakraborty wrote:
> >> On 2024-06-15 13:49, Jonathan Cameron wrote:
> >> > On Sat, 15 Jun 2024 09:46:59 +0000
> >> > Kaustabh Chakraborty <kauschluss@disroot.org> wrote:
> >> > 
> >> >> On 2024-06-11 18:17, Conor Dooley wrote:
> >> >> > On Tue, Jun 11, 2024 at 09:35:53PM +0530, Kaustabh Chakraborty wrote:  
> >> >> >> Add compatible for LIS2DS12 accelerometer by STMicroelectronics.  
> >> >> > 
> >> >> > I can see that! Your commit message should mention why this device
> >> >> > is not compatible with existing variants.  
> >> >> 
> >> >> Sure, is adding the WhoAmI value enough? Or do I also have to
> >> >> explain the different registers and sensor settings.
> >> >> 
> >> > Who ami is not enough, but a statement along the lines of
> >> > "The register interface is not compatible with existing parts, for
> >> > example addresses and contents of numerous registers are different"
> >> > 
> >> > With whatever the actual differences are.
> >> 
> >> "LIS2DS12 is an accelerometer by STMicroelectronics. It is identifiable by
> >> its WhoAmI value 0x43.
> >> 
> >> Its register interface is not compatible with existing parts. For example: 
> >> 
> >> - The full-scale values are present in register 0x20, in bits 2 and 3
> >>   (mask 0x0c).
> >> 
> >> - The full-scale values 2G, 4G, 8G, and 16G correspond to the register
> >>   values 0x00, 0x02, 0x03, 0x01 respectively.
> >> 
> >> Add the compatible string without any fallback."
> >> 
> >> Is this good enough?
> > 
> > I don't know the other devices, so please highlight how the examples you
> > give here are different to the existing ones please.
> 
> Are these acceptable?
> 
> - The full-scale values are present in register 0x20, in bits 2 and 3
>   (mask 0x0c).
>   Most other supported sensors have the register address set to 0x21,
>   0x23, 0x24, or 0x25.
>   There is one sensor setting though (bearing WhoAmI 0x3b) which has
>   it's address set to 0x20, but the mask is set to 0x20, not 0x0c.
> 
> - The full-scale values 2G, 4G, 8G, and 16G correspond to the register
>   values 0x00, 0x02, 0x03, 0x01 respectively.
>   None of the sensor settings have the value 0x01 associated with 16G.

Yeah, that sounds good to me. Thanks for the updates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]