[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZuqyuvZ6tdzp5XSW@skv.local>
Date: Wed, 18 Sep 2024 14:00:10 +0300
From: Andrey Skvortsov <andrej.skvortzov@...il.com>
To: Dragan Simic <dsimic@...jaro.org>
Cc: Chen-Yu Tsai <wens@...e.org>, Jernej Skrabec <jernej.skrabec@...il.com>,
Samuel Holland <samuel@...lland.org>,
linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
linux-kernel@...r.kernel.org, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Ondřej Jirman <megi@....cz>
Subject: Re: [PATCH] arm64: dts: sun50i-a64-pinephone: Add mount matrix for
accelerometer
Hi Dragan,
On 24-09-18 11:27, Dragan Simic wrote:
> Hello Andrey,
>
>
> arm64: dts: allwinner: pinephone: Add mount matrix to accelerometer
>
> The patch description should be reworded like this, reflown into
> proper line lengths, of course:
>
> The way InvenSense MPU-6050 accelerometer is mounted on the
> user-facing side of the Pine64 PinePhone mainboard requires
> the accelerometer's x- and y-axis to be swapped, and the
> direction of the accelerometer's y-axis to be inverted.
>
> Rectify this by adding a mount-matrix to the accelerometer
> definition in the PinePhone dtsi file.
>
> [andrey: Picked the patch description provided by dsimic]
> Fixes: 91f480d40942 ("arm64: dts: allwinner: Add initial support for
> Pine64 PinePhone")
> Cc: stable@...r.kernel.org
Thanks for the commit description, it's much better, than original
one.
> Please note the Fixes tag, which will submit this bugfix patch
> for inclusion into the long-term/stable kernels.
>
> Also note that the patch description corrects the way inversion
> of the axis direction is described, which should also be corrected
> in the patch itself, as described further below.
>
> After going through the InvenSense MPU-6050 datasheet, [1] the
> MPU-6050 evaluation board user guide, the PinePhone schematic,
> the PinePhone mainboard component placement, [2] and the kernel
> bindings documentation for mount-matrix, [3] I can conslude that
> only the direction of the accelerometer's y-axis is inverted,
> while the direction of the z-axis remain unchanged, according
> to the right-hand rule.
yes, it looks so on the first glance, but in MPU-6050 datasheet there
is also following information:
7.8 Three-Axis MEMS Accelerometer with 16-bit ADCs and Signal
Conditioning
When the device is placed on a flat surface, it will measure
0g on the X- and Y-axes and +1g on the Z-axis.
So sensors reports positive acceleration values for Z-axis, when
the gravity points to Z-minus. I see the same on device. positive
values are returned, when screen and IC point upwards (not the center
for gravity).
In device tree mount-matrix documentation [3] there is
users would likely expect a value of 9.81 m/s^2 upwards along the (z)
axis, i.e. out of the screen when the device is held with its screen
flat on the planets surface.
According to that, it looks like Z-axis here has to be inverted.
It applies to other axes as well. And because of that I came from (only Y-axis is inverted)
x' = -y
y' = x
z' = z
to inverted solution (Y-axis is kept, but X and Z are inverted).
x' = y
y' = -x
z' = -z
probably should put this information into commit description.
> > > > Signed-off-by: Ondrej Jirman <megi@....cz>
> > > > Signed-off-by: Andrey Skvortsov <andrej.skvortzov@...il.com>
> > > > ---
> > > > arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > > b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > > index bc6af17e9267a..1da7506c38cd0 100644
> > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> > > > @@ -229,6 +229,9 @@ accelerometer@68 {
> > > > interrupts = <7 5 IRQ_TYPE_EDGE_RISING>; /* PH5 */
> > > > vdd-supply = <®_dldo1>;
> > > > vddio-supply = <®_dldo1>;
> > > > + mount-matrix = "0", "1", "0",
> > > > + "-1", "0", "0",
> > > > + "0", "0", "-1";
> > > > };
> > > > };
>
> With the above-described analysis in mind, the mount-matrix
> should be defined like this instead:
>
> mount-matrix = "0", "1", "0",
> "-1", "0", "0",
> "0", "0", "1";
x' = 0 * x + 1 * y + 0 * z = y
y' = -1 * x + 1 * y + 0 * z = -x
z' = 0 * z + 0 * y + 1 * z = z
your description says, that only Y-axis is inverted, but this matrix,
imho, inverts original X axis as it was in original description.
> Please also note the line indentation that was changed a bit.
>
> [1] https://rimgo.reallyaweso.me/vrBXQPq.png
> [2] https://rimgo.reallyaweso.me/uTmT1pr.png
> [3] https://www.kernel.org/doc/Documentation/devicetree/bindings/iio/mount-matrix.txt
--
Best regards,
Andrey Skvortsov
Powered by blists - more mailing lists