2024-03-20 06:08:54

by quic_zijuhu

Subject: [PATCH v1 0/2] Bluetooth: qca: Add tool btattach support for more QCA soc types

Kernel header drivers/bluetooth/btqca.h defines many soc types as following:
enum qca_btsoc_type {
QCA_INVALID = -1,
QCA_AR3002,
QCA_ROME,
QCA_WCN3988,
QCA_WCN3990,
QCA_WCN3998,
QCA_WCN3991,
QCA_QCA2066,
QCA_QCA6390,
QCA_WCN6750,
QCA_WCN6855,
QCA_WCN7850,
QCA_MAX,
};
and every soc type stands for a kind of QCA BT controller, but tool btattach currenlty
only supports default soc type QCA_ROME, this patch series are to add support for other
all other QCA soc types by adding a option for tool btattach to specify soc type.

Zijun Hu (2):
Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA
Bluetooth: qca: Fix wrong soc_type returned for tool btattach

drivers/bluetooth/btqca.h | 1 +
drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
drivers/bluetooth/hci_qca.c | 8 +++++++-
drivers/bluetooth/hci_uart.h | 3 +++
4 files changed, 21 insertions(+), 1 deletion(-)

--
2.7.4

2024-03-20 06:09:02

by quic_zijuhu

Subject: [PATCH v1 1/2] Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA

HCIUARTSETPROTODATA is introduced to specify protocol specific settings
prior to HCIUARTSETPROTO, for protocal QCA, it is used by tool btattch
to specify soc_type.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
drivers/bluetooth/hci_uart.h | 3 +++
2 files changed, 13 insertions(+)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index a26367e9fb19..4be09c61bae5 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -506,6 +506,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
/* disable alignment support by default */
hu->alignment = 1;
hu->padding = 0;
+ hu->proto_data = 0;

INIT_WORK(&hu->init_ready, hci_uart_init_work);
INIT_WORK(&hu->write_work, hci_uart_write_work);
@@ -795,6 +796,15 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
err = hu->hdev_flags;
break;

+ case HCIUARTSETPROTODATA:
+ if (test_bit(HCI_UART_PROTO_SET, &hu->flags)) {
+ err = -EBUSY;
+ } else {
+ hu->proto_data = arg;
+ BT_DBG("HCIUARTSETPROTODATA %lu okay.", arg);
+ }
+ break;
+
default:
err = n_tty_ioctl_helper(tty, cmd, arg);
break;
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 68c8c7e95d64..fc35e9bd4398 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -18,6 +18,8 @@
#define HCIUARTGETDEVICE _IOR('U', 202, int)
#define HCIUARTSETFLAGS _IOW('U', 203, int)
#define HCIUARTGETFLAGS _IOR('U', 204, int)
+/* Used prior to HCIUARTSETPROTO */
+#define HCIUARTSETPROTODATA _IOW('U', 205, unsigned long)

/* UART protocols */
#define HCI_UART_MAX_PROTO 12
@@ -71,6 +73,7 @@ struct hci_uart {
struct work_struct init_ready;
struct work_struct write_work;

+ unsigned long proto_data;
const struct hci_uart_proto *proto;
struct percpu_rw_semaphore proto_lock; /* Stop work for proto close */
void *priv;
--
2.7.4

2024-03-20 06:09:10

by quic_zijuhu

Subject: [PATCH v1 2/2] Bluetooth: qca: Fix wrong soc_type returned for tool btattach

qca_soc_type() returns wrong soc_type QCA_ROME when use tool btattach
for any other soc_type such as QCA_QCA2066, and wrong soc_type will
cause later initialization failure, fixed by using user specified
soc_type for hci_uart derived from tool btattach.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/btqca.h | 1 +
drivers/bluetooth/hci_qca.c | 8 +++++++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index dc31984f71dc..a148d4c4e1bd 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -153,6 +153,7 @@ enum qca_btsoc_type {
QCA_WCN6750,
QCA_WCN6855,
QCA_WCN7850,
+ QCA_MAX,
};

#if IS_ENABLED(CONFIG_BT_QCA)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 8a60ad7acd70..06bbcc678f88 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -238,12 +238,17 @@ static void qca_dmp_hdr(struct hci_dev *hdev, struct sk_buff *skb);

static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
{
+ /* For Non-serdev device, hu->proto_data records soc_type
+ * set by ioctl HCIUARTSETPROTODATA.
+ */
+ int proto_data = (int)hu->proto_data;
enum qca_btsoc_type soc_type;

if (hu->serdev) {
struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
-
soc_type = qsd->btsoc_type;
+ } else if ((proto_data > 0) && (proto_data < QCA_MAX)) {
+ soc_type = (enum qca_btsoc_type)proto_data;
} else {
soc_type = QCA_ROME;
}
@@ -2286,6 +2291,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return -ENOMEM;

qcadev->serdev_hu.serdev = serdev;
+ qcadev->serdev_hu.proto_data = 0;
data = device_get_match_data(&serdev->dev);
serdev_device_set_drvdata(serdev, qcadev);
device_property_read_string(&serdev->dev, "firmware-name",
--
2.7.4

2024-03-20 06:35:02

by bluez.test.bot

Subject: RE: Bluetooth: qca: Add tool btattach support for more QCA soc types

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=836622

---Test result---

Test Summary:
CheckPatch PASS 1.69 seconds
GitLint PASS 0.65 seconds
SubjectPrefix PASS 0.25 seconds
BuildKernel PASS 28.99 seconds
CheckAllWarning PASS 30.68 seconds
CheckSparse PASS 36.37 seconds
CheckSmatch PASS 98.95 seconds
BuildKernel32 PASS 27.13 seconds
TestRunnerSetup PASS 510.86 seconds
TestRunner_l2cap-tester PASS 19.99 seconds
TestRunner_iso-tester PASS 30.17 seconds
TestRunner_bnep-tester PASS 4.72 seconds
TestRunner_mgmt-tester PASS 110.58 seconds
TestRunner_rfcomm-tester PASS 7.29 seconds
TestRunner_sco-tester PASS 14.92 seconds
TestRunner_ioctl-tester PASS 7.73 seconds
TestRunner_mesh-tester PASS 5.78 seconds
TestRunner_smp-tester PASS 6.91 seconds
TestRunner_userchan-tester PASS 4.89 seconds
IncrementalBuild PASS 31.17 seconds



---
Regards,
Linux Bluetooth

2024-03-20 07:28:57

by quic_zijuhu

Subject: [PATCH v1] tools/btattach: Add support for all QCA soc_types

Tool btattach currently only supports QCA default soc_type
QCA_ROME, this change adds support for all other QCA soc_types
by adding a option to specify soc_type.
---
tools/btattach.c | 29 ++++++++++++++++++++++++-----
tools/btattach.rst | 2 ++
tools/hciattach.h | 2 ++
3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/tools/btattach.c b/tools/btattach.c
index 4ce1be78d69c..024b0c7a289c 100644
--- a/tools/btattach.c
+++ b/tools/btattach.c
@@ -97,7 +97,8 @@ static void local_version_callback(const void *data, uint8_t size,
}

static int attach_proto(const char *path, unsigned int proto,
- unsigned int speed, bool flowctl, unsigned int flags)
+ unsigned int speed, bool flowctl, unsigned int flags,
+ unsigned long soc_type)
{
int fd, dev_id;

@@ -111,6 +112,16 @@ static int attach_proto(const char *path, unsigned int proto,
return -1;
}

+ if ((proto == HCI_UART_QCA) && (soc_type > 0)) {
+ if (ioctl(fd, HCIUARTSETPROTODATA, soc_type) < 0) {
+ fprintf(stderr,
+ "Failed to set soc_type(%lu) for protocol qca\n",
+ soc_type);
+ close(fd);
+ return -1;
+ }
+ }
+
if (ioctl(fd, HCIUARTSETPROTO, proto) < 0) {
perror("Failed to set protocol");
close(fd);
@@ -181,6 +192,7 @@ static void usage(void)
"\t-A, --amp <device> Attach AMP controller\n"
"\t-P, --protocol <proto> Specify protocol type\n"
"\t-S, --speed <baudrate> Specify which baudrate to use\n"
+ "\t-T, --type <soc_type> Specify soc_type for protocol qca\n"
"\t-N, --noflowctl Disable flow control\n"
"\t-h, --help Show help options\n");
}
@@ -190,6 +202,7 @@ static const struct option main_options[] = {
{ "amp", required_argument, NULL, 'A' },
{ "protocol", required_argument, NULL, 'P' },
{ "speed", required_argument, NULL, 'S' },
+ { "type", required_argument, NULL, 'T' },
{ "noflowctl",no_argument, NULL, 'N' },
{ "version", no_argument, NULL, 'v' },
{ "help", no_argument, NULL, 'h' },
@@ -221,12 +234,13 @@ int main(int argc, char *argv[])
bool flowctl = true, raw_device = false;
int exit_status, count = 0, proto_id = HCI_UART_H4;
unsigned int speed = B115200;
+ unsigned long soc_type = 0;

for (;;) {
int opt;

- opt = getopt_long(argc, argv, "B:A:P:S:NRvh",
- main_options, NULL);
+ opt = getopt_long(argc, argv, "B:A:P:S:T:NRvh",
+ main_options, NULL);
if (opt < 0)
break;

@@ -237,6 +251,9 @@ int main(int argc, char *argv[])
case 'A':
amp_path = optarg;
break;
+ case 'T':
+ soc_type = strtoul(optarg, NULL, 0);
+ break;
case 'P':
proto = optarg;
break;
@@ -298,7 +315,8 @@ int main(int argc, char *argv[])
if (raw_device)
flags = (1 << HCI_UART_RAW_DEVICE);

- fd = attach_proto(bredr_path, proto_id, speed, flowctl, flags);
+ fd = attach_proto(bredr_path, proto_id, speed, flowctl, flags,
+ soc_type);
if (fd >= 0) {
mainloop_add_fd(fd, 0, uart_callback, NULL, NULL);
count++;
@@ -317,7 +335,8 @@ int main(int argc, char *argv[])
if (raw_device)
flags = (1 << HCI_UART_RAW_DEVICE);

- fd = attach_proto(amp_path, proto_id, speed, flowctl, flags);
+ fd = attach_proto(amp_path, proto_id, speed, flowctl, flags,
+ soc_type);
if (fd >= 0) {
mainloop_add_fd(fd, 0, uart_callback, NULL, NULL);
count++;
diff --git a/tools/btattach.rst b/tools/btattach.rst
index 787d5c49e3bb..4aad3b915641 100644
--- a/tools/btattach.rst
+++ b/tools/btattach.rst
@@ -62,6 +62,8 @@ OPTIONS

-S baudrate, --speed baudrate Specify wich baudrate to use

+-T soc_type, --type soc_type Specify soc_type for protocol qca
+
-N, --noflowctl Disable flow control

-v, --version Show version
diff --git a/tools/hciattach.h b/tools/hciattach.h
index dfa4c1e7abe7..998a2a9a8460 100644
--- a/tools/hciattach.h
+++ b/tools/hciattach.h
@@ -19,6 +19,8 @@
#define HCIUARTGETDEVICE _IOR('U', 202, int)
#define HCIUARTSETFLAGS _IOW('U', 203, int)
#define HCIUARTGETFLAGS _IOR('U', 204, int)
+#define HCIUARTSETPROTODATA _IOW('U', 205, unsigned long)
+

#define HCI_UART_H4 0
#define HCI_UART_BCSP 1
--
2.7.4

2024-03-20 09:03:03

by bluez.test.bot

Subject: RE: [v1] tools/btattach: Add support for all QCA soc_types

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=836647

---Test result---

Test Summary:
CheckPatch PASS 0.43 seconds
GitLint PASS 0.29 seconds
BuildEll PASS 24.20 seconds
BluezMake PASS 1678.26 seconds
MakeCheck PASS 13.19 seconds
MakeDistcheck PASS 174.98 seconds
CheckValgrind PASS 243.07 seconds
CheckSmatch PASS 348.13 seconds
bluezmakeextell PASS 119.57 seconds
IncrementalBuild PASS 1431.75 seconds
ScanBuild PASS 982.21 seconds



---
Regards,
Linux Bluetooth

2024-04-12 16:37:19

by quic_zijuhu

Subject: [PATCH v1 0/3] Fix 2 tool btattach issues for QCA controllers

There are many QCA soc types defined by drivers/bluetooth/btqca.h
enum qca_btsoc_type {
QCA_INVALID = -1,
QCA_AR3002,
QCA_ROME,
QCA_WCN3988,
QCA_WCN3990,
QCA_WCN3998,
QCA_WCN3991,
QCA_QCA2066,
QCA_QCA6390,
QCA_WCN6750,
QCA_WCN6855,
QCA_WCN7850,
QCA_MAX,
};
and every soc type stands for a kind of QCA BT controller, this patch
series are to solve below 2 tool btattach issues for QCA soc types:
1) tool btattach will cause kernel crash when used for QCA_ROME
2) tool btattach does not support any other soc type except QCA_ROME

For hci_uart derived from tool btattach, it is allocated by hci_ldisc
and is Non-serdev device, its @serdev is NULL and its soc type is
currenlty hard coded as QCA_ROME.

The 1st issue is caused by dereferencing nullptr hu->serdev, in order to
solve the 2nd issue, a new ioctl is introduced for user to specify soc
type by a new added tool btattach option.

Zijun Hu (3):
Bluetooth: qca: Fix crash caused by tool btattach for QCA_ROME
Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA
Bluetooth: qca: Fix wrong soc type returned for tool btattach

drivers/bluetooth/btqca.h | 1 +
drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
drivers/bluetooth/hci_qca.c | 10 ++++++++--
drivers/bluetooth/hci_uart.h | 3 +++
4 files changed, 22 insertions(+), 2 deletions(-)

--
2.7.4

2024-04-12 16:37:59

by quic_zijuhu

Subject: [PATCH v1 3/3] Bluetooth: qca: Fix wrong soc type returned for tool btattach

qca_soc_type() returns wrong soc type QCA_ROME when use tool btattach
for any other soc type such as QCA_QCA2066, and wrong soc type will
cause later initialization failure, fixed by using user specified
soc type for hci_uart derived from tool btattach.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/btqca.h | 1 +
drivers/bluetooth/hci_qca.c | 8 +++++++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index dc31984f71dc..a148d4c4e1bd 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -153,6 +153,7 @@ enum qca_btsoc_type {
QCA_WCN6750,
QCA_WCN6855,
QCA_WCN7850,
+ QCA_MAX,
};

#if IS_ENABLED(CONFIG_BT_QCA)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 158567774cec..45492456f4f2 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -238,12 +238,17 @@ static void qca_dmp_hdr(struct hci_dev *hdev, struct sk_buff *skb);

static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
{
+ /* For Non-serdev device, hu->proto_data records soc type
+ * set by ioctl HCIUARTSETPROTODATA.
+ */
+ int proto_data = (int)hu->proto_data;
enum qca_btsoc_type soc_type;

if (hu->serdev) {
struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
-
soc_type = qsd->btsoc_type;
+ } else if ((proto_data > 0) && (proto_data < QCA_MAX)) {
+ soc_type = (enum qca_btsoc_type)proto_data;
} else {
soc_type = QCA_ROME;
}
@@ -2282,6 +2287,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return -ENOMEM;

qcadev->serdev_hu.serdev = serdev;
+ qcadev->serdev_hu.proto_data = 0;
data = device_get_match_data(&serdev->dev);
serdev_device_set_drvdata(serdev, qcadev);
device_property_read_string(&serdev->dev, "firmware-name",
--
2.7.4

2024-04-12 16:38:34

by quic_zijuhu

Subject: [PATCH v1 2/3] Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA

HCIUARTSETPROTODATA is introduced to specify protocol specific settings
prior to HCIUARTSETPROTO, for protocal QCA, it is used by tool btattch
to specify soc type.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
drivers/bluetooth/hci_uart.h | 3 +++
2 files changed, 13 insertions(+)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index a26367e9fb19..4be09c61bae5 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -506,6 +506,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
/* disable alignment support by default */
hu->alignment = 1;
hu->padding = 0;
+ hu->proto_data = 0;

INIT_WORK(&hu->init_ready, hci_uart_init_work);
INIT_WORK(&hu->write_work, hci_uart_write_work);
@@ -795,6 +796,15 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
err = hu->hdev_flags;
break;

+ case HCIUARTSETPROTODATA:
+ if (test_bit(HCI_UART_PROTO_SET, &hu->flags)) {
+ err = -EBUSY;
+ } else {
+ hu->proto_data = arg;
+ BT_DBG("HCIUARTSETPROTODATA %lu okay.", arg);
+ }
+ break;
+
default:
err = n_tty_ioctl_helper(tty, cmd, arg);
break;
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 68c8c7e95d64..fc35e9bd4398 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -18,6 +18,8 @@
#define HCIUARTGETDEVICE _IOR('U', 202, int)
#define HCIUARTSETFLAGS _IOW('U', 203, int)
#define HCIUARTGETFLAGS _IOR('U', 204, int)
+/* Used prior to HCIUARTSETPROTO */
+#define HCIUARTSETPROTODATA _IOW('U', 205, unsigned long)

/* UART protocols */
#define HCI_UART_MAX_PROTO 12
@@ -71,6 +73,7 @@ struct hci_uart {
struct work_struct init_ready;
struct work_struct write_work;

+ unsigned long proto_data;
const struct hci_uart_proto *proto;
struct percpu_rw_semaphore proto_lock; /* Stop work for proto close */
void *priv;
--
2.7.4

2024-04-17 12:53:02

by quic_zijuhu

Subject: [PATCH v2 0/4] Fix 2 tool btattach issues for QCA controllers

BT chip vendor and customer often needs to use tool btattach to perform
various development,verification and evaluation test for vendor's BT
module, but tool btattach is not working fine for any QCA BT controller
currently, this patch series are to solve this issue.

There are many QCA soc types defined by drivers/bluetooth/btqca.h
enum qca_btsoc_type {
QCA_INVALID = -1,
QCA_AR3002,
QCA_ROME,
QCA_WCN3988,
QCA_WCN3990,
QCA_WCN3998,
QCA_WCN3991,
QCA_QCA2066,
QCA_QCA6390,
QCA_WCN6750,
QCA_WCN6855,
QCA_WCN7850,
QCA_MAX,
};
and every soc type stands for a kind of QCA BT controller, this patch
series are to solve below 2 tool btattach issues for QCA soc types:
1) tool btattach will cause kernel crash when used for QCA_ROME
2) tool btattach does not support any other soc type except QCA_ROME

For hci_uart derived from tool btattach, it is allocated by hci_ldisc
and is Non-serdev device, its @serdev is NULL and its soc type is
currenlty hard coded as QCA_ROME.

The 1st issue is caused by dereferencing nullptr hu->serdev, in order to
solve the 2nd issue, a new ioctl is introduced for user to specify soc
type by a new added tool btattach option.

Changes v1 -> v2
- Add patch 2/4
- Correct cover letter

Zijun Hu (4):
Bluetooth: qca: Fix crash caused by tool btattach for QCA_ROME
Bluetooth: qca: Fix nullptr dereference for non-serdev devices
Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA
Bluetooth: qca: Fix wrong soc type returned for tool btattach

drivers/bluetooth/btqca.h | 1 +
drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
drivers/bluetooth/hci_qca.c | 19 +++++++++++++------
drivers/bluetooth/hci_uart.h | 3 +++
4 files changed, 27 insertions(+), 6 deletions(-)

--
2.7.4

2024-04-17 12:53:12

by quic_zijuhu

Subject: [PATCH v2 1/4] Bluetooth: qca: Fix crash caused by tool btattach for QCA_ROME

A kernel crash will happen when use tool btattach for a BT controller
with soc type QCA_ROME, and it is caused by dereferencing nullptr
hu->serdev, fixed by null check before access.

sudo btattach -B /dev/ttyUSB0 -P qca
Bluetooth: hci1: QCA setup on UART is completed
BUG: kernel NULL pointer dereference, address: 00000000000002f0
......
Workqueue: hci1 hci_power_on [bluetooth]
RIP: 0010:qca_setup+0x7c1/0xe30 [hci_uart]
......
Call Trace:
<TASK>
? show_regs+0x72/0x90
? __die+0x25/0x80
? page_fault_oops+0x154/0x4c0
? srso_alias_return_thunk+0x5/0xfbef5
? kmem_cache_alloc+0x16b/0x310
? do_user_addr_fault+0x330/0x6e0
? srso_alias_return_thunk+0x5/0xfbef5
? exc_page_fault+0x84/0x1b0
? asm_exc_page_fault+0x27/0x30
? qca_setup+0x7c1/0xe30 [hci_uart]
hci_uart_setup+0x5c/0x1a0 [hci_uart]
hci_dev_open_sync+0xee/0xca0 [bluetooth]
hci_dev_do_open+0x2a/0x70 [bluetooth]
hci_power_on+0x46/0x210 [bluetooth]
process_one_work+0x17b/0x360
worker_thread+0x307/0x430
? __pfx_worker_thread+0x10/0x10
kthread+0xf7/0x130
? __pfx_kthread+0x10/0x10
ret_from_fork+0x46/0x70
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1b/0x30
</TASK>

Fixes: 03b0093f7b31 ("Bluetooth: hci_qca: get wakeup status from serdev device handle")
Signed-off-by: Zijun Hu <[email protected]>
Tested-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 92fa20f5ac7d..fdaf83d817af 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1955,7 +1955,7 @@ static int qca_setup(struct hci_uart *hu)
qca_debugfs_init(hdev);
hu->hdev->hw_error = qca_hw_error;
hu->hdev->cmd_timeout = qca_cmd_timeout;
- if (device_can_wakeup(hu->serdev->ctrl->dev.parent))
+ if (hu->serdev && device_can_wakeup(hu->serdev->ctrl->dev.parent))
hu->hdev->wakeup = qca_wakeup;
} else if (ret == -ENOENT) {
/* No patch/nvm-config found, run with original fw/config */
--
2.7.4

2024-04-17 12:53:16

by quic_zijuhu

Subject: [PATCH v2 2/4] Bluetooth: qca: Fix nullptr dereference for non-serdev devices

hu->serdev is nullptr and will cause nullptr dereference if qca_setup()
is called by non-serdev device, fixed by null check before access.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index fdaf83d817af..c04b97332bca 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1905,10 +1905,11 @@ static int qca_setup(struct hci_uart *hu)
case QCA_WCN6750:
case QCA_WCN6855:
case QCA_WCN7850:
- qcadev = serdev_device_get_drvdata(hu->serdev);
- if (qcadev->bdaddr_property_broken)
- set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
-
+ if (hu->serdev) {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+ if (qcadev->bdaddr_property_broken)
+ set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
+ }
hci_set_aosp_capable(hdev);

ret = qca_read_soc_version(hdev, &ver, soc_type);
--
2.7.4

2024-04-17 12:53:23

by quic_zijuhu

Subject: [PATCH v2 4/4] Bluetooth: qca: Fix wrong soc type returned for tool btattach

qca_soc_type() returns wrong soc type QCA_ROME when use tool btattach
for any other soc type such as QCA_QCA2066, and wrong soc type will
cause later initialization failure, fixed by using user specified
soc type for hci_uart derived from tool btattach.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/btqca.h | 1 +
drivers/bluetooth/hci_qca.c | 8 +++++++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index dc31984f71dc..a148d4c4e1bd 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -153,6 +153,7 @@ enum qca_btsoc_type {
QCA_WCN6750,
QCA_WCN6855,
QCA_WCN7850,
+ QCA_MAX,
};

#if IS_ENABLED(CONFIG_BT_QCA)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index c04b97332bca..7c3577a4887c 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -238,12 +238,17 @@ static void qca_dmp_hdr(struct hci_dev *hdev, struct sk_buff *skb);

static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
{
+ /* For Non-serdev device, hu->proto_data records soc type
+ * set by ioctl HCIUARTSETPROTODATA.
+ */
+ int proto_data = (int)hu->proto_data;
enum qca_btsoc_type soc_type;

if (hu->serdev) {
struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
-
soc_type = qsd->btsoc_type;
+ } else if ((proto_data > 0) && (proto_data < QCA_MAX)) {
+ soc_type = (enum qca_btsoc_type)proto_data;
} else {
soc_type = QCA_ROME;
}
@@ -2281,6 +2286,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return -ENOMEM;

qcadev->serdev_hu.serdev = serdev;
+ qcadev->serdev_hu.proto_data = 0;
data = device_get_match_data(&serdev->dev);
serdev_device_set_drvdata(serdev, qcadev);
device_property_read_string(&serdev->dev, "firmware-name",
--
2.7.4

2024-04-17 12:55:37

by quic_zijuhu

Subject: [PATCH v2 3/4] Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA

HCIUARTSETPROTODATA is introduced to specify protocol specific settings
prior to HCIUARTSETPROTO, for protocal QCA, it is used by tool btattch
to specify soc type.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
drivers/bluetooth/hci_uart.h | 3 +++
2 files changed, 13 insertions(+)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index a26367e9fb19..4be09c61bae5 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -506,6 +506,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
/* disable alignment support by default */
hu->alignment = 1;
hu->padding = 0;
+ hu->proto_data = 0;

INIT_WORK(&hu->init_ready, hci_uart_init_work);
INIT_WORK(&hu->write_work, hci_uart_write_work);
@@ -795,6 +796,15 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
err = hu->hdev_flags;
break;

+ case HCIUARTSETPROTODATA:
+ if (test_bit(HCI_UART_PROTO_SET, &hu->flags)) {
+ err = -EBUSY;
+ } else {
+ hu->proto_data = arg;
+ BT_DBG("HCIUARTSETPROTODATA %lu okay.", arg);
+ }
+ break;
+
default:
err = n_tty_ioctl_helper(tty, cmd, arg);
break;
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 68c8c7e95d64..fc35e9bd4398 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -18,6 +18,8 @@
#define HCIUARTGETDEVICE _IOR('U', 202, int)
#define HCIUARTSETFLAGS _IOW('U', 203, int)
#define HCIUARTGETFLAGS _IOR('U', 204, int)
+/* Used prior to HCIUARTSETPROTO */
+#define HCIUARTSETPROTODATA _IOW('U', 205, unsigned long)

/* UART protocols */
#define HCI_UART_MAX_PROTO 12
@@ -71,6 +73,7 @@ struct hci_uart {
struct work_struct init_ready;
struct work_struct write_work;

+ unsigned long proto_data;
const struct hci_uart_proto *proto;
struct percpu_rw_semaphore proto_lock; /* Stop work for proto close */
void *priv;
--
2.7.4

2024-04-17 21:27:56

by Luiz Augusto von Dentz

Subject: Re: [PATCH v2 3/4] Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA

Hi Zijun,

On Wed, Apr 17, 2024 at 8:53 AM Zijun Hu <[email protected]> wrote:
>
> HCIUARTSETPROTODATA is introduced to specify protocol specific settings
> prior to HCIUARTSETPROTO, for protocal QCA, it is used by tool btattch
> to specify soc type.
>
> Signed-off-by: Zijun Hu <[email protected]>
> ---
> drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
> drivers/bluetooth/hci_uart.h | 3 +++
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index a26367e9fb19..4be09c61bae5 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -506,6 +506,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
> /* disable alignment support by default */
> hu->alignment = 1;
> hu->padding = 0;
> + hu->proto_data = 0;
>
> INIT_WORK(&hu->init_ready, hci_uart_init_work);
> INIT_WORK(&hu->write_work, hci_uart_write_work);
> @@ -795,6 +796,15 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
> err = hu->hdev_flags;
> break;
>
> + case HCIUARTSETPROTODATA:
> + if (test_bit(HCI_UART_PROTO_SET, &hu->flags)) {
> + err = -EBUSY;
> + } else {
> + hu->proto_data = arg;
> + BT_DBG("HCIUARTSETPROTODATA %lu okay.", arg);
> + }
> + break;
> +
> default:
> err = n_tty_ioctl_helper(tty, cmd, arg);
> break;
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index 68c8c7e95d64..fc35e9bd4398 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -18,6 +18,8 @@
> #define HCIUARTGETDEVICE _IOR('U', 202, int)
> #define HCIUARTSETFLAGS _IOW('U', 203, int)
> #define HCIUARTGETFLAGS _IOR('U', 204, int)
> +/* Used prior to HCIUARTSETPROTO */
> +#define HCIUARTSETPROTODATA _IOW('U', 205, unsigned long)

Don't think this will gonna fly, Im not going to introduce vendor
specific like this, besides if the kernel is not able to discover this
data why would userspace be?

> /* UART protocols */
> #define HCI_UART_MAX_PROTO 12
> @@ -71,6 +73,7 @@ struct hci_uart {
> struct work_struct init_ready;
> struct work_struct write_work;
>
> + unsigned long proto_data;
> const struct hci_uart_proto *proto;
> struct percpu_rw_semaphore proto_lock; /* Stop work for proto close */
> void *priv;
> --
> 2.7.4
>


--
Luiz Augusto von Dentz

2024-04-17 21:40:21

by Luiz Augusto von Dentz

Subject: Re: [PATCH v2 4/4] Bluetooth: qca: Fix wrong soc type returned for tool btattach

Hi Zijun,

On Wed, Apr 17, 2024 at 8:53 AM Zijun Hu <[email protected]> wrote:
>
> qca_soc_type() returns wrong soc type QCA_ROME when use tool btattach
> for any other soc type such as QCA_QCA2066, and wrong soc type will
> cause later initialization failure, fixed by using user specified
> soc type for hci_uart derived from tool btattach.

Then we have to fix qca_soc_type or explain what is going on that it
can't detect the soc_type if initialized via btattach?

>
> Signed-off-by: Zijun Hu <[email protected]>
> ---
> drivers/bluetooth/btqca.h | 1 +
> drivers/bluetooth/hci_qca.c | 8 +++++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
> index dc31984f71dc..a148d4c4e1bd 100644
> --- a/drivers/bluetooth/btqca.h
> +++ b/drivers/bluetooth/btqca.h
> @@ -153,6 +153,7 @@ enum qca_btsoc_type {
> QCA_WCN6750,
> QCA_WCN6855,
> QCA_WCN7850,
> + QCA_MAX,
> };
>
> #if IS_ENABLED(CONFIG_BT_QCA)
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index c04b97332bca..7c3577a4887c 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -238,12 +238,17 @@ static void qca_dmp_hdr(struct hci_dev *hdev, struct sk_buff *skb);
>
> static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
> {
> + /* For Non-serdev device, hu->proto_data records soc type
> + * set by ioctl HCIUARTSETPROTODATA.
> + */
> + int proto_data = (int)hu->proto_data;
> enum qca_btsoc_type soc_type;
>
> if (hu->serdev) {
> struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
> -
> soc_type = qsd->btsoc_type;
> + } else if ((proto_data > 0) && (proto_data < QCA_MAX)) {
> + soc_type = (enum qca_btsoc_type)proto_data;

Like I said a vendor specific ioctl will not gonna fly with me,
specially since each vendor may need a different size to describe
their controller version, etc,

> } else {
> soc_type = QCA_ROME;
> }
> @@ -2281,6 +2286,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> return -ENOMEM;
>
> qcadev->serdev_hu.serdev = serdev;
> + qcadev->serdev_hu.proto_data = 0;
> data = device_get_match_data(&serdev->dev);
> serdev_device_set_drvdata(serdev, qcadev);
> device_property_read_string(&serdev->dev, "firmware-name",
> --
> 2.7.4
>


--
Luiz Augusto von Dentz

2024-04-18 00:44:28

by quic_zijuhu

Subject: Re: [PATCH v2 3/4] Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA

On 4/18/2024 5:27 AM, Luiz Augusto von Dentz wrote:
> Hi Zijun,
>
> On Wed, Apr 17, 2024 at 8:53 AM Zijun Hu <[email protected]> wrote:
>>
>> HCIUARTSETPROTODATA is introduced to specify protocol specific settings
>> prior to HCIUARTSETPROTO, for protocal QCA, it is used by tool btattch
>> to specify soc type.
>>
>> Signed-off-by: Zijun Hu <[email protected]>
>> ---
>> drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
>> drivers/bluetooth/hci_uart.h | 3 +++
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
>> index a26367e9fb19..4be09c61bae5 100644
>> --- a/drivers/bluetooth/hci_ldisc.c
>> +++ b/drivers/bluetooth/hci_ldisc.c
>> @@ -506,6 +506,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
>> /* disable alignment support by default */
>> hu->alignment = 1;
>> hu->padding = 0;
>> + hu->proto_data = 0;
>>
>> INIT_WORK(&hu->init_ready, hci_uart_init_work);
>> INIT_WORK(&hu->write_work, hci_uart_write_work);
>> @@ -795,6 +796,15 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
>> err = hu->hdev_flags;
>> break;
>>
>> + case HCIUARTSETPROTODATA:
>> + if (test_bit(HCI_UART_PROTO_SET, &hu->flags)) {
>> + err = -EBUSY;
>> + } else {
>> + hu->proto_data = arg;
>> + BT_DBG("HCIUARTSETPROTODATA %lu okay.", arg);
>> + }
>> + break;
>> +
>> default:
>> err = n_tty_ioctl_helper(tty, cmd, arg);
>> break;
>> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
>> index 68c8c7e95d64..fc35e9bd4398 100644
>> --- a/drivers/bluetooth/hci_uart.h
>> +++ b/drivers/bluetooth/hci_uart.h
>> @@ -18,6 +18,8 @@
>> #define HCIUARTGETDEVICE _IOR('U', 202, int)
>> #define HCIUARTSETFLAGS _IOW('U', 203, int)
>> #define HCIUARTGETFLAGS _IOR('U', 204, int)
>> +/* Used prior to HCIUARTSETPROTO */
>> +#define HCIUARTSETPROTODATA _IOW('U', 205, unsigned long)
>
> Don't think this will gonna fly, Im not going to introduce vendor
> specific like this, besides if the kernel is not able to discover this
> data why would userspace be?
>
i don't think so as explained below.
1)
For the final product, BT device will get many configuration info from board configuration such DT|ACPI during
driver probe phase, But for tool btattach case, it has no way to get such configuration info due to derived from hci_ldisc.
so i introduce a new ioctl to let user specify some such required info when possible to make btattach work.

2) present ioctl HCIUARTSETPROTO has been introduced specify vendor protocol, why can't introduce a new ioctl to specify
protocol specific settings ? is HCIUARTSETPROTO vendor specific?

3) ioctl()'s designed purpose is for variant non-standard settings, do you have suggestions about how to specify device driver specify settings from user
if ioct() is not suitable?

4)
hci_ldisc driver don't parse and touch such user specified settings and pass it into vendor driver directly
does it has any problem?

>> /* UART protocols */
>> #define HCI_UART_MAX_PROTO 12
>> @@ -71,6 +73,7 @@ struct hci_uart {
>> struct work_struct init_ready;
>> struct work_struct write_work;
>>
>> + unsigned long proto_data;
>> const struct hci_uart_proto *proto;
>> struct percpu_rw_semaphore proto_lock; /* Stop work for proto close */
>> void *priv;
>> --
>> 2.7.4
>>
>
>

2024-04-18 01:26:41

by quic_zijuhu

Subject: Re: [PATCH v2 4/4] Bluetooth: qca: Fix wrong soc type returned for tool btattach

On 4/18/2024 5:39 AM, Luiz Augusto von Dentz wrote:
> Hi Zijun,
>
> On Wed, Apr 17, 2024 at 8:53 AM Zijun Hu <[email protected]> wrote:
>>
>> qca_soc_type() returns wrong soc type QCA_ROME when use tool btattach
>> for any other soc type such as QCA_QCA2066, and wrong soc type will
>> cause later initialization failure, fixed by using user specified
>> soc type for hci_uart derived from tool btattach.
>
> Then we have to fix qca_soc_type or explain what is going on that it
> can't detect the soc_type if initialized via btattach?
>
perhaps, my commit message is not precise and cause some mistook.

for tool btattach, only default QCA_ROME is used, there are no way to
get right soc type for other BT controllers. so i introduce a ioctl to let user specify
soc type info used. so i fix qca_soc_type() to use user specified soc type for tool btattach
case.

1) different soc types have different responses for VSC which is used to detect soc type
as shown by. so soc_type is can't be detected and it is needed by config by DT|ACPI or user specified.
int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
enum qca_btsoc_type soc_type)

2) soc type is a critical info, and it is used everywhere by hci_qca driver, it is also used to
decide which BT firmware to download as shown qca_uart_setup(), it soc type is not right. it will download
error BT firmware and cause serious results.

i will correct commit message for this patch.

>>
>> Signed-off-by: Zijun Hu <[email protected]>
>> ---
>> drivers/bluetooth/btqca.h | 1 +
>> drivers/bluetooth/hci_qca.c | 8 +++++++-
>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
>> index dc31984f71dc..a148d4c4e1bd 100644
>> --- a/drivers/bluetooth/btqca.h
>> +++ b/drivers/bluetooth/btqca.h
>> @@ -153,6 +153,7 @@ enum qca_btsoc_type {
>> QCA_WCN6750,
>> QCA_WCN6855,
>> QCA_WCN7850,
>> + QCA_MAX,
>> };
>>
>> #if IS_ENABLED(CONFIG_BT_QCA)
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index c04b97332bca..7c3577a4887c 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -238,12 +238,17 @@ static void qca_dmp_hdr(struct hci_dev *hdev, struct sk_buff *skb);
>>
>> static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
>> {
>> + /* For Non-serdev device, hu->proto_data records soc type
>> + * set by ioctl HCIUARTSETPROTODATA.
>> + */
>> + int proto_data = (int)hu->proto_data;
>> enum qca_btsoc_type soc_type;
>>
>> if (hu->serdev) {
>> struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
>> -
>> soc_type = qsd->btsoc_type;
>> + } else if ((proto_data > 0) && (proto_data < QCA_MAX)) {
>> + soc_type = (enum qca_btsoc_type)proto_data;
>
> Like I said a vendor specific ioctl will not gonna fly with me,
> specially since each vendor may need a different size to describe
> their controller version, etc,
>
i have comments about this part of this question in reply for [PATCH v2 3/4]

hci_uart->proto_data is a protocol specified unsigned long data, it is parsed
by specific protocol, for protocol, it is parsed as soc type. so force cast to
(enum qca_btsoc_type).

hci_uart->proto_data is mostly similar as @data of struct struct of_device_id defined by
below header file. it is assigned with misc data type and explained by specific device driver.
include/linux/mod_devicetable.h:
struct of_device_id {
char name[32];
char type[32];
char compatible[128];
const void *data;
};


>> } else {
>> soc_type = QCA_ROME;
>> }
>> @@ -2281,6 +2286,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>> return -ENOMEM;
>>
>> qcadev->serdev_hu.serdev = serdev;
>> + qcadev->serdev_hu.proto_data = 0;
>> data = device_get_match_data(&serdev->dev);
>> serdev_device_set_drvdata(serdev, qcadev);
>> device_property_read_string(&serdev->dev, "firmware-name",
>> --
>> 2.7.4
>>
>
>

2024-04-18 03:12:17

by quic_zijuhu

Subject: [PATCH v3 0/4] Fix 2 tool btattach issues for QCA controllers

BT chip vendor and customer often needs to use tool btattach to perform
various development,verification and evaluation test for vendor's BT
module, but tool btattach is not working fine for any QCA BT controller
currently, this patch series are to solve this issue.

There are many QCA soc types defined by drivers/bluetooth/btqca.h
enum qca_btsoc_type {
QCA_INVALID = -1,
QCA_AR3002,
QCA_ROME,
QCA_WCN3988,
QCA_WCN3990,
QCA_WCN3998,
QCA_WCN3991,
QCA_QCA2066,
QCA_QCA6390,
QCA_WCN6750,
QCA_WCN6855,
QCA_WCN7850,
QCA_MAX,
};
and every soc type stands for a kind of QCA BT controller, this patch
series are to solve below 2 tool btattach issues for QCA soc types:
1) tool btattach will cause kernel crash when used for QCA_ROME
2) tool btattach does not support any other soc type except QCA_ROME

For hci_uart derived from tool btattach, it is allocated by hci_ldisc
and is Non-serdev device, its @serdev is NULL and its soc type is
currenlty hard coded as QCA_ROME.

The 1st issue is caused by dereferencing nullptr hu->serdev, in order to
solve the 2nd issue, a new ioctl is introduced for user to specify soc
type by a new added tool btattach option.

Changes:
V2 -> V3: Correct commit message
V1 -> V2: Correct commit message

Zijun Hu (4):
Bluetooth: qca: Fix crash caused by tool btattach for QCA_ROME
Bluetooth: qca: Fix nullptr dereference for non-serdev devices
Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA
Bluetooth: qca: Support more soc types for non-serdev devices

drivers/bluetooth/btqca.h | 1 +
drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
drivers/bluetooth/hci_qca.c | 19 +++++++++++++------
drivers/bluetooth/hci_uart.h | 3 +++
4 files changed, 27 insertions(+), 6 deletions(-)

--
2.7.4

2024-04-18 03:12:17

by quic_zijuhu

Subject: [PATCH v3 1/4] Bluetooth: qca: Fix crash caused by tool btattach for QCA_ROME

A kernel crash will happen when use tool btattach for a BT controller
with soc type QCA_ROME, and it is caused by dereferencing nullptr
hu->serdev, fixed by null check before access.

sudo btattach -B /dev/ttyUSB0 -P qca
Bluetooth: hci1: QCA setup on UART is completed
BUG: kernel NULL pointer dereference, address: 00000000000002f0
......
Workqueue: hci1 hci_power_on [bluetooth]
RIP: 0010:qca_setup+0x7c1/0xe30 [hci_uart]
......
Call Trace:
<TASK>
? show_regs+0x72/0x90
? __die+0x25/0x80
? page_fault_oops+0x154/0x4c0
? srso_alias_return_thunk+0x5/0xfbef5
? kmem_cache_alloc+0x16b/0x310
? do_user_addr_fault+0x330/0x6e0
? srso_alias_return_thunk+0x5/0xfbef5
? exc_page_fault+0x84/0x1b0
? asm_exc_page_fault+0x27/0x30
? qca_setup+0x7c1/0xe30 [hci_uart]
hci_uart_setup+0x5c/0x1a0 [hci_uart]
hci_dev_open_sync+0xee/0xca0 [bluetooth]
hci_dev_do_open+0x2a/0x70 [bluetooth]
hci_power_on+0x46/0x210 [bluetooth]
process_one_work+0x17b/0x360
worker_thread+0x307/0x430
? __pfx_worker_thread+0x10/0x10
kthread+0xf7/0x130
? __pfx_kthread+0x10/0x10
ret_from_fork+0x46/0x70
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1b/0x30
</TASK>

Fixes: 03b0093f7b31 ("Bluetooth: hci_qca: get wakeup status from serdev device handle")
Signed-off-by: Zijun Hu <[email protected]>
Tested-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 92fa20f5ac7d..fdaf83d817af 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1955,7 +1955,7 @@ static int qca_setup(struct hci_uart *hu)
qca_debugfs_init(hdev);
hu->hdev->hw_error = qca_hw_error;
hu->hdev->cmd_timeout = qca_cmd_timeout;
- if (device_can_wakeup(hu->serdev->ctrl->dev.parent))
+ if (hu->serdev && device_can_wakeup(hu->serdev->ctrl->dev.parent))
hu->hdev->wakeup = qca_wakeup;
} else if (ret == -ENOENT) {
/* No patch/nvm-config found, run with original fw/config */
--
2.7.4

2024-04-18 03:12:24

by quic_zijuhu

Subject: [PATCH v3 3/4] Bluetooth: hci_ldisc: Add a ioctl HCIUARTSETPROTODATA

HCIUARTSETPROTODATA is introduced to specify protocol specific settings
prior to HCIUARTSETPROTO, for protocal QCA, it is used by tool btattch
to specify soc type.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_ldisc.c | 10 ++++++++++
drivers/bluetooth/hci_uart.h | 3 +++
2 files changed, 13 insertions(+)

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index a26367e9fb19..4be09c61bae5 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -506,6 +506,7 @@ static int hci_uart_tty_open(struct tty_struct *tty)
/* disable alignment support by default */
hu->alignment = 1;
hu->padding = 0;
+ hu->proto_data = 0;

INIT_WORK(&hu->init_ready, hci_uart_init_work);
INIT_WORK(&hu->write_work, hci_uart_write_work);
@@ -795,6 +796,15 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
err = hu->hdev_flags;
break;

+ case HCIUARTSETPROTODATA:
+ if (test_bit(HCI_UART_PROTO_SET, &hu->flags)) {
+ err = -EBUSY;
+ } else {
+ hu->proto_data = arg;
+ BT_DBG("HCIUARTSETPROTODATA %lu okay.", arg);
+ }
+ break;
+
default:
err = n_tty_ioctl_helper(tty, cmd, arg);
break;
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 68c8c7e95d64..fc35e9bd4398 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -18,6 +18,8 @@
#define HCIUARTGETDEVICE _IOR('U', 202, int)
#define HCIUARTSETFLAGS _IOW('U', 203, int)
#define HCIUARTGETFLAGS _IOR('U', 204, int)
+/* Used prior to HCIUARTSETPROTO */
+#define HCIUARTSETPROTODATA _IOW('U', 205, unsigned long)

/* UART protocols */
#define HCI_UART_MAX_PROTO 12
@@ -71,6 +73,7 @@ struct hci_uart {
struct work_struct init_ready;
struct work_struct write_work;

+ unsigned long proto_data;
const struct hci_uart_proto *proto;
struct percpu_rw_semaphore proto_lock; /* Stop work for proto close */
void *priv;
--
2.7.4

2024-04-18 03:12:29

by quic_zijuhu

Subject: [PATCH v3 4/4] Bluetooth: qca: Support more soc types for non-serdev devices

For non-serdev devices which are derived from tool btattach, only
default soc type QCA_ROME is supported currently since there are no
way to get soc type from DT or ACPI, this change supports more soc
types by using user specified soc type for non-serdev devices.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/btqca.h | 1 +
drivers/bluetooth/hci_qca.c | 8 +++++++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index dc31984f71dc..a148d4c4e1bd 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -153,6 +153,7 @@ enum qca_btsoc_type {
QCA_WCN6750,
QCA_WCN6855,
QCA_WCN7850,
+ QCA_MAX,
};

#if IS_ENABLED(CONFIG_BT_QCA)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index c04b97332bca..7c3577a4887c 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -238,12 +238,17 @@ static void qca_dmp_hdr(struct hci_dev *hdev, struct sk_buff *skb);

static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
{
+ /* For Non-serdev device, hu->proto_data records soc type
+ * set by ioctl HCIUARTSETPROTODATA.
+ */
+ int proto_data = (int)hu->proto_data;
enum qca_btsoc_type soc_type;

if (hu->serdev) {
struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
-
soc_type = qsd->btsoc_type;
+ } else if ((proto_data > 0) && (proto_data < QCA_MAX)) {
+ soc_type = (enum qca_btsoc_type)proto_data;
} else {
soc_type = QCA_ROME;
}
@@ -2281,6 +2286,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
return -ENOMEM;

qcadev->serdev_hu.serdev = serdev;
+ qcadev->serdev_hu.proto_data = 0;
data = device_get_match_data(&serdev->dev);
serdev_device_set_drvdata(serdev, qcadev);
device_property_read_string(&serdev->dev, "firmware-name",
--
2.7.4

2024-04-18 03:13:27

by quic_zijuhu

Subject: [PATCH v3 2/4] Bluetooth: qca: Fix nullptr dereference for non-serdev devices

hu->serdev is nullptr and will cause nullptr dereference if qca_setup()
is called by non-serdev device, fixed by nullptr checking before access.

Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index fdaf83d817af..c04b97332bca 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1905,10 +1905,11 @@ static int qca_setup(struct hci_uart *hu)
case QCA_WCN6750:
case QCA_WCN6855:
case QCA_WCN7850:
- qcadev = serdev_device_get_drvdata(hu->serdev);
- if (qcadev->bdaddr_property_broken)
- set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
-
+ if (hu->serdev) {
+ qcadev = serdev_device_get_drvdata(hu->serdev);
+ if (qcadev->bdaddr_property_broken)
+ set_bit(HCI_QUIRK_BDADDR_PROPERTY_BROKEN, &hdev->quirks);
+ }
hci_set_aosp_capable(hdev);

ret = qca_read_soc_version(hdev, &ver, soc_type);
--
2.7.4

2024-04-18 03:57:33

by bluez.test.bot

Subject: RE: Fix 2 tool btattach issues for QCA controllers

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=845651

---Test result---

Test Summary:
CheckPatch PASS 2.69 seconds
GitLint PASS 0.72 seconds
SubjectPrefix PASS 0.21 seconds
BuildKernel PASS 29.90 seconds
CheckAllWarning PASS 32.87 seconds
CheckSparse PASS 42.68 seconds
CheckSmatch FAIL 36.57 seconds
BuildKernel32 PASS 28.94 seconds
TestRunnerSetup PASS 521.39 seconds
TestRunner_l2cap-tester PASS 18.30 seconds
TestRunner_iso-tester FAIL 35.36 seconds
TestRunner_bnep-tester PASS 4.63 seconds
TestRunner_mgmt-tester PASS 111.32 seconds
TestRunner_rfcomm-tester PASS 7.22 seconds
TestRunner_sco-tester PASS 15.05 seconds
TestRunner_ioctl-tester PASS 7.58 seconds
TestRunner_mesh-tester PASS 5.69 seconds
TestRunner_smp-tester PASS 6.65 seconds
TestRunner_userchan-tester PASS 4.85 seconds
IncrementalBuild PASS 45.41 seconds

Details
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bpa10x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bpa10x.o'
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 122, Passed: 121 (99.2%), Failed: 1, Not Run: 0

Failed Test Cases
ISO Connect Suspend - Success Failed 4.179 seconds


---
Regards,
Linux Bluetooth

2024-04-18 16:09:22

by Johan Hovold

Subject: Re: [PATCH v3 2/4] Bluetooth: qca: Fix nullptr dereference for non-serdev devices

On Thu, Apr 18, 2024 at 11:11:51AM +0800, Zijun Hu wrote:
> hu->serdev is nullptr and will cause nullptr dereference if qca_setup()
> is called by non-serdev device, fixed by nullptr checking before access.

As I explained elsewhere, this is not a fix. It is only something you
need *after* you added the later patches in this series. This needs to
be reflected in the commit summary and commit message as I already told
you:

https://lore.kernel.org/all/[email protected]/

Johan

2024-04-18 22:17:52

by quic_zijuhu

Subject: Re: [PATCH v3 2/4] Bluetooth: qca: Fix nullptr dereference for non-serdev devices

On 4/19/2024 12:08 AM, Johan Hovold wrote:
> On Thu, Apr 18, 2024 at 11:11:51AM +0800, Zijun Hu wrote:
>> hu->serdev is nullptr and will cause nullptr dereference if qca_setup()
>> is called by non-serdev device, fixed by nullptr checking before access.
>
> As I explained elsewhere, this is not a fix. It is only something you
> need *after* you added the later patches in this series. This needs to
> be reflected in the commit summary and commit message as I already told
> you:
>
> https://lore.kernel.org/all/[email protected]/
>
i have removed below fix commit sentence from commit message.
Fixes: 77f45cca8bc5 ("Bluetooth: qca: fix device-address endianness")

let me also remove work Fix|fix.
> Johan