[PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

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 exists 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>
---
 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 reserved.
- *  Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
  *
  *  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 property
-		 * 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->quirks);
-			bt_dev_info(hdev, "setting quirk bit to read BDA from fwnode later");
-		} else {
-			bt_dev_dbg(hdev, "local-bd-address` is not present 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 = qca_read_soc_version(hdev, &ver, soc_type);
-- 
2.43.2

RE: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=835223

---Test result---

Test Summary:
CheckPatch                    PASS      0.65 seconds
GitLint                       PASS      0.30 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      27.60 seconds
CheckAllWarning               PASS      30.36 seconds
CheckSparse                   PASS      36.00 seconds
CheckSmatch                   PASS      98.23 seconds
BuildKernel32                 PASS      26.97 seconds
TestRunnerSetup               PASS      507.94 seconds
TestRunner_l2cap-tester       PASS      20.06 seconds
TestRunner_iso-tester         PASS      30.00 seconds
TestRunner_bnep-tester        PASS      4.83 seconds
TestRunner_mgmt-tester        PASS      112.23 seconds
TestRunner_rfcomm-tester      PASS      7.45 seconds
TestRunner_sco-tester         PASS      15.00 seconds
TestRunner_ioctl-tester       PASS      7.73 seconds
TestRunner_mesh-tester        PASS      5.90 seconds
TestRunner_smp-tester         PASS      6.85 seconds
TestRunner_userchan-tester    PASS      5.62 seconds
IncrementalBuild              PASS      25.94 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Luiz Augusto von Dentz]

Hi Johan,

On Thu, Mar 14, 2024 at 4:44 AM Johan Hovold <johan+linaro@kernel.org> 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 exists 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 reserved.
> - *  Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>   *
>   *  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 property
> -                * 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->quirks);
> -                       bt_dev_info(hdev, "setting quirk bit to read BDA from fwnode later");
> -               } else {
> -                       bt_dev_dbg(hdev, "local-bd-address` is not present 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 = qca_read_soc_version(hdev, &ver, soc_type);
> --
> 2.43.2
>


-- 
Luiz Augusto von Dentz

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Thu, Mar 14, 2024 at 10:30:36AM -0400, Luiz Augusto von Dentz wrote:
> On Thu, Mar 14, 2024 at 4:44 AM Johan Hovold <johan+linaro@kernel.org> 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 exists 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?

Well, in this case we actually want the controller to remain
unconfigured (e.g. to avoid having every user of the X13s unknowingly
use the same default address). 

I'm not sure why Qualcomm insists on breaking these quirks, but I guess
they just haven't understood why they exist. It's of course convenient
to be able to use the default address during development without first
having to provide an address, but that's not a valid reason to break the
driver.

From what I hear the Qualcomm developers only care about Android and I
believe they have some out-of-tree hack for retrieving the device
address directly from the rootfs.

For the X13s, and as I think I've mentioned before, we have been trying
to get Qualcomm to tell us how to access the assigned addresses that are
stored in some secure world storage so that we can set it directly from
the driver. But until we figure that out, users will need to continue
setting the address manually.

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Linux regression tracking (Thorsten Leemhuis)]

Bluetooth Maintainers, what's...

On 14.03.24 16:07, Johan Hovold wrote:
> On Thu, Mar 14, 2024 at 10:30:36AM -0400, Luiz Augusto von Dentz wrote:
>> On Thu, Mar 14, 2024 at 4:44 AM Johan Hovold <johan+linaro@kernel.org> 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 exists 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?
> 
> Well, in this case we actually want the controller to remain
> unconfigured (e.g. to avoid having every user of the X13s unknowingly
> use the same default address). 
> 
> I'm not sure why Qualcomm insists on breaking these quirks, but I guess
> they just haven't understood why they exist. It's of course convenient
> to be able to use the default address during development without first
> having to provide an address, but that's not a valid reason to break the
> driver.
> 
> From what I hear the Qualcomm developers only care about Android and I
> believe they have some out-of-tree hack for retrieving the device
> address directly from the rootfs.
> 
> For the X13s, and as I think I've mentioned before, we have been trying
> to get Qualcomm to tell us how to access the assigned addresses that are
> stored in some secure world storage so that we can set it directly from
> the driver. But until we figure that out, users will need to continue
> setting the address manually.

...the plan forward here? This to me sounds like a case where a quick
revert is the right (interim?) solution, but nevertheless nothing
happened for ~10 days now afaics. Or am I missing something?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Luiz Augusto von Dentz]

Hi Johan,

On Mon, Mar 25, 2024 at 9:57 AM Linux regression tracking (Thorsten
Leemhuis) <regressions@leemhuis.info> wrote:
>
> Bluetooth Maintainers, what's...
>
> On 14.03.24 16:07, Johan Hovold wrote:
> > On Thu, Mar 14, 2024 at 10:30:36AM -0400, Luiz Augusto von Dentz wrote:
> >> On Thu, Mar 14, 2024 at 4:44 AM Johan Hovold <johan+linaro@kernel.org> 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 exists 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?
> >
> > Well, in this case we actually want the controller to remain
> > unconfigured (e.g. to avoid having every user of the X13s unknowingly
> > use the same default address).
> >
> > I'm not sure why Qualcomm insists on breaking these quirks, but I guess
> > they just haven't understood why they exist. It's of course convenient
> > to be able to use the default address during development without first
> > having to provide an address, but that's not a valid reason to break the
> > driver.
> >
> > From what I hear the Qualcomm developers only care about Android and I
> > believe they have some out-of-tree hack for retrieving the device
> > address directly from the rootfs.
> >
> > For the X13s, and as I think I've mentioned before, we have been trying
> > to get Qualcomm to tell us how to access the assigned addresses that are
> > stored in some secure world storage so that we can set it directly from
> > the driver. But until we figure that out, users will need to continue
> > setting the address manually.
>
> ...the plan forward here? This to me sounds like a case where a quick
> revert is the right (interim?) solution, but nevertheless nothing
> happened for ~10 days now afaics. Or am I missing something?
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke

I guess the following is the latest version:

https://patchwork.kernel.org/project/bluetooth/list/?series=836664

Or are you working on a v5?

-- 
Luiz Augusto von Dentz

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Mon, Mar 25, 2024 at 01:10:13PM -0400, Luiz Augusto von Dentz wrote:
> On Mon, Mar 25, 2024 at 9:57 AM Linux regression tracking (Thorsten
> Leemhuis) <regressions@leemhuis.info> wrote:
> > On 14.03.24 16:07, Johan Hovold wrote:
> > > On Thu, Mar 14, 2024 at 10:30:36AM -0400, Luiz Augusto von Dentz wrote:
> > >> On Thu, Mar 14, 2024 at 4:44 AM Johan Hovold <johan+linaro@kernel.org> 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 exists 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>

> > ...the plan forward here? This to me sounds like a case where a quick
> > revert is the right (interim?) solution, but nevertheless nothing
> > happened for ~10 days now afaics. Or am I missing something?

> I guess the following is the latest version:
> 
> https://patchwork.kernel.org/project/bluetooth/list/?series=836664
> 
> Or are you working on a v5?

This patch (revert) fixes a separate issue than the series you link to
above, but it is also a prerequisite for that series.

v4 is indeed the latest version, and it has been acked by Rob and Bjorn
so you can take it all through the Bluetooth tree. Just remember to
apply this patch (the revert) first.

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Luiz Augusto von Dentz]

Hi Johan

On Mon, Mar 25, 2024 at 1:24 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 25, 2024 at 01:10:13PM -0400, Luiz Augusto von Dentz wrote:
> > On Mon, Mar 25, 2024 at 9:57 AM Linux regression tracking (Thorsten
> > Leemhuis) <regressions@leemhuis.info> wrote:
> > > On 14.03.24 16:07, Johan Hovold wrote:
> > > > On Thu, Mar 14, 2024 at 10:30:36AM -0400, Luiz Augusto von Dentz wrote:
> > > >> On Thu, Mar 14, 2024 at 4:44 AM Johan Hovold <johan+linaro@kernel.org> 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 exists 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>
>
> > > ...the plan forward here? This to me sounds like a case where a quick
> > > revert is the right (interim?) solution, but nevertheless nothing
> > > happened for ~10 days now afaics. Or am I missing something?
>
> > I guess the following is the latest version:
> >
> > https://patchwork.kernel.org/project/bluetooth/list/?series=836664
> >
> > Or are you working on a v5?
>
> This patch (revert) fixes a separate issue than the series you link to
> above, but it is also a prerequisite for that series.
>
> v4 is indeed the latest version, and it has been acked by Rob and Bjorn
> so you can take it all through the Bluetooth tree. Just remember to
> apply this patch (the revert) first.

Doesn't seem to apply cleanly:

Applying: Bluetooth: qca: fix device-address endianness
error: patch failed: drivers/bluetooth/hci_qca.c:1904
error: drivers/bluetooth/hci_qca.c: patch does not apply
Patch failed at 0004 Bluetooth: qca: fix device-address endianness

> Johan



-- 
Luiz Augusto von Dentz

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Mon, Mar 25, 2024 at 03:39:03PM -0400, Luiz Augusto von Dentz wrote:
> On Mon, Mar 25, 2024 at 1:24 PM Johan Hovold <johan@kernel.org> wrote:
> > On Mon, Mar 25, 2024 at 01:10:13PM -0400, Luiz Augusto von Dentz wrote:

> > > I guess the following is the latest version:
> > >
> > > https://patchwork.kernel.org/project/bluetooth/list/?series=836664
> > >
> > > Or are you working on a v5?
> >
> > This patch (revert) fixes a separate issue than the series you link to
> > above, but it is also a prerequisite for that series.
> >
> > v4 is indeed the latest version, and it has been acked by Rob and Bjorn
> > so you can take it all through the Bluetooth tree. Just remember to
> > apply this patch (the revert) first.
> 
> Doesn't seem to apply cleanly:
> 
> Applying: Bluetooth: qca: fix device-address endianness
> error: patch failed: drivers/bluetooth/hci_qca.c:1904
> error: drivers/bluetooth/hci_qca.c: patch does not apply
> Patch failed at 0004 Bluetooth: qca: fix device-address endianness

Did you apply this patch (the revert) before trying to apply the series?

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Luiz Augusto von Dentz]

Hi Johan,

On Mon, Mar 25, 2024 at 3:48 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 25, 2024 at 03:39:03PM -0400, Luiz Augusto von Dentz wrote:
> > On Mon, Mar 25, 2024 at 1:24 PM Johan Hovold <johan@kernel.org> wrote:
> > > On Mon, Mar 25, 2024 at 01:10:13PM -0400, Luiz Augusto von Dentz wrote:
>
> > > > I guess the following is the latest version:
> > > >
> > > > https://patchwork.kernel.org/project/bluetooth/list/?series=836664
> > > >
> > > > Or are you working on a v5?
> > >
> > > This patch (revert) fixes a separate issue than the series you link to
> > > above, but it is also a prerequisite for that series.
> > >
> > > v4 is indeed the latest version, and it has been acked by Rob and Bjorn
> > > so you can take it all through the Bluetooth tree. Just remember to
> > > apply this patch (the revert) first.
> >
> > Doesn't seem to apply cleanly:
> >
> > Applying: Bluetooth: qca: fix device-address endianness
> > error: patch failed: drivers/bluetooth/hci_qca.c:1904
> > error: drivers/bluetooth/hci_qca.c: patch does not apply
> > Patch failed at 0004 Bluetooth: qca: fix device-address endianness
>
> Did you apply this patch (the revert) before trying to apply the series?

Probably needs rebasing:

Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
error: drivers/bluetooth/hci_qca.c: does not match index
Patch failed at 0001 Revert "Bluetooth: hci_qca: Set BDA quirk bit if
fwnode exists in DT"

> Johan



-- 
Luiz Augusto von Dentz

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Mon, Mar 25, 2024 at 04:07:03PM -0400, Luiz Augusto von Dentz wrote:
> On Mon, Mar 25, 2024 at 3:48 PM Johan Hovold <johan@kernel.org> wrote:
> > On Mon, Mar 25, 2024 at 03:39:03PM -0400, Luiz Augusto von Dentz wrote:
> > > On Mon, Mar 25, 2024 at 1:24 PM Johan Hovold <johan@kernel.org> wrote:
> > > > On Mon, Mar 25, 2024 at 01:10:13PM -0400, Luiz Augusto von Dentz wrote:
> >
> > > > > I guess the following is the latest version:
> > > > >
> > > > > https://patchwork.kernel.org/project/bluetooth/list/?series=836664
> > > > >
> > > > > Or are you working on a v5?
> > > >
> > > > This patch (revert) fixes a separate issue than the series you link to
> > > > above, but it is also a prerequisite for that series.
> > > >
> > > > v4 is indeed the latest version, and it has been acked by Rob and Bjorn
> > > > so you can take it all through the Bluetooth tree. Just remember to
> > > > apply this patch (the revert) first.
> > >
> > > Doesn't seem to apply cleanly:
> > >
> > > Applying: Bluetooth: qca: fix device-address endianness
> > > error: patch failed: drivers/bluetooth/hci_qca.c:1904
> > > error: drivers/bluetooth/hci_qca.c: patch does not apply
> > > Patch failed at 0004 Bluetooth: qca: fix device-address endianness
> >
> > Did you apply this patch (the revert) before trying to apply the series?
> 
> Probably needs rebasing:
> 
> Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
> error: drivers/bluetooth/hci_qca.c: does not match index
> Patch failed at 0001 Revert "Bluetooth: hci_qca: Set BDA quirk bit if
> fwnode exists in DT"

