[U-Boot] [PATCH v2 2/4] arm: sunxi: add a config option to fixup a Bluetooth address

[Ondřej Jirman]

On Tue, Nov 26, 2019 at 01:15:42PM +0100, Andre Heider wrote:
> On 26/11/2019 11:46, Ondřej Jirman wrote:
> > Hello Andre,
> > 
> > On Tue, Nov 26, 2019 at 09:16:00AM +0100, Andre Heider wrote:
> > > Some Bluetooth controllers, like the BCM4345C5 of the Orange Pi 3,
> > > ship with the controller default address.
> > > 
> > > Add a config option to fix it up so it can function properly.
> > 
> > I tried it on TBS A711 tablet that also has bluetooth, but that doesn't have
> > ethernet (ethernet alias is not set up) so no ethaddr gets generated and
> > bdaddr setting fails due to dependency on ethernet alias being present.
> > 
> > So for this mechanism to be more universal, sid->mac_addr code probably
> > needs to be moved to it's own function, and used both from setup_environment
> > and fixup_bd_address, to avoid dependency on boards having ethernet.
> 
> Okay, I extracted a helper function locally, so it doesn't rely on
> "ethaddr". That should work on your tablet then I think. I'll wait a bit for
> further feedback before sending out v3.
> 
> > 
> > Otherwise, on Orange Pi 3:
> > 
> > Tested-by: Ondrej Jirman <megous at megous.com>
> 
> Nice, thanks!
> 
> > 
> > Also see the comment below:
> > 
> > > Signed-off-by: Andre Heider <a.heider at gmail.com>
> > > ---
> > >   arch/arm/mach-sunxi/Kconfig | 12 ++++++++++++
> > >   board/sunxi/board.c         | 30 ++++++++++++++++++++++++++++++
> > >   2 files changed, 42 insertions(+)
> > > 
> > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> > > index 16d41b83af..b41c64870e 100644
> > > --- a/arch/arm/mach-sunxi/Kconfig
> > > +++ b/arch/arm/mach-sunxi/Kconfig
> > > @@ -1009,4 +1009,16 @@ config PINE64_DT_SELECTION
> > >   	  option, the device tree selection code specific to Pine64 which
> > >   	  utilizes the DRAM size will be enabled.
> > > +config FIXUP_BDADDR
> > > +	string "Fixup the Bluetooth controller address"
> > > +	depends on MACH_SUN50I_H6
> > 
> > ^ This should be more sunxi generic or perhaps just removed, because the code is
> > not specific to H6.
> 
> I didn't see a way to exclude fixup_bd_address() from compilation via
> #ifdefs when FIXUP_BDADDR is empty. And I didn't want to use 2 knobs (one to
> enable, one for the string).
> 
> Currently the function body has "if (strlen(CONFIG_FIXUP_BDADDR) < 1)
> return", and I *think* the compiler should be smart enough to just drop the
> whole thing if the string is empty. So the "depends on" is just a safety net
> to protect against code bloat for archs where it's currently unused anyway.
> 
> But I can drop it if there are no concerns.

I understand, but this is useful for many more sunxi SoCs, so I think the
small increase in main uboot size for sunxi boards is justified.

Anyway, I'd think using `if (CONFIG_FIXUP_BDADDR[0])` would be a safer bet
for compiler dropping the if body, rather than strlen.

regards,
	o.

> Thanks,
> Andre
> 
> > 
> > thank you and regards,
> > 	o.
> > 
> > > +	default ""
> > > +	help
> > > +	  This option specifies the DT compatible name of the Bluetooth
> > > +	  controller for which to set the "local-bd-address" property.
> > > +	  Set this option if your device ships with the Bluetooth controller
> > > +	  default address.
> > > +	  The used address is "bdaddr" if set, and "ethaddr" with the LSB
> > > +	  flipped elsewise.
> > > +
> > >   endif
> > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> > > index bb35d6b66e..89851102d1 100644
> > > --- a/board/sunxi/board.c
> > > +++ b/board/sunxi/board.c
> > > @@ -856,6 +856,34 @@ int misc_init_r(void)
> > >   	return 0;
> > >   }
> > > +static void fixup_bd_address(void *blob)
> > > +{
> > > +#ifdef CONFIG_FIXUP_BDADDR
> > > +	/* Some devices ship with a Bluetooth controller default address.
> > > +	 * Set a valid address through the device tree.
> > > +	 */
> > > +	uchar tmp[ETH_ALEN], bdaddr[ETH_ALEN];
> > > +	int i;
> > > +
> > > +	if (strlen(CONFIG_FIXUP_BDADDR) < 1)
> > > +		return;
> > > +
> > > +	if (!eth_env_get_enetaddr("bdaddr", tmp)) {
> > > +		if (!eth_env_get_enetaddr("ethaddr", tmp))
> > > +			return;
> > > +
> > > +		tmp[ETH_ALEN - 1] ^= 1;
> > > +	}
> > > +
> > > +	/* Addresses need to be in the binary format of the corresponding stack */
> > > +	for (i = 0; i < ETH_ALEN; ++i)
> > > +		bdaddr[i] = tmp[ETH_ALEN - i - 1];
> > > +
> > > +	do_fixup_by_compat(blob, CONFIG_FIXUP_BDADDR,
> > > +			   "local-bd-address", bdaddr, ETH_ALEN, 1);
> > > +#endif
> > > +}
> > > +
> > >   int ft_board_setup(void *blob, bd_t *bd)
> > >   {
> > >   	int __maybe_unused r;
> > > @@ -866,6 +894,8 @@ int ft_board_setup(void *blob, bd_t *bd)
> > >   	 */
> > >   	setup_environment(blob);
> > > +	fixup_bd_address(blob);
> > > +
> > >   #ifdef CONFIG_VIDEO_DT_SIMPLEFB
> > >   	r = sunxi_simplefb_setup(blob);
> > >   	if (r)
> > > -- 
> > > 2.24.0
> > > 
>