Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-2518-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240314084412.1127-1-johan+linaro@kernel.org>
In-Reply-To: <20240314084412.1127-1-johan+linaro@kernel.org>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Thu, 14 Mar 2024 10:30:36 -0400
Message-ID: <CABBYNZJV1htg46Gyu=7_iUWdukM+rHLitsLjxmWWYFGXty3tVw@mail.gmail.com>
Subject: Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode
 exists in DT"
To: Johan Hovold <johan+linaro@kernel.org>
Cc: Marcel Holtmann <marcel@holtmann.org>, linux-bluetooth@vger.kernel.org, 
	linux-kernel@vger.kernel.org, stable@vger.kernel.org, 
	Janaki Ramaiah Thota <quic_janathot@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Johan,

On Thu, Mar 14, 2024 at 4:44=E2=80=AFAM Johan Hovold <johan+linaro@kernel.o=
rg> wrote:
>
> This reverts commit 7dcd3e014aa7faeeaf4047190b22d8a19a0db696.
>
> Qualcomm Bluetooth controllers like WCN6855 do not have persistent
> storage for the Bluetooth address and must therefore start as
> unconfigured to allow the user to set a valid address unless one has
> been provided by the boot firmware in the devicetree.
>
> A recent change snuck into v6.8-rc7 and incorrectly started marking the
> default (non-unique) address as valid. This specifically also breaks the
> Bluetooth setup for some user of the Lenovo ThinkPad X13s.
>
> Note that this is the second time Qualcomm breaks the driver this way
> and that this was fixed last year by commit 6945795bc81a ("Bluetooth:
> fix use-bdaddr-property quirk"), which also has some further details.
>
> Fixes: 7dcd3e014aa7 ("Bluetooth: hci_qca: Set BDA quirk bit if fwnode exi=
sts in DT")
> Cc: stable@vger.kernel.org      # 6.8
> Cc: Janaki Ramaiah Thota <quic_janathot@quicinc.com>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Well I guess I will need to start asking for evidence that this works
on regular Linux distros then, because it looks like that is not the
environment Janaki and others Qualcomm folks are testing with.

What I probably would consider as evidence is bluetoothd logs showing
that the controller has been configured correctly or perhaps there is
a simpler way?

> ---
>  drivers/bluetooth/hci_qca.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index edd2a81b4d5e..f989c05f8177 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -7,7 +7,6 @@
>   *
>   *  Copyright (C) 2007 Texas Instruments, Inc.
>   *  Copyright (c) 2010, 2012, 2018 The Linux Foundation. All rights rese=
rved.
> - *  Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reser=
ved.
>   *
>   *  Acknowledgements:
>   *  This file is based on hci_ll.c, which was...
> @@ -1904,17 +1903,7 @@ static int qca_setup(struct hci_uart *hu)
>         case QCA_WCN6750:
>         case QCA_WCN6855:
>         case QCA_WCN7850:
> -
> -               /* Set BDA quirk bit for reading BDA value from fwnode pr=
operty
> -                * only if that property exist in DT.
> -                */
> -               if (fwnode_property_present(dev_fwnode(hdev->dev.parent),=
 "local-bd-address")) {
> -                       set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->qui=
rks);
> -                       bt_dev_info(hdev, "setting quirk bit to read BDA =
from fwnode later");
> -               } else {
> -                       bt_dev_dbg(hdev, "local-bd-address` is not presen=
t in the devicetree so not setting quirk bit for BDA");
> -               }
> -
> +               set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>                 hci_set_aosp_capable(hdev);
>
>                 ret =3D qca_read_soc_version(hdev, &ver, soc_type);
> --
> 2.43.2
>


--=20
Luiz Augusto von Dentz