I just verified that it applies cleanly to 6.9-rc1.

	$ git checkout tmp v6.9-rc1
	$ b4 am -sl ZgHVFjAZ1uqEiUa2@hovoldconsulting.com
	...
	$ git am ./20240314_johan_linaro_revert_bluetooth_hci_qca_set_bda_quirk_bit_if_fwnode_exists_in_dt.mbx
	Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
	$ b4 am -sl 20240320075554.8178-2-johan+linaro@kernel.org
	...
	$ git am ./v4_20240320_johan_linaro_bluetooth_qca_fix_device_address_endianness.mbx
	Applying: dt-bindings: bluetooth: add 'qcom,local-bd-address-broken'
	Applying: arm64: dts: qcom: sc7180-trogdor: mark bluetooth address as broken
	Applying: Bluetooth: add quirk for broken address properties
	Applying: Bluetooth: qca: fix device-address endianness

Do you have anything else in your tree which may interfere? What tree is
that exactly?

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Luiz Augusto von Dentz]

Hi Johan,

On Mon, Mar 25, 2024 at 4:14 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 25, 2024 at 04:07:03PM -0400, Luiz Augusto von Dentz wrote:
> > On Mon, Mar 25, 2024 at 3:48 PM Johan Hovold <johan@kernel.org> wrote:
> > > On Mon, Mar 25, 2024 at 03:39:03PM -0400, Luiz Augusto von Dentz wrote:
> > > > On Mon, Mar 25, 2024 at 1:24 PM Johan Hovold <johan@kernel.org> wrote:
> > > > > On Mon, Mar 25, 2024 at 01:10:13PM -0400, Luiz Augusto von Dentz wrote:
> > >
> > > > > > I guess the following is the latest version:
> > > > > >
> > > > > > https://patchwork.kernel.org/project/bluetooth/list/?series=836664
> > > > > >
> > > > > > Or are you working on a v5?
> > > > >
> > > > > This patch (revert) fixes a separate issue than the series you link to
> > > > > above, but it is also a prerequisite for that series.
> > > > >
> > > > > v4 is indeed the latest version, and it has been acked by Rob and Bjorn
> > > > > so you can take it all through the Bluetooth tree. Just remember to
> > > > > apply this patch (the revert) first.
> > > >
> > > > Doesn't seem to apply cleanly:
> > > >
> > > > Applying: Bluetooth: qca: fix device-address endianness
> > > > error: patch failed: drivers/bluetooth/hci_qca.c:1904
> > > > error: drivers/bluetooth/hci_qca.c: patch does not apply
> > > > Patch failed at 0004 Bluetooth: qca: fix device-address endianness
> > >
> > > Did you apply this patch (the revert) before trying to apply the series?
> >
> > Probably needs rebasing:
> >
> > Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
> > error: drivers/bluetooth/hci_qca.c: does not match index
> > Patch failed at 0001 Revert "Bluetooth: hci_qca: Set BDA quirk bit if
> > fwnode exists in DT"
>
> I just verified that it applies cleanly to 6.9-rc1.
>
>         $ git checkout tmp v6.9-rc1
>         $ b4 am -sl ZgHVFjAZ1uqEiUa2@hovoldconsulting.com
>         ...
>         $ git am ./20240314_johan_linaro_revert_bluetooth_hci_qca_set_bda_quirk_bit_if_fwnode_exists_in_dt.mbx
>         Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
>         $ b4 am -sl 20240320075554.8178-2-johan+linaro@kernel.org
>         ...
>         $ git am ./v4_20240320_johan_linaro_bluetooth_qca_fix_device_address_endianness.mbx
>         Applying: dt-bindings: bluetooth: add 'qcom,local-bd-address-broken'
>         Applying: arm64: dts: qcom: sc7180-trogdor: mark bluetooth address as broken
>         Applying: Bluetooth: add quirk for broken address properties
>         Applying: Bluetooth: qca: fix device-address endianness
>
> Do you have anything else in your tree which may interfere? What tree is
> that exactly?

bluetooth-next tree, why would it be anything other than that? All the
CI automation is done on bluetooth-next and if you are asking to be
done via bluetooth tree which is based on the latest rc that is not
how things works here, we usually first apply to bluetooth-next and in
case it needs to be backported then it later done via pull-request.

> Johan



-- 
Luiz Augusto von Dentz

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Mon, Mar 25, 2024 at 04:31:53PM -0400, Luiz Augusto von Dentz wrote:
> On Mon, Mar 25, 2024 at 4:14 PM Johan Hovold <johan@kernel.org> wrote:
> > On Mon, Mar 25, 2024 at 04:07:03PM -0400, Luiz Augusto von Dentz wrote:

> > > Probably needs rebasing:
> > >
> > > Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
> > > error: drivers/bluetooth/hci_qca.c: does not match index
> > > Patch failed at 0001 Revert "Bluetooth: hci_qca: Set BDA quirk bit if
> > > fwnode exists in DT"
> >
> > I just verified that it applies cleanly to 6.9-rc1.
> >
> >         $ git checkout tmp v6.9-rc1
> >         $ b4 am -sl ZgHVFjAZ1uqEiUa2@hovoldconsulting.com
> >         ...
> >         $ git am ./20240314_johan_linaro_revert_bluetooth_hci_qca_set_bda_quirk_bit_if_fwnode_exists_in_dt.mbx
> >         Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
> >         $ b4 am -sl 20240320075554.8178-2-johan+linaro@kernel.org
> >         ...
> >         $ git am ./v4_20240320_johan_linaro_bluetooth_qca_fix_device_address_endianness.mbx
> >         Applying: dt-bindings: bluetooth: add 'qcom,local-bd-address-broken'
> >         Applying: arm64: dts: qcom: sc7180-trogdor: mark bluetooth address as broken
> >         Applying: Bluetooth: add quirk for broken address properties
> >         Applying: Bluetooth: qca: fix device-address endianness
> >
> > Do you have anything else in your tree which may interfere? What tree is
> > that exactly?
> 
> bluetooth-next tree, why would it be anything other than that?

I ask because I did not see anything in either the bluetooth or
bluetooth-next tree which should interfere.

And I just verified that by applying the revert followed by the series
to bluetooth-next. In that order it applies just fine, as expected.

