[PATCH 0/2] Fix magnetometers on PinePhone

[PATCH 0/2] Fix magnetometers on PinePhone

[Andrey Skvortsov]

Introduce support for alternative magnetometer (AF8133J) used on new
batches of PinePhones and define mount-matrix for both magnetometers.

Icenowy Zheng (1):
  arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone

Shoji Keita (1):
  arm64: dts: sun50i-a64-pinephone: Add mount-matrix for PinePhone
    magnetometers

 .../dts/allwinner/sun50i-a64-pinephone.dtsi    | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

-- 
2.45.2

[PATCH 1/2] arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone

[Andrey Skvortsov]

From: Icenowy Zheng <icenowy@aosc.io>

New batches of PinePhones switched the magnetometer to AF8133J from
LIS3MDL because lack of ST components.

Both chips use the same PB1 pin, but in different modes.
LIS3MDL uses it as an gpio input to handle interrupt.
AF8133J uses it as an gpio output as a reset signal.

It wasn't possible at runtime to enable both device tree nodes and
detect supported sensor at probe time, because both drivers try to
acquire the same gpio in different modes.

Device tree fixup will be done in firmware without introducing new board
revision and new dts.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
Link: https://patchwork.ozlabs.org/project/uboot/patch/20240211092824.395155-1-andrej.skvortzov@gmail.com/

---
 .../boot/dts/allwinner/sun50i-a64-pinephone.dtsi     | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index 6eab61a12cd8f..66fbb35a7fae9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -188,6 +188,18 @@ touchscreen@5d {
 &i2c1 {
 	status = "okay";
 
+	/* Alternative magnetometer */
+	af8133j: magnetometer@1c {
+		compatible = "voltafield,af8133j";
+		reg = <0x1c>;
+		reset-gpios = <&pio 1 1 GPIO_ACTIVE_LOW>;
+		avdd-supply = <&reg_dldo1>;
+		dvdd-supply = <&reg_dldo1>;
+
+		/* status will be fixed up in firmware */
+		status = "disabled";
+	};
+
 	/* Magnetometer */
 	lis3mdl: magnetometer@1e {
 		compatible = "st,lis3mdl-magn";
-- 
2.45.2

Re: [PATCH 1/2] arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone

[Chen-Yu Tsai]

On Mon, Sep 9, 2024 at 5:48 AM Andrey Skvortsov
<andrej.skvortzov@gmail.com> wrote:
>
> From: Icenowy Zheng <icenowy@aosc.io>
>
> New batches of PinePhones switched the magnetometer to AF8133J from
> LIS3MDL because lack of ST components.
>
> Both chips use the same PB1 pin, but in different modes.
> LIS3MDL uses it as an gpio input to handle interrupt.
> AF8133J uses it as an gpio output as a reset signal.
>
> It wasn't possible at runtime to enable both device tree nodes and
> detect supported sensor at probe time, because both drivers try to
> acquire the same gpio in different modes.
>
> Device tree fixup will be done in firmware without introducing new board
> revision and new dts.

FYI I've been working on an in-kernel prober [1] for such alternative
components. This does not require firmware support.

[1] https://lore.kernel.org/all/20240904090016.2841572-1-wenst@chromium.org/

> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> Link: https://patchwork.ozlabs.org/project/uboot/patch/20240211092824.395155-1-andrej.skvortzov@gmail.com/
>
> ---
>  .../boot/dts/allwinner/sun50i-a64-pinephone.dtsi     | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> index 6eab61a12cd8f..66fbb35a7fae9 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
> @@ -188,6 +188,18 @@ touchscreen@5d {
>  &i2c1 {
>         status = "okay";
>
> +       /* Alternative magnetometer */
> +       af8133j: magnetometer@1c {
> +               compatible = "voltafield,af8133j";
> +               reg = <0x1c>;
> +               reset-gpios = <&pio 1 1 GPIO_ACTIVE_LOW>;
> +               avdd-supply = <&reg_dldo1>;
> +               dvdd-supply = <&reg_dldo1>;
> +
> +               /* status will be fixed up in firmware */
> +               status = "disabled";
> +       };
> +
>         /* Magnetometer */
>         lis3mdl: magnetometer@1e {
>                 compatible = "st,lis3mdl-magn";
> --
> 2.45.2
>

Re: [PATCH 1/2] arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone

[Andrey Skvortsov]

Hi Chen-Yu Tsai,

On 24-09-09 16:08, Chen-Yu Tsai wrote:
> On Mon, Sep 9, 2024 at 5:48 AM Andrey Skvortsov
> <andrej.skvortzov@gmail.com> wrote:
> >
> > From: Icenowy Zheng <icenowy@aosc.io>
> >
> > New batches of PinePhones switched the magnetometer to AF8133J from
> > LIS3MDL because lack of ST components.
> >
> > Both chips use the same PB1 pin, but in different modes.
> > LIS3MDL uses it as an gpio input to handle interrupt.
> > AF8133J uses it as an gpio output as a reset signal.
> >
> > It wasn't possible at runtime to enable both device tree nodes and
> > detect supported sensor at probe time, because both drivers try to
> > acquire the same gpio in different modes.
> >
> > Device tree fixup will be done in firmware without introducing new board
> > revision and new dts.
> 
> FYI I've been working on an in-kernel prober [1] for such alternative
> components. This does not require firmware support.
> 
> [1] https://lore.kernel.org/all/20240904090016.2841572-1-wenst@chromium.org/

Thank you for the information.

I've tried to use in-kernel prober from your v7 patchset [1] on top of
-next and it worked without any changes to firmware.

Since there is still on-going review of your patches it looks like
it's to early to submit my changes for review. But I'm ready to test
your new patches.

[1] https://lore.kernel.org/all/20240911072751.365361-1-wenst@chromium.org/

-- 
Best regards,
Andrey Skvortsov

Re: [PATCH 1/2] arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone

[Chen-Yu Tsai]

On Sun, Sep 15, 2024 at 6:12 PM Andrey Skvortsov
<andrej.skvortzov@gmail.com> wrote:
>
> Hi Chen-Yu Tsai,
>
> On 24-09-09 16:08, Chen-Yu Tsai wrote:
> > On Mon, Sep 9, 2024 at 5:48 AM Andrey Skvortsov
> > <andrej.skvortzov@gmail.com> wrote:
> > >
> > > From: Icenowy Zheng <icenowy@aosc.io>
> > >
> > > New batches of PinePhones switched the magnetometer to AF8133J from
> > > LIS3MDL because lack of ST components.
> > >
> > > Both chips use the same PB1 pin, but in different modes.
> > > LIS3MDL uses it as an gpio input to handle interrupt.
> > > AF8133J uses it as an gpio output as a reset signal.
> > >
> > > It wasn't possible at runtime to enable both device tree nodes and
> > > detect supported sensor at probe time, because both drivers try to
> > > acquire the same gpio in different modes.
> > >
> > > Device tree fixup will be done in firmware without introducing new board
> > > revision and new dts.
> >
> > FYI I've been working on an in-kernel prober [1] for such alternative
> > components. This does not require firmware support.
> >
> > [1] https://lore.kernel.org/all/20240904090016.2841572-1-wenst@chromium.org/
>
> Thank you for the information.
>
> I've tried to use in-kernel prober from your v7 patchset [1] on top of
> -next and it worked without any changes to firmware.
>
> Since there is still on-going review of your patches it looks like
> it's to early to submit my changes for review. But I'm ready to test
> your new patches.

FYI I'm open to either approach. If the firmware can do it, that is also
fine. I don't know if it makes sense to have both disabled by default
though? That would break existing users, but so would the in-kernel
prober approach, which requires both components be marked as
"fail-needs-probe", and also requires that the kernel driver be enabled.

In other words, I think the firmware approach is friendlier for existing
users that have the original batches.


ChenYu


> [1] https://lore.kernel.org/all/20240911072751.365361-1-wenst@chromium.org/
>
> --
> Best regards,
> Andrey Skvortsov
>

Re: [PATCH 1/2] arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone

[Andrey Skvortsov]

Hi Chen-Yu Tsai,

On 24-10-19 10:04, Chen-Yu Tsai wrote:
> On Sun, Sep 15, 2024 at 6:12 PM Andrey Skvortsov
> <andrej.skvortzov@gmail.com> wrote:
> >
> > Hi Chen-Yu Tsai,
> >
> > On 24-09-09 16:08, Chen-Yu Tsai wrote:
> > > On Mon, Sep 9, 2024 at 5:48 AM Andrey Skvortsov
> > > <andrej.skvortzov@gmail.com> wrote:
> > > >
> > > > From: Icenowy Zheng <icenowy@aosc.io>
> > > >
> > > > New batches of PinePhones switched the magnetometer to AF8133J from
> > > > LIS3MDL because lack of ST components.
> > > >
> > > > Both chips use the same PB1 pin, but in different modes.
> > > > LIS3MDL uses it as an gpio input to handle interrupt.
> > > > AF8133J uses it as an gpio output as a reset signal.
> > > >
> > > > It wasn't possible at runtime to enable both device tree nodes and
> > > > detect supported sensor at probe time, because both drivers try to
> > > > acquire the same gpio in different modes.
> > > >
> > > > Device tree fixup will be done in firmware without introducing new board
> > > > revision and new dts.
> > >
> > > FYI I've been working on an in-kernel prober [1] for such alternative
> > > components. This does not require firmware support.
> > >
> > > [1] https://lore.kernel.org/all/20240904090016.2841572-1-wenst@chromium.org/
> >
> > Thank you for the information.
> >
> > I've tried to use in-kernel prober from your v7 patchset [1] on top of
> > -next and it worked without any changes to firmware.
> >
> > Since there is still on-going review of your patches it looks like
> > it's to early to submit my changes for review. But I'm ready to test
> > your new patches.
> 
> FYI I'm open to either approach. If the firmware can do it, that is also
> fine. I don't know if it makes sense to have both disabled by default
> though? That would break existing users, but so would the in-kernel
> prober approach, which requires both components be marked as
> "fail-needs-probe", and also requires that the kernel driver be enabled.
> In other words, I think the firmware approach is friendlier for existing
> users that have the original batches.

Current patches leave original magnetometer enabled as before. So only
the new alternative magnetometer is disabled. Firmware prober will set
the correct status. So you are right firmware approach is a bit nicer
for existing users, nothing will change for them with any combination
of kernel and firmware. Let's go with a firmware approach with current
patches then, if nobody 

But I like your in-kernel approach as well. 
JFYI I've applied your v10 patches [1] on top of next-20241105 and retested
it with patches for magnetometer. It's available here [2]. 

1. https://lore.kernel.org/lkml/20241030072229.1013235-1-wenst@chromium.org/#t
2. https://github.com/AndreySV/linux-stable/commits/in-kernel-hwprober-magnetometer/

-- 
Best regards,
Andrey Skvortsov

Re: [PATCH 1/2] arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone

[Chen-Yu Tsai]

On Wed, Nov 6, 2024 at 3:51 AM Andrey Skvortsov
<andrej.skvortzov@gmail.com> wrote:
>
> Hi Chen-Yu Tsai,
>
> On 24-10-19 10:04, Chen-Yu Tsai wrote:
> > On Sun, Sep 15, 2024 at 6:12 PM Andrey Skvortsov
> > <andrej.skvortzov@gmail.com> wrote:
> > >
> > > Hi Chen-Yu Tsai,
> > >
> > > On 24-09-09 16:08, Chen-Yu Tsai wrote:
> > > > On Mon, Sep 9, 2024 at 5:48 AM Andrey Skvortsov
> > > > <andrej.skvortzov@gmail.com> wrote:
> > > > >
> > > > > From: Icenowy Zheng <icenowy@aosc.io>
> > > > >
> > > > > New batches of PinePhones switched the magnetometer to AF8133J from
> > > > > LIS3MDL because lack of ST components.
> > > > >
> > > > > Both chips use the same PB1 pin, but in different modes.
> > > > > LIS3MDL uses it as an gpio input to handle interrupt.
> > > > > AF8133J uses it as an gpio output as a reset signal.
> > > > >
> > > > > It wasn't possible at runtime to enable both device tree nodes and
> > > > > detect supported sensor at probe time, because both drivers try to
> > > > > acquire the same gpio in different modes.
> > > > >
> > > > > Device tree fixup will be done in firmware without introducing new board
> > > > > revision and new dts.
> > > >
> > > > FYI I've been working on an in-kernel prober [1] for such alternative
> > > > components. This does not require firmware support.
> > > >
> > > > [1] https://lore.kernel.org/all/20240904090016.2841572-1-wenst@chromium.org/
> > >
> > > Thank you for the information.
> > >
> > > I've tried to use in-kernel prober from your v7 patchset [1] on top of
> > > -next and it worked without any changes to firmware.
> > >
> > > Since there is still on-going review of your patches it looks like
> > > it's to early to submit my changes for review. But I'm ready to test
> > > your new patches.
> >
> > FYI I'm open to either approach. If the firmware can do it, that is also
> > fine. I don't know if it makes sense to have both disabled by default
> > though? That would break existing users, but so would the in-kernel
> > prober approach, which requires both components be marked as
> > "fail-needs-probe", and also requires that the kernel driver be enabled.
> > In other words, I think the firmware approach is friendlier for existing
> > users that have the original batches.
>
> Current patches leave original magnetometer enabled as before. So only
> the new alternative magnetometer is disabled. Firmware prober will set
> the correct status. So you are right firmware approach is a bit nicer
> for existing users, nothing will change for them with any combination
> of kernel and firmware. Let's go with a firmware approach with current
> patches then, if nobody

If?

I'll wait a day before applying this patch then.

ChenYu

> But I like your in-kernel approach as well.
> JFYI I've applied your v10 patches [1] on top of next-20241105 and retested
> it with patches for magnetometer. It's available here [2].
>
> 1. https://lore.kernel.org/lkml/20241030072229.1013235-1-wenst@chromium.org/#t
> 2. https://github.com/AndreySV/linux-stable/commits/in-kernel-hwprober-magnetometer/
>
> --
> Best regards,
> Andrey Skvortsov

Re: [PATCH 1/2] arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone

[Andrey Skvortsov]

Hi,

On 24-11-06 10:31, Chen-Yu Tsai wrote:
> On Wed, Nov 6, 2024 at 3:51 AM Andrey Skvortsov
> <andrej.skvortzov@gmail.com> wrote:

> >
> > >
> > > FYI I'm open to either approach. If the firmware can do it, that is also
> > > fine. I don't know if it makes sense to have both disabled by default
> > > though? That would break existing users, but so would the in-kernel
> > > prober approach, which requires both components be marked as
> > > "fail-needs-probe", and also requires that the kernel driver be enabled.
> > > In other words, I think the firmware approach is friendlier for existing
> > > users that have the original batches.
> >
> > Current patches leave original magnetometer enabled as before. So only
> > the new alternative magnetometer is disabled. Firmware prober will set
> > the correct status. So you are right firmware approach is a bit nicer
> > for existing users, nothing will change for them with any combination
> > of kernel and firmware. Let's go with a firmware approach with current
> > patches then, if nobody
> 
> If?
if no one has anything against that.


> I'll wait a day before applying this patch then.
Sure, thanks.

-- 
Best regards,
Andrey Skvortsov

[PATCH 2/2] arm64: dts: sun50i-a64-pinephone: Add mount-matrix for PinePhone magnetometers

[Andrey Skvortsov]

From: Shoji Keita <awaittrot@shjk.jp>

For lis3mdl, values are based on datasheet and PCB drawing
and tested on a real device.

For af8133j, values are from testing on a real device.

Signed-off-by: Shoji Keita <awaittrot@shjk.jp>
Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>

---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index 66fbb35a7fae9..d35d747292ce6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -195,6 +195,9 @@ af8133j: magnetometer@1c {
 		reset-gpios = <&pio 1 1 GPIO_ACTIVE_LOW>;
 		avdd-supply = <&reg_dldo1>;
 		dvdd-supply = <&reg_dldo1>;
+		mount-matrix = "0", "-1", "0",
+				"-1", "0", "0",
+				"0", "0", "-1";
 
 		/* status will be fixed up in firmware */
 		status = "disabled";
@@ -206,6 +209,9 @@ lis3mdl: magnetometer@1e {
 		reg = <0x1e>;
 		vdd-supply = <&reg_dldo1>;
 		vddio-supply = <&reg_dldo1>;
+		mount-matrix = "0", "1", "0",
+				"-1", "0", "0",
+				"0", "0", "1";
 	};
 
 	/* Light/proximity sensor */
-- 
2.45.2

Re: [PATCH 0/2] Fix magnetometers on PinePhone

[Chen-Yu Tsai]

On Mon, 09 Sep 2024 00:47:16 +0300, Andrey Skvortsov wrote:
> Introduce support for alternative magnetometer (AF8133J) used on new
> batches of PinePhones and define mount-matrix for both magnetometers.
> 
> Icenowy Zheng (1):
>   arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone
> 
> Shoji Keita (1):
>   arm64: dts: sun50i-a64-pinephone: Add mount-matrix for PinePhone
>     magnetometers
> 
> [...]

Applied to dt-for-6.13 in git@github.com:linux-sunxi/linux-sunxi.git, thanks!

[1/2] arm64: dts: sun50i-a64-pinephone: Add AF8133J to PinePhone
      commit: 193b199a925b04e2d9bc9dde8bb63f6d55483b7d
[2/2] arm64: dts: sun50i-a64-pinephone: Add mount-matrix for PinePhone magnetometers
      commit: 844c35cea6b2d36608c3026c7d29a4f5350b7ad9

Best regards,
-- 
Chen-Yu Tsai <wens@csie.org>