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

[Andre Heider]

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.

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
>>