> All the
> CI automation is done on bluetooth-next and if you are asking to be
> done via bluetooth tree which is based on the latest rc that is not
> how things works here, we usually first apply to bluetooth-next and in
> case it needs to be backported then it later done via pull-request.

The revert fixes a regression in 6.7-rc7 and should get to Linus as soon
as possible and I assume you have some way to get fixes into mainline
for the current development cycle.

The series fixes a critical bug in the Qualcomm driver and should
similarly get into mainline as soon as possible to avoid having people
unknowingly start relying on the broken behaviour (reversed address).
The bug in this case is older, but since the bug is severe and we're
only at rc1, I don't think this one should wait for 6.10 either.

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Luiz Augusto von Dentz]

Hi Johan,

On Tue, Mar 26, 2024 at 3:09 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 25, 2024 at 04:31:53PM -0400, Luiz Augusto von Dentz wrote:
> > On Mon, Mar 25, 2024 at 4:14 PM Johan Hovold <johan@kernel.org> wrote:
> > > On Mon, Mar 25, 2024 at 04:07:03PM -0400, Luiz Augusto von Dentz wrote:
>
> > > > Probably needs rebasing:
> > > >
> > > > Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
> > > > error: drivers/bluetooth/hci_qca.c: does not match index
> > > > Patch failed at 0001 Revert "Bluetooth: hci_qca: Set BDA quirk bit if
> > > > fwnode exists in DT"
> > >
> > > I just verified that it applies cleanly to 6.9-rc1.
> > >
> > >         $ git checkout tmp v6.9-rc1
> > >         $ b4 am -sl ZgHVFjAZ1uqEiUa2@hovoldconsulting.com
> > >         ...
> > >         $ git am ./20240314_johan_linaro_revert_bluetooth_hci_qca_set_bda_quirk_bit_if_fwnode_exists_in_dt.mbx
> > >         Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
> > >         $ b4 am -sl 20240320075554.8178-2-johan+linaro@kernel.org
> > >         ...
> > >         $ git am ./v4_20240320_johan_linaro_bluetooth_qca_fix_device_address_endianness.mbx
> > >         Applying: dt-bindings: bluetooth: add 'qcom,local-bd-address-broken'
> > >         Applying: arm64: dts: qcom: sc7180-trogdor: mark bluetooth address as broken
> > >         Applying: Bluetooth: add quirk for broken address properties
> > >         Applying: Bluetooth: qca: fix device-address endianness
> > >
> > > Do you have anything else in your tree which may interfere? What tree is
> > > that exactly?
> >
> > bluetooth-next tree, why would it be anything other than that?
>
> I ask because I did not see anything in either the bluetooth or
> bluetooth-next tree which should interfere.
>
> And I just verified that by applying the revert followed by the series
> to bluetooth-next. In that order it applies just fine, as expected.
>
> > All the
> > CI automation is done on bluetooth-next and if you are asking to be
> > done via bluetooth tree which is based on the latest rc that is not
> > how things works here, we usually first apply to bluetooth-next and in
> > case it needs to be backported then it later done via pull-request.
>
> The revert fixes a regression in 6.7-rc7 and should get to Linus as soon
> as possible and I assume you have some way to get fixes into mainline
> for the current development cycle.

Yeah I will send it later today to be included in the next rc release
and since it is marked for stable that shall trigger the process of
backporting it.

> The series fixes a critical bug in the Qualcomm driver and should
> similarly get into mainline as soon as possible to avoid having people
> unknowingly start relying on the broken behaviour (reversed address).
> The bug in this case is older, but since the bug is severe and we're
> only at rc1, I don't think this one should wait for 6.10 either.

The revert is now pushed, I had to apply it manually though since it
didn't apply cleanly, that said the other set still don't apply:

Applying: Bluetooth: qca: fix device-address endianness
error: patch failed: drivers/bluetooth/btqca.c:826
error: drivers/bluetooth/btqca.c: patch does not apply
error: drivers/bluetooth/hci_qca.c: does not match index
Patch failed at 0004 Bluetooth: qca: fix device-address endianness

So please rebase and send a v5.

-- 
Luiz Augusto von Dentz

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Tue, Mar 26, 2024 at 10:17:13AM -0400, Luiz Augusto von Dentz wrote:
> On Tue, Mar 26, 2024 at 3:09 AM Johan Hovold <johan@kernel.org> wrote:
> > On Mon, Mar 25, 2024 at 04:31:53PM -0400, Luiz Augusto von Dentz wrote:
> > > On Mon, Mar 25, 2024 at 4:14 PM Johan Hovold <johan@kernel.org> wrote:

> > > > I just verified that it applies cleanly to 6.9-rc1.
> > > >
> > > >         $ git checkout tmp v6.9-rc1
> > > >         $ b4 am -sl ZgHVFjAZ1uqEiUa2@hovoldconsulting.com
> > > >         ...
> > > >         $ git am ./20240314_johan_linaro_revert_bluetooth_hci_qca_set_bda_quirk_bit_if_fwnode_exists_in_dt.mbx
> > > >         Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
> > > >         $ b4 am -sl 20240320075554.8178-2-johan+linaro@kernel.org
> > > >         ...
> > > >         $ git am ./v4_20240320_johan_linaro_bluetooth_qca_fix_device_address_endianness.mbx
> > > >         Applying: dt-bindings: bluetooth: add 'qcom,local-bd-address-broken'
> > > >         Applying: arm64: dts: qcom: sc7180-trogdor: mark bluetooth address as broken
> > > >         Applying: Bluetooth: add quirk for broken address properties
> > > >         Applying: Bluetooth: qca: fix device-address endianness
> > > >
> > > > Do you have anything else in your tree which may interfere? What tree is
> > > > that exactly?
> > >
> > > bluetooth-next tree, why would it be anything other than that?
> >
> > I ask because I did not see anything in either the bluetooth or
> > bluetooth-next tree which should interfere.
> >
> > And I just verified that by applying the revert followed by the series
> > to bluetooth-next. In that order it applies just fine, as expected.
> >
> > > All the
> > > CI automation is done on bluetooth-next and if you are asking to be
> > > done via bluetooth tree which is based on the latest rc that is not
> > > how things works here, we usually first apply to bluetooth-next and in
> > > case it needs to be backported then it later done via pull-request.
> >
> > The revert fixes a regression in 6.7-rc7 and should get to Linus as soon
> > as possible and I assume you have some way to get fixes into mainline
> > for the current development cycle.
> 
> Yeah I will send it later today to be included in the next rc release
> and since it is marked for stable that shall trigger the process of
> backporting it.
> 
> > The series fixes a critical bug in the Qualcomm driver and should
> > similarly get into mainline as soon as possible to avoid having people
> > unknowingly start relying on the broken behaviour (reversed address).
> > The bug in this case is older, but since the bug is severe and we're
> > only at rc1, I don't think this one should wait for 6.10 either.
> 
> The revert is now pushed, I had to apply it manually though since it
> didn't apply cleanly, that said the other set still don't apply:

