From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 512D5CCD1A0 for ; Wed, 18 Sep 2024 11:28:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:Message-ID:References:In-Reply-To:Subject:Cc:To:From:Date: MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=nfH01sB+DBUpTSjL37MwEOYgAbC2BjpMcYHpPwSx4fY=; b=Q18LR1JEiPExdINSV4vFb9mZtD ETDDu+YO7LUWUi9FJhtzIeJw0w57Nthxxx7/OuzySLB3QXLUh7IJ2iwKLy85dgnyHs/+yb9Cc+Ynp ZLfItfdHWYCO+g7AUsWdB5ESxiAsKgcttwkGIpU7HvwvKt74ODIQlIX3cOXqDk0H4PywZa2o/UZ1w /Dk2Xg7haD+eXycHWH/ADMcmlEOvc5lXcYMZ5t0f3QMwovqlp45PFEQ4fM1aHnuJZKqyzvgiD3mm6 +CZfYY+hgRqLPnmzHQ2x6vywUcsZ/+rsLAAh0xCQPM5IPbhlrP+qrv+WGWLq7Fvy8FX7uEOTjznaI ywZU2A9w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1sqsrC-000000088wA-3miE; Wed, 18 Sep 2024 11:28:26 +0000 Received: from mail.manjaro.org ([116.203.91.91]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1sqsq6-000000088jp-1jWC for linux-arm-kernel@lists.infradead.org; Wed, 18 Sep 2024 11:27:20 +0000 MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=manjaro.org; s=2021; t=1726658836; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nfH01sB+DBUpTSjL37MwEOYgAbC2BjpMcYHpPwSx4fY=; b=CtsfmNVLqtJ+WQam3YvWbhBJhaWK/pE93Z78JTKYcbMqgNBnzqYJO50ASggcDMRXG9GhAG uAs+K/jGWO7Aote38bJpbYv9Bmcm7gnqtU0KVtWerqoqv8S4/WdyLtKMuTkG30ivTmeazG s7nRL0AlNVZmiQeVR/CU7qqdFHVUzwRMpy8+4FkqrosQV0PietQZPx0G6ctiCte7GKEK9E vw7s4tFxBSArSEv44Veg6Jv7ubptMHdLQv0M1B3pBfTKXYQOAEFmLBjd3w9PQioDR080y5 9rOS8gFVVjQvlzUTp7wYTNqi/3xYh39FItlTa5TdkbqTClxvKtKNhOoEkZvy1g== Date: Wed, 18 Sep 2024 13:27:16 +0200 From: Dragan Simic To: Andrey Skvortsov Cc: Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Rob Herring , Krzysztof Kozlowski , Conor Dooley , =?UTF-8?Q?Ond=C5=99ej_Jirman?= Subject: Re: [PATCH] arm64: dts: sun50i-a64-pinephone: Add mount matrix for accelerometer In-Reply-To: References: <20240916204521.2033218-1-andrej.skvortzov@gmail.com> <6e5d0e9978bff30559c17f30d1495b59@manjaro.org> Message-ID: <8df5fc79a3e899738aa944a290774c72@manjaro.org> X-Sender: dsimic@manjaro.org Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Authentication-Results: ORIGINATING; auth=pass smtp.auth=dsimic@manjaro.org smtp.mailfrom=dsimic@manjaro.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240918_042718_786046_982D3691 X-CRM114-Status: GOOD ( 35.56 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org [Sorry for the noise, the webmail I'm using somehow managed to mess up the recipients list. Sending again with the list fixed.] Hello Andrey, On 2024-09-18 13:00, Andrey Skvortsov wrote: > On 24-09-18 11:27, Dragan Simic wrote: >> 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@vger.kernel.org > > Thanks for the commit description, it's much better, than original > one. Thanks, I'm glad that you like it. >> 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. Yes, reporting +1 g on the z-axis with the device remaining stationary on a level surface is the normal behavior, and the returned positive value actually goes along with the quoted description from the kernel documentation. The z-axis of the MPU-6050 goes upward and out of the screen, the way the MPU-6050 is placed inside the PinePhone. > 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. Wouldn't inverting the direction of the z-axis go against the above-quoted description from the kernel documentation? >> > > > Signed-off-by: Ondrej Jirman >> > > > Signed-off-by: Andrey Skvortsov >> > > > --- >> > > > 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. The way it's specified, it actually swaps the x- and y-axis, and inverts the direction of the y-axis, all that from the viewpoint of the accelerometer, which matches the proposed patch description. IOW, the description keeps the original names of the axes. >> 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