| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABBYNZLMFYaEjgRhO7J+sDRdp=JPVhgLdLrUNWkum5YTc5dv_w@mail.gmail.com>
Date: Fri, 28 Feb 2025 20:41:25 +0500
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
Cc: marcel@...tmann.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, linux-bluetooth@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
amitkumar.karwar@....com, sherry.sun@....com, ziniu.wang_1@....com,
johan.korsnes@...arkable.no, kristian.krohn@...arkable.no,
manjeet.gupta@....com
Subject: Re: [PATCH v6 2/2] Bluetooth: btnxpuart: Add support to set BD address
Hi Neeraj,
On Fri, Feb 28, 2025 at 10:27 AM Neeraj Sanjay Kale
<neeraj.sanjaykale@....com> wrote:
>
> This adds support for setting BD address during hci registration. NXP
> FW does not allow vendor commands unless it receives a reset command
> after FW download and initialization done.
>
> As a workaround, the .set_bdaddr callback function will first send the
> HCI reset command, followed by the actual vendor command to set BD
> address.
>
> The driver checks for the local-bd-address property in device tree, and
> if preset, it sets the HCI_QUIRK_USE_BDADDR_PROPERTY quirk.
>
> With this quirk set, the driver's set_bdaddr callback function is called
> after FW download is complete and before HCI initialization, which sends
> the hci reset and 3f 22 commands. During initialization, kernel reads
> the newly set BD address from the controller.
>
> Signed-off-by: Loic Poulain <loic.poulain@...aro.org>
> Signed-off-by: Johan Korsnes <johan.korsnes@...arkable.no>
> Signed-off-by: Kristian Krohn <kristian.krohn@...arkable.no>
> Tested-by: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
> ---
> v4: hci0 interface shows RAW mode if 'local-bd-address' not defined and
> HCI_QUIRK_USE_BDADDR_PROPERTY is set. Add Quirk only if device tree
> property 'local-bd-address' found. (Neeraj)
> v5: Initialize local variable ba, update Copywrite year. (Kristian)
> v6: Elaborate commit message, add User Manual reference. (Paul Menzel)
> ---
> drivers/bluetooth/btnxpuart.c | 63 ++++++++++++++++++++++++++++++++---
> 1 file changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index 1230045d78a5..2eb14b9beb70 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> * NXP Bluetooth driver
> - * Copyright 2023 NXP
> + * Copyright 2023-2025 NXP
> */
>
> #include <linux/module.h>
> @@ -98,13 +98,16 @@
> #define PS_STATE_AWAKE 0
> #define PS_STATE_SLEEP 1
>
> -/* Bluetooth vendor command : Sleep mode */
> +/* NXP Vendor Commands. Refer user manual UM11628 on nxp.com */
> +/* Set custom BD Address */
> +#define HCI_NXP_SET_BD_ADDR 0xfc22
> +/* Set Auto-Sleep mode */
> #define HCI_NXP_AUTO_SLEEP_MODE 0xfc23
> -/* Bluetooth vendor command : Wakeup method */
> +/* Set Wakeup method */
> #define HCI_NXP_WAKEUP_METHOD 0xfc53
> -/* Bluetooth vendor command : Set operational baudrate */
> +/* Set operational baudrate */
> #define HCI_NXP_SET_OPER_SPEED 0xfc09
> -/* Bluetooth vendor command: Independent Reset */
> +/* Independent Reset (Soft Reset) */
> #define HCI_NXP_IND_RESET 0xfcfc
>
> /* Bluetooth Power State : Vendor cmd params */
> @@ -310,6 +313,15 @@ union nxp_v3_rx_timeout_nak_u {
> u8 buf[6];
> };
>
> +union nxp_set_bd_addr_payload {
> + struct {
> + u8 param_id;
> + u8 param_len;
> + u8 param[6];
> + } __packed data;
> + u8 buf[8];
> +};
> +
> static u8 crc8_table[CRC8_TABLE_SIZE];
>
> /* Default configurations */
> @@ -1197,6 +1209,38 @@ static int nxp_set_ind_reset(struct hci_dev *hdev, void *data)
> return hci_recv_frame(hdev, skb);
> }
>
> +static int nxp_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> +{
> + union nxp_set_bd_addr_payload pcmd;
> + struct sk_buff *skb;
> + int err;
> +
> + pcmd.data.param_id = 0xfe;
> + pcmd.data.param_len = 6;
> + memcpy(pcmd.data.param, bdaddr, 6);
> +
> + /* BD address can be assigned only after first reset command. */
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + bt_dev_err(hdev, "Reset before setting local-bd-addr failed (%ld)",
> + PTR_ERR(skb));
> + return err;
> + }
> + kfree_skb(skb);
If you don't care about the response, just the status, it is probably
better to use __hci_cmd_sync_status, also since the hdev->set_bdaddr
comes after hdev->setup doesn't the later do perform a reset anyway?
If you end up with 2 resets in a row it problems means you don't need
to reset again.
> +
> + skb = __hci_cmd_sync(hdev, HCI_NXP_SET_BD_ADDR, sizeof(pcmd),
> + pcmd.buf, HCI_CMD_TIMEOUT);
Ditto.
> + if (IS_ERR(skb)) {
> + err = PTR_ERR(skb);
> + bt_dev_err(hdev, "Changing device address failed (%d)", err);
> + return err;
> + }
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> /* NXP protocol */
> static int nxp_setup(struct hci_dev *hdev)
> {
> @@ -1500,6 +1544,7 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
> {
> struct hci_dev *hdev;
> struct btnxpuart_dev *nxpdev;
> + bdaddr_t ba = {0};
>
> nxpdev = devm_kzalloc(&serdev->dev, sizeof(*nxpdev), GFP_KERNEL);
> if (!nxpdev)
> @@ -1547,8 +1592,16 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
> hdev->send = nxp_enqueue;
> hdev->hw_error = nxp_hw_err;
> hdev->shutdown = nxp_shutdown;
> + hdev->set_bdaddr = nxp_set_bdaddr;
> +
> SET_HCIDEV_DEV(hdev, &serdev->dev);
>
> + device_property_read_u8_array(&nxpdev->serdev->dev,
> + "local-bd-address",
> + (u8 *)&ba, sizeof(ba));
> + if (bacmp(&ba, BDADDR_ANY))
> + set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
> +
> if (hci_register_dev(hdev) < 0) {
> dev_err(&serdev->dev, "Can't register HCI device\n");
> goto probe_fail;
> --
> 2.25.1
>
--
Luiz Augusto von Dentz
Powered by blists - more mailing lists