I have no idea what you did here but you need to drop that commit
immediately, it's completely messed up:

	https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=1b2cc5c2e5a666bdaa57a7a3964a2fe2afefb667

This is the revert:

	https://lore.kernel.org/lkml/20240314084412.1127-1-johan+linaro@kernel.org/

and as you can see your commit bears no resemblance to the revert
(instead it includes code from the below series which obviously then
fails to apply):

> Applying: Bluetooth: qca: fix device-address endianness
> error: patch failed: drivers/bluetooth/btqca.c:826
> error: drivers/bluetooth/btqca.c: patch does not apply
> error: drivers/bluetooth/hci_qca.c: does not match index
> Patch failed at 0004 Bluetooth: qca: fix device-address endianness
> 
> So please rebase and send a v5.

There should be no need to resend as the problem is clearly on your end.

Just drop whatever you applied and start fresh:

> > > >         $ b4 am -sl ZgHVFjAZ1uqEiUa2@hovoldconsulting.com
> > > >         ...
> > > >         $ git am ./20240314_johan_linaro_revert_bluetooth_hci_qca_set_bda_quirk_bit_if_fwnode_exists_in_dt.mbx
> > > >         Applying: Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

This is the revert from this thread.

> > > >         $ b4 am -sl 20240320075554.8178-2-johan+linaro@kernel.org
> > > >         ...
> > > >         $ git am ./v4_20240320_johan_linaro_bluetooth_qca_fix_device_address_endianness.mbx
> > > >         Applying: dt-bindings: bluetooth: add 'qcom,local-bd-address-broken'
> > > >         Applying: arm64: dts: qcom: sc7180-trogdor: mark bluetooth address as broken
> > > >         Applying: Bluetooth: add quirk for broken address properties
> > > >         Applying: Bluetooth: qca: fix device-address endianness

And this is the follow-on series that depends on the revert.

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

Hi Luiz,

On Tue, Mar 26, 2024 at 04:18:16PM +0100, Johan Hovold wrote:
> On Tue, Mar 26, 2024 at 10:17:13AM -0400, Luiz Augusto von Dentz wrote:
> > On Tue, Mar 26, 2024 at 3:09 AM Johan Hovold <johan@kernel.org> wrote:
> > > On Mon, Mar 25, 2024 at 04:31:53PM -0400, Luiz Augusto von Dentz wrote:

> > > > All the
> > > > CI automation is done on bluetooth-next and if you are asking to be
> > > > done via bluetooth tree which is based on the latest rc that is not
> > > > how things works here, we usually first apply to bluetooth-next and in
> > > > case it needs to be backported then it later done via pull-request.
> > >
> > > The revert fixes a regression in 6.7-rc7 and should get to Linus as soon
> > > as possible and I assume you have some way to get fixes into mainline
> > > for the current development cycle.
> > 
> > Yeah I will send it later today to be included in the next rc release
> > and since it is marked for stable that shall trigger the process of
> > backporting it.
> > 
> > > The series fixes a critical bug in the Qualcomm driver and should
> > > similarly get into mainline as soon as possible to avoid having people
> > > unknowingly start relying on the broken behaviour (reversed address).
> > > The bug in this case is older, but since the bug is severe and we're
> > > only at rc1, I don't think this one should wait for 6.10 either.

I just double checked the bluetooth-next branch and everything looks
good now (revert + endianness fix series). Thanks!

Did I understand you correctly that you'll be able to get all five
commits into 6.9 during this development cycle (e.g. 6.9-rc2)?
 
Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Luiz Augusto von Dentz]

Hi Johan,

On Tue, Mar 26, 2024 at 12:20 PM Johan Hovold <johan@kernel.org> wrote:
>
> Hi Luiz,
>
> On Tue, Mar 26, 2024 at 04:18:16PM +0100, Johan Hovold wrote:
> > On Tue, Mar 26, 2024 at 10:17:13AM -0400, Luiz Augusto von Dentz wrote:
> > > On Tue, Mar 26, 2024 at 3:09 AM Johan Hovold <johan@kernel.org> wrote:
> > > > On Mon, Mar 25, 2024 at 04:31:53PM -0400, Luiz Augusto von Dentz wrote:
>
> > > > > All the
> > > > > CI automation is done on bluetooth-next and if you are asking to be
> > > > > done via bluetooth tree which is based on the latest rc that is not
> > > > > how things works here, we usually first apply to bluetooth-next and in
> > > > > case it needs to be backported then it later done via pull-request.
> > > >
> > > > The revert fixes a regression in 6.7-rc7 and should get to Linus as soon
> > > > as possible and I assume you have some way to get fixes into mainline
> > > > for the current development cycle.
> > >
> > > Yeah I will send it later today to be included in the next rc release
> > > and since it is marked for stable that shall trigger the process of
> > > backporting it.
> > >
> > > > The series fixes a critical bug in the Qualcomm driver and should
> > > > similarly get into mainline as soon as possible to avoid having people
> > > > unknowingly start relying on the broken behaviour (reversed address).
> > > > The bug in this case is older, but since the bug is severe and we're
> > > > only at rc1, I don't think this one should wait for 6.10 either.
>
> I just double checked the bluetooth-next branch and everything looks
> good now (revert + endianness fix series). Thanks!
>
> Did I understand you correctly that you'll be able to get all five
> commits into 6.9 during this development cycle (e.g. 6.9-rc2)?

Yep, I will be preparing a pull-request with them later this week,
there are some other fixes that I want to get in as well.


-- 
Luiz Augusto von Dentz

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Tue, Mar 26, 2024 at 12:58:12PM -0400, Luiz Augusto von Dentz wrote:
> On Tue, Mar 26, 2024 at 12:20 PM Johan Hovold <johan@kernel.org> wrote:

> > Did I understand you correctly that you'll be able to get all five
> > commits into 6.9 during this development cycle (e.g. 6.9-rc2)?
> 
> Yep, I will be preparing a pull-request with them later this week,
> there are some other fixes that I want to get in as well.

Perfect, thanks!

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Clayton Craft]

On Thu, 14 Mar 2024 09:44:12 +0100 Johan Hovold <johan+linaro@kernel.org> 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 exists 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>

Thanks Johan, this revert does indeed fix Bluetooth for me on the X13s.

Reported-by: Clayton Craft <clayton@craftyguy.net>
Tested-by: Clayton Craft <clayton@craftyguy.net>

-Clayton

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[patchwork-bot+bluetooth]

Hello:

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

On Thu, 14 Mar 2024 09:44:12 +0100 you 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.
> 
> [...]

Here is the summary with links:
  - Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
    https://git.kernel.org/bluetooth/bluetooth-next/c/ac0cf3552972

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

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Janaki Ramaiah Thota]

Hi Johan,
We made this change to configure the device which supports persistent memory for the BD-Address
So to make device functional in both scenarios we are adding a new property in dts file to distinguish persistent and non-persistent support of BD Address and set HCI_QUIRK_USE_BDADDR_PROPERTY bit accordingly

On 3/26/2024 9:00 PM, patchwork-bot+bluetooth@kernel.org wrote:
> Hello:
> 
> This patch was applied to bluetooth/bluetooth-next.git (master)
> by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
> 
> On Thu, 14 Mar 2024 09:44:12 +0100 you 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.
>>
>> [...]
> 
> Here is the summary with links:
>    - Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"
>      https://git.kernel.org/bluetooth/bluetooth-next/c/ac0cf3552972
> 
> You are awesome, thank you!

Thanks,
Janaki Ram

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

[ Please wrap your emails at 72 columns or so. ]

On Thu, Mar 28, 2024 at 08:25:16PM +0530, Janaki Ramaiah Thota wrote:
> We made this change to configure the device which supports persistent
> memory for the BD-Address

Can you say something more about which devices support persistent
storage for the address? Is that all or just some of the chip variants?

> So to make device functional in both scenarios we are adding a new
> property in dts file to distinguish persistent and non-persistent
> support of BD Address and set HCI_QUIRK_USE_BDADDR_PROPERTY bit
> accordingly

Depending on the answer to my questions above, you may be able to infer
this from the compatible string and/or you can read out the address from
the device and only set the quirk if it's set to the default address.

You should not need to add a new property for this.

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Janaki Ramaiah Thota]

On 3/28/2024 8:53 PM, Johan Hovold wrote:
Hi Johan,
Thanks for the valuable inputs.
> [ Please wrap your emails at 72 columns or so. ]
>

Noted.

> On Thu, Mar 28, 2024 at 08:25:16PM +0530, Janaki Ramaiah Thota wrote:
>> We made this change to configure the device which supports persistent
>> memory for the BD-Address
> 
> Can you say something more about which devices support persistent
> storage for the address? Is that all or just some of the chip variants?
> 

Most of the devices support persistent storage, and bd-address storage
is chosen based on the OEM and Target.

>> So to make device functional in both scenarios we are adding a new
>> property in dts file to distinguish persistent and non-persistent
>> support of BD Address and set HCI_QUIRK_USE_BDADDR_PROPERTY bit
>> accordingly
> 
> Depending on the answer to my questions above, you may be able to infer
> this from the compatible string and/or you can read out the address from
> the device and only set the quirk if it's set to the default address.
> 
> You should not need to add a new property for this.
> 
> Johan

As per my understanding, altering the compatible string may cause duplicate
configuration, right ?

Thanks,
JanakiRam

  

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Fri, Mar 29, 2024 at 12:55:40PM +0530, Janaki Ramaiah Thota wrote:
> On 3/28/2024 8:53 PM, Johan Hovold wrote:
> > On Thu, Mar 28, 2024 at 08:25:16PM +0530, Janaki Ramaiah Thota wrote:

> >> We made this change to configure the device which supports persistent
> >> memory for the BD-Address
> > 
> > Can you say something more about which devices support persistent
> > storage for the address? Is that all or just some of the chip variants?

> Most of the devices support persistent storage, and bd-address storage
> is chosen based on the OEM and Target.

Can you be more specific about which devices support it (or say which do
not)?

Is the address stored in some external eeprom or similar which the OEM
can choose to populate?

> >> So to make device functional in both scenarios we are adding a new
> >> property in dts file to distinguish persistent and non-persistent
> >> support of BD Address and set HCI_QUIRK_USE_BDADDR_PROPERTY bit
> >> accordingly
> > 
> > Depending on the answer to my questions above, you may be able to infer
> > this from the compatible string and/or you can read out the address from
> > the device and only set the quirk if it's set to the default address.
> > 
> > You should not need to add a new property for this.

> As per my understanding, altering the compatible string may cause duplicate
> configuration, right ?

If it's the same device and just a different configuration then we can't
use the compatible string for this.

It seems we need a patch like the below to address this. But please
provide some more details (e.g. answers to the questions above) so I can
determine what the end result should look like.

Johan


From 9719effe80fcc17518131816fdfeb1824cfa08b6 Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan+linaro@kernel.org>
Date: Thu, 20 Apr 2023 14:10:55 +0200
Subject: [PATCH] Bluetooth: btqca: add invalid device address check

Some Qualcomm Bluetooth controllers lack persistent storage for the
device address and therefore end up using the default address
00:00:00:00:5a:ad.

Apparently this depends on how the controller has been integrated so we
can not use the device type alone to determine when the address is
valid.

Instead read back the address during setup() and only set the
HCI_QUIRK_USE_BDADDR_PROPERTY flag when needed.

Fixes: de79a9df1692 ("Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY")
Fixes: e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")
Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
Cc: stable@vger.kernel.org	# 6.5
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/bluetooth/btqca.c   | 33 +++++++++++++++++++++++++++++++++
 drivers/bluetooth/hci_qca.c |  2 --
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 19cfc342fc7b..15124157372c 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -15,6 +15,8 @@
 
 #define VERSION "0.1"
 
+#define QCA_BDADDR_DEFAULT (&(bdaddr_t) {{ 0xad, 0x5a, 0x00, 0x00, 0x00, 0x00 }})
+
 int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
 			 enum qca_btsoc_type soc_type)
 {
@@ -612,6 +614,35 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
 }
 EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
 
+static void qca_check_bdaddr(struct hci_dev *hdev)
+{
+	struct hci_rp_read_bd_addr *bda;
+	struct sk_buff *skb;
+	int err;
+
+	if (bacmp(&hdev->public_addr, BDADDR_ANY))
+		return;
+
+	skb = __hci_cmd_sync(hdev, HCI_OP_READ_BD_ADDR, 0, NULL,
+			     HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		err = PTR_ERR(skb);
+		bt_dev_err(hdev, "Failed to read device address (%d)", err);
+		return;
+	}
+
+	if (skb->len != sizeof(*bda)) {
+		bt_dev_err(hdev, "Device address length mismatch");
+		goto free_skb;
+	}
+
+	bda = (struct hci_rp_read_bd_addr *)skb->data;
+	if (!bacmp(&bda->bdaddr, QCA_BDADDR_DEFAULT))
+		set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
+free_skb:
+	kfree_skb(skb);
+}
+
 static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
 		struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
 {
@@ -818,6 +849,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
 		break;
 	}
 
