ARM: dts: n900: switch accelerometer to iio driver

ARM: dts: n900: switch accelerometer to iio driver

[Sicelo A. Mhlongo]

[PATCH] ARM: dts: n900: switch accelerometer to iio driver

The lis302dl accelerometer is now supported by an iio driver, so the N900 can
work with modern iio-based userspace. This patch provides the required dts
changes for the switch

[PATCH] ARM: dts: n900: switch accelerometer to iio driver

[Sicelo A. Mhlongo]

Since 8a7449d68670a8f9033d57b9e7997af77a900d53, lis302dl is supported by an iio
driver. Make the switch, to accommodate modern userspace, even though the iio
interface lacks some of the extended features of the older driver

Signed-off-by: Sicelo A. Mhlongo <absicsz@gmail.com>
---
 arch/arm/boot/dts/omap3-n900.dts | 53 +++++---------------------------
 1 file changed, 8 insertions(+), 45 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 6ba2e8f81973..94fa1d492fb4 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -767,56 +767,19 @@ &i2c3 {
 
 	clock-frequency = <400000>;
 
-	lis302dl: lis3lv02d@1d {
-		compatible = "st,lis3lv02d";
+	lis302dl: lis302dl@1d {
+		compatible = "st,lis302dl";
 		reg = <0x1d>;
 
-		Vdd-supply = <&vaux1>;
-		Vdd_IO-supply = <&vio>;
+		vdd-supply = <&vaux1>;
+		vddio-supply = <&vio>;
 
 		interrupt-parent = <&gpio6>;
-		interrupts = <21 20>; /* 181 and 180 */
-
-		/* click flags */
-		st,click-single-x;
-		st,click-single-y;
-		st,click-single-z;
-
-		/* Limits are 0.5g * value */
-		st,click-threshold-x = <8>;
-		st,click-threshold-y = <8>;
-		st,click-threshold-z = <10>;
-
-		/* Click must be longer than time limit */
-		st,click-time-limit = <9>;
-
-		/* Kind of debounce filter */
-		st,click-latency = <50>;
-
-		/* Interrupt line 2 for click detection */
-		st,irq2-click;
-
-		st,wakeup-x-hi;
-		st,wakeup-y-hi;
-		st,wakeup-threshold = <(800/18)>; /* millig-value / 18 to get HW values */
-
-		st,wakeup2-z-hi;
-		st,wakeup2-threshold = <(900/18)>; /* millig-value / 18 to get HW values */
-
-		st,hipass1-disable;
-		st,hipass2-disable;
-
-		st,axis-x = <1>;    /* LIS3_DEV_X */
-		st,axis-y = <(-2)>; /* LIS3_INV_DEV_Y */
-		st,axis-z = <(-3)>; /* LIS3_INV_DEV_Z */
-
-		st,min-limit-x = <(-32)>;
-		st,min-limit-y = <3>;
-		st,min-limit-z = <3>;
+		interrupts = <21 IRQ_TYPE_EDGE_RISING>, <20 IRQ_TYPE_EDGE_RISING>; /* 181 and 180 */
 
-		st,max-limit-x = <(-3)>;
-		st,max-limit-y = <32>;
-		st,max-limit-z = <32>;
+		mount-matrix =	 "-1",  "0",  "0",
+				  "0",  "1",  "0",
+				  "0",  "0",  "1";
 	};
 
 	cam1: camera@3e {
-- 
2.39.0

Re: [PATCH] ARM: dts: n900: switch accelerometer to iio driver

[Krzysztof Kozlowski]

On 27/12/2022 23:38, Sicelo A. Mhlongo wrote:
> Since 8a7449d68670a8f9033d57b9e7997af77a900d53, lis302dl is supported by an iio

Use commit SHA ("title") format, as suggested by checkpatch.

> driver. Make the switch, to accommodate modern userspace, even though the iio
> interface lacks some of the extended features of the older driver
> 
> Signed-off-by: Sicelo A. Mhlongo <absicsz@gmail.com>
> ---
>  arch/arm/boot/dts/omap3-n900.dts | 53 +++++---------------------------
>  1 file changed, 8 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
> index 6ba2e8f81973..94fa1d492fb4 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -767,56 +767,19 @@ &i2c3 {
>  
>  	clock-frequency = <400000>;
>  
> -	lis302dl: lis3lv02d@1d {
> -		compatible = "st,lis3lv02d";
> +	lis302dl: lis302dl@1d {

That's not really explained in commit msg and does not look related to
your goal. If changing - in separate patch - make the node name generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +		compatible = "st,lis302dl";
>  		reg = <0x1d>;
>  
> -		Vdd-supply = <&vaux1>;
> -		Vdd_IO-supply = <&vio>;
> +		vdd-supply = <&vaux1>;
> +		vddio-supply = <&vio>;

Does not look related/explained in commit msg.

>  
>  		interrupt-parent = <&gpio6>;
> -		interrupts = <21 20>; /* 181 and 180 */
> -
> -		/* click flags */
> -		st,click-single-x;
> -		st,click-single-y;
> -		st,click-single-z;
> -
> -		/* Limits are 0.5g * value */
> -		st,click-threshold-x = <8>;
> -		st,click-threshold-y = <8>;
> -		st,click-threshold-z = <10>;
> -
> -		/* Click must be longer than time limit */
> -		st,click-time-limit = <9>;
> -
> -		/* Kind of debounce filter */
> -		st,click-latency = <50>;
> -
> -		/* Interrupt line 2 for click detection */
> -		st,irq2-click;
> -
> -		st,wakeup-x-hi;
> -		st,wakeup-y-hi;
> -		st,wakeup-threshold = <(800/18)>; /* millig-value / 18 to get HW values */
> -
> -		st,wakeup2-z-hi;
> -		st,wakeup2-threshold = <(900/18)>; /* millig-value / 18 to get HW values */
> -
> -		st,hipass1-disable;
> -		st,hipass2-disable;
> -
> -		st,axis-x = <1>;    /* LIS3_DEV_X */
> -		st,axis-y = <(-2)>; /* LIS3_INV_DEV_Y */
> -		st,axis-z = <(-3)>; /* LIS3_INV_DEV_Z */
> -
> -		st,min-limit-x = <(-32)>;
> -		st,min-limit-y = <3>;
> -		st,min-limit-z = <3>;
> +		interrupts = <21 IRQ_TYPE_EDGE_RISING>, <20 IRQ_TYPE_EDGE_RISING>; /* 181 and 180 */

Does not fit in 80-wrap length.

>  
> -		st,max-limit-x = <(-3)>;
> -		st,max-limit-y = <32>;
> -		st,max-limit-z = <32>;
> +		mount-matrix =	 "-1",  "0",  "0",
> +				  "0",  "1",  "0",
> +				  "0",  "0",  "1";
>  	};
>  
>  	cam1: camera@3e {

Best regards,
Krzysztof

Re: [PATCH] ARM: dts: n900: switch accelerometer to iio driver

[Sicelo]

Thank you for the review.

> > +	lis302dl: lis302dl@1d {
> 
> That's not really explained in commit msg and does not look related to
> your goal. If changing - in separate patch - make the node name generic.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
 
Now I understand that it should just be `accelerometer@1d`. To be clear,
are you saying this change should have a separate patch, i.e. not part
of the switch to iio driver?

> > -		Vdd-supply = <&vaux1>;
> > -		Vdd_IO-supply = <&vio>;
> > +		vdd-supply = <&vaux1>;
> > +		vddio-supply = <&vio>;
> 
> Does not look related/explained in commit msg.

This is from Documentation/devicetree/bindings/iio/st,st-sensors.yaml,
i.e. lowercase. I will look for a way to explain it in v2.

Sincerely
Sicelo

Re: [PATCH] ARM: dts: n900: switch accelerometer to iio driver

[Krzysztof Kozlowski]

On 28/12/2022 11:28, Sicelo wrote:
> Thank you for the review.
> 
>>> +	lis302dl: lis302dl@1d {
>>
>> That's not really explained in commit msg and does not look related to
>> your goal. If changing - in separate patch - make the node name generic.
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>  
> Now I understand that it should just be `accelerometer@1d`. To be clear,
> are you saying this change should have a separate patch, i.e. not part
> of the switch to iio driver?

Yes, such cleanup is not related to changing compatible.

> 
>>> -		Vdd-supply = <&vaux1>;
>>> -		Vdd_IO-supply = <&vio>;
>>> +		vdd-supply = <&vaux1>;
>>> +		vddio-supply = <&vio>;
>>
>> Does not look related/explained in commit msg.
> 
> This is from Documentation/devicetree/bindings/iio/st,st-sensors.yaml,
> i.e. lowercase. I will look for a way to explain it in v2.

Ah, ok, then maybe mention in commit msg that you are changing
properties to match bindings of new compatible.

Best regards,
Krzysztof

Re: ARM: dts: n900: switch accelerometer to iio driver

[Krzysztof Kozlowski]

On 27/12/2022 23:38, Sicelo A. Mhlongo wrote:
> [PATCH] ARM: dts: n900: switch accelerometer to iio driver

That's not correct placement of title. This should be in the subject.

> 
> The lis302dl accelerometer is now supported by an iio driver, so the N900 can
> work with modern iio-based userspace. This patch provides the required dts
> changes for the switch

You miss the actual patch here...

Best regards,
Krzysztof

[PATCH v2 0/2] ARM: dts: n900: use iio driver for accelerometer

[Sicelo A. Mhlongo]

The accelerometer in the N900 is now supported by the iio framework. This patch
series makes the switch to the new compatible.

The iio framework does not support some of the extended properties in the
previous driver, but the change is useful for modern userspace, which expects
accelerometers to be exposed via iio

Sicelo A. Mhlongo (2):
  ARM: dts: n900: rename accelerometer node
  ARM: dts: n900: use iio driver for accelerometer

 arch/arm/boot/dts/omap3-n900.dts | 54 ++++++--------------------------
 1 file changed, 9 insertions(+), 45 deletions(-)

-- 
2.39.0

[PATCH v2 1/2] ARM: dts: n900: rename accelerometer node

[Sicelo A. Mhlongo]

Use generic node naming for lis302dl accelerometer, and drop its
label that is not used anywhere else

Signed-off-by: Sicelo A. Mhlongo <absicsz@gmail.com>
---
 arch/arm/boot/dts/omap3-n900.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 6ba2e8f81973..20d7a7bb6b04 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -767,7 +767,7 @@ &i2c3 {
 
 	clock-frequency = <400000>;
 
-	lis302dl: lis3lv02d@1d {
+	accelerometer@1d {
 		compatible = "st,lis3lv02d";
 		reg = <0x1d>;
 
-- 
2.39.0

Re: [PATCH v2 1/2] ARM: dts: n900: rename accelerometer node

[Krzysztof Kozlowski]

On 29/12/2022 15:52, Sicelo A. Mhlongo wrote:
> Use generic node naming for lis302dl accelerometer, and drop its
> label that is not used anywhere else

Full stop.

Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

> 
> Signed-off-by: Sicelo A. Mhlongo <absicsz@gmail.com>



Best regards,
Krzysztof

[PATCH v2 2/2] ARM: dts: n900: use iio driver for accelerometer

[Sicelo A. Mhlongo]

The accelerometer in the N900 is supported by the iio-framework since commit
8a7449d68670a8f9 ("iio: accel: add support for LIS302DL variant). This commit
switches to it and updates node properties to match the bindings of the new
compatible

Signed-off-by: Sicelo A. Mhlongo <absicsz@gmail.com>
---
 arch/arm/boot/dts/omap3-n900.dts | 52 +++++---------------------------
 1 file changed, 8 insertions(+), 44 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 20d7a7bb6b04..adee3da93421 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -768,55 +768,19 @@ &i2c3 {
 	clock-frequency = <400000>;
 
 	accelerometer@1d {
-		compatible = "st,lis3lv02d";
+		compatible = "st,lis302dl";
 		reg = <0x1d>;
 
-		Vdd-supply = <&vaux1>;
-		Vdd_IO-supply = <&vio>;
+		vdd-supply = <&vaux1>;
+		vddio-supply = <&vio>;
 
 		interrupt-parent = <&gpio6>;
-		interrupts = <21 20>; /* 181 and 180 */
-
-		/* click flags */
-		st,click-single-x;
-		st,click-single-y;
-		st,click-single-z;
-
-		/* Limits are 0.5g * value */
-		st,click-threshold-x = <8>;
-		st,click-threshold-y = <8>;
-		st,click-threshold-z = <10>;
-
-		/* Click must be longer than time limit */
-		st,click-time-limit = <9>;
-
-		/* Kind of debounce filter */
-		st,click-latency = <50>;
-
-		/* Interrupt line 2 for click detection */
-		st,irq2-click;
-
-		st,wakeup-x-hi;
-		st,wakeup-y-hi;
-		st,wakeup-threshold = <(800/18)>; /* millig-value / 18 to get HW values */
-
-		st,wakeup2-z-hi;
-		st,wakeup2-threshold = <(900/18)>; /* millig-value / 18 to get HW values */
-
-		st,hipass1-disable;
-		st,hipass2-disable;
-
-		st,axis-x = <1>;    /* LIS3_DEV_X */
-		st,axis-y = <(-2)>; /* LIS3_INV_DEV_Y */
-		st,axis-z = <(-3)>; /* LIS3_INV_DEV_Z */
-
-		st,min-limit-x = <(-32)>;
-		st,min-limit-y = <3>;
-		st,min-limit-z = <3>;
+		interrupts = <21 IRQ_TYPE_EDGE_RISING>,
+			     <20 IRQ_TYPE_EDGE_RISING>; /* 181 and 180 */
 
-		st,max-limit-x = <(-3)>;
-		st,max-limit-y = <32>;
-		st,max-limit-z = <32>;
+		mount-matrix =	 "-1",  "0",  "0",
+				  "0",  "1",  "0",
+				  "0",  "0",  "1";
 	};
 
 	cam1: camera@3e {
-- 
2.39.0

Re: [PATCH v2 2/2] ARM: dts: n900: use iio driver for accelerometer

[Krzysztof Kozlowski]

On 29/12/2022 15:52, Sicelo A. Mhlongo wrote:
> The accelerometer in the N900 is supported by the iio-framework since commit
> 8a7449d68670a8f9 ("iio: accel: add support for LIS302DL variant). This commit
> switches to it and updates node properties to match the bindings of the new
> compatible
> 
> Signed-off-by: Sicelo A. Mhlongo <absicsz@gmail.com>
> ---
>  arch/arm/boot/dts/omap3-n900.dts | 52 +++++---------------------------
>  1 file changed, 8 insertions(+), 44 deletions(-)
> 


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof