[PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

[PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

[Andrey Skvortsov]

In newer 1.2 PinePhone board revisions LIS3MDL magnetometer was replaced by
AF8133J. They use the same PB1 pin 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.

AF8133J has reset pin (PB1) connected to the SoC. By default AF8133J
is in a reset state and don't respond to probe request on I2C
bus. Extra code would be needed to handle reset signal. Therefore this
code uses LIS3MDL magnetometer instead of AF8133J.

Introducing new dts 1.2b with AF8133J sensor would require probing in
SPL. That would lead to pulling in into SPL I2C controller driver,
RSB controller driver, introducing new AXP803 driver to power-up
sensors for probe. It's working, but SPL is pretty size-constrained on
A64 and doesn't have much space. Therefore fdt fixup is done in U-Boot
proper without introducing new board revision and new dts.

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---

AF8133J's driver isn't upstreamed yet, but it will be soon. Therefore this
is RFC patch. I'd like to know whether selected approach will be accepted
in u-boot before submiting coresponding dts changes to the Linux kernel.
Any feedback on this change would be very welcome.

 board/sunxi/board.c         | 26 ++++++++++++++++++++++++++
 configs/pinephone_defconfig |  1 +
 2 files changed, 27 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 1305302060..a4bfa24400 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -15,6 +15,7 @@
 #include <dm.h>
 #include <env.h>
 #include <hang.h>
+#include <i2c.h>
 #include <image.h>
 #include <init.h>
 #include <log.h>
@@ -920,6 +921,28 @@ static void bluetooth_dt_fixup(void *blob)
 			   "local-bd-address", bdaddr, ETH_ALEN, 1);
 }
 
+#ifdef CONFIG_PINEPHONE_DT_SELECTION
+
+#define PINEPHONE_LIS3MDL_I2C_ADDR	0x1E
+#define PINEPHONE_LIS3MDL_I2C_BUS	1 /* I2C1 */
+
+static	void board_dt_fixup(void *blob)
+{
+	struct udevice *bus, *dev;
+
+	if (!fdt_node_check_compatible(blob, 0, "pine64,pinephone-1.2"))
+		if (!uclass_get_device_by_seq(UCLASS_I2C, PINEPHONE_LIS3MDL_I2C_BUS, &bus)) {
+			dm_i2c_probe(bus, PINEPHONE_LIS3MDL_I2C_ADDR, 0, &dev);
+			fdt_set_status_by_compatible(
+				blob, "st,lis3mdl-magn",
+				dev ? FDT_STATUS_OKAY  : FDT_STATUS_DISABLED);
+			fdt_set_status_by_compatible(
+				blob, "voltafield,af8133j",
+				dev ? FDT_STATUS_DISABLED : FDT_STATUS_OKAY);
+		}
+}
+#endif
+
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
 	int __maybe_unused r;
@@ -934,6 +957,9 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 
 	bluetooth_dt_fixup(blob);
 
+#ifdef CONFIG_PINEPHONE_DT_SELECTION
+	board_dt_fixup(blob);
+#endif
 #ifdef CONFIG_VIDEO_DT_SIMPLEFB
 	r = sunxi_simplefb_setup(blob);
 	if (r)
diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
index eebc676901..457e7ee1e7 100644
--- a/configs/pinephone_defconfig
+++ b/configs/pinephone_defconfig
@@ -12,6 +12,7 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2
 CONFIG_PINEPHONE_DT_SELECTION=y
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2"
+CONFIG_SYS_I2C_MVTWSI=y
 CONFIG_LED_STATUS=y
 CONFIG_LED_STATUS_GPIO=y
 CONFIG_LED_STATUS0=y
-- 
2.43.0