+	qca_check_bdaddr(hdev);
+
 	bt_dev_info(hdev, "QCA setup on UART is completed");
 
 	return 0;
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index b266db18c8cc..b621a0a40ea4 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1908,8 +1908,6 @@ static int qca_setup(struct hci_uart *hu)
 	case QCA_WCN6750:
 	case QCA_WCN6855:
 	case QCA_WCN7850:
-		set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
-
 		qcadev = serdev_device_get_drvdata(hu->serdev);
 		if (qcadev->bdaddr_property_broken)
 			set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
-- 
2.43.2

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Janaki Ramaiah Thota]

On 4/3/2024 5:54 PM, Johan Hovold wrote:
> On Fri, Mar 29, 2024 at 12:55:40PM +0530, Janaki Ramaiah Thota wrote:
>> On 3/28/2024 8:53 PM, Johan Hovold wrote:
>>> On Thu, Mar 28, 2024 at 08:25:16PM +0530, Janaki Ramaiah Thota wrote:
> 
>>>> We made this change to configure the device which supports persistent
>>>> memory for the BD-Address
>>>
>>> Can you say something more about which devices support persistent
>>> storage for the address? Is that all or just some of the chip variants?
> 
>> Most of the devices support persistent storage, and bd-address storage
>> is chosen based on the OEM and Target.
> 
> Can you be more specific about which devices support it (or say which do
> not)?
> 

We know below BT chipsets supports persistent storage(OTP) for BDA
WCN7850, WCN6855, WCN6750

> Is the address stored in some external eeprom or similar which the OEM
> can choose to populate?
>
  
This persistent storage is One Time Programmable (OTP) reserved memory
which resides in the BT chip.

>>>> So to make device functional in both scenarios we are adding a new
>>>> property in dts file to distinguish persistent and non-persistent
>>>> support of BD Address and set HCI_QUIRK_USE_BDADDR_PROPERTY bit
>>>> accordingly
>>>
>>> Depending on the answer to my questions above, you may be able to infer
>>> this from the compatible string and/or you can read out the address from
>>> the device and only set the quirk if it's set to the default address.
>>>
>>> You should not need to add a new property for this.
> 
>> As per my understanding, altering the compatible string may cause duplicate
>> configuration, right ?
> 

Yes, we are correct.

> If it's the same device and just a different configuration then we can't
> use the compatible string for this.
> 
> It seems we need a patch like the below to address this. But please
> provide some more details (e.g. answers to the questions above) so I can
> determine what the end result should look like.
> 
> Johan
> 
> 
>  From 9719effe80fcc17518131816fdfeb1824cfa08b6 Mon Sep 17 00:00:00 2001
> From: Johan Hovold <johan+linaro@kernel.org>
> Date: Thu, 20 Apr 2023 14:10:55 +0200
> Subject: [PATCH] Bluetooth: btqca: add invalid device address check
> 
> Some Qualcomm Bluetooth controllers lack persistent storage for the
> device address and therefore end up using the default address
> 00:00:00:00:5a:ad.
> 
> Apparently this depends on how the controller has been integrated so we
> can not use the device type alone to determine when the address is
> valid.
> 
> Instead read back the address during setup() and only set the
> HCI_QUIRK_USE_BDADDR_PROPERTY flag when needed.
> 
> Fixes: de79a9df1692 ("Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY")
> Fixes: e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")
> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
> Cc: stable@vger.kernel.org	# 6.5
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/bluetooth/btqca.c   | 33 +++++++++++++++++++++++++++++++++
>   drivers/bluetooth/hci_qca.c |  2 --
>   2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index 19cfc342fc7b..15124157372c 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -15,6 +15,8 @@
>   
>   #define VERSION "0.1"
>   
> +#define QCA_BDADDR_DEFAULT (&(bdaddr_t) {{ 0xad, 0x5a, 0x00, 0x00, 0x00, 0x00 }})
> +
>   int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
>   			 enum qca_btsoc_type soc_type)
>   {
> @@ -612,6 +614,35 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
>   }
>   EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
>   
> +static void qca_check_bdaddr(struct hci_dev *hdev)
> +{
> +	struct hci_rp_read_bd_addr *bda;
> +	struct sk_buff *skb;
> +	int err;
> +
> +	if (bacmp(&hdev->public_addr, BDADDR_ANY))
> +		return;
> +
> +	skb = __hci_cmd_sync(hdev, HCI_OP_READ_BD_ADDR, 0, NULL,
> +			     HCI_INIT_TIMEOUT);
> +	if (IS_ERR(skb)) {
> +		err = PTR_ERR(skb);
> +		bt_dev_err(hdev, "Failed to read device address (%d)", err);
> +		return;
> +	}
> +
> +	if (skb->len != sizeof(*bda)) {
> +		bt_dev_err(hdev, "Device address length mismatch");
> +		goto free_skb;
> +	}
> +
> +	bda = (struct hci_rp_read_bd_addr *)skb->data;
> +	if (!bacmp(&bda->bdaddr, QCA_BDADDR_DEFAULT))
> +		set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
> +free_skb:
> +	kfree_skb(skb);
> +}
> +
>   static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
>   		struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
>   {
> @@ -818,6 +849,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>   		break;
>   	}
>   
> +	qca_check_bdaddr(hdev);
> +
>   	bt_dev_info(hdev, "QCA setup on UART is completed");
>   
>   	return 0;
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index b266db18c8cc..b621a0a40ea4 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -1908,8 +1908,6 @@ static int qca_setup(struct hci_uart *hu)
>   	case QCA_WCN6750:
>   	case QCA_WCN6855:
>   	case QCA_WCN7850:
> -		set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
> -
>   		qcadev = serdev_device_get_drvdata(hu->serdev);
>   		if (qcadev->bdaddr_property_broken)
>   			set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);

Thanks for the patch. This change looks fine and it will resolve the current OTP issue.

--
Thanks,
JanakiRam

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Janaki Ramaiah Thota]

Hi Johan,
Are you planing to merge your below patch ?

