linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension
@ 2021-10-15  5:09 Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 2/7] monitor: Make use of MSFT packet definitions Luiz Augusto von Dentz
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-15  5:09 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds proper packet definitions for command and response of MSFT
extension.
---
 monitor/msft.h | 148 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 148 insertions(+)

diff --git a/monitor/msft.h b/monitor/msft.h
index a268f4bc7..90a64117a 100644
--- a/monitor/msft.h
+++ b/monitor/msft.h
@@ -24,6 +24,154 @@
 
 #include <stdint.h>
 
+#define MSFT_SUBCMD_READ_SUPPORTED_FEATURES	0x00
+
+struct msft_cmd_read_supported_features {
+	uint8_t subcmd;
+} __attribute__((packed));
+
+struct msft_rsp_read_supported_features {
+	uint8_t  status;
+	uint8_t  subcmd;
+	uint8_t  features[8];
+	uint8_t  evt_prefix_len;
+	uint8_t  evt_prefix[];
+} __attribute__((packed));
+
+#define MSFT_SUBCMD_MONITOR_RSSI		0x01
+
+struct msft_cmd_monitor_rssi {
+	uint8_t  subcmd;
+	uint16_t handle;
+	int8_t   rssi_high;
+	int8_t   rssi_low;
+	uint8_t  rssi_low_interval;
+	uint8_t  rssi_period;
+} __attribute__((packed));
+
+struct msft_rsp_monitor_rssi {
+	uint8_t  status;
+	uint8_t  subcmd;
+} __attribute__((packed));
+
+#define MSFT_SUBCMD_CANCEL_MONITOR_RSSI		0x02
+
+struct msft_cmd_cancel_monitor_rssi {
+	uint8_t  subcmd;
+	uint16_t handle;
+} __attribute__((packed));
+
+struct msft_rsp_cancel_monitor_rssi {
+	uint8_t  status;
+	uint8_t  subcmd;
+} __attribute__((packed));
+
+#define MSFT_SUBCMD_LE_MONITOR_ADV		0x03
+
+struct msft_le_monitor_pattern {
+	uint8_t  len;
+	uint8_t  type;
+	uint8_t  start;
+	uint8_t  data[];
+} __attribute__((packed));
+
+struct msft_le_monitor_adv_pattern_type {
+	uint8_t num_patterns;
+	struct msft_le_monitor_pattern data[];
+} __attribute__((packed));
+
+struct msft_le_monitor_adv_uuid_type {
+	uint8_t  type;
+	union {
+		uint16_t u16;
+		uint32_t u32;
+		uint8_t  u128[8];
+	} value;
+} __attribute__((packed));
+
+struct msft_le_monitor_adv_irk_type {
+	uint8_t  irk[8];
+} __attribute__((packed));
+
+#define MSFT_SUBCMD_LE_MONITOR_ADV_ADDR		0x04
+struct msft_le_monitor_adv_addr {
+	uint8_t  type;
+	uint8_t  addr[6];
+} __attribute__((packed));
+
+struct msft_cmd_le_monitor_adv {
+	uint8_t  subcmd;
+	int8_t   rssi_low;
+	int8_t   rssi_high;
+	uint8_t  rssi_low_interval;
+	uint8_t  rssi_period;
+	uint8_t  type;
+	uint8_t  data[];
+} __attribute__((packed));
+
+struct msft_rsp_le_monitor_adv {
+	uint8_t  status;
+	uint8_t  subcmd;
+	uint8_t  handle;
+} __attribute__((packed));
+
+#define MSFT_SUBCMD_LE_CANCEL_MONITOR_ADV	0x04
+
+struct msft_cmd_le_cancel_monitor_adv {
+	uint8_t  subcmd;
+	uint8_t  handle;
+} __attribute__((packed));
+
+struct msft_rsp_le_cancel_monitor_adv {
+	uint8_t  status;
+	uint8_t  subcmd;
+} __attribute__((packed));
+
+#define MSFT_SUBCMD_LE_MONITOR_ADV_ENABLE	0x05
+
+struct msft_cmd_le_monitor_adv_enable {
+	uint8_t  subcmd;
+	uint8_t  enable;
+} __attribute__((packed));
+
+struct msft_rsp_le_monitor_adv_enable {
+	uint8_t  status;
+	uint8_t  subcmd;
+} __attribute__((packed));
+
+#define MSFT_SUBCMD_READ_ABS_RSSI		0x06
+
+struct msft_cmd_read_abs_rssi {
+	uint8_t  subcmd;
+	uint16_t handle;
+} __attribute__((packed));
+
+struct msft_rsp_read_abs_rssi {
+	uint8_t  status;
+	uint8_t  subcmd;
+	uint16_t handle;
+	uint8_t  rssi;
+} __attribute__((packed));
+
+#define MSFT_SUBEVT_RSSI			0x01
+
+struct msft_evt_rssi {
+	uint8_t  subevt;
+	uint8_t  status;
+	uint16_t handle;
+	uint8_t  rssi;
+} __attribute__((packed));
+
+#define MSFT_SUBEVT_MONITOR_DEVICE		0x02
+
+struct msft_evt_monitor_device {
+	uint8_t  subevt;
+	uint8_t  addr_type;
+	uint8_t  addr[6];
+	uint8_t  handle;
+	uint8_t  state;
+} __attribute__((packed));
+
 struct vendor_ocf;
 struct vendor_evt;
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH BlueZ 2/7] monitor: Make use of MSFT packet definitions
  2021-10-15  5:09 [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension Luiz Augusto von Dentz
@ 2021-10-15  5:09 ` Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 3/7] vhci: Read the controller index Luiz Augusto von Dentz
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-15  5:09 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This make use of the packet definitions for MSFT extension.
---
 monitor/msft.c | 144 ++++++++++++++++++++++++++++++++++++++++---------
 monitor/msft.h |  11 ++--
 2 files changed, 125 insertions(+), 30 deletions(-)

diff --git a/monitor/msft.c b/monitor/msft.c
index 46e765489..ccb232f3b 100644
--- a/monitor/msft.c
+++ b/monitor/msft.c
@@ -30,6 +30,9 @@
 #include <stdio.h>
 #include <inttypes.h>
 
+#include "lib/bluetooth.h"
+#include "lib/uuid.h"
+
 #include "src/shared/util.h"
 #include "display.h"
 #include "packet.h"
@@ -49,46 +52,132 @@ static void null_rsp(const void *data, uint16_t size)
 
 static void read_supported_features_rsp(const void *data, uint16_t size)
 {
-	uint8_t evt_prefix_len = get_u8(data + 8);
+	const struct msft_rsp_read_supported_features *rsp = data;
+
+	packet_print_features_msft(rsp->features);
+	print_field("Event prefix length: %u", rsp->evt_prefix_len);
+	print_field("Event prefix:");
+	packet_hexdump(rsp->evt_prefix, rsp->evt_prefix_len);
+	packet_set_msft_evt_prefix(rsp->evt_prefix, rsp->evt_prefix_len);
+}
+
+static void monitor_rssi_cmd(const void *data, uint16_t size)
+{
+	const struct msft_cmd_monitor_rssi *cmd = data;
+
+	print_field("Connection handle: 0x%04x", cmd->handle);
+	packet_print_rssi("RSSI threshold high", cmd->rssi_high);
+	packet_print_rssi("RSSI threshold low", cmd->rssi_low);
+	print_field("RSSI threshold low time interval: %u sec (0x%2.2x)",
+						cmd->rssi_low_interval,
+						cmd->rssi_low_interval);
+	print_field("RSSI sampling period: %u msec (0x%2.2x)",
+						cmd->rssi_period * 100,
+						cmd->rssi_period);
+}
 
-	packet_print_features_msft(data);
-	print_field("Event prefix length: %u", evt_prefix_len);
-	packet_hexdump(data + 9, size - 9);
+static void cancel_monitor_rssi_cmd(const void *data, uint16_t size)
+{
+	const struct msft_cmd_cancel_monitor_rssi *cmd = data;
 
-	packet_set_msft_evt_prefix(data + 9, evt_prefix_len);
+	print_field("Connection handle: 0x%04x", cmd->handle);
 }
 
 static void le_monitor_advertisement_cmd(const void *data, uint16_t size)
 {
-	int8_t threshold_high = get_s8(data);
-	int8_t threshold_low = get_s8(data + 1);
-	uint8_t threshold_low_time_interval = get_u8(data + 2);
-	uint8_t sampling_period = get_u8(data + 3);
+	const struct msft_cmd_le_monitor_adv *cmd = data;
+	const struct msft_le_monitor_adv_patterns *patterns;
+	const struct msft_le_monitor_adv_uuid *uuid;
+	const struct msft_le_monitor_adv_irk *irk;
+	const struct msft_le_monitor_adv_addr *addr;
+	const char *str;
+	char uuidstr[MAX_LEN_UUID_STR];
 
-	packet_print_rssi("RSSI threshold high", threshold_high);
-	packet_print_rssi("RSSI threshold low", threshold_low);
+	packet_print_rssi("RSSI threshold high", cmd->rssi_high);
+	packet_print_rssi("RSSI threshold low", cmd->rssi_low);
 	print_field("RSSI threshold low time interval: %u sec (0x%2.2x)",
-						threshold_low_time_interval,
-						threshold_low_time_interval);
+						cmd->rssi_low_interval,
+						cmd->rssi_low_interval);
 	print_field("RSSI sampling period: %u msec (0x%2.2x)",
-						sampling_period * 100,
-						sampling_period);
-	packet_hexdump(data + 4, size - 4);
+						cmd->rssi_period * 100,
+						cmd->rssi_period);
+
+	switch (cmd->type) {
+	case MSFT_SUBCMD_LE_MONITOR_ADV_PATTERN:
+		print_field("Type: Pattern (0x%2.2x)", cmd->type);
+		patterns = (void *)cmd->data;
+		print_field("Number of patterns: %u", patterns->num);
+		packet_hexdump((void *)patterns->data,
+			       size - (sizeof(*cmd) + sizeof(*patterns)));
+		break;
+	case MSFT_SUBCMD_LE_MONITOR_ADV_UUID:
+		print_field("Type: UUID (0x%2.2x)", cmd->type);
+		uuid = (void *)cmd->data;
+
+		switch (uuid->type) {
+		case 0x01:
+			str = bt_uuid16_to_str(uuid->value.u16);
+			print_field("UUID: %s (0x%4.4x)", str, uuid->value.u16);
+			break;
+		case 0x02:
+			str = bt_uuid32_to_str(uuid->value.u32);
+			print_field("UUID: %s (0x%8.8x)", str, uuid->value.u32);
+			break;
+		case 0x03:
+			sprintf(uuidstr, "%8.8x-%4.4x-%4.4x-%4.4x-%8.8x%4.4x",
+				get_le32(uuid->value.u128 + 12),
+				get_le16(uuid->value.u128 + 10),
+				get_le16(uuid->value.u128 + 8),
+				get_le16(uuid->value.u128 + 6),
+				get_le32(uuid->value.u128 + 2),
+				get_le16(uuid->value.u128 + 0));
+			str = bt_uuidstr_to_str(uuidstr);
+			print_field("UUID: %s (%s)", str, uuidstr);
+			break;
+		default:
+			packet_hexdump((void *)&uuid->value,
+					size - sizeof(*cmd));
+			break;
+		}
+		break;
+	case MSFT_SUBCMD_LE_MONITOR_ADV_IRK:
+		print_field("Type: IRK (0x%2.2x)", cmd->type);
+		irk = (void *)cmd->data;
+		print_field("IRK:");
+		packet_hexdump(irk->irk, size - sizeof(*cmd));
+		break;
+	case MSFT_SUBCMD_LE_MONITOR_ADV_ADDR:
+		print_field("Type: Adderss (0x%2.2x)", cmd->type);
+		addr = (void *)cmd->data;
+		packet_print_addr(NULL, addr->addr, addr->type);
+		break;
+	default:
+		print_field("Type: Unknown (0x%2.2x)", cmd->type);
+		packet_hexdump(cmd->data, size - sizeof(*cmd));
+		break;
+	}
 }
 
 static void le_monitor_advertisement_rsp(const void *data, uint16_t size)
 {
-	uint8_t handle = get_u8(data);
+	const struct msft_rsp_le_monitor_adv *rsp = data;
+
+	print_field("Monitor handle: %u", rsp->handle);
+}
+
+static void le_cancel_monitor_adv_cmd(const void *data, uint16_t size)
+{
+	const struct msft_cmd_le_cancel_monitor_adv *cmd = data;
 
-	print_field("Monitor handle: %u", handle);
+	print_field("Monitor handle: %u", cmd->handle);
 }
 
 static void set_adv_filter_enable_cmd(const void *data, uint16_t size)
 {
-	uint8_t enable = get_u8(data);
+	const struct msft_cmd_le_monitor_adv_enable *cmd = data;
 	const char *str;
 
-	switch (enable) {
+	switch (cmd->enable) {
 	case 0x00:
 		str = "Current allow list";
 		break;
@@ -100,7 +189,7 @@ static void set_adv_filter_enable_cmd(const void *data, uint16_t size)
 		break;
 	}
 
-	print_field("Enable: %s (0x%2.2x)", str, enable);
+	print_field("Enable: %s (0x%2.2x)", str, cmd->enable);
 }
 
 typedef void (*func_t) (const void *data, uint16_t size);
@@ -114,12 +203,15 @@ static const struct {
 	{ 0x00, "Read Supported Features",
 			null_cmd,
 			read_supported_features_rsp },
-	{ 0x01, "Monitor RSSI" },
-	{ 0x02, "Cancel Monitor RSSI" },
+	{ 0x01, "Monitor RSSI",
+			monitor_rssi_cmd },
+	{ 0x02, "Cancel Monitor RSSI",
+			cancel_monitor_rssi_cmd },
 	{ 0x03, "LE Monitor Advertisement",
 			le_monitor_advertisement_cmd,
 			le_monitor_advertisement_rsp },
-	{ 0x04, "LE Cancel Monitor Advertisement" },
+	{ 0x04, "LE Cancel Monitor Advertisement",
+			le_cancel_monitor_adv_cmd },
 	{ 0x05, "LE Set Advertisement Filter Enable",
 			set_adv_filter_enable_cmd,
 			null_rsp },
@@ -156,7 +248,7 @@ static void msft_cmd(const void *data, uint8_t size)
 						" (0x%2.2x)", code);
 
 	if (code_func)
-		code_func(data + 1, size - 1);
+		code_func(data, size);
 	else
 		packet_hexdump(data + 1, size - 1);
 }
@@ -193,7 +285,7 @@ static void msft_rsp(const void *data, uint8_t size)
 	packet_print_error("Status", status);
 
 	if (code_func)
-		code_func(data + 2, size - 2);
+		code_func(data, size);
 	else
 		packet_hexdump(data + 2, size - 2);
 }
diff --git a/monitor/msft.h b/monitor/msft.h
index 90a64117a..4f4bdd992 100644
--- a/monitor/msft.h
+++ b/monitor/msft.h
@@ -68,6 +68,7 @@ struct msft_rsp_cancel_monitor_rssi {
 
 #define MSFT_SUBCMD_LE_MONITOR_ADV		0x03
 
+#define MSFT_SUBCMD_LE_MONITOR_ADV_PATTERN	0x01
 struct msft_le_monitor_pattern {
 	uint8_t  len;
 	uint8_t  type;
@@ -75,12 +76,13 @@ struct msft_le_monitor_pattern {
 	uint8_t  data[];
 } __attribute__((packed));
 
-struct msft_le_monitor_adv_pattern_type {
-	uint8_t num_patterns;
+struct msft_le_monitor_adv_patterns {
+	uint8_t num;
 	struct msft_le_monitor_pattern data[];
 } __attribute__((packed));
 
-struct msft_le_monitor_adv_uuid_type {
+#define MSFT_SUBCMD_LE_MONITOR_ADV_UUID		0x02
+struct msft_le_monitor_adv_uuid {
 	uint8_t  type;
 	union {
 		uint16_t u16;
@@ -89,7 +91,8 @@ struct msft_le_monitor_adv_uuid_type {
 	} value;
 } __attribute__((packed));
 
-struct msft_le_monitor_adv_irk_type {
+#define MSFT_SUBCMD_LE_MONITOR_ADV_IRK		0x03
+struct msft_le_monitor_adv_irk {
 	uint8_t  irk[8];
 } __attribute__((packed));
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH BlueZ 3/7] vhci: Read the controller index
  2021-10-15  5:09 [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 2/7] monitor: Make use of MSFT packet definitions Luiz Augusto von Dentz
@ 2021-10-15  5:09 ` Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 4/7] vhci: Use io.h instead of mainloop.h Luiz Augusto von Dentz
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-15  5:09 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes vhci instance read its controller index assigned by the
kernel and also introduces vhci_get_btdev so it can be used by the
likes of hciemu.
---
 emulator/main.c | 12 ++++----
 emulator/vhci.c | 75 +++++++++++++++++++++++++++++--------------------
 emulator/vhci.h | 11 ++------
 3 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/emulator/main.c b/emulator/main.c
