Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3893-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240416091509.19995-1-johan+linaro@kernel.org>
In-Reply-To: <20240416091509.19995-1-johan+linaro@kernel.org>
From: Doug Anderson <dianders@chromium.org>
Date: Mon, 22 Apr 2024 10:50:33 -0700
Message-ID: <CAD=FV=UBHvz2S5bd8eso030-E=rhbAypz_BnO-vmB1vNo+4Uvw@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: qca: fix invalid device address check
To: Johan Hovold <johan+linaro@kernel.org>
Cc: Marcel Holtmann <marcel@holtmann.org>, Luiz Augusto von Dentz <luiz.dentz@gmail.com>, 
	Matthias Kaehlcke <mka@chromium.org>, linux-bluetooth@vger.kernel.org, 
	linux-kernel@vger.kernel.org, 
	Janaki Ramaiah Thota <quic_janathot@quicinc.com>, stable@vger.kernel.org, 
	Stephen Boyd <swboyd@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Tue, Apr 16, 2024 at 2:17=E2=80=AFAM Johan Hovold <johan+linaro@kernel.o=
rg> wrote:
>
> Qualcomm Bluetooth controllers may not have been provisioned with a
> valid device address and instead end up using the default address
> 00:00:00:00:5a:ad.
>
> This was previously believed to be due to lack of persistent storage for
> the address but it may also be due to integrators opting to not use the
> on-chip OTP memory and instead store the address elsewhere (e.g. in
> storage managed by secure world firmware).
>
> According to Qualcomm, at least WCN6750, WCN6855 and WCN7850 have
> on-chip OTP storage for the address.
>
> As the device type alone cannot be used 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.
>
> This specifically makes sure that controllers that have been provisioned
> with an address do not start as unconfigured.
>
> Reported-by: Janaki Ramaiah Thota <quic_janathot@quicinc.com>
> Link: https://lore.kernel.org/r/124a7d54-5a18-4be7-9a76-a12017f6cce5@quic=
inc.com/
> Fixes: 5971752de44c ("Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPER=
TY for wcn3990")
> Fixes: e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD addres=
s missing in dts")
> Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
> Cc: stable@vger.kernel.org      # 6.5
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/bluetooth/btqca.c   | 38 +++++++++++++++++++++++++++++++++++++
>  drivers/bluetooth/hci_qca.c |  2 --
>  2 files changed, 38 insertions(+), 2 deletions(-)
>
>
> Matthias and Doug,
>
> As Chromium is the only known user of the 'local-bd-address' property,
> could you please confirm that your controllers use the 00:00:00:00:5a:ad
> address by default so that the quirk continues to be set as intended?

I was at EOSS last week so didn't get a chance to test this, but I
just tested it now and I can confirm that it breaks trogdor. It
appears that trogdor devices seem to have a variant of your "default"
address. Instead of:

00:00:00:00:5a:ad

We seem to have a default of this:

39:98:00:00:5a:ad

...so almost the same, but not enough the same to make it work with
your code. I checked 3 different trogdor boards and they were all the
same, though I can't 100% commit to saying that every trogdor device
out there has that same default address...

Given that this breaks devices and also that it's already landed and
tagged for stable, what's the plan here? Do we revert? Do we add the
second address in and hope that there aren't trogdor devices out in
the wild that somehow have a different default?


-Doug