Re: [PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

[Andre Przywara]

On Sun, 11 Feb 2024 12:28:24 +0300
Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:

Hi Andrey,

thanks for taking care of this upstream!

> In newer 1.2 PinePhone board revisions LIS3MDL magnetometer was replaced by
> AF8133J. They use the same PB1 pin 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.
> 
> AF8133J has reset pin (PB1) connected to the SoC. By default AF8133J
> is in a reset state and don't respond to probe request on I2C
> bus. Extra code would be needed to handle reset signal. Therefore this
> code uses LIS3MDL magnetometer instead of AF8133J.

Thanks for the research and explanation, that makes it much easier to
reason about!

> Introducing new dts 1.2b with AF8133J sensor would require probing in
> SPL. That would lead to pulling in into SPL I2C controller driver,
> RSB controller driver, introducing new AXP803 driver to power-up
> sensors for probe. It's working, but SPL is pretty size-constrained on
> A64 and doesn't have much space. Therefore fdt fixup is done in U-Boot
> proper without introducing new board revision and new dts.

I agree on that, especially if it's indeed just a matter of flipping
two "status" properties.

> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> ---
> 
> AF8133J's driver isn't upstreamed yet, but it will be soon. Therefore this
> is RFC patch. I'd like to know whether selected approach will be accepted
> in u-boot before submiting coresponding dts changes to the Linux kernel.
> Any feedback on this change would be very welcome.

So I think this is the right approach: Since the SPL has no use of this
device, and it's a rather small DT change, we definitely want to do this
in U-Boot proper. And I believe that U-Boot (proper) is indeed the best
place to do those adjustments, since it's built for one board anyway,
so anything board specific belongs into there (and not into TF-A or the
SPL, which are more tailored to one *SoC*). Also we are not so size
sensitive in proper.
And I also like the fact that it's protected by a board specific
definition, and even better that it's an already existing one.

Oh, and I am not really convinced this is the right approach here, but
maybe have a look at doc/usage/cmd/extension.rst, for another related
mechanism.

Some smaller comments below ...

> 
>  board/sunxi/board.c         | 26 ++++++++++++++++++++++++++
>  configs/pinephone_defconfig |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index 1305302060..a4bfa24400 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -15,6 +15,7 @@
>  #include <dm.h>
>  #include <env.h>
>  #include <hang.h>
> +#include <i2c.h>
>  #include <image.h>
>  #include <init.h>
>  #include <log.h>
> @@ -920,6 +921,28 @@ static void bluetooth_dt_fixup(void *blob)
>  			   "local-bd-address", bdaddr, ETH_ALEN, 1);
>  }
>  
> +#ifdef CONFIG_PINEPHONE_DT_SELECTION

Can you move that line into the function? That would for one avoid the
protection in the caller below, and secondly make it easier to add
other boards, if a need arises for them. The two #defines don't hurt or
change the code, so just keep them outside.