index aa269c3f0..f64d46a5e 100644
--- a/emulator/main.c
+++ b/emulator/main.c
@@ -17,12 +17,14 @@
 #include <stdlib.h>
 #include <stdbool.h>
 #include <getopt.h>
+#include <sys/uio.h>
 
 #include "src/shared/mainloop.h"
 #include "src/shared/util.h"
 
 #include "serial.h"
 #include "server.h"
+#include "btdev.h"
 #include "vhci.h"
 #include "amp.h"
 #include "le.h"
@@ -90,7 +92,7 @@ int main(int argc, char *argv[])
 	int letest_count = 0;
 	int amptest_count = 0;
 	int vhci_count = 0;
-	enum vhci_type vhci_type = VHCI_TYPE_BREDRLE;
+	enum btdev_type type = BTDEV_TYPE_BREDRLE52;
 	int i;
 
 	mainloop_init();
@@ -120,13 +122,13 @@ int main(int argc, char *argv[])
 				vhci_count = 1;
 			break;
 		case 'L':
-			vhci_type = VHCI_TYPE_LE;
+			type = BTDEV_TYPE_LE;
 			break;
 		case 'B':
-			vhci_type = VHCI_TYPE_BREDR;
+			type = BTDEV_TYPE_BREDR;
 			break;
 		case 'A':
-			vhci_type = VHCI_TYPE_AMP;
+			type = BTDEV_TYPE_AMP;
 			break;
 		case 'U':
 			if (optarg)
