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

[Andre Heider]

On 26/11/2019 13:40, Ondřej Jirman wrote:
> 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.

Okay let me remove it then, but I'll go with what the maintainers deem 
acceptable.

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

Yeah, that might just work, thanks!

Andre