> +
> +#define PINEPHONE_LIS3MDL_I2C_ADDR	0x1E
> +#define PINEPHONE_LIS3MDL_I2C_BUS	1 /* I2C1 */
> +
> +static	void board_dt_fixup(void *blob)
> +{
> +	struct udevice *bus, *dev;
> +

(have the #ifdef here)

> +	if (!fdt_node_check_compatible(blob, 0, "pine64,pinephone-1.2"))

Please have curly braces around that.

So I'd say please send the (disabled) DT node addition patch to the
kernel MLs, then send this patch to U-Boot.

Cheers,
Andre

> +		if (!uclass_get_device_by_seq(UCLASS_I2C, PINEPHONE_LIS3MDL_I2C_BUS, &bus)) {
> +			dm_i2c_probe(bus, PINEPHONE_LIS3MDL_I2C_ADDR, 0, &dev);
> +			fdt_set_status_by_compatible(
> +				blob, "st,lis3mdl-magn",
> +				dev ? FDT_STATUS_OKAY  : FDT_STATUS_DISABLED);
> +			fdt_set_status_by_compatible(
> +				blob, "voltafield,af8133j",
> +				dev ? FDT_STATUS_DISABLED : FDT_STATUS_OKAY);
> +		}
> +}
> +#endif
> +
>  int ft_board_setup(void *blob, struct bd_info *bd)
>  {
>  	int __maybe_unused r;
> @@ -934,6 +957,9 @@ int ft_board_setup(void *blob, struct bd_info *bd)
>  
>  	bluetooth_dt_fixup(blob);
>  
> +#ifdef CONFIG_PINEPHONE_DT_SELECTION
> +	board_dt_fixup(blob);
> +#endif
>  #ifdef CONFIG_VIDEO_DT_SIMPLEFB
>  	r = sunxi_simplefb_setup(blob);
>  	if (r)
> diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
> index eebc676901..457e7ee1e7 100644
> --- a/configs/pinephone_defconfig
> +++ b/configs/pinephone_defconfig
> @@ -12,6 +12,7 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2
>  CONFIG_PINEPHONE_DT_SELECTION=y
>  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>  CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2"
> +CONFIG_SYS_I2C_MVTWSI=y
>  CONFIG_LED_STATUS=y
>  CONFIG_LED_STATUS_GPIO=y
>  CONFIG_LED_STATUS0=y

Re: [PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

[Andrey Skvortsov]

Hi Andre,

thank you for the valuable feedback!

On 24-02-11 13:13, Andre Przywara wrote:
> On Sun, 11 Feb 2024 12:28:24 +0300
> Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:
> 
> Hi Andrey,
> 
> thanks for taking care of this upstream!
> 
> > In newer 1.2 PinePhone board revisions LIS3MDL magnetometer was replaced by
> > AF8133J. They use the same PB1 pin in different modes.

> > 
> > AF8133J's driver isn't upstreamed yet, but it will be soon. Therefore this
> > is RFC patch. I'd like to know whether selected approach will be accepted
> > in u-boot before submiting coresponding dts changes to the Linux kernel.
> > Any feedback on this change would be very welcome.
> 
> So I think this is the right approach: Since the SPL has no use of this
> device, and it's a rather small DT change, we definitely want to do this
> in U-Boot proper. And I believe that U-Boot (proper) is indeed the best
> place to do those adjustments, since it's built for one board anyway,
> so anything board specific belongs into there (and not into TF-A or the
> SPL, which are more tailored to one *SoC*). Also we are not so size
> sensitive in proper.
> And I also like the fact that it's protected by a board specific
> definition, and even better that it's an already existing one.
> 
> Oh, and I am not really convinced this is the right approach here, but
> maybe have a look at doc/usage/cmd/extension.rst, for another related
> mechanism.

Thanks for the tip. I've looked at it after your suggestion and tried
to implement the same logic using extension command. Here are my
thoughts about that:

1. integrated magnetometer is part of the device and making it
extension with separate dtbo sounds a bit strange.

2. if I'd create dtbo for new AF8133J magnetometer, then I'd call it
like other dts files for the device:
'sun50i-a64-pinephone-1.2-af8133j.dtbo. This is 38 characters and 
currently overlay name is limited to 32 bytes. What do you think about
increasing overlay name?

3. extensions are not supported for extlinux boot flow at all. This is Debian
case, that I'm working with. And this is a major problem for me.

4. I've looked how EFI boot flow is made and I see that extensions are
not applied to fdtcontroladdr, only to loaded fdt to
fdt_addr_r. Extlinux uses fdtcontroladdr always.

5. When distribution supplies fdt with extlinux, when supplied fdt is
used and no extensions are applied to it. This is my case as well.

6. I can't apply extensions to fdtcontroladdr. When I've tried to
apply working extension to fdtcontroladdr, then I get a crash.
I have to copy fdt from fdtcontroladdr to fdt_addr_r
and then apply extension to fdt_addr_r and when it works. Maybe this
is something sunxi-specific.

Overall extensions seems like a nice feature for capes, extension boards for
pogo pins and so on. But I'm not sure, that it's the right choice in
this case.

> Some smaller comments below ...
> 
> > 
> >  board/sunxi/board.c         | 26 ++++++++++++++++++++++++++
> >  configs/pinephone_defconfig |  1 +
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > index 1305302060..a4bfa24400 100644
> > --- a/board/sunxi/board.c
> > +++ b/board/sunxi/board.c
> > @@ -15,6 +15,7 @@
> >  #include <dm.h>
> >  #include <env.h>
> >  #include <hang.h>
> > +#include <i2c.h>
> >  #include <image.h>
> >  #include <init.h>
> >  #include <log.h>
> > @@ -920,6 +921,28 @@ static void bluetooth_dt_fixup(void *blob)
> >  			   "local-bd-address", bdaddr, ETH_ALEN, 1);
> >  }
> >  
> > +#ifdef CONFIG_PINEPHONE_DT_SELECTION
> 
> Can you move that line into the function? That would for one avoid the
> protection in the caller below, and secondly make it easier to add
> other boards, if a need arises for them. The two #defines don't hurt or
> change the code, so just keep them outside.

> > +
> > +#define PINEPHONE_LIS3MDL_I2C_ADDR	0x1E
> > +#define PINEPHONE_LIS3MDL_I2C_BUS	1 /* I2C1 */
> > +
> > +static	void board_dt_fixup(void *blob)
> > +{
> > +	struct udevice *bus, *dev;
> > +
> 
> (have the #ifdef here)

Ok, then I mark local variables with __maybe_unused and remove #ifdef
at board_dt_fixup call.

> 
> > +	if (!fdt_node_check_compatible(blob, 0, "pine64,pinephone-1.2"))
> 
> Please have curly braces around that.

I'll fix that in v2. 

> So I'd say please send the (disabled) DT node addition patch to the
> kernel MLs, then send this patch to U-Boot.

Do you mean patch with disabled AF8133J DT node? Right?
If so, then it was the plan.

1. upstream AF8133J driver to the Linux kernel (on-going)
2. find acceptable solution for u-boot to handle different
   magnetometers (on-going in this thread) 
3. upstream necessary dts changes to the Linux kernel
4. upstream previously discussed changes to u-boot.


> 
> Cheers,
> Andre
> 
> > +		if (!uclass_get_device_by_seq(UCLASS_I2C, PINEPHONE_LIS3MDL_I2C_BUS, &bus)) {
> > +			dm_i2c_probe(bus, PINEPHONE_LIS3MDL_I2C_ADDR, 0, &dev);
> > +			fdt_set_status_by_compatible(
> > +				blob, "st,lis3mdl-magn",
> > +				dev ? FDT_STATUS_OKAY  : FDT_STATUS_DISABLED);
> > +			fdt_set_status_by_compatible(
> > +				blob, "voltafield,af8133j",
> > +				dev ? FDT_STATUS_DISABLED : FDT_STATUS_OKAY);
> > +		}
> > +}
> > +#endif
> > +
> >  int ft_board_setup(void *blob, struct bd_info *bd)
> >  {
> >  	int __maybe_unused r;
> > @@ -934,6 +957,9 @@ int ft_board_setup(void *blob, struct bd_info *bd)
> >  
> >  	bluetooth_dt_fixup(blob);
> >  
> > +#ifdef CONFIG_PINEPHONE_DT_SELECTION
> > +	board_dt_fixup(blob);
> > +#endif

I'll remove #ifdef here to make code a bit cleaner. I've applied all
your suggestions and make it available here [1].

> >  #ifdef CONFIG_VIDEO_DT_SIMPLEFB
> >  	r = sunxi_simplefb_setup(blob);
> >  	if (r)
> > diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
> > index eebc676901..457e7ee1e7 100644
> > --- a/configs/pinephone_defconfig
> > +++ b/configs/pinephone_defconfig
> > @@ -12,6 +12,7 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2
> >  CONFIG_PINEPHONE_DT_SELECTION=y
> >  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> >  CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2"
> > +CONFIG_SYS_I2C_MVTWSI=y
> >  CONFIG_LED_STATUS=y
> >  CONFIG_LED_STATUS_GPIO=y
> >  CONFIG_LED_STATUS0=y
> 

1. https://github.com/AndreySV/u-boot/commit/84a9f1c14d1850c5559c5d888c814eee8886b04f

-- 
Best regards,
Andrey Skvortsov

Re: [PATCH RFC] sunxi: pinephone: detect existed magnetometer and fixup dtb

[Andre Przywara]

On Mon, 12 Feb 2024 01:38:36 +0300
Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:

Hi Andrey,

> thank you for the valuable feedback!
> 
> On 24-02-11 13:13, Andre Przywara wrote:
> > On Sun, 11 Feb 2024 12:28:24 +0300
> > Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:
> > 
> > Hi Andrey,
> > 
> > thanks for taking care of this upstream!
> >   
> > > In newer 1.2 PinePhone board revisions LIS3MDL magnetometer was replaced by
> > > AF8133J. They use the same PB1 pin in different modes.  
> 
> > > 
> > > AF8133J's driver isn't upstreamed yet, but it will be soon. Therefore this
> > > is RFC patch. I'd like to know whether selected approach will be accepted
> > > in u-boot before submiting coresponding dts changes to the Linux kernel.
> > > Any feedback on this change would be very welcome.  
> > 
> > So I think this is the right approach: Since the SPL has no use of this
> > device, and it's a rather small DT change, we definitely want to do this
> > in U-Boot proper. And I believe that U-Boot (proper) is indeed the best
> > place to do those adjustments, since it's built for one board anyway,
> > so anything board specific belongs into there (and not into TF-A or the
> > SPL, which are more tailored to one *SoC*). Also we are not so size
> > sensitive in proper.
> > And I also like the fact that it's protected by a board specific
> > definition, and even better that it's an already existing one.
> > 
> > Oh, and I am not really convinced this is the right approach here, but
> > maybe have a look at doc/usage/cmd/extension.rst, for another related
> > mechanism.  
> 
> Thanks for the tip. I've looked at it after your suggestion and tried
> to implement the same logic using extension command. Here are my
> thoughts about that:
> 
> 1. integrated magnetometer is part of the device and making it
> extension with separate dtbo sounds a bit strange.
> 
> 2. if I'd create dtbo for new AF8133J magnetometer, then I'd call it
> like other dts files for the device:
> 'sun50i-a64-pinephone-1.2-af8133j.dtbo. This is 38 characters and 
> currently overlay name is limited to 32 bytes. What do you think about
> increasing overlay name?
> 
> 3. extensions are not supported for extlinux boot flow at all. This is Debian
> case, that I'm working with. And this is a major problem for me.
> 
> 4. I've looked how EFI boot flow is made and I see that extensions are
> not applied to fdtcontroladdr, only to loaded fdt to
> fdt_addr_r. Extlinux uses fdtcontroladdr always.
> 
> 5. When distribution supplies fdt with extlinux, when supplied fdt is
> used and no extensions are applied to it. This is my case as well.
> 
> 6. I can't apply extensions to fdtcontroladdr. When I've tried to
> apply working extension to fdtcontroladdr, then I get a crash.
> I have to copy fdt from fdtcontroladdr to fdt_addr_r
> and then apply extension to fdt_addr_r and when it works. Maybe this
> is something sunxi-specific.

Not really. The DTB is appended to the end of the U-Boot image, and I
believe there is something important immediately after it, like the
heap or the gd or something. Some years back this used to work, but not
anymore, for quite a while now.
So to apply overlays, do a "fdt move $fdtcontroladdr $fdt_addr_r"
first, then use $fdt_addr_r.
But that's just for clarification, nothing that solves the above
problems.

> Overall extensions seems like a nice feature for capes, extension boards for
> pogo pins and so on. But I'm not sure, that it's the right choice in
> this case.

Yeah, many thanks for the extensive research and nice summary! I was
already expecting something along those lines, but wasn't sure. So
thanks for saving me some time.

This confirms that we should go the route you already sketched.

> > Some smaller comments below ...
> >   
> > > 
> > >  board/sunxi/board.c         | 26 ++++++++++++++++++++++++++
> > >  configs/pinephone_defconfig |  1 +
> > >  2 files changed, 27 insertions(+)
> > > 
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index 1305302060..a4bfa24400 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -15,6 +15,7 @@
> > >  #include <dm.h>
> > >  #include <env.h>
> > >  #include <hang.h>
> > > +#include <i2c.h>
> > >  #include <image.h>
> > >  #include <init.h>
> > >  #include <log.h>
> > > @@ -920,6 +921,28 @@ static void bluetooth_dt_fixup(void *blob)
> > >  			   "local-bd-address", bdaddr, ETH_ALEN, 1);
> > >  }
> > >  
> > > +#ifdef CONFIG_PINEPHONE_DT_SELECTION  
> > 
> > Can you move that line into the function? That would for one avoid the
> > protection in the caller below, and secondly make it easier to add
> > other boards, if a need arises for them. The two #defines don't hurt or
> > change the code, so just keep them outside.  
> 
> > > +
> > > +#define PINEPHONE_LIS3MDL_I2C_ADDR	0x1E
> > > +#define PINEPHONE_LIS3MDL_I2C_BUS	1 /* I2C1 */
> > > +
> > > +static	void board_dt_fixup(void *blob)
> > > +{
> > > +	struct udevice *bus, *dev;
> > > +  
> > 
> > (have the #ifdef here)  
> 
> Ok, then I mark local variables with __maybe_unused and remove #ifdef
> at board_dt_fixup call.

Ah right, there are warnings.
I've got an even better idea, use:
      if (IS_ENABLED(CONFIG_PINEPHONE_DT_SELECTION) &&
          !fdt_node_check_compatible(blob, 0, "pine64,pinephone-1.2")) {

Then you can keep the variables without the tag, and avoid the #ifdef
altogether. The compiler optimises this away, for an unrelated board
"size board.o" reports exactly the same numbers.

> > > +	if (!fdt_node_check_compatible(blob, 0, "pine64,pinephone-1.2"))  
> > 
> > Please have curly braces around that.  
> 
> I'll fix that in v2. 
> 
> > So I'd say please send the (disabled) DT node addition patch to the
> > kernel MLs, then send this patch to U-Boot.  
> 
> Do you mean patch with disabled AF8133J DT node? Right?
> If so, then it was the plan.
> 
> 1. upstream AF8133J driver to the Linux kernel (on-going)
> 2. find acceptable solution for u-boot to handle different
>    magnetometers (on-going in this thread) 
> 3. upstream necessary dts changes to the Linux kernel

I'd say we settled 2., so feel free to append the DT change to the
series in 1), or send it as a follow-up patch if it's out there already.
This gives us also a user in the kernel DT tree.
You should add a comment to the disabled node, that this will be fixed
up in firmware.

Thanks,
Andre

> 4. upstream previously discussed changes to u-boot.
> 
> 
> > 
> > Cheers,
> > Andre
> >   
> > > +		if (!uclass_get_device_by_seq(UCLASS_I2C, PINEPHONE_LIS3MDL_I2C_BUS, &bus)) {
> > > +			dm_i2c_probe(bus, PINEPHONE_LIS3MDL_I2C_ADDR, 0, &dev);
> > > +			fdt_set_status_by_compatible(
> > > +				blob, "st,lis3mdl-magn",
> > > +				dev ? FDT_STATUS_OKAY  : FDT_STATUS_DISABLED);
> > > +			fdt_set_status_by_compatible(
> > > +				blob, "voltafield,af8133j",
> > > +				dev ? FDT_STATUS_DISABLED : FDT_STATUS_OKAY);
> > > +		}
> > > +}
> > > +#endif
> > > +
> > >  int ft_board_setup(void *blob, struct bd_info *bd)
> > >  {
> > >  	int __maybe_unused r;
> > > @@ -934,6 +957,9 @@ int ft_board_setup(void *blob, struct bd_info *bd)
> > >  
> > >  	bluetooth_dt_fixup(blob);
> > >  
> > > +#ifdef CONFIG_PINEPHONE_DT_SELECTION
> > > +	board_dt_fixup(blob);
> > > +#endif  
> 
> I'll remove #ifdef here to make code a bit cleaner. I've applied all
> your suggestions and make it available here [1].
> 
> > >  #ifdef CONFIG_VIDEO_DT_SIMPLEFB
> > >  	r = sunxi_simplefb_setup(blob);
> > >  	if (r)
> > > diff --git a/configs/pinephone_defconfig b/configs/pinephone_defconfig
> > > index eebc676901..457e7ee1e7 100644
> > > --- a/configs/pinephone_defconfig
> > > +++ b/configs/pinephone_defconfig
> > > @@ -12,6 +12,7 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2
> > >  CONFIG_PINEPHONE_DT_SELECTION=y
> > >  # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> > >  CONFIG_OF_LIST="sun50i-a64-pinephone-1.1 sun50i-a64-pinephone-1.2"
> > > +CONFIG_SYS_I2C_MVTWSI=y
> > >  CONFIG_LED_STATUS=y
> > >  CONFIG_LED_STATUS_GPIO=y
> > >  CONFIG_LED_STATUS0=y  
> >   
> 
> 1. https://github.com/AndreySV/u-boot/commit/84a9f1c14d1850c5559c5d888c814eee8886b04f
>