@@ -182,7 +184,7 @@ int main(int argc, char *argv[])
 	for (i = 0; i < vhci_count; i++) {
 		struct vhci *vhci;
 
-		vhci = vhci_open(vhci_type);
+		vhci = vhci_open(type);
 		if (!vhci) {
 			fprintf(stderr, "Failed to open Virtual HCI device\n");
 			return EXIT_FAILURE;
diff --git a/emulator/vhci.c b/emulator/vhci.c
index 33f674d79..28cdef633 100644
--- a/emulator/vhci.c
+++ b/emulator/vhci.c
@@ -29,10 +29,8 @@
 #include "btdev.h"
 #include "vhci.h"
 
-#define uninitialized_var(x) x = x
-
 struct vhci {
-	enum vhci_type type;
+	enum btdev_type type;
 	int fd;
 	struct btdev *btdev;
 };
@@ -91,32 +89,22 @@ bool vhci_set_debug(struct vhci *vhci, vhci_debug_func_t callback,
 	return btdev_set_debug(vhci->btdev, callback, user_data, destroy);
 }
 
-struct vhci *vhci_open(enum vhci_type type)
+struct vhci_create_req {
+	uint8_t  pkt_type;
+	uint8_t  opcode;
+} __attribute__((packed));
+
+struct vhci_create_rsp {
+	uint8_t  pkt_type;
+	uint8_t  opcode;
+	uint16_t index;
+} __attribute__((packed));
+
+struct vhci *vhci_open(uint8_t type)
 {
 	struct vhci *vhci;
-	enum btdev_type uninitialized_var(btdev_type);
-	unsigned char uninitialized_var(ctrl_type);
-	unsigned char setup_cmd[2];
-	static uint8_t id = 0x23;
-
-	switch (type) {
-	case VHCI_TYPE_BREDRLE:
-		btdev_type = BTDEV_TYPE_BREDRLE52;
-		ctrl_type = HCI_PRIMARY;
-		break;
-	case VHCI_TYPE_BREDR:
-		btdev_type = BTDEV_TYPE_BREDR;
-		ctrl_type = HCI_PRIMARY;
-		break;
-	case VHCI_TYPE_LE:
-		btdev_type = BTDEV_TYPE_LE;
-		ctrl_type = HCI_PRIMARY;
-		break;
-	case VHCI_TYPE_AMP:
-		btdev_type = BTDEV_TYPE_AMP;
-		ctrl_type = HCI_AMP;
-		break;
-	}
+	struct vhci_create_req req;
+	struct vhci_create_rsp rsp;
 
 	vhci = malloc(sizeof(*vhci));
 	if (!vhci)
@@ -131,16 +119,33 @@ struct vhci *vhci_open(enum vhci_type type)
 		return NULL;
 	}
 
-	setup_cmd[0] = HCI_VENDOR_PKT;
-	setup_cmd[1] = ctrl_type;
+	memset(&req, 0, sizeof(req));
+	req.pkt_type = HCI_VENDOR_PKT;
 
-	if (write(vhci->fd, setup_cmd, sizeof(setup_cmd)) < 0) {
+	switch (type) {
+	case BTDEV_TYPE_AMP:
+		req.opcode = HCI_AMP;
+		break;
+	default:
+		req.opcode = HCI_PRIMARY;
+		break;
+	}
+
+	if (write(vhci->fd, &req, sizeof(req)) < 0) {
+		close(vhci->fd);
+		free(vhci);
+		return NULL;
+	}
+
+	memset(&rsp, 0, sizeof(rsp));
+
+	if (read(vhci->fd, &rsp, sizeof(rsp)) < 0) {
 		close(vhci->fd);
 		free(vhci);
 		return NULL;
 	}
 
-	vhci->btdev = btdev_create(btdev_type, id++);
+	vhci->btdev = btdev_create(type, rsp.index);
 	if (!vhci->btdev) {
 		close(vhci->fd);
 		free(vhci);
@@ -167,3 +172,11 @@ void vhci_close(struct vhci *vhci)
 
 	mainloop_remove_fd(vhci->fd);
 }
+
+struct btdev *vhci_get_btdev(struct vhci *vhci)
+{
+	if (!vhci)
+		return NULL;
+
+	return vhci->btdev;
+}
diff --git a/emulator/vhci.h b/emulator/vhci.h
index 7dfea25df..0554121e8 100644
--- a/emulator/vhci.h
+++ b/emulator/vhci.h
@@ -11,13 +11,6 @@
 
 #include <stdint.h>
 
-enum vhci_type {
-	VHCI_TYPE_BREDRLE,
-	VHCI_TYPE_BREDR,
-	VHCI_TYPE_LE,
-	VHCI_TYPE_AMP,
-};
-
 struct vhci;
 
 typedef void (*vhci_debug_func_t)(const char *str, void *user_data);
@@ -25,5 +18,7 @@ typedef void (*vhci_destroy_func_t)(void *user_data);
 bool vhci_set_debug(struct vhci *vhci, vhci_debug_func_t callback,
 			void *user_data, vhci_destroy_func_t destroy);
 
-struct vhci *vhci_open(enum vhci_type type);
+struct vhci *vhci_open(uint8_t type);
 void vhci_close(struct vhci *vhci);
+
+struct btdev *vhci_get_btdev(struct vhci *vhci);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH BlueZ 4/7] vhci: Use io.h instead of mainloop.h
  2021-10-15  5:09 [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 2/7] monitor: Make use of MSFT packet definitions Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 3/7] vhci: Read the controller index Luiz Augusto von Dentz
@ 2021-10-15  5:09 ` Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 5/7] hciemu: Use vhci_open to instanciate a vhci btdev Luiz Augusto von Dentz
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-15  5:09 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

The likes of mainloop_add_fd is not implemented in mainloop-glib.c while
io_set_read_handler so this makes it possible to use vhci instance with
both libshared-glib and libshared-mainloop.
---
 emulator/vhci.c | 67 +++++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/emulator/vhci.c b/emulator/vhci.c
index 28cdef633..97fbcb8c4 100644
--- a/emulator/vhci.c
+++ b/emulator/vhci.c
@@ -24,14 +24,14 @@
 #include "lib/bluetooth.h"
 #include "lib/hci.h"
 
-#include "src/shared/mainloop.h"
+#include "src/shared/io.h"
 #include "monitor/bt.h"
 #include "btdev.h"
 #include "vhci.h"
 
 struct vhci {
 	enum btdev_type type;
-	int fd;
+	struct io *io;
 	struct btdev *btdev;
 };
 
@@ -40,8 +40,7 @@ static void vhci_destroy(void *user_data)
 	struct vhci *vhci = user_data;
 
 	btdev_destroy(vhci->btdev);
-
-	close(vhci->fd);
+	io_destroy(vhci->io);
 
 	free(vhci);
 }
@@ -52,23 +51,21 @@ static void vhci_write_callback(const struct iovec *iov, int iovlen,
 	struct vhci *vhci = user_data;
 	ssize_t written;
 
-	written = writev(vhci->fd, iov, iovlen);
+	written = io_send(vhci->io, iov, iovlen);
 	if (written < 0)
 		return;
 }
 
-static void vhci_read_callback(int fd, uint32_t events, void *user_data)
+static bool vhci_read_callback(struct io *io, void *user_data)
 {
 	struct vhci *vhci = user_data;
+	int fd = io_get_fd(vhci->io);
 	unsigned char buf[4096];
 	ssize_t len;
 
-	if (events & (EPOLLERR | EPOLLHUP))
-		return;
-
-	len = read(vhci->fd, buf, sizeof(buf));
+	len = read(fd, buf, sizeof(buf));
 	if (len < 1)
-		return;
+		return false;
 
 	switch (buf[0]) {
 	case BT_H4_CMD_PKT:
@@ -78,6 +75,8 @@ static void vhci_read_callback(int fd, uint32_t events, void *user_data)
 		btdev_receive_h4(vhci->btdev, buf, len);
 		break;
 	}
+
+	return true;
 }
 
 bool vhci_set_debug(struct vhci *vhci, vhci_debug_func_t callback,
@@ -105,19 +104,11 @@ struct vhci *vhci_open(uint8_t type)
 	struct vhci *vhci;
 	struct vhci_create_req req;
 	struct vhci_create_rsp rsp;
+	int fd;
 
-	vhci = malloc(sizeof(*vhci));
-	if (!vhci)
-		return NULL;
-
-	memset(vhci, 0, sizeof(*vhci));
-	vhci->type = type;
-
-	vhci->fd = open("/dev/vhci", O_RDWR | O_NONBLOCK);
-	if (vhci->fd < 0) {
-		free(vhci);
+	fd = open("/dev/vhci", O_RDWR | O_NONBLOCK);
+	if (fd < 0)
 		return NULL;
-	}
 
 	memset(&req, 0, sizeof(req));
 	req.pkt_type = HCI_VENDOR_PKT;
@@ -131,34 +122,38 @@ struct vhci *vhci_open(uint8_t type)
 		break;
 	}
 
-	if (write(vhci->fd, &req, sizeof(req)) < 0) {
-		close(vhci->fd);
-		free(vhci);
+	if (write(fd, &req, sizeof(req)) < 0) {
+		close(fd);
 		return NULL;
 	}
 
 	memset(&rsp, 0, sizeof(rsp));
 
-	if (read(vhci->fd, &rsp, sizeof(rsp)) < 0) {
-		close(vhci->fd);
-		free(vhci);
+	if (read(fd, &rsp, sizeof(rsp)) < 0) {
+		close(fd);
 		return NULL;
 	}
 
+	vhci = malloc(sizeof(*vhci));
+	if (!vhci)
+		return NULL;
+
+	memset(vhci, 0, sizeof(*vhci));
+	vhci->type = type;
+	vhci->io = io_new(fd);
+
+	io_set_close_on_destroy(vhci->io, true);
+
 	vhci->btdev = btdev_create(type, rsp.index);
 	if (!vhci->btdev) {
-		close(vhci->fd);
-		free(vhci);
+		vhci_destroy(vhci);
 		return NULL;
 	}
 
 	btdev_set_send_handler(vhci->btdev, vhci_write_callback, vhci);
 
-	if (mainloop_add_fd(vhci->fd, EPOLLIN, vhci_read_callback,
-						vhci, vhci_destroy) < 0) {
-		btdev_destroy(vhci->btdev);
-		close(vhci->fd);
-		free(vhci);
+	if (!io_set_read_handler(vhci->io, vhci_read_callback, vhci, NULL)) {
+		vhci_destroy(vhci);
 		return NULL;
 	}
 
@@ -170,7 +165,7 @@ void vhci_close(struct vhci *vhci)
 	if (!vhci)
 		return;
 
-	mainloop_remove_fd(vhci->fd);
+	vhci_destroy(vhci);
 }
 
 struct btdev *vhci_get_btdev(struct vhci *vhci)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH BlueZ 5/7] hciemu: Use vhci_open to instanciate a vhci btdev
  2021-10-15  5:09 [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension Luiz Augusto von Dentz
                   ` (2 preceding siblings ...)
  2021-10-15  5:09 ` [PATCH BlueZ 4/7] vhci: Use io.h instead of mainloop.h Luiz Augusto von Dentz
@ 2021-10-15  5:09 ` Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 6/7] vhci: Add functions to interface with debugfs Luiz Augusto von Dentz
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-15  5:09 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes use of vhci_open to isntanciate the vhci btdev since that
has proper support for reading the index assigned to it.
---
 Makefile.tools      |   8 +++
 android/Makefile.am |   2 +
 emulator/btdev.c    |   8 ++-
 emulator/hciemu.c   | 127 ++++++++++++++++++++++++++------------------
 4 files changed, 91 insertions(+), 54 deletions(-)

diff --git a/Makefile.tools b/Makefile.tools
index 8cfaf8972..c7bdff83f 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -115,6 +115,7 @@ tools_3dsp_LDADD = src/libshared-mainloop.la
 
 tools_mgmt_tester_SOURCES = tools/mgmt-tester.c monitor/bt.h \
 				emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c
@@ -123,6 +124,7 @@ tools_mgmt_tester_LDADD = lib/libbluetooth-internal.la \
 
 tools_l2cap_tester_SOURCES = tools/l2cap-tester.c monitor/bt.h \
 				emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c
@@ -131,6 +133,7 @@ tools_l2cap_tester_LDADD = lib/libbluetooth-internal.la \
 
 tools_rfcomm_tester_SOURCES = tools/rfcomm-tester.c monitor/bt.h \
 				emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c
@@ -139,6 +142,7 @@ tools_rfcomm_tester_LDADD = lib/libbluetooth-internal.la \
 
 tools_bnep_tester_SOURCES = tools/bnep-tester.c monitor/bt.h \
 				emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c
@@ -147,6 +151,7 @@ tools_bnep_tester_LDADD = lib/libbluetooth-internal.la \
 
 tools_smp_tester_SOURCES = tools/smp-tester.c monitor/bt.h \
 				emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c
