Bluetooth: hci_bcm: do not mark valid bd_addr as invalid

[PATCH] Bluetooth: hci_bcm: do not mark valid bd_addr as invalid

Posted by Johan Hovold 2 years, 8 months ago

A recent commit restored the original (and still documented) semantics
for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device address
is considered invalid unless an address is provided by firmware.

This specifically means that this flag must only be set for devices with
invalid addresses, but the Broadcom driver has so far been setting this
flag unconditionally.

Fortunately the driver already checks for invalid addresses during setup
and sets the HCI_QUIRK_INVALID_BDADDR flag. Use this flag to indicate
when the address can be overridden by firmware (long term, this should
probably just always be allowed).

Fixes: 6ac517d8cf8b ("Bluetooth: fix use-bdaddr-property quirk")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Link: https://lore.kernel.org/lkml/ecef83c8-497f-4011-607b-a63c24764867@samsung.com
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/bluetooth/hci_bcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 83bf5d4330c4..874d23089b39 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -643,7 +643,8 @@ static int bcm_setup(struct hci_uart *hu)
 	 * Allow the bootloader to set a valid address through the
 	 * device tree.
 	 */
-	set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hu->hdev->quirks);
+	if (test_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks))
+		set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hu->hdev->quirks);
 
 	if (!bcm_request_irq(bcm))
 		err = bcm_setup_sleep(hu);
-- 
2.39.3

Re: [PATCH] Bluetooth: hci_bcm: do not mark valid bd_addr as invalid

Posted by patchwork-bot+bluetooth@kernel.org 2 years, 8 months ago

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri,  2 Jun 2023 10:19:12 +0200 you wrote:
> A recent commit restored the original (and still documented) semantics
> for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device address
> is considered invalid unless an address is provided by firmware.
> 
> This specifically means that this flag must only be set for devices with
> invalid addresses, but the Broadcom driver has so far been setting this
> flag unconditionally.
> 
> [...]

Here is the summary with links:
  - Bluetooth: hci_bcm: do not mark valid bd_addr as invalid
    https://git.kernel.org/bluetooth/bluetooth-next/c/758be2398ff6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] Bluetooth: hci_bcm: do not mark valid bd_addr as invalid

Posted by Marek Szyprowski 2 years, 8 months ago

On 02.06.2023 10:19, Johan Hovold wrote:
> A recent commit restored the original (and still documented) semantics
> for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device address
> is considered invalid unless an address is provided by firmware.
>
> This specifically means that this flag must only be set for devices with
> invalid addresses, but the Broadcom driver has so far been setting this
> flag unconditionally.
>
> Fortunately the driver already checks for invalid addresses during setup
> and sets the HCI_QUIRK_INVALID_BDADDR flag. Use this flag to indicate
> when the address can be overridden by firmware (long term, this should
> probably just always be allowed).
>
> Fixes: 6ac517d8cf8b ("Bluetooth: fix use-bdaddr-property quirk")
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Link: https://lore.kernel.org/lkml/ecef83c8-497f-4011-607b-a63c24764867@samsung.com
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   drivers/bluetooth/hci_bcm.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 83bf5d4330c4..874d23089b39 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -643,7 +643,8 @@ static int bcm_setup(struct hci_uart *hu)
>   	 * Allow the bootloader to set a valid address through the
>   	 * device tree.
>   	 */
> -	set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hu->hdev->quirks);
> +	if (test_bit(HCI_QUIRK_INVALID_BDADDR, &hu->hdev->quirks))
> +		set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hu->hdev->quirks);
>   
>   	if (!bcm_request_irq(bcm))
>   		err = bcm_setup_sleep(hu);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland