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

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

[Zijun Hu]

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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

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

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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

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

[Zijun Hu]

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

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

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 948 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=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

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

[Zijun Hu]

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

[PATCH v1 1/3] Bluetooth: qca: Fix crash caused by tool btattach for QCA_ROME

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
Tested-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 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 ecbc52eaf101..158567774cec 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1957,7 +1957,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

RE: Fix 2 tool btattach issues for QCA controllers

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2577 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=844120

---Test result---

Test Summary:
CheckPatch                    PASS      1.73 seconds
GitLint                       PASS      0.56 seconds
SubjectPrefix                 PASS      0.17 seconds
BuildKernel                   PASS      29.94 seconds
CheckAllWarning               PASS      32.59 seconds
CheckSparse                   PASS      37.83 seconds
CheckSmatch                   FAIL      34.84 seconds
BuildKernel32                 PASS      28.64 seconds
TestRunnerSetup               PASS      519.51 seconds
TestRunner_l2cap-tester       PASS      18.38 seconds
TestRunner_iso-tester         PASS      32.85 seconds
TestRunner_bnep-tester        PASS      4.64 seconds
TestRunner_mgmt-tester        PASS      108.95 seconds
TestRunner_rfcomm-tester      PASS      7.20 seconds
TestRunner_sco-tester         PASS      14.86 seconds
TestRunner_ioctl-tester       PASS      9.88 seconds
TestRunner_mesh-tester        FAIL      5.85 seconds
TestRunner_smp-tester         PASS      6.65 seconds
TestRunner_userchan-tester    PASS      4.80 seconds
IncrementalBuild              PASS      40.47 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....
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_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Failed       0.100 seconds


---
Regards,
Linux Bluetooth

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

[PATCH BlueZ v1] tools/btattach: Add support for more QCA soc types

[Zijun Hu]

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

RE: [BlueZ,v1] tools/btattach: Add support for more QCA soc types

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 949 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=844121

---Test result---

Test Summary:
CheckPatch                    PASS      0.47 seconds
GitLint                       PASS      0.31 seconds
BuildEll                      PASS      24.16 seconds
BluezMake                     PASS      1661.75 seconds
MakeCheck                     PASS      13.36 seconds
MakeDistcheck                 PASS      177.85 seconds
CheckValgrind                 PASS      248.58 seconds
CheckSmatch                   PASS      355.23 seconds
bluezmakeextell               PASS      120.37 seconds
IncrementalBuild              PASS      1459.70 seconds
ScanBuild                     PASS      1035.21 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ v1] tools/btattach: Add support for more QCA soc types

[Wren Turkal]

On 4/12/24 9:26 AM, Zijun Hu wrote:
> 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' },

I am guessing this means that there is no way to determine the soc from 
the kernel without the assist of the IOCTL? I also see this is a 
required parm. Is this not something that can use something like a 
devicetree for discovery so that the type of soc can be a property of 
the system instead of being manually specified?

>   	{ "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

-- 
You're more amazing than you think!

Re: [PATCH BlueZ v1] tools/btattach: Add support for more QCA soc types

[quic_zijuhu]

On 4/13/2024 4:10 AM, Wren Turkal wrote:
> On 4/12/24 9:26 AM, Zijun Hu wrote:
>> 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' },
> 
> I am guessing this means that there is no way to determine the soc from the kernel without the assist of the IOCTL? I also see this is a required parm. Is this not something that can use something like a devicetree for discovery so that the type of soc can be a property of the system instead of being manually specified?
> 
yes for tool btattch scenario. tool btattch is mainly used before the final board configuration phase(change DT|APCI to enabel BT), so it can't get such soc type info from board configuration.
for tool btattach, it doesn't need to touch any system configuration and is mainly used for variant evaluation tests before the final product implementation phase

let me take below process to explain its usage scenario.
Customer often buys a BT module from a vendor for BT evaluation, the BT module have BT chip embedded and are externally powered, the module also has a BT UART converted mini usb port,
they connects the BT module to generic ubntu PC by a USB cable, then they run tool btattach at the machine to enable BT and perform variants BT tests, when the evaluation results is expected,
they maybe buy the embedded chip and integrated into there target product's PCB, then change and compile DT to enable BT formally.

thanks 
>>       { "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
> 

Re: [PATCH BlueZ v1] tools/btattach: Add support for more QCA soc types

[Wren Turkal]

On 4/12/24 7:44 PM, quic_zijuhu wrote:
> On 4/13/2024 4:10 AM, Wren Turkal wrote:
>> On 4/12/24 9:26 AM, Zijun Hu wrote:
>>> 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' },
>>
>> I am guessing this means that there is no way to determine the soc from the kernel without the assist of the IOCTL? I also see this is a required parm. Is this not something that can use something like a devicetree for discovery so that the type of soc can be a property of the system instead of being manually specified?
>>
> yes for tool btattch scenario. tool btattch is mainly used before the final board configuration phase(change DT|APCI to enabel BT), so it can't get such soc type info from board configuration.
> for tool btattach, it doesn't need to touch any system configuration and is mainly used for variant evaluation tests before the final product implementation phase
> 
> let me take below process to explain its usage scenario.
> Customer often buys a BT module from a vendor for BT evaluation, the BT module have BT chip embedded and are externally powered, the module also has a BT UART converted mini usb port,
> they connects the BT module to generic ubntu PC by a USB cable, then they run tool btattach at the machine to enable BT and perform variants BT tests, when the evaluation results is expected,
> they maybe buy the embedded chip and integrated into there target product's PCB, then change and compile DT to enable BT formally.
Thanks for the explanation for a total newb. This is really cool to 
learn about. Really appreciate your taking the time to help me out.

> thanks
>>>        { "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
>>
> 

-- 
You're more amazing than you think!

[PATCH BlueZ v2] tools/btattach: Add support for more QCA soc types

[Zijun Hu]

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.
---
V1->V2: rename variable names

 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..08c648f44248 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 proto_data)
 {
 	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) && (proto_data > 0)) {
+		if (ioctl(fd, HCIUARTSETPROTODATA, proto_data) < 0) {
+			fprintf(stderr,
+				"Failed to set proto_data(%lu) for protocol qca\n",
+				proto_data);
+			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..8f554a38181f 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

RE: [BlueZ,v2] tools/btattach: Add support for more QCA soc types

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 949 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=845653

---Test result---

Test Summary:
CheckPatch                    PASS      0.46 seconds
GitLint                       PASS      0.31 seconds
BuildEll                      PASS      24.35 seconds
BluezMake                     PASS      1651.99 seconds
MakeCheck                     PASS      13.44 seconds
MakeDistcheck                 PASS      176.71 seconds
CheckValgrind                 PASS      246.94 seconds
CheckSmatch                   PASS      348.94 seconds
bluezmakeextell               PASS      119.09 seconds
IncrementalBuild              PASS      1406.00 seconds
ScanBuild                     PASS      1020.09 seconds



---
Regards,
Linux Bluetooth

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

[Zijun Hu]

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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
Tested-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

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

[Luiz Augusto von Dentz]

Hi Zijun,

On Wed, Apr 17, 2024 at 8:53 AM Zijun Hu <quic_zijuhu@quicinc.com> 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 <quic_zijuhu@quicinc.com>
> ---
>  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

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

[quic_zijuhu]

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 <quic_zijuhu@quicinc.com> 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 <quic_zijuhu@quicinc.com>
>> ---
>>  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
>>
> 
> 

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

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

[Luiz Augusto von Dentz]

Hi Zijun,

On Wed, Apr 17, 2024 at 8:53 AM Zijun Hu <quic_zijuhu@quicinc.com> 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 <quic_zijuhu@quicinc.com>
> ---
>  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

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

[quic_zijuhu]

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 <quic_zijuhu@quicinc.com> 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 <quic_zijuhu@quicinc.com>
>> ---
>>  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
>>
> 
> 

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

[Zijun Hu]

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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
Tested-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 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

RE: Fix 2 tool btattach issues for QCA controllers

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2747 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=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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

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

[Johan Hovold]

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/Zh91zq13nZvH3-Yj@hovoldconsulting.com/

Johan

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

[quic_zijuhu]

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/Zh91zq13nZvH3-Yj@hovoldconsulting.com/
> 
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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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

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

[Zijun Hu]

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 <quic_zijuhu@quicinc.com>
---
 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