@@ -155,6 +160,7 @@ tools_smp_tester_LDADD = lib/libbluetooth-internal.la \
 
 tools_gap_tester_SOURCES = tools/gap-tester.c monitor/bt.h \
 				emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c
@@ -165,6 +171,7 @@ tools_gap_tester_LDADD =  lib/libbluetooth-internal.la \
 
 tools_sco_tester_SOURCES = tools/sco-tester.c monitor/bt.h \
 				emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c
@@ -176,6 +183,7 @@ tools_hci_tester_LDADD = src/libshared-glib.la $(GLIB_LIBS)
 
 tools_userchan_tester_SOURCES = tools/userchan-tester.c monitor/bt.h \
 				emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c
diff --git a/android/Makefile.am b/android/Makefile.am
index a370598a2..309910147 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -148,6 +148,7 @@ android_haltest_LDADD = -ldl -lm
 noinst_PROGRAMS += android/android-tester
 
 android_android_tester_SOURCES = emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c \
@@ -172,6 +173,7 @@ android_android_tester_LDFLAGS = $(AM_LDFLAGS) -pthread
 noinst_PROGRAMS += android/ipc-tester
 
 android_ipc_tester_SOURCES = emulator/hciemu.h emulator/hciemu.c \
+				emulator/vhci.h emulator/vhci.c \
 				emulator/btdev.h emulator/btdev.c \
 				emulator/bthost.h emulator/bthost.c \
 				emulator/smp.c \
diff --git a/emulator/btdev.c b/emulator/btdev.c
index 148e32b7d..0c0ebde40 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -21,6 +21,8 @@
 #include <sys/uio.h>
 #include <stdint.h>
 #include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
 
 #include "lib/bluetooth.h"
 #include "lib/hci.h"
@@ -44,6 +46,8 @@
 #define ISO_HANDLE 257
 #define SCO_HANDLE 257
 
+#define DEBUGFS_PATH "/sys/kernel/debug/bluetooth"
+
 struct hook {
 	btdev_hook_func handler;
 	void *user_data;
@@ -93,6 +97,7 @@ struct le_ext_adv {
 
 struct btdev {
 	enum btdev_type type;
+	uint16_t id;
 
 	struct queue *conns;
 
@@ -135,6 +140,7 @@ struct btdev {
 	uint8_t  le_features[8];
 	uint8_t  le_states[8];
 	const struct btdev_cmd *cmds;
+	uint16_t msft_opcode;
 
 	uint16_t default_link_policy;
 	uint8_t  event_mask[8];
@@ -6230,7 +6236,7 @@ struct btdev *btdev_create(enum btdev_type type, uint16_t id)
 	}
 
 	btdev->type = type;
-
+	btdev->id = id;
 	btdev->manufacturer = 63;
 	btdev->revision = 0x0000;
 
diff --git a/emulator/hciemu.c b/emulator/hciemu.c
index bd6bf1e63..3557efc93 100644
--- a/emulator/hciemu.c
+++ b/emulator/hciemu.c
@@ -28,6 +28,7 @@
 #include "lib/hci.h"
 
 #include "monitor/bt.h"
+#include "emulator/vhci.h"
 #include "emulator/btdev.h"
 #include "emulator/bthost.h"
 #include "src/shared/util.h"
@@ -45,9 +46,8 @@ struct hciemu_client {
 struct hciemu {
 	int ref_count;
 	enum btdev_type btdev_type;
-	struct btdev *dev;
+	struct vhci *vhci;
 	struct queue *clients;
-	guint source;
 	struct queue *post_command_hooks;
 	char bdaddr_str[18];
 
@@ -221,37 +221,15 @@ static guint create_source_btdev(int fd, struct btdev *btdev)
 
 static bool create_vhci(struct hciemu *hciemu)
 {
-	struct btdev *btdev;
-	uint8_t create_req[2];
-	ssize_t written;
-	int fd;
-
-	btdev = btdev_create(hciemu->btdev_type, 0x00);
-	if (!btdev)
-		return false;
+	struct vhci *vhci;
 
-	btdev_set_command_handler(btdev, central_command_callback, hciemu);
-
-	fd = open("/dev/vhci", O_RDWR | O_NONBLOCK | O_CLOEXEC);
-	if (fd < 0) {
-		perror("Opening /dev/vhci failed");
-		btdev_destroy(btdev);
-		return false;
-	}
-
-	create_req[0] = HCI_VENDOR_PKT;
-	create_req[1] = HCI_PRIMARY;
-
-	written = write(fd, create_req, sizeof(create_req));
-	if (written < 0) {
-		close(fd);
-		btdev_destroy(btdev);
+	vhci = vhci_open(hciemu->btdev_type);
+	if (!vhci)
 		return false;
-	}
 
-	hciemu->dev = btdev;
-
-	hciemu->source = create_source_btdev(fd, btdev);
+	btdev_set_command_handler(vhci_get_btdev(vhci),
+					central_command_callback, hciemu);
+	hciemu->vhci = vhci;
 
 	return true;
 }
@@ -448,8 +426,7 @@ void hciemu_unref(struct hciemu *hciemu)
 	queue_destroy(hciemu->post_command_hooks, destroy_command_hook);
 	queue_destroy(hciemu->clients, hciemu_client_destroy);
 
-	g_source_remove(hciemu->source);
-	btdev_destroy(hciemu->dev);
+	vhci_close(hciemu->vhci);
 
 	free(hciemu);
 }
@@ -462,12 +439,12 @@ static void bthost_print(const char *str, void *user_data)
 					"bthost: %s", str);
 }
 
-static void btdev_central_debug(const char *str, void *user_data)
+static void vhci_debug(const char *str, void *user_data)
 {
 	struct hciemu *hciemu = user_data;
 
 	util_debug(hciemu->debug_callback, hciemu->debug_data,
-					"btdev: %s", str);
+					"vhci: %s", str);
 }
 
 static void btdev_client_debug(const char *str, void *user_data)
@@ -475,7 +452,7 @@ static void btdev_client_debug(const char *str, void *user_data)
 	struct hciemu *hciemu = user_data;
 
 	util_debug(hciemu->debug_callback, hciemu->debug_data,
-					"btdev[bthost]: %s", str);
+					"btdev: %s", str);
 }
 
 static void hciemu_client_set_debug(void *data, void *user_data)
