| Message ID | 20221227223841.2990847-2-absicsz@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: dts: n900: switch accelerometer to iio driver | expand |
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
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
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
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 {
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(-)