The WCN6855 firmware on the Lenovo ThinkPad X13s expects the Bluetooth
device address in MSB order when setting it using the
EDL_WRITE_BD_ADDR_OPCODE command.
Presumably, this is the case for all non-ROME devices which all use the
EDL_WRITE_BD_ADDR_OPCODE command for this (unlike the ROME devices which
use a different command and expect the address in LSB order).
Reverse the little-endian address before setting it to make sure that
the address can be configured using tools like btmgmt or using the
'local-bd-address' devicetree property.
Note that this can potentially break systems with boot firmware which
has started relying on the broken behaviour and is incorrectly passing
the address via devicetree in MSB order.
Fixes: 5c0a1001c8be ("Bluetooth: hci_qca: Add helper to set device address")
Cc: [email protected] # 5.1
Cc: Balakrishna Godavarthi <[email protected]>
Cc: Matthias Kaehlcke <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
Hi Qualcomm people,
Could you please verify with your documentation that all non-ROME
devices expect the address provided in the EDL_WRITE_BD_ADDR_OPCODE
command in MSB order?
I assume this is not something that anyone would change between firmware
revisions, but if that turns out to be the case, we'd need to reverse
the address based on firmware revision or similar.
Johan
drivers/bluetooth/btqca.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index fdb0fae88d1c..29035daf21bc 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -826,11 +826,15 @@ EXPORT_SYMBOL_GPL(qca_uart_setup);
int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
{
+ bdaddr_t bdaddr_swapped;
struct sk_buff *skb;
int err;
- skb = __hci_cmd_sync_ev(hdev, EDL_WRITE_BD_ADDR_OPCODE, 6, bdaddr,
- HCI_EV_VENDOR, HCI_INIT_TIMEOUT);
+ baswap(&bdaddr_swapped, bdaddr);
+
+ skb = __hci_cmd_sync_ev(hdev, EDL_WRITE_BD_ADDR_OPCODE, 6,
+ &bdaddr_swapped, HCI_EV_VENDOR,
+ HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
bt_dev_err(hdev, "QCA Change address cmd failed (%d)", err);
--
2.41.0
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=813083
---Test result---
Test Summary:
CheckPatch PASS 0.62 seconds
GitLint PASS 0.37 seconds
SubjectPrefix PASS 0.12 seconds
BuildKernel PASS 27.83 seconds
CheckAllWarning PASS 30.66 seconds
CheckSparse PASS 37.40 seconds
CheckSmatch PASS 99.70 seconds
BuildKernel32 PASS 27.34 seconds
TestRunnerSetup PASS 434.37 seconds
TestRunner_l2cap-tester PASS 22.76 seconds
TestRunner_iso-tester PASS 48.67 seconds
TestRunner_bnep-tester PASS 6.93 seconds
TestRunner_mgmt-tester PASS 159.41 seconds
TestRunner_rfcomm-tester PASS 11.22 seconds
TestRunner_sco-tester PASS 14.91 seconds
TestRunner_ioctl-tester PASS 12.03 seconds
TestRunner_mesh-tester PASS 8.75 seconds
TestRunner_smp-tester PASS 11.73 seconds
TestRunner_userchan-tester PASS 7.16 seconds
IncrementalBuild PASS 26.43 seconds
---
Regards,
Linux Bluetooth
On Wed, Dec 27, 2023 at 07:03:06PM +0100, Johan Hovold wrote:
> The WCN6855 firmware on the Lenovo ThinkPad X13s expects the Bluetooth
> device address in MSB order when setting it using the
> EDL_WRITE_BD_ADDR_OPCODE command.
>
> Presumably, this is the case for all non-ROME devices which all use the
> EDL_WRITE_BD_ADDR_OPCODE command for this (unlike the ROME devices which
> use a different command and expect the address in LSB order).
>
> Reverse the little-endian address before setting it to make sure that
> the address can be configured using tools like btmgmt or using the
> 'local-bd-address' devicetree property.
>
> Note that this can potentially break systems with boot firmware which
> has started relying on the broken behaviour and is incorrectly passing
> the address via devicetree in MSB order.
>
> Fixes: 5c0a1001c8be ("Bluetooth: hci_qca: Add helper to set device address")
> Cc: [email protected] # 5.1
> Cc: Balakrishna Godavarthi <[email protected]>
> Cc: Matthias Kaehlcke <[email protected]>
> Signed-off-by: Johan Hovold <[email protected]>
The same issue was present on sc7180 (qcom,wcn3991-bt) and this patch
fixes it.
Tested-by: Nikita Travkin <[email protected]> # sc7180
Thanks!
> ---
>
> Hi Qualcomm people,
>
> Could you please verify with your documentation that all non-ROME
> devices expect the address provided in the EDL_WRITE_BD_ADDR_OPCODE
> command in MSB order?
>
> I assume this is not something that anyone would change between firmware
> revisions, but if that turns out to be the case, we'd need to reverse
> the address based on firmware revision or similar.
>
> Johan
>
>
> drivers/bluetooth/btqca.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index fdb0fae88d1c..29035daf21bc 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -826,11 +826,15 @@ EXPORT_SYMBOL_GPL(qca_uart_setup);
>
> int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> {
> + bdaddr_t bdaddr_swapped;
> struct sk_buff *skb;
> int err;
>
> - skb = __hci_cmd_sync_ev(hdev, EDL_WRITE_BD_ADDR_OPCODE, 6, bdaddr,
> - HCI_EV_VENDOR, HCI_INIT_TIMEOUT);
> + baswap(&bdaddr_swapped, bdaddr);
> +
> + skb = __hci_cmd_sync_ev(hdev, EDL_WRITE_BD_ADDR_OPCODE, 6,
> + &bdaddr_swapped, HCI_EV_VENDOR,
> + HCI_INIT_TIMEOUT);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> bt_dev_err(hdev, "QCA Change address cmd failed (%d)", err);
> --
> 2.41.0
Hi Johan,
On Wed, Dec 27, 2023 at 07:03:06PM +0100, Johan Hovold wrote:
> The WCN6855 firmware on the Lenovo ThinkPad X13s expects the Bluetooth
> device address in MSB order when setting it using the
> EDL_WRITE_BD_ADDR_OPCODE command.
>
> Presumably, this is the case for all non-ROME devices which all use the
> EDL_WRITE_BD_ADDR_OPCODE command for this (unlike the ROME devices which
> use a different command and expect the address in LSB order).
>
> Reverse the little-endian address before setting it to make sure that
> the address can be configured using tools like btmgmt or using the
> 'local-bd-address' devicetree property.
>
> Note that this can potentially break systems with boot firmware which
> has started relying on the broken behaviour and is incorrectly passing
> the address via devicetree in MSB order.
We should not break existing devices. Their byte order for
'local-bd-address' may not adhere to the 'spec', however in practice
it is the correct format for existing kernels.
I suggest adding a quirk like 'local-bd-address-msb-quirk' or
'qcom,local-bd-address-msb-quirk' to make sure existing devices keep
working properly.
Thanks
Matthias
>
> Fixes: 5c0a1001c8be ("Bluetooth: hci_qca: Add helper to set device address")
> Cc: [email protected] # 5.1
> Cc: Balakrishna Godavarthi <[email protected]>
> Cc: Matthias Kaehlcke <[email protected]>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
>
> Hi Qualcomm people,
>
> Could you please verify with your documentation that all non-ROME
> devices expect the address provided in the EDL_WRITE_BD_ADDR_OPCODE
> command in MSB order?
>
> I assume this is not something that anyone would change between firmware
> revisions, but if that turns out to be the case, we'd need to reverse
> the address based on firmware revision or similar.
>
> Johan
>
>
> drivers/bluetooth/btqca.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index fdb0fae88d1c..29035daf21bc 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -826,11 +826,15 @@ EXPORT_SYMBOL_GPL(qca_uart_setup);
>
> int qca_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> {
> + bdaddr_t bdaddr_swapped;
> struct sk_buff *skb;
> int err;
>
> - skb = __hci_cmd_sync_ev(hdev, EDL_WRITE_BD_ADDR_OPCODE, 6, bdaddr,
> - HCI_EV_VENDOR, HCI_INIT_TIMEOUT);
> + baswap(&bdaddr_swapped, bdaddr);
> +
> + skb = __hci_cmd_sync_ev(hdev, EDL_WRITE_BD_ADDR_OPCODE, 6,
> + &bdaddr_swapped, HCI_EV_VENDOR,
> + HCI_INIT_TIMEOUT);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> bt_dev_err(hdev, "QCA Change address cmd failed (%d)", err);
> --
> 2.41.0
>
On Tue, Jan 09, 2024 at 04:50:59PM +0000, Matthias Kaehlcke wrote:
> On Wed, Dec 27, 2023 at 07:03:06PM +0100, Johan Hovold wrote:
> > The WCN6855 firmware on the Lenovo ThinkPad X13s expects the Bluetooth
> > device address in MSB order when setting it using the
> > EDL_WRITE_BD_ADDR_OPCODE command.
> >
> > Presumably, this is the case for all non-ROME devices which all use the
> > EDL_WRITE_BD_ADDR_OPCODE command for this (unlike the ROME devices which
> > use a different command and expect the address in LSB order).
> >
> > Reverse the little-endian address before setting it to make sure that
> > the address can be configured using tools like btmgmt or using the
> > 'local-bd-address' devicetree property.
> >
> > Note that this can potentially break systems with boot firmware which
> > has started relying on the broken behaviour and is incorrectly passing
> > the address via devicetree in MSB order.
>
> We should not break existing devices. Their byte order for
> 'local-bd-address' may not adhere to the 'spec', however in practice
> it is the correct format for existing kernels.
That depends on in what way the current devices are broken.
Any machines that correctly specify their address in little-endian order
in the devicetree would no longer be configured using the wrong address.
So no problem there (except requiring users to re-pair their gadgets).
And tools like btgmt is broken on all of these Qualcomm machine in any
case and would now start working as expected. So no problem there either
(unless user space had adapted an inverted the addresses to btmgmt).
So the first question is whether there actually is any boot firmware out
there which passes the BD_ADDR in reverse order?
> I suggest adding a quirk like 'local-bd-address-msb-quirk' or
> 'qcom,local-bd-address-msb-quirk' to make sure existing devices keep
> working properly.
I don't think that would work. If this is something that we really need
to handle, then there's probably no way around introducing new
compatible strings for boot firmware that isn't broken while maintaining
the current broken behaviour with respect to 'local-bd-address' for some
of the current ones.
Johan
On Tue, Jan 09, 2024 at 05:54:01PM +0000, Matthias Kaehlcke wrote:
> On Tue, Jan 09, 2024 at 06:12:26PM +0100, Johan Hovold wrote:
> > That depends on in what way the current devices are broken.
> >
> > Any machines that correctly specify their address in little-endian order
> > in the devicetree would no longer be configured using the wrong address.
> > So no problem there (except requiring users to re-pair their gadgets).
> >
> > And tools like btgmt is broken on all of these Qualcomm machine in any
> > case and would now start working as expected. So no problem there either
> > (unless user space had adapted an inverted the addresses to btmgmt).
> >
> > So the first question is whether there actually is any boot firmware out
> > there which passes the BD_ADDR in reverse order?
>
> Yes, (at least) the boot firmware for sc7180-trogdor devices.
>
> hexdump -C /proc/device-tree/soc\@0/geniqup\@8c0000/serial\@88c000/bluetooth/local-bd-address
> 00000000 8c fd f0 40 15 dc
Indeed, this should have been LE order.
> hciconfig
> hci0: Type: Primary Bus: UART
> BD Address: 8C:FD:F0:40:15:DC ACL MTU: 1024:8 SCO MTU: 240:8
> UP RUNNING
> RX bytes:1700 acl:0 sco:0 events:95 errors:0
> TX bytes:128949 acl:0 sco:0 commands:578 errors:0
And any user space tool overriding the address would currently need to
provide the address in reverse order on Qualcomm platforms like this
one (e.g. if generating the address for privacy reasons).
> > > I suggest adding a quirk like 'local-bd-address-msb-quirk' or
> > > 'qcom,local-bd-address-msb-quirk' to make sure existing devices keep
> > > working properly.
> >
> > I don't think that would work. If this is something that we really need
> > to handle, then there's probably no way around introducing new
> > compatible strings for boot firmware that isn't broken while maintaining
> > the current broken behaviour with respect to 'local-bd-address' for some
> > of the current ones.
>
> I think it should work for sc7180-trogdor. For these devices the device tree
> is bundled with the kernel image and can be updated. That might not be true
> for other devices though.
Thanks for confirming.
I'm still hoping we can get away with not having to add quirks to
Bluetooth core for broken Qualcomm boot firmware. Let's see if anyone
knows of a use case that makes that impossible to avoid.
Johan
On Wed, Jan 17, 2024 at 01:52:08PM -0800, Doug Anderson wrote:
> Hi,
>
> On Wed, Jan 10, 2024 at 12:12 AM Johan Hovold <[email protected]> wrote:
> >
> > > > So the first question is whether there actually is any boot firmware out
> > > > there which passes the BD_ADDR in reverse order?
> > >
> > > Yes, (at least) the boot firmware for sc7180-trogdor devices.
> > >
> > > hexdump -C /proc/device-tree/soc\@0/geniqup\@8c0000/serial\@88c000/bluetooth/local-bd-address
> > > 00000000 8c fd f0 40 15 dc
> >
> > Indeed, this should have been LE order.
>
> In case it adds any extra data points, we also do similar with the
> WiFi MAC address and it also seems to be big endian.
>
> lazor-rev9 /proc/device-tree/soc@0/wifi@18800000 # hexdump -C local-mac-address
> 00000000 8c fd f0 3e 3e 86 |...>>.|
> 00000006
>
> lazor-rev9 /proc/device-tree/soc@0/wifi@18800000 # ifconfig wlan0 | grep ether
> ether 8c:fd:f0:3e:3e:86 txqueuelen 1000 (Ethernet)
Yes, wifi and ethernet MAC addresses are always big endian (i.e. on the
wire as well as in UIs).
When the corresponding devicetree property for Bluetooth device
addresses was added, Marcel explicitly requested that the address be
provided in little-endian order:
"I would prefer the boot loader actually providing the BD
Address in the correct byte order as the protocol expects and
not yet another form. The string representation is just for
reference since that is what most people have seen so far."
https://lore.kernel.org/all/[email protected]/
and this is also what made it into the binding:
28517c02e1dd ("dt-bindings: net: document Bluetooth bindings in one place")
Perhaps someone should have pushed back at the time to avoid this
(apparent) inconsistency, but this is what we have since 2017.
Johan
On Wed, Jan 17, 2024 at 05:49:07PM -0500, Luiz Augusto von Dentz wrote:
> On Wed, Jan 10, 2024 at 3:12 AM Johan Hovold <[email protected]> wrote:
> > On Tue, Jan 09, 2024 at 05:54:01PM +0000, Matthias Kaehlcke wrote:
> > > hciconfig
> > > hci0: Type: Primary Bus: UART
> > > BD Address: 8C:FD:F0:40:15:DC ACL MTU: 1024:8 SCO MTU: 240:8
> > > UP RUNNING
> > > RX bytes:1700 acl:0 sco:0 events:95 errors:0
> > > TX bytes:128949 acl:0 sco:0 commands:578 errors:0
> >
> > And any user space tool overriding the address would currently need to
> > provide the address in reverse order on Qualcomm platforms like this
> > one (e.g. if generating the address for privacy reasons).
>
> Perhaps we could attempt to resolve the address byteorder, in
> userspace we use hwdb_get_company to resolve the company but since
> this shall only really care about Qualcomm range(s) perhaps we can
> hardcode them check in which order the address is, that said if the
> device is configured with a Static Random Address then that would not
> work, but that is only really possible for BLE only devices.
It's not just Qualcomm ranges; The Lenovo ThinkPad X13s that I noticed
this on has been assigned a Wistron OUI, for example.
We're still hoping to learn how to retrieve this address (from the
secure world firmware) so that we can set it directly from the driver,
but for now it needs to be set using btmgmt (or the local-bd-address
devicetree property).
As was discussed here:
https://github.com/bluez/bluez/issues/107
it would be useful to teach bluetoothd to (generate and) set an address
for devices that lack (accessible) persistent storage. And any such
generic tool would need to work using the standard interfaces and the
address endianness that those interfaces expect.
And from skimming the Bluetooth spec, I was under the impression that
random addresses applied also to non-BLE devices (e.g. requiring the two
most-significants bits to be 1).
But to summarise, I don't really see any way around fixing the Qualcomm
driver.
Johan
Hi Johan,
On Thu, Jan 18, 2024 at 3:40 AM Johan Hovold <[email protected]> wrote:
>
> On Wed, Jan 17, 2024 at 05:49:07PM -0500, Luiz Augusto von Dentz wrote:
> > On Wed, Jan 10, 2024 at 3:12 AM Johan Hovold <[email protected]> wrote:
> > > On Tue, Jan 09, 2024 at 05:54:01PM +0000, Matthias Kaehlcke wrote:
>
> > > > hciconfig
> > > > hci0: Type: Primary Bus: UART
> > > > BD Address: 8C:FD:F0:40:15:DC ACL MTU: 1024:8 SCO MTU: 240:8
> > > > UP RUNNING
> > > > RX bytes:1700 acl:0 sco:0 events:95 errors:0
> > > > TX bytes:128949 acl:0 sco:0 commands:578 errors:0
> > >
> > > And any user space tool overriding the address would currently need to
> > > provide the address in reverse order on Qualcomm platforms like this
> > > one (e.g. if generating the address for privacy reasons).
> >
> > Perhaps we could attempt to resolve the address byteorder, in
> > userspace we use hwdb_get_company to resolve the company but since
> > this shall only really care about Qualcomm range(s) perhaps we can
> > hardcode them check in which order the address is, that said if the
> > device is configured with a Static Random Address then that would not
> > work, but that is only really possible for BLE only devices.
>
> It's not just Qualcomm ranges; The Lenovo ThinkPad X13s that I noticed
> this on has been assigned a Wistron OUI, for example.
Well we could still attempt to check if it has a valid OUI and then it
fail swap and check again.
> We're still hoping to learn how to retrieve this address (from the
> secure world firmware) so that we can set it directly from the driver,
> but for now it needs to be set using btmgmt (or the local-bd-address
> devicetree property).
>
> As was discussed here:
>
> https://github.com/bluez/bluez/issues/107
>
> it would be useful to teach bluetoothd to (generate and) set an address
> for devices that lack (accessible) persistent storage. And any such
> generic tool would need to work using the standard interfaces and the
> address endianness that those interfaces expect.
Yep, patches are welcome in this regard, note that we do something like this:
https://github.com/bluez/bluez/blob/master/src/adapter.c#L9847
But the first thing it checks is if the controller supports BR/EDR, so
if you want to extend that we need at least the OUI portion to be able
to allocate a valid public address, we could perhaps attempt to fetch
the manufacturer somehow or use the controller manufacturer
(adapter->manufacturer) in case there is nothing else to use.
> And from skimming the Bluetooth spec, I was under the impression that
> random addresses applied also to non-BLE devices (e.g. requiring the two
> most-significants bits to be 1).
Not really, BR/EDR/classic addresses are always considered public
addresses, the HCI interface doesn't even have an address type to be
able to handle something like a random address or privacy for the same
reason.
> But to summarise, I don't really see any way around fixing the Qualcomm
> driver.
>
> Johan
--
Luiz Augusto von Dentz
On Thu, Jan 18, 2024 at 10:30:50AM -0500, Luiz Augusto von Dentz wrote:
> On Thu, Jan 18, 2024 at 3:40 AM Johan Hovold <[email protected]> wrote:
> > On Wed, Jan 17, 2024 at 05:49:07PM -0500, Luiz Augusto von Dentz wrote:
> > > On Wed, Jan 10, 2024 at 3:12 AM Johan Hovold <[email protected]> wrote:
> > > > On Tue, Jan 09, 2024 at 05:54:01PM +0000, Matthias Kaehlcke wrote:
> > > > And any user space tool overriding the address would currently need to
> > > > provide the address in reverse order on Qualcomm platforms like this
> > > > one (e.g. if generating the address for privacy reasons).
> > >
> > > Perhaps we could attempt to resolve the address byteorder, in
> > > userspace we use hwdb_get_company to resolve the company but since
> > > this shall only really care about Qualcomm range(s) perhaps we can
> > > hardcode them check in which order the address is, that said if the
> > > device is configured with a Static Random Address then that would not
> > > work, but that is only really possible for BLE only devices.
> >
> > It's not just Qualcomm ranges; The Lenovo ThinkPad X13s that I noticed
> > this on has been assigned a Wistron OUI, for example.
>
> Well we could still attempt to check if it has a valid OUI and then it
> fail swap and check again.
So in the kernel you would parse any address coming from firmware or
user space to try to determine if it's given in reverse order? I don't
see how this would work as presumably some of the least significant
bytes would occasionally match a valid OUI even if you were somehow able
to determine that.
> > We're still hoping to learn how to retrieve this address (from the
> > secure world firmware) so that we can set it directly from the driver,
> > but for now it needs to be set using btmgmt (or the local-bd-address
> > devicetree property).
> >
> > As was discussed here:
> >
> > https://github.com/bluez/bluez/issues/107
> >
> > it would be useful to teach bluetoothd to (generate and) set an address
> > for devices that lack (accessible) persistent storage. And any such
> > generic tool would need to work using the standard interfaces and the
> > address endianness that those interfaces expect.
>
> Yep, patches are welcome in this regard, note that we do something like this:
>
> https://github.com/bluez/bluez/blob/master/src/adapter.c#L9847
>
> But the first thing it checks is if the controller supports BR/EDR, so
> if you want to extend that we need at least the OUI portion to be able
> to allocate a valid public address, we could perhaps attempt to fetch
> the manufacturer somehow or use the controller manufacturer
> (adapter->manufacturer) in case there is nothing else to use.
Thanks for the pointer. I'm trying nudge some of the distro folks to
look into this.
> > And from skimming the Bluetooth spec, I was under the impression that
> > random addresses applied also to non-BLE devices (e.g. requiring the two
> > most-significants bits to be 1).
>
> Not really, BR/EDR/classic addresses are always considered public
> addresses, the HCI interface doesn't even have an address type to be
> able to handle something like a random address or privacy for the same
> reason.
Ah, ok. Then generating an address is perhaps not an option, but reading
one out from a file and setting it would still be useful for cases like
the X13s which do have an address assigned (e.g. accessible through
windows or written on the box the machine came in).
Johan
On Tue, Feb 13, 2024 at 03:41:56PM +0100, Johan Hovold wrote:
> Hi Luiz,
>
> On Wed, Dec 27, 2023 at 07:03:06PM +0100, Johan Hovold wrote:
> > The WCN6855 firmware on the Lenovo ThinkPad X13s expects the Bluetooth
> > device address in MSB order when setting it using the
> > EDL_WRITE_BD_ADDR_OPCODE command.
> >
> > Presumably, this is the case for all non-ROME devices which all use the
> > EDL_WRITE_BD_ADDR_OPCODE command for this (unlike the ROME devices which
> > use a different command and expect the address in LSB order).
> >
> > Reverse the little-endian address before setting it to make sure that
> > the address can be configured using tools like btmgmt or using the
> > 'local-bd-address' devicetree property.
> >
> > Note that this can potentially break systems with boot firmware which
> > has started relying on the broken behaviour and is incorrectly passing
> > the address via devicetree in MSB order.
> >
> > Fixes: 5c0a1001c8be ("Bluetooth: hci_qca: Add helper to set device address")
> > Cc: [email protected] # 5.1
> > Cc: Balakrishna Godavarthi <[email protected]>
> > Cc: Matthias Kaehlcke <[email protected]>
> > Signed-off-by: Johan Hovold <[email protected]>
>
> Can we go ahead and merge this one to get this fixed in 6.8?
>
> I've spoken to Bjorn Andersson at Qualcomm about this and he is in
> favour of doing so. The only people actually using the devicetree
> property should be the Chromium team and they control their own boot
> firmware and should be able to update it in lockstep (and Android uses
> some custom hacks to set the address that are not in mainline).
Unfortunately it's not as trivial as it sounds for Chrome OS. The boot
firmware is controlled by Chrome OS, however for any baseboard (e.g.
'trogdor') there is a larger number binary firmware packages, one
for every model derived from that baseboard. There can be dozens of
models. Chrome OS Firmware releases are qualified and rolled out per
model. FW qual may involve the ODM, usually there are multiple ODMs
per board. In an absolute emergency it would be possible to coordinate
a qual and synced rollout for all models, but it's definitely
non-trivial in terms of operations.
On Tue, Feb 13, 2024 at 05:03:15PM +0100, Johan Hovold wrote:
> On Tue, Feb 13, 2024 at 03:55:06PM +0000, Matthias Kaehlcke wrote:
> > On Tue, Feb 13, 2024 at 03:41:56PM +0100, Johan Hovold wrote:
> > > On Wed, Dec 27, 2023 at 07:03:06PM +0100, Johan Hovold wrote:
> > > > The WCN6855 firmware on the Lenovo ThinkPad X13s expects the Bluetooth
> > > > device address in MSB order when setting it using the
> > > > EDL_WRITE_BD_ADDR_OPCODE command.
>
> > > > Reverse the little-endian address before setting it to make sure that
> > > > the address can be configured using tools like btmgmt or using the
> > > > 'local-bd-address' devicetree property.
> > > >
> > > > Note that this can potentially break systems with boot firmware which
> > > > has started relying on the broken behaviour and is incorrectly passing
> > > > the address via devicetree in MSB order.
> > > >
> > > > Fixes: 5c0a1001c8be ("Bluetooth: hci_qca: Add helper to set device address")
> > > > Cc: [email protected] # 5.1
> > > > Cc: Balakrishna Godavarthi <[email protected]>
> > > > Cc: Matthias Kaehlcke <[email protected]>
> > > > Signed-off-by: Johan Hovold <[email protected]>
> > >
> > > Can we go ahead and merge this one to get this fixed in 6.8?
> > >
> > > I've spoken to Bjorn Andersson at Qualcomm about this and he is in
> > > favour of doing so. The only people actually using the devicetree
> > > property should be the Chromium team and they control their own boot
> > > firmware and should be able to update it in lockstep (and Android uses
> > > some custom hacks to set the address that are not in mainline).
> >
> > Unfortunately it's not as trivial as it sounds for Chrome OS. The boot
> > firmware is controlled by Chrome OS, however for any baseboard (e.g.
> > 'trogdor') there is a larger number binary firmware packages, one
> > for every model derived from that baseboard. There can be dozens of
> > models. Chrome OS Firmware releases are qualified and rolled out per
> > model. FW qual may involve the ODM, usually there are multiple ODMs
> > per board. In an absolute emergency it would be possible to coordinate
> > a qual and synced rollout for all models, but it's definitely
> > non-trivial in terms of operations.
>
> Ok, fair enough.
>
> Could you please provide a list of the compatible strings that you guys
> currently use and I can add new compatible strings for those, while
> keeping the current ones for backwards compatibility with older boot
> firmware?
'qcom,wcn3991-bt' should be the only impacted compatible string for
released devices.
Thanks for maintaining backwards compatibility, and sorry for the
inconvenience.
On Tue, Feb 13, 2024 at 03:55:06PM +0000, Matthias Kaehlcke wrote:
> On Tue, Feb 13, 2024 at 03:41:56PM +0100, Johan Hovold wrote:
> > On Wed, Dec 27, 2023 at 07:03:06PM +0100, Johan Hovold wrote:
> > > The WCN6855 firmware on the Lenovo ThinkPad X13s expects the Bluetooth
> > > device address in MSB order when setting it using the
> > > EDL_WRITE_BD_ADDR_OPCODE command.
> > > Reverse the little-endian address before setting it to make sure that
> > > the address can be configured using tools like btmgmt or using the
> > > 'local-bd-address' devicetree property.
> > >
> > > Note that this can potentially break systems with boot firmware which
> > > has started relying on the broken behaviour and is incorrectly passing
> > > the address via devicetree in MSB order.
> > >
> > > Fixes: 5c0a1001c8be ("Bluetooth: hci_qca: Add helper to set device address")
> > > Cc: [email protected] # 5.1
> > > Cc: Balakrishna Godavarthi <[email protected]>
> > > Cc: Matthias Kaehlcke <[email protected]>
> > > Signed-off-by: Johan Hovold <[email protected]>
> >
> > Can we go ahead and merge this one to get this fixed in 6.8?
> >
> > I've spoken to Bjorn Andersson at Qualcomm about this and he is in
> > favour of doing so. The only people actually using the devicetree
> > property should be the Chromium team and they control their own boot
> > firmware and should be able to update it in lockstep (and Android uses
> > some custom hacks to set the address that are not in mainline).
>
> Unfortunately it's not as trivial as it sounds for Chrome OS. The boot
> firmware is controlled by Chrome OS, however for any baseboard (e.g.
> 'trogdor') there is a larger number binary firmware packages, one
> for every model derived from that baseboard. There can be dozens of
> models. Chrome OS Firmware releases are qualified and rolled out per
> model. FW qual may involve the ODM, usually there are multiple ODMs
> per board. In an absolute emergency it would be possible to coordinate
> a qual and synced rollout for all models, but it's definitely
> non-trivial in terms of operations.
Ok, fair enough.
Could you please provide a list of the compatible strings that you guys
currently use and I can add new compatible strings for those, while
keeping the current ones for backwards compatibility with older boot
firmware?
Johan