@@ -500,7 +477,7 @@ bool hciemu_set_debug(struct hciemu *hciemu, hciemu_debug_func_t callback,
 	hciemu->debug_destroy = destroy;
 	hciemu->debug_data = user_data;
 
-	btdev_set_debug(hciemu->dev, btdev_central_debug, hciemu, NULL);
+	vhci_set_debug(hciemu->vhci, vhci_debug, hciemu, NULL);
 
 	queue_foreach(hciemu->clients, hciemu_client_set_debug, hciemu);
 
@@ -510,11 +487,16 @@ bool hciemu_set_debug(struct hciemu *hciemu, hciemu_debug_func_t callback,
 const char *hciemu_get_address(struct hciemu *hciemu)
 {
 	const uint8_t *addr;
+	struct btdev *dev;
+
+	if (!hciemu || !hciemu->vhci)
+		return NULL;
 
-	if (!hciemu || !hciemu->dev)
+	dev = vhci_get_btdev(hciemu->vhci);
+	if (!dev)
 		return NULL;
 
-	addr = btdev_get_bdaddr(hciemu->dev);
+	addr = btdev_get_bdaddr(dev);
 	sprintf(hciemu->bdaddr_str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
 			addr[5], addr[4], addr[3], addr[2], addr[1], addr[0]);
 	return hciemu->bdaddr_str;
@@ -522,18 +504,30 @@ const char *hciemu_get_address(struct hciemu *hciemu)
 
 uint8_t *hciemu_get_features(struct hciemu *hciemu)
 {
-	if (!hciemu || !hciemu->dev)
+	struct btdev *dev;
+
+	if (!hciemu || !hciemu->vhci)
 		return NULL;
 
-	return btdev_get_features(hciemu->dev);
+	dev = vhci_get_btdev(hciemu->vhci);
+	if (!dev)
+		return NULL;
+
+	return btdev_get_features(dev);
 }
 
 const uint8_t *hciemu_get_central_bdaddr(struct hciemu *hciemu)
 {
-	if (!hciemu || !hciemu->dev)
+	struct btdev *dev;
+
+	if (!hciemu || !hciemu->vhci)
+		return NULL;
+
+	dev = vhci_get_btdev(hciemu->vhci);
+	if (!dev)
 		return NULL;
 
-	return btdev_get_bdaddr(hciemu->dev);
+	return btdev_get_bdaddr(dev);
 }
 
 const uint8_t *hciemu_client_bdaddr(struct hciemu_client *client)
@@ -558,27 +552,45 @@ const uint8_t *hciemu_get_client_bdaddr(struct hciemu *hciemu)
 
 uint8_t hciemu_get_central_scan_enable(struct hciemu *hciemu)
 {
-	if (!hciemu || !hciemu->dev)
+	struct btdev *dev;
+
+	if (!hciemu || !hciemu->vhci)
+		return 0;
+
+	dev = vhci_get_btdev(hciemu->vhci);
+	if (!dev)
 		return 0;
 
-	return btdev_get_scan_enable(hciemu->dev);
+	return btdev_get_scan_enable(dev);
 }
 
 uint8_t hciemu_get_central_le_scan_enable(struct hciemu *hciemu)
 {
-	if (!hciemu || !hciemu->dev)
+	struct btdev *dev;
+
+	if (!hciemu || !hciemu->vhci)
 		return 0;
 
-	return btdev_get_le_scan_enable(hciemu->dev);
+	dev = vhci_get_btdev(hciemu->vhci);
+	if (!dev)
+		return 0;
+
+	return btdev_get_le_scan_enable(dev);
 }
 
 void hciemu_set_central_le_states(struct hciemu *hciemu,
 						const uint8_t *le_states)
 {
-	if (!hciemu || !hciemu->dev)
+	struct btdev *dev;
+
+	if (!hciemu || !hciemu->vhci)
+		return;
+
+	dev = vhci_get_btdev(hciemu->vhci);
+	if (!dev)
 		return;
 
-	btdev_set_le_states(hciemu->dev, le_states);
+	btdev_set_le_states(dev, le_states);
 }
 
 bool hciemu_add_central_post_command_hook(struct hciemu *hciemu,
@@ -619,10 +631,15 @@ int hciemu_add_hook(struct hciemu *hciemu, enum hciemu_hook_type type,
 				void *user_data)
 {
 	enum btdev_hook_type hook_type;
+	struct btdev *dev;
 
-	if (!hciemu)
+	if (!hciemu || !hciemu->vhci)
 		return -1;
 
+	dev = vhci_get_btdev(hciemu->vhci);
+	if (!dev)
+		return 0;
+
 	switch (type) {
 	case HCIEMU_HOOK_PRE_CMD:
 		hook_type = BTDEV_HOOK_PRE_CMD;
@@ -640,16 +657,20 @@ int hciemu_add_hook(struct hciemu *hciemu, enum hciemu_hook_type type,
 		return -1;
 	}
 
-	return btdev_add_hook(hciemu->dev, hook_type, opcode, function,
-								user_data);
+	return btdev_add_hook(dev, hook_type, opcode, function, user_data);
 }
 
 bool hciemu_del_hook(struct hciemu *hciemu, enum hciemu_hook_type type,
 								uint16_t opcode)
 {
 	enum btdev_hook_type hook_type;
+	struct btdev *dev;
 
-	if (!hciemu)
+	if (!hciemu || !hciemu->vhci)
+		return false;
+
+	dev = vhci_get_btdev(hciemu->vhci);
+	if (!dev)
 		return false;
 
 	switch (type) {
@@ -669,5 +690,5 @@ bool hciemu_del_hook(struct hciemu *hciemu, enum hciemu_hook_type type,
 		return false;
 	}
 
-	return btdev_del_hook(hciemu->dev, hook_type, opcode);
+	return btdev_del_hook(dev, hook_type, opcode);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH BlueZ 6/7] vhci: Add functions to interface with debugfs
  2021-10-15  5:09 [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension Luiz Augusto von Dentz
                   ` (3 preceding siblings ...)
  2021-10-15  5:09 ` [PATCH BlueZ 5/7] hciemu: Use vhci_open to instanciate a vhci btdev Luiz Augusto von Dentz
@ 2021-10-15  5:09 ` Luiz Augusto von Dentz
  2021-10-15  5:09 ` [PATCH BlueZ 7/7] mgmt-tester: Make use of vhci_set_force_suspend/vhci_set_force_wakeup Luiz Augusto von Dentz
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-15  5:09 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds functions that can be used to set debugfs options.
---
 emulator/btdev.c |  23 +++++++++-
 emulator/btdev.h |   3 ++
 emulator/vhci.c  | 108 +++++++++++++++++++++++++++++++++++++++++++++++
 emulator/vhci.h  |   5 +++
 4 files changed, 137 insertions(+), 2 deletions(-)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index 0c0ebde40..f28187362 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -46,8 +46,6 @@
 #define ISO_HANDLE 257
 #define SCO_HANDLE 257
 
-#define DEBUGFS_PATH "/sys/kernel/debug/bluetooth"
-
 struct hook {
 	btdev_hook_func handler;
 	void *user_data;
@@ -141,6 +139,7 @@ struct btdev {
 	uint8_t  le_states[8];
 	const struct btdev_cmd *cmds;
 	uint16_t msft_opcode;
+	bool aosp_capable;
 
 	uint16_t default_link_policy;
 	uint8_t  event_mask[8];
@@ -6677,3 +6676,23 @@ bool btdev_del_hook(struct btdev *btdev, enum btdev_hook_type type,
 
 	return false;
 }
+
+int btdev_set_msft_opcode(struct btdev *btdev, uint16_t opcode)
+{
+	if (!btdev)
+		return -EINVAL;
+
+	btdev->msft_opcode = opcode;
+
+	return 0;
+}
+
+int btdev_set_aosp_capable(struct btdev *btdev, bool enable)
+{
+	if (!btdev)
+		return -EINVAL;
+
+	btdev->aosp_capable = enable;
+
+	return 0;
+}
diff --git a/emulator/btdev.h b/emulator/btdev.h
index f7cba149a..412bfd158 100644
--- a/emulator/btdev.h
+++ b/emulator/btdev.h
@@ -93,3 +93,6 @@ int btdev_add_hook(struct btdev *btdev, enum btdev_hook_type type,
 
 bool btdev_del_hook(struct btdev *btdev, enum btdev_hook_type type,
 							uint16_t opcode);
+
+int btdev_set_msft_opcode(struct btdev *btdev, uint16_t opcode);
+int btdev_set_aosp_capable(struct btdev *btdev, bool enable);
diff --git a/emulator/vhci.c b/emulator/vhci.c
index 97fbcb8c4..e016a1ac5 100644
--- a/emulator/vhci.c
+++ b/emulator/vhci.c
@@ -20,6 +20,8 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/uio.h>
+#include <fcntl.h>
+#include <unistd.h>
 
 #include "lib/bluetooth.h"
 #include "lib/hci.h"
@@ -29,8 +31,11 @@
 #include "btdev.h"
 #include "vhci.h"
 
+#define DEBUGFS_PATH "/sys/kernel/debug/bluetooth"
+
 struct vhci {
 	enum btdev_type type;
+	uint16_t index;
 	struct io *io;
 	struct btdev *btdev;
 };
@@ -140,6 +145,7 @@ struct vhci *vhci_open(uint8_t type)
 
 	memset(vhci, 0, sizeof(*vhci));
 	vhci->type = type;
+	vhci->index = rsp.index;
 	vhci->io = io_new(fd);
 
 	io_set_close_on_destroy(vhci->io, true);
@@ -175,3 +181,105 @@ struct btdev *vhci_get_btdev(struct vhci *vhci)
 
 	return vhci->btdev;
 }
+
+static int vhci_debugfs_open(struct vhci *vhci, char *option)
+{
+	char path[64];
+
+	if (!vhci)
+		return -EINVAL;
+
+	memset(path, 0, sizeof(path));
+	sprintf(path, DEBUGFS_PATH "/hci%d/%s", vhci->index, option);
+
+	return open(path, O_RDWR);
+}
+
+int vhci_set_force_suspend(struct vhci *vhci, bool enable)
+{
+	int fd, err;
+	char val;
+
+	fd = vhci_debugfs_open(vhci, "force_suspend");
+	if (fd < 0)
+		return -errno;
+
+	val = (enable) ? 'Y' : 'N';
+
+	err = write(fd, &val, sizeof(val));
+	if (err < 0) {
+		err = -errno;
+		goto done;
+	}
+
+done:
+	close(fd);
+	return err;
+}
+
+int vhci_set_force_wakeup(struct vhci *vhci, bool enable)
+{
+	int fd, err;
+	char val;
+
+	fd = vhci_debugfs_open(vhci, "force_wakeup");
+	if (fd < 0)
+		return -errno;
+
+	val = (enable) ? 'Y' : 'N';
+
+	err = write(fd, &val, sizeof(val));
+	if (err < 0) {
+		err = -errno;
+		goto done;
+	}
+
+done:
+	close(fd);
+	return err;
+}
+
+int vhci_set_msft_opcode(struct vhci *vhci, uint16_t opcode)
+{
+	int fd, err;
+
+	fd = vhci_debugfs_open(vhci, "msft_opcode");
+	if (fd < 0)
+		return -errno;
+
+	err = write(fd, &opcode, sizeof(opcode));
+	if (err < 0) {
+		err = -errno;
+		goto done;
+	}
+
+	btdev_set_msft_opcode(vhci->btdev, opcode);
+
+done:
+	close(fd);
+	return err;
+}
+
+int vhci_set_aosp_capable(struct vhci *vhci, bool enable)
+{
+	int fd, err;
+	char val;
+
+	fd = vhci_debugfs_open(vhci, "aosp_capable");
+	if (fd < 0)
+		return -errno;
+
+	val = (enable) ? 'Y' : 'N';
+
+	err = write(fd, &val, sizeof(val));
+	if (err < 0) {
+		err = -errno;
+		goto done;
+	}
+
+	btdev_set_aosp_capable(vhci->btdev, enable);
+
+done:
+	close(fd);
+	return err;
+}
diff --git a/emulator/vhci.h b/emulator/vhci.h
index 0554121e8..a601d3934 100644
--- a/emulator/vhci.h
+++ b/emulator/vhci.h
@@ -22,3 +22,8 @@ struct vhci *vhci_open(uint8_t type);
 void vhci_close(struct vhci *vhci);
 
 struct btdev *vhci_get_btdev(struct vhci *vhci);
+
+int vhci_set_force_suspend(struct vhci *vhci, bool enable);
+int vhci_set_force_wakeup(struct vhci *vhci, bool enable);
+int vhci_set_msft_opcode(struct vhci *vhci, uint16_t opcode);
+int vhci_set_aosp_capable(struct vhci *vhci, bool enable);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH BlueZ 7/7] mgmt-tester: Make use of vhci_set_force_suspend/vhci_set_force_wakeup
  2021-10-15  5:09 [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension Luiz Augusto von Dentz
                   ` (4 preceding siblings ...)
  2021-10-15  5:09 ` [PATCH BlueZ 6/7] vhci: Add functions to interface with debugfs Luiz Augusto von Dentz
@ 2021-10-15  5:09 ` Luiz Augusto von Dentz
  2021-10-15  5:36 ` [BlueZ,1/7] monitor: Add packet definitions for MSFT extension bluez.test.bot
       [not found] ` <CAGPPCLDFYFeiwfiyRX=6PquYYQ-Fp_LpN4Gw2745jyWzQKEBRQ@mail.gmail.com>
  7 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-15  5:09 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This replaces the direct setting debugfs to use vhci instance which
properly stores the controller index so it can be used even if there
are real controllers in the system.
---
 emulator/hciemu.c   |   8 ++++
 emulator/hciemu.h   |   1 +
 tools/mgmt-tester.c | 105 ++++++++++----------------------------------
 3 files changed, 33 insertions(+), 81 deletions(-)

diff --git a/emulator/hciemu.c b/emulator/hciemu.c
index 3557efc93..4752c8a4d 100644
--- a/emulator/hciemu.c
+++ b/emulator/hciemu.c
@@ -234,6 +234,14 @@ static bool create_vhci(struct hciemu *hciemu)
 	return true;
 }
 
+struct vhci *hciemu_get_vhci(struct hciemu *hciemu)
+{
+	if (!hciemu)
+		return NULL;
+
+	return hciemu->vhci;
+}
+
 struct hciemu_client *hciemu_get_client(struct hciemu *hciemu, int num)
 {
 	const struct queue_entry *entry;
diff --git a/emulator/hciemu.h b/emulator/hciemu.h
index 3d3d93b4b..338fa844d 100644
--- a/emulator/hciemu.h
+++ b/emulator/hciemu.h
@@ -45,6 +45,7 @@ typedef void (*hciemu_destroy_func_t)(void *user_data);
 bool hciemu_set_debug(struct hciemu *hciemu, hciemu_debug_func_t callback,
 			void *user_data, hciemu_destroy_func_t destroy);
 
+struct vhci *hciemu_get_vhci(struct hciemu *hciemu);
 struct bthost *hciemu_client_get_host(struct hciemu *hciemu);
 
 const char *hciemu_get_address(struct hciemu *hciemu);
diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index 8bddf6b03..62218b431 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -30,6 +30,7 @@
 #include "lib/l2cap.h"
 
 #include "monitor/bt.h"
+#include "emulator/vhci.h"
 #include "emulator/bthost.h"
 #include "emulator/hciemu.h"
 
@@ -313,62 +314,6 @@ struct generic_data {
 	uint8_t adv_data_len;
 };
 
-static int set_debugfs_force_suspend(int index, bool enable)
-{
-	int fd, n, err;
-	char val, path[64];
-
-	err = 0;
-
-	/* path for the debugfs file
-	 * /sys/kernel/debug/bluetooth/hciX/force_suspend
-	 */
-	memset(path, 0, sizeof(path));
-	sprintf(path, "/sys/kernel/debug/bluetooth/hci%d/force_suspend", index);
-
-	fd = open(path, O_RDWR);
-	if (fd < 0)
-		return -errno;
-
-	val = (enable) ? 'Y' : 'N';
-
-	n = write(fd, &val, sizeof(val));
-	if (n < (ssize_t) sizeof(val))
-		err = -errno;
-
-	close(fd);
-
-	return err;
-}
-
-static int set_debugfs_force_wakeup(int index, bool enable)
-{
-	int fd, n, err;
-	char val, path[64];
-
-	err = 0;
-
-	/* path for the debugfs file
-	 * /sys/kernel/debug/bluetooth/hciX/force_suspend
-	 */
-	memset(path, 0, sizeof(path));
-	sprintf(path, "/sys/kernel/debug/bluetooth/hci%d/force_wakeup", index);
-
-	fd = open(path, O_RDWR);
-	if (fd < 0)
-		return -errno;
-
-	val = (enable) ? 'Y' : 'N';
-
-	n = write(fd, &val, sizeof(val));
-	if (n < (ssize_t) sizeof(val))
-		err = -errno;
-
-	close(fd);
-
-	return err;
-}
-
 static const uint8_t set_exp_feat_param_debug[] = {
 	0x1c, 0xda, 0x47, 0x1c, 0x48, 0x6c, 0x01, 0xab, /* UUID - Debug */
 	0x9f, 0x46, 0xec, 0xb9, 0x30, 0x25, 0x99, 0xd4,
@@ -10512,12 +10457,12 @@ static const struct generic_data suspend_resume_success_1 = {
 
 static void test_suspend_resume_success_1(const void *test_data)
 {
-	bool suspend;
+	struct test_data *data = tester_get_data();
+	struct vhci *vhci = hciemu_get_vhci(data->hciemu);
 	int err;
 
 	/* Triggers the suspend */
-	suspend = true;
-	err = set_debugfs_force_suspend(0, suspend);
+	err = vhci_set_force_suspend(vhci, true);
 	if (err) {
 		tester_warn("Unable to enable the force_suspend");
 		tester_test_failed();
@@ -10535,12 +10480,12 @@ static const struct generic_data suspend_resume_success_2 = {
 
 static void test_suspend_resume_success_2(const void *test_data)
 {
-	bool suspend;
+	struct test_data *data = tester_get_data();
+	struct vhci *vhci = hciemu_get_vhci(data->hciemu);
 	int err;
 
 	/* Triggers the suspend */
-	suspend = true;
-	err = set_debugfs_force_suspend(0, suspend);
+	err = vhci_set_force_suspend(vhci, true);
 	if (err) {
 		tester_warn("Unable to enable the force_suspend");
 		tester_test_failed();
@@ -10548,8 +10493,7 @@ static void test_suspend_resume_success_2(const void *test_data)
 	}
 
 	/* Triggers the resume */
-	suspend = false;
-	err = set_debugfs_force_suspend(0, suspend);
+	err = vhci_set_force_suspend(vhci, false);
 	if (err) {
 		tester_warn("Unable to enable the force_suspend");
 		tester_test_failed();
@@ -10586,12 +10530,12 @@ static void setup_suspend_resume_success_3(const void *test_data)
 
 static void test_suspend_resume_success_3(const void *test_data)
 {
-	bool suspend;
+	struct test_data *data = tester_get_data();
+	struct vhci *vhci = hciemu_get_vhci(data->hciemu);
 	int err;
 
 	/* Triggers the suspend */
-	suspend = true;
-	err = set_debugfs_force_suspend(0, suspend);
+	err = vhci_set_force_suspend(vhci, true);
 	if (err) {
 		tester_warn("Unable to enable the force_suspend");
 		tester_test_failed();
@@ -10635,15 +10579,15 @@ static void setup_suspend_resume_success_4(const void *test_data)
 
 static void test_suspend_resume_success_4(const void *test_data)
 {
-	bool suspend;
+	struct test_data *data = tester_get_data();
+	struct vhci *vhci = hciemu_get_vhci(data->hciemu);
 	int err;
 
 	test_command_generic(test_data);
 
 	/* Triggers the suspend */
-	suspend = true;
 	tester_print("Set the system into Suspend via force_suspend");
-	err = set_debugfs_force_suspend(0, suspend);
+	err = vhci_set_force_suspend(vhci, true);
 	if (err) {
 		tester_warn("Unable to enable the force_suspend");
 		tester_test_failed();
@@ -10664,13 +10608,13 @@ static const struct generic_data suspend_resume_success_5 = {
 
 static void trigger_force_suspend(void *user_data)
 {
-	bool suspend;
+	struct test_data *data = tester_get_data();
+	struct vhci *vhci = hciemu_get_vhci(data->hciemu);
 	int err;
 
 	/* Triggers the suspend */
-	suspend = true;
 	tester_print("Set the system into Suspend via force_suspend");
-	err = set_debugfs_force_suspend(0, suspend);
+	err = vhci_set_force_suspend(vhci, true);
 	if (err) {
 		tester_warn("Unable to enable the force_suspend");
 		return;
@@ -10679,13 +10623,13 @@ static void trigger_force_suspend(void *user_data)
 
 static void trigger_force_resume(void *user_data)
 {
-	bool suspend;
+	struct test_data *data = tester_get_data();
+	struct vhci *vhci = hciemu_get_vhci(data->hciemu);
 	int err;
 
 	/* Triggers the suspend */
-	suspend = false;
 	tester_print("Set the system into Resume via force_suspend");
-	err = set_debugfs_force_suspend(0, suspend);
+	err = vhci_set_force_suspend(vhci, false);
 	if (err) {
 		tester_warn("Unable to disable the force_suspend");
 		return;
@@ -10720,12 +10664,12 @@ static const struct generic_data suspend_resume_success_7 = {
 
 static void test_suspend_resume_success_7(const void *test_data)
 {
-	bool suspend;
+	struct test_data *data = tester_get_data();
+	struct vhci *vhci = hciemu_get_vhci(data->hciemu);
 	int err;
 
 	/* Set Force Wakeup */
-	suspend = true;
-	err = set_debugfs_force_wakeup(0, suspend);
+	err = vhci_set_force_wakeup(vhci, true);
 	if (err) {
 		tester_warn("Unable to enable the force_wakeup");
 		tester_test_failed();
@@ -10733,8 +10677,7 @@ static void test_suspend_resume_success_7(const void *test_data)
 	}
 
 	/* Triggers the suspend */
-	suspend = true;
-	err = set_debugfs_force_suspend(0, suspend);
+	err = vhci_set_force_wakeup(vhci, false);
 	if (err) {
 		tester_warn("Unable to enable the force_suspend");
 		tester_test_failed();
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* RE: [BlueZ,1/7] monitor: Add packet definitions for MSFT extension
  2021-10-15  5:09 [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension Luiz Augusto von Dentz
                   ` (5 preceding siblings ...)
  2021-10-15  5:09 ` [PATCH BlueZ 7/7] mgmt-tester: Make use of vhci_set_force_suspend/vhci_set_force_wakeup Luiz Augusto von Dentz
@ 2021-10-15  5:36 ` bluez.test.bot
  2021-10-15 20:10   ` Luiz Augusto von Dentz
       [not found] ` <CAGPPCLDFYFeiwfiyRX=6PquYYQ-Fp_LpN4Gw2745jyWzQKEBRQ@mail.gmail.com>
  7 siblings, 1 reply; 11+ messages in thread
From: bluez.test.bot @ 2021-10-15  5:36 UTC (permalink / raw)
  To: linux-bluetooth, luiz.dentz

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

---Test result---

Test Summary:
CheckPatch                    FAIL      4.45 seconds
GitLint                       FAIL      2.34 seconds
Prep - Setup ELL              PASS      56.30 seconds
Build - Prep                  PASS      0.25 seconds
Build - Configure             PASS      10.00 seconds
Build - Make                  PASS      238.32 seconds
Make Check                    PASS      9.34 seconds
Make Distcheck                PASS      279.48 seconds
Build w/ext ELL - Configure   PASS      10.29 seconds
Build w/ext ELL - Make        PASS      226.33 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,1/7] monitor: Add packet definitions for MSFT extension
WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#101: FILE: monitor/msft.h:31:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#109: FILE: monitor/msft.h:39:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#120: FILE: monitor/msft.h:50:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#125: FILE: monitor/msft.h:55:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#132: FILE: monitor/msft.h:62:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#137: FILE: monitor/msft.h:67:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#146: FILE: monitor/msft.h:76:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#151: FILE: monitor/msft.h:81:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#160: FILE: monitor/msft.h:90:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#164: FILE: monitor/msft.h:94:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#170: FILE: monitor/msft.h:100:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#180: FILE: monitor/msft.h:110:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#186: FILE: monitor/msft.h:116:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#193: FILE: monitor/msft.h:123:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#198: FILE: monitor/msft.h:128:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#205: FILE: monitor/msft.h:135:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#210: FILE: monitor/msft.h:140:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#217: FILE: monitor/msft.h:147:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#224: FILE: monitor/msft.h:154:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#233: FILE: monitor/msft.h:163:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#243: FILE: monitor/msft.h:173:
+} __attribute__((packed));

/github/workspace/src/12560019.patch total: 0 errors, 21 warnings, 154 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12560019.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

[BlueZ,3/7] vhci: Read the controller index
WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#173: FILE: emulator/vhci.c:95:
+} __attribute__((packed));

WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#179: FILE: emulator/vhci.c:101:
+} __attribute__((packed));

/github/workspace/src/12560021.patch total: 0 errors, 2 warnings, 172 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12560021.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

[BlueZ,5/7] hciemu: Use vhci_open to instanciate a vhci btdev
WARNING:TYPO_SPELLING: 'instanciate' may be misspelled - perhaps 'instantiate'?
#72: 
Subject: [PATCH BlueZ 5/7] hciemu: Use vhci_open to instanciate a vhci btdev
                                                    ^^^^^^^^^^^

/github/workspace/src/12560027.patch total: 0 errors, 1 warnings, 355 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12560027.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint with rule in .gitlint
Output:
[BlueZ,7/7] mgmt-tester: Make use of vhci_set_force_suspend/vhci_set_force_wakeup
1: T1 Title exceeds max length (81>80): "[BlueZ,7/7] mgmt-tester: Make use of vhci_set_force_suspend/vhci_set_force_wakeup"




---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [BlueZ,1/7] monitor: Add packet definitions for MSFT extension
  2021-10-15  5:36 ` [BlueZ,1/7] monitor: Add packet definitions for MSFT extension bluez.test.bot
@ 2021-10-15 20:10   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-15 20:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Tedd Ho-Jeong An, Manish Mandlik

Hi Tedd, Manish,

On Thu, Oct 14, 2021 at 10:36 PM <bluez.test.bot@gmail.com> wrote:
>
> 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=563859

Do you guys have any comments on this set? Id like to push it but I
haven't been able to validate if the PDU definitions, etc, are set
properly.

> ---Test result---
>
> Test Summary:
> CheckPatch                    FAIL      4.45 seconds
> GitLint                       FAIL      2.34 seconds
> Prep - Setup ELL              PASS      56.30 seconds
> Build - Prep                  PASS      0.25 seconds
> Build - Configure             PASS      10.00 seconds
> Build - Make                  PASS      238.32 seconds
> Make Check                    PASS      9.34 seconds
> Make Distcheck                PASS      279.48 seconds
> Build w/ext ELL - Configure   PASS      10.29 seconds
> Build w/ext ELL - Make        PASS      226.33 seconds
>
> Details
> ##############################
> Test: CheckPatch - FAIL
> Desc: Run checkpatch.pl script with rule in .checkpatch.conf
> Output:
> [BlueZ,1/7] monitor: Add packet definitions for MSFT extension
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #101: FILE: monitor/msft.h:31:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #109: FILE: monitor/msft.h:39:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #120: FILE: monitor/msft.h:50:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #125: FILE: monitor/msft.h:55:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #132: FILE: monitor/msft.h:62:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #137: FILE: monitor/msft.h:67:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #146: FILE: monitor/msft.h:76:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #151: FILE: monitor/msft.h:81:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #160: FILE: monitor/msft.h:90:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #164: FILE: monitor/msft.h:94:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #170: FILE: monitor/msft.h:100:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #180: FILE: monitor/msft.h:110:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #186: FILE: monitor/msft.h:116:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #193: FILE: monitor/msft.h:123:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #198: FILE: monitor/msft.h:128:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #205: FILE: monitor/msft.h:135:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #210: FILE: monitor/msft.h:140:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #217: FILE: monitor/msft.h:147:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #224: FILE: monitor/msft.h:154:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #233: FILE: monitor/msft.h:163:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #243: FILE: monitor/msft.h:173:
> +} __attribute__((packed));
>
> /github/workspace/src/12560019.patch total: 0 errors, 21 warnings, 154 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
>
> /github/workspace/src/12560019.patch has style problems, please review.
>
> NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
>
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> [BlueZ,3/7] vhci: Read the controller index
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #173: FILE: emulator/vhci.c:95:
> +} __attribute__((packed));
>
> WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
> #179: FILE: emulator/vhci.c:101:
> +} __attribute__((packed));
>
> /github/workspace/src/12560021.patch total: 0 errors, 2 warnings, 172 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
>
> /github/workspace/src/12560021.patch has style problems, please review.
>
> NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
>
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> [BlueZ,5/7] hciemu: Use vhci_open to instanciate a vhci btdev
> WARNING:TYPO_SPELLING: 'instanciate' may be misspelled - perhaps 'instantiate'?
> #72:
> Subject: [PATCH BlueZ 5/7] hciemu: Use vhci_open to instanciate a vhci btdev
>                                                     ^^^^^^^^^^^
>
> /github/workspace/src/12560027.patch total: 0 errors, 1 warnings, 355 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
>
> /github/workspace/src/12560027.patch has style problems, please review.
>
> NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
>
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
>
> ##############################
> Test: GitLint - FAIL
> Desc: Run gitlint with rule in .gitlint
> Output:
> [BlueZ,7/7] mgmt-tester: Make use of vhci_set_force_suspend/vhci_set_force_wakeup
> 1: T1 Title exceeds max length (81>80): "[BlueZ,7/7] mgmt-tester: Make use of vhci_set_force_suspend/vhci_set_force_wakeup"
>
>
>
>
> ---
> Regards,
> Linux Bluetooth
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension
       [not found] ` <CAGPPCLDFYFeiwfiyRX=6PquYYQ-Fp_LpN4Gw2745jyWzQKEBRQ@mail.gmail.com>
@ 2021-10-18 16:24   ` Luiz Augusto von Dentz
  2021-10-18 20:05     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-18 16:24 UTC (permalink / raw)
  To: Manish Mandlik; +Cc: linux-bluetooth, Tedd Ho-Jeong An

Hi Manish,

On Mon, Oct 18, 2021 at 7:53 AM Manish Mandlik <mmandlik@google.com> wrote:
>
> Hi Luiz,
>
> On Fri, Oct 15, 2021 at 1:09 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This adds proper packet definitions for command and response of MSFT
>> extension.
>> ---
>>  monitor/msft.h | 148 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 148 insertions(+)
>>
>> diff --git a/monitor/msft.h b/monitor/msft.h
>> index a268f4bc7..90a64117a 100644
>> --- a/monitor/msft.h
>> +++ b/monitor/msft.h
>> @@ -24,6 +24,154 @@
>>
>>  #include <stdint.h>
>>
>> +#define MSFT_SUBCMD_READ_SUPPORTED_FEATURES    0x00
>> +
>> +struct msft_cmd_read_supported_features {
>> +       uint8_t subcmd;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_read_supported_features {
>> +       uint8_t  status;
>> +       uint8_t  subcmd;
>> +       uint8_t  features[8];
>> +       uint8_t  evt_prefix_len;
>> +       uint8_t  evt_prefix[];
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_MONITOR_RSSI               0x01
>> +
>> +struct msft_cmd_monitor_rssi {
>> +       uint8_t  subcmd;
>> +       uint16_t handle;
>> +       int8_t   rssi_high;
>> +       int8_t   rssi_low;
>> +       uint8_t  rssi_low_interval;
>> +       uint8_t  rssi_period;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_monitor_rssi {
>> +       uint8_t  status;
>> +       uint8_t  subcmd;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_CANCEL_MONITOR_RSSI                0x02
>> +
>> +struct msft_cmd_cancel_monitor_rssi {
>> +       uint8_t  subcmd;
>> +       uint16_t handle;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_cancel_monitor_rssi {
>> +       uint8_t  status;
>> +       uint8_t  subcmd;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_LE_MONITOR_ADV             0x03
>> +
>> +struct msft_le_monitor_pattern {
>> +       uint8_t  len;
>> +       uint8_t  type;
>> +       uint8_t  start;
>> +       uint8_t  data[];
>> +} __attribute__((packed));
>> +
>
>
> +    #define MSFT_COND_LE_MONITOR_ADV_PATTERN                0x01
>>
>> +struct msft_le_monitor_adv_pattern_type {
>> +       uint8_t num_patterns;
>> +       struct msft_le_monitor_pattern data[];
>> +} __attribute__((packed));
>> +
>
>
> +    #define MSFT_COND_LE_MONITOR_ADV_UUID                0x02
>>
>> +struct msft_le_monitor_adv_uuid_type {
>> +       uint8_t  type;
>> +       union {
>> +               uint16_t u16;
>> +               uint32_t u32;
>> +               uint8_t  u128[8];
>> +       } value;
>> +} __attribute__((packed));
>> +
>
>
> +   #define MSFT_COND_LE_MONITOR_ADV_IRK                0x03
>>
>> +struct msft_le_monitor_adv_irk_type {
>> +       uint8_t  irk[8];
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_LE_MONITOR_ADV_ADDR                0x04
>
> I think this is not a subcommand. Instead, it is a condition type. So we can rename this to something else, e.g. MSFT_COND_LE_MONITOR_ADV_ADDR.
> Similarly, we'll have to define other three condition types as well for msft_le_monitor_adv_pattern_type, msft_le_monitor_adv_uuid_type and msft_le_monitor_adv_irk_type as mentioned above.

Right I will fix it since the intent was to have it as conditions,
thanks for reviewing.

>> +struct msft_le_monitor_adv_addr {
>> +       uint8_t  type;
>> +       uint8_t  addr[6];
>> +} __attribute__((packed));
>> +
>> +struct msft_cmd_le_monitor_adv {
>> +       uint8_t  subcmd;
>> +       int8_t   rssi_low;
>> +       int8_t   rssi_high;
>
> Order should be:
> +       int8_t   rssi_high;
> +       int8_t   rssi_low;
>>
>> +       uint8_t  rssi_low_interval;
>> +       uint8_t  rssi_period;
>> +       uint8_t  type;
>> +       uint8_t  data[];
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_le_monitor_adv {
>> +       uint8_t  status;
>> +       uint8_t  subcmd;
>> +       uint8_t  handle;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_LE_CANCEL_MONITOR_ADV      0x04
>> +
>> +struct msft_cmd_le_cancel_monitor_adv {
>> +       uint8_t  subcmd;
>> +       uint8_t  handle;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_le_cancel_monitor_adv {
>> +       uint8_t  status;
>> +       uint8_t  subcmd;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_LE_MONITOR_ADV_ENABLE      0x05
>> +
>> +struct msft_cmd_le_monitor_adv_enable {
>> +       uint8_t  subcmd;
>> +       uint8_t  enable;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_le_monitor_adv_enable {
>> +       uint8_t  status;
>> +       uint8_t  subcmd;
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBCMD_READ_ABS_RSSI              0x06
>> +
>> +struct msft_cmd_read_abs_rssi {
>> +       uint8_t  subcmd;
>> +       uint16_t handle;
>> +} __attribute__((packed));
>> +
>> +struct msft_rsp_read_abs_rssi {
>> +       uint8_t  status;
>> +       uint8_t  subcmd;
>> +       uint16_t handle;
>> +       uint8_t  rssi;

Ack.

> 'int8_t rssi' instead of 'uint8_t rssi'
>
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBEVT_RSSI                       0x01
>> +
>> +struct msft_evt_rssi {
>> +       uint8_t  subevt;
>> +       uint8_t  status;
>> +       uint16_t handle;
>> +       uint8_t  rssi;
>
> same as above - 'int8_t rssi' instead of 'uint8_t rssi'

Ack.

>
>> +} __attribute__((packed));
>> +
>> +#define MSFT_SUBEVT_MONITOR_DEVICE             0x02
>> +
>> +struct msft_evt_monitor_device {
>> +       uint8_t  subevt;
>> +       uint8_t  addr_type;
>> +       uint8_t  addr[6];
>> +       uint8_t  handle;
>> +       uint8_t  state;
>> +} __attribute__((packed));
>> +
>>  struct vendor_ocf;
>>  struct vendor_evt;
>>
>> --
>> 2.31.1
>>
>
> Rest of the changes look good to me.
>
> Thanks,
> Manish.



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension
  2021-10-18 16:24   ` [PATCH BlueZ 1/7] " Luiz Augusto von Dentz
@ 2021-10-18 20:05     ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-10-18 20:05 UTC (permalink / raw)
  To: Manish Mandlik; +Cc: linux-bluetooth, Tedd Ho-Jeong An

Hi,

On Mon, Oct 18, 2021 at 9:24 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Manish,
>
> On Mon, Oct 18, 2021 at 7:53 AM Manish Mandlik <mmandlik@google.com> wrote:
> >
> > Hi Luiz,
> >
> > On Fri, Oct 15, 2021 at 1:09 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> >>
> >> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>
> >> This adds proper packet definitions for command and response of MSFT
> >> extension.
> >> ---
> >>  monitor/msft.h | 148 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 148 insertions(+)
> >>
> >> diff --git a/monitor/msft.h b/monitor/msft.h
> >> index a268f4bc7..90a64117a 100644
> >> --- a/monitor/msft.h
> >> +++ b/monitor/msft.h
> >> @@ -24,6 +24,154 @@
> >>
> >>  #include <stdint.h>
> >>
> >> +#define MSFT_SUBCMD_READ_SUPPORTED_FEATURES    0x00
> >> +
> >> +struct msft_cmd_read_supported_features {
> >> +       uint8_t subcmd;
> >> +} __attribute__((packed));
> >> +
> >> +struct msft_rsp_read_supported_features {
> >> +       uint8_t  status;
> >> +       uint8_t  subcmd;
> >> +       uint8_t  features[8];
> >> +       uint8_t  evt_prefix_len;
> >> +       uint8_t  evt_prefix[];
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBCMD_MONITOR_RSSI               0x01
> >> +
> >> +struct msft_cmd_monitor_rssi {
> >> +       uint8_t  subcmd;
> >> +       uint16_t handle;
> >> +       int8_t   rssi_high;
> >> +       int8_t   rssi_low;
> >> +       uint8_t  rssi_low_interval;
> >> +       uint8_t  rssi_period;
> >> +} __attribute__((packed));
> >> +
> >> +struct msft_rsp_monitor_rssi {
> >> +       uint8_t  status;
> >> +       uint8_t  subcmd;
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBCMD_CANCEL_MONITOR_RSSI                0x02
> >> +
> >> +struct msft_cmd_cancel_monitor_rssi {
> >> +       uint8_t  subcmd;
> >> +       uint16_t handle;
> >> +} __attribute__((packed));
> >> +
> >> +struct msft_rsp_cancel_monitor_rssi {
> >> +       uint8_t  status;
> >> +       uint8_t  subcmd;
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBCMD_LE_MONITOR_ADV             0x03
> >> +
> >> +struct msft_le_monitor_pattern {
> >> +       uint8_t  len;
> >> +       uint8_t  type;
> >> +       uint8_t  start;
> >> +       uint8_t  data[];
> >> +} __attribute__((packed));
> >> +
> >
> >
> > +    #define MSFT_COND_LE_MONITOR_ADV_PATTERN                0x01
> >>
> >> +struct msft_le_monitor_adv_pattern_type {
> >> +       uint8_t num_patterns;
> >> +       struct msft_le_monitor_pattern data[];
> >> +} __attribute__((packed));
> >> +
> >
> >
> > +    #define MSFT_COND_LE_MONITOR_ADV_UUID                0x02
> >>
> >> +struct msft_le_monitor_adv_uuid_type {
> >> +       uint8_t  type;
> >> +       union {
> >> +               uint16_t u16;
> >> +               uint32_t u32;
> >> +               uint8_t  u128[8];
> >> +       } value;
> >> +} __attribute__((packed));
> >> +
> >
> >
> > +   #define MSFT_COND_LE_MONITOR_ADV_IRK                0x03
> >>
> >> +struct msft_le_monitor_adv_irk_type {
> >> +       uint8_t  irk[8];
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBCMD_LE_MONITOR_ADV_ADDR                0x04
> >
> > I think this is not a subcommand. Instead, it is a condition type. So we can rename this to something else, e.g. MSFT_COND_LE_MONITOR_ADV_ADDR.
> > Similarly, we'll have to define other three condition types as well for msft_le_monitor_adv_pattern_type, msft_le_monitor_adv_uuid_type and msft_le_monitor_adv_irk_type as mentioned above.
>
> Right I will fix it since the intent was to have it as conditions,
> thanks for reviewing.
>
> >> +struct msft_le_monitor_adv_addr {
> >> +       uint8_t  type;
> >> +       uint8_t  addr[6];
> >> +} __attribute__((packed));
> >> +
> >> +struct msft_cmd_le_monitor_adv {
> >> +       uint8_t  subcmd;
> >> +       int8_t   rssi_low;
> >> +       int8_t   rssi_high;
> >
> > Order should be:
> > +       int8_t   rssi_high;
> > +       int8_t   rssi_low;
> >>
> >> +       uint8_t  rssi_low_interval;
> >> +       uint8_t  rssi_period;
> >> +       uint8_t  type;
> >> +       uint8_t  data[];
> >> +} __attribute__((packed));
> >> +
> >> +struct msft_rsp_le_monitor_adv {
> >> +       uint8_t  status;
> >> +       uint8_t  subcmd;
> >> +       uint8_t  handle;
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBCMD_LE_CANCEL_MONITOR_ADV      0x04
> >> +
> >> +struct msft_cmd_le_cancel_monitor_adv {
> >> +       uint8_t  subcmd;
> >> +       uint8_t  handle;
> >> +} __attribute__((packed));
> >> +
> >> +struct msft_rsp_le_cancel_monitor_adv {
> >> +       uint8_t  status;
> >> +       uint8_t  subcmd;
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBCMD_LE_MONITOR_ADV_ENABLE      0x05
> >> +
> >> +struct msft_cmd_le_monitor_adv_enable {
> >> +       uint8_t  subcmd;
> >> +       uint8_t  enable;
> >> +} __attribute__((packed));
> >> +
> >> +struct msft_rsp_le_monitor_adv_enable {
> >> +       uint8_t  status;
> >> +       uint8_t  subcmd;
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBCMD_READ_ABS_RSSI              0x06
> >> +
> >> +struct msft_cmd_read_abs_rssi {
> >> +       uint8_t  subcmd;
> >> +       uint16_t handle;
> >> +} __attribute__((packed));
> >> +
> >> +struct msft_rsp_read_abs_rssi {
> >> +       uint8_t  status;
> >> +       uint8_t  subcmd;
> >> +       uint16_t handle;
> >> +       uint8_t  rssi;
>
> Ack.
>
> > 'int8_t rssi' instead of 'uint8_t rssi'
> >
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBEVT_RSSI                       0x01
> >> +
> >> +struct msft_evt_rssi {
> >> +       uint8_t  subevt;
> >> +       uint8_t  status;
> >> +       uint16_t handle;
> >> +       uint8_t  rssi;
> >
> > same as above - 'int8_t rssi' instead of 'uint8_t rssi'
>
> Ack.
>
> >
> >> +} __attribute__((packed));
> >> +
> >> +#define MSFT_SUBEVT_MONITOR_DEVICE             0x02
> >> +
> >> +struct msft_evt_monitor_device {
> >> +       uint8_t  subevt;
> >> +       uint8_t  addr_type;
> >> +       uint8_t  addr[6];
> >> +       uint8_t  handle;
> >> +       uint8_t  state;
> >> +} __attribute__((packed));
> >> +
> >>  struct vendor_ocf;
> >>  struct vendor_evt;
> >>
> >> --
> >> 2.31.1
> >>
> >
> > Rest of the changes look good to me.
> >
> > Thanks,
> > Manish.
>
>
>
> --
> Luiz Augusto von Dentz

Pushed after making the suggested changes.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-10-18 20:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15  5:09 [PATCH BlueZ 1/7] monitor: Add packet definitions for MSFT extension Luiz Augusto von Dentz
2021-10-15  5:09 ` [PATCH BlueZ 2/7] monitor: Make use of MSFT packet definitions Luiz Augusto von Dentz
2021-10-15  5:09 ` [PATCH BlueZ 3/7] vhci: Read the controller index Luiz Augusto von Dentz
2021-10-15  5:09 ` [PATCH BlueZ 4/7] vhci: Use io.h instead of mainloop.h Luiz Augusto von Dentz
2021-10-15  5:09 ` [PATCH BlueZ 5/7] hciemu: Use vhci_open to instanciate a vhci btdev Luiz Augusto von Dentz
2021-10-15  5:09 ` [PATCH BlueZ 6/7] vhci: Add functions to interface with debugfs Luiz Augusto von Dentz
2021-10-15  5:09 ` [PATCH BlueZ 7/7] mgmt-tester: Make use of vhci_set_force_suspend/vhci_set_force_wakeup Luiz Augusto von Dentz
2021-10-15  5:36 ` [BlueZ,1/7] monitor: Add packet definitions for MSFT extension bluez.test.bot
2021-10-15 20:10   ` Luiz Augusto von Dentz
     [not found] ` <CAGPPCLDFYFeiwfiyRX=6PquYYQ-Fp_LpN4Gw2745jyWzQKEBRQ@mail.gmail.com>
2021-10-18 16:24   ` [PATCH BlueZ 1/7] " Luiz Augusto von Dentz
2021-10-18 20:05     ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).