On 4/5/2024 6:29 PM, Janaki Ramaiah Thota wrote:
> 
> 
> On 4/3/2024 5:54 PM, Johan Hovold wrote:
>> On Fri, Mar 29, 2024 at 12:55:40PM +0530, Janaki Ramaiah Thota wrote:
>>> On 3/28/2024 8:53 PM, Johan Hovold wrote:
>>>> On Thu, Mar 28, 2024 at 08:25:16PM +0530, Janaki Ramaiah Thota wrote:
>>
>>>>> We made this change to configure the device which supports persistent
>>>>> memory for the BD-Address
>>>>
>>>> Can you say something more about which devices support persistent
>>>> storage for the address? Is that all or just some of the chip variants?
>>
>>> Most of the devices support persistent storage, and bd-address storage
>>> is chosen based on the OEM and Target.
>>
>> Can you be more specific about which devices support it (or say which do
>> not)?
>>
> 
> We know below BT chipsets supports persistent storage(OTP) for BDA
> WCN7850, WCN6855, WCN6750
> 
>> Is the address stored in some external eeprom or similar which the OEM
>> can choose to populate?
>>
> 
> This persistent storage is One Time Programmable (OTP) reserved memory
> which resides in the BT chip.
> 
>>>>> So to make device functional in both scenarios we are adding a new
>>>>> property in dts file to distinguish persistent and non-persistent
>>>>> support of BD Address and set HCI_QUIRK_USE_BDADDR_PROPERTY bit
>>>>> accordingly
>>>>
>>>> Depending on the answer to my questions above, you may be able to infer
>>>> this from the compatible string and/or you can read out the address from
>>>> the device and only set the quirk if it's set to the default address.
>>>>
>>>> You should not need to add a new property for this.
>>
>>> As per my understanding, altering the compatible string may cause duplicate
>>> configuration, right ?
>>
> 
> Yes, we are correct.
> 
>> If it's the same device and just a different configuration then we can't
>> use the compatible string for this.
>>
>> It seems we need a patch like the below to address this. But please
>> provide some more details (e.g. answers to the questions above) so I can
>> determine what the end result should look like.
>>
>> Johan
>>
>>
>>  From 9719effe80fcc17518131816fdfeb1824cfa08b6 Mon Sep 17 00:00:00 2001
>> From: Johan Hovold <johan+linaro@kernel.org>
>> Date: Thu, 20 Apr 2023 14:10:55 +0200
>> Subject: [PATCH] Bluetooth: btqca: add invalid device address check
>>
>> Some Qualcomm Bluetooth controllers lack persistent storage for the
>> device address and therefore end up using the default address
>> 00:00:00:00:5a:ad.
>>
>> Apparently this depends on how the controller has been integrated so we
>> can not use the device type alone to determine when the address is
>> valid.
>>
>> Instead read back the address during setup() and only set the
>> HCI_QUIRK_USE_BDADDR_PROPERTY flag when needed.
>>
>> Fixes: de79a9df1692 ("Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY")
>> Fixes: e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")
>> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
>> Cc: stable@vger.kernel.org    # 6.5
>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> ---
>>   drivers/bluetooth/btqca.c   | 33 +++++++++++++++++++++++++++++++++
>>   drivers/bluetooth/hci_qca.c |  2 --
>>   2 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
>> index 19cfc342fc7b..15124157372c 100644
>> --- a/drivers/bluetooth/btqca.c
>> +++ b/drivers/bluetooth/btqca.c
>> @@ -15,6 +15,8 @@
>>   #define VERSION "0.1"
>> +#define QCA_BDADDR_DEFAULT (&(bdaddr_t) {{ 0xad, 0x5a, 0x00, 0x00, 0x00, 0x00 }})
>> +
>>   int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
>>                enum qca_btsoc_type soc_type)
>>   {
>> @@ -612,6 +614,35 @@ int qca_set_bdaddr_rome(struct hci_dev *hdev, const bdaddr_t *bdaddr)
>>   }
>>   EXPORT_SYMBOL_GPL(qca_set_bdaddr_rome);
>> +static void qca_check_bdaddr(struct hci_dev *hdev)
>> +{
>> +    struct hci_rp_read_bd_addr *bda;
>> +    struct sk_buff *skb;
>> +    int err;
>> +
>> +    if (bacmp(&hdev->public_addr, BDADDR_ANY))
>> +        return;
>> +
>> +    skb = __hci_cmd_sync(hdev, HCI_OP_READ_BD_ADDR, 0, NULL,
>> +                 HCI_INIT_TIMEOUT);
>> +    if (IS_ERR(skb)) {
>> +        err = PTR_ERR(skb);
>> +        bt_dev_err(hdev, "Failed to read device address (%d)", err);
>> +        return;
>> +    }
>> +
>> +    if (skb->len != sizeof(*bda)) {
>> +        bt_dev_err(hdev, "Device address length mismatch");
>> +        goto free_skb;
>> +    }
>> +
>> +    bda = (struct hci_rp_read_bd_addr *)skb->data;
>> +    if (!bacmp(&bda->bdaddr, QCA_BDADDR_DEFAULT))
>> +        set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>> +free_skb:
>> +    kfree_skb(skb);
>> +}
>> +
>>   static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
>>           struct qca_btsoc_version ver, u8 rom_ver, u16 bid)
>>   {
>> @@ -818,6 +849,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>>           break;
>>       }
>> +    qca_check_bdaddr(hdev);
>> +
>>       bt_dev_info(hdev, "QCA setup on UART is completed");
>>       return 0;
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index b266db18c8cc..b621a0a40ea4 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -1908,8 +1908,6 @@ static int qca_setup(struct hci_uart *hu)
>>       case QCA_WCN6750:
>>       case QCA_WCN6855:
>>       case QCA_WCN7850:
>> -        set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
>> -
>>           qcadev = serdev_device_get_drvdata(hu->serdev);
>>           if (qcadev->bdaddr_property_broken)
>>               set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
> 
> Thanks for the patch. This change looks fine and it will resolve the current OTP issue.
> 
> -- 
> Thanks,
> JanakiRam

Regards,
Janaki Ram

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Mon, Apr 15, 2024 at 04:22:51PM +0530, Janaki Ramaiah Thota wrote:

> Are you planing to merge your below patch ?

Yes, sorry about the delay. Was busy with other things last week.

I'll revisit and post it shortly.

Johan

Re: [PATCH] Revert "Bluetooth: hci_qca: Set BDA quirk bit if fwnode exists in DT"

[Johan Hovold]

On Mon, Apr 15, 2024 at 01:20:36PM +0200, Johan Hovold wrote:
> On Mon, Apr 15, 2024 at 04:22:51PM +0530, Janaki Ramaiah Thota wrote:
> 
> > Are you planing to merge your below patch ?
> 
> Yes, sorry about the delay. Was busy with other things last week.
> 
> I'll revisit and post it shortly.

For the record, I've now posted a fix for this here:

	https://lore.kernel.org/r/20240416091509.19995-1-johan+linaro@kernel.org

Johan