linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ 0/7] Support for Adapter's Default PHY Configuration
       [not found] <CGME20210722052737epcas5p1682a7f66d42470c3cdb8dd04160912ab@epcas5p1.samsung.com>
@ 2021-07-22  5:26 ` Ayush Garg
       [not found]   ` <CGME20210722052738epcas5p3b6074e49f088159e8282bba3457d56ba@epcas5p3.samsung.com>
                     ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Ayush Garg @ 2021-07-22  5:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: anupam.r, nitin.j

Hello Maintainers,

This patch set contains a proposed approach to enable
user to get/set adapter's default PHYs.

It will introduce two new adapter properties:
1. "SupportedPhyConfiguration": to get PHYs supported
by the adapter.
2. "PhyConfiguration": The user can use this property
to get/set the default adapter PHYs which will be used
in all subsequent connections.

These properties are based on "MGMT_OP_SET_PHY_CONFIGURATION"
and "MGMT_OP_GET_PHY_CONFIGURATION" MGMT commands.

Ayush Garg (7):
  doc/adapter-api: Add SupportedPhyConfiguration property
  doc/adapter-api: Add PhyConfiguration property
  adapter: Add support for get/set phy configuration property
  adapter: Add support for the get supported phy property
  adapter: Add support for PHY Configuration Changed event
  client: Add support for get/set PHY configuration in bluetoothctl
  adapter: Save PHY Configuration in storage and read it at init

 client/main.c       |  39 +++++
 doc/adapter-api.txt |  35 +++++
 src/adapter.c       | 355 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 429 insertions(+)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 1/7] doc/adapter-api: Add SupportedPhyConfiguration property
       [not found]   ` <CGME20210722052738epcas5p3b6074e49f088159e8282bba3457d56ba@epcas5p3.samsung.com>
@ 2021-07-22  5:26     ` Ayush Garg
  2021-07-22  6:45       ` Support for Adapter's Default PHY Configuration bluez.test.bot
  2021-07-22 14:46       ` [PATCH BlueZ 1/7] doc/adapter-api: Add SupportedPhyConfiguration property Marcel Holtmann
  0 siblings, 2 replies; 11+ messages in thread
From: Ayush Garg @ 2021-07-22  5:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: anupam.r, nitin.j

This change add a new property to retrieve the PHYs
supported by the controller.

Reviewed-by: Anupam Roy <anupam.r@samsung.com>
---
 doc/adapter-api.txt | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index 464434a81..25e370d75 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -335,3 +335,24 @@ Properties	string Address [readonly]
 				"peripheral": Supports the peripheral role.
 				"central-peripheral": Supports both roles
 						      concurrently.
+
+		array{string} SupportedPhyConfiguration [readonly]
+
+			List of PHYs supported by the controller.
+
+			Possible values:
+				"BR1M1SLOT"
+				"BR1M3SLOT"
+				"BR1M5SLOT"
+				"EDR2M1SLOT"
+				"EDR2M3SLOT"
+				"EDR2M5SLOT"
+				"EDR3M1SLOT"
+				"EDR3M3SLOT"
+				"EDR3M5SLOT"
+				"LE1MTX"
+				"LE1MRX"
+				"LE2MTX"
+				"LE2MRX"
+				"LECODEDTX"
+				"LECODEDRX"
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 2/7] doc/adapter-api: Add PhyConfiguration property
       [not found]   ` <CGME20210722052739epcas5p4412d38b394f51ab195c77c887f0a0aaf@epcas5p4.samsung.com>
@ 2021-07-22  5:26     ` Ayush Garg
  2021-07-22 17:20       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 11+ messages in thread
From: Ayush Garg @ 2021-07-22  5:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: anupam.r, nitin.j

This change add a new property which will allow the user
to get the controller's current selected PHYs and to
change it at any point of time.

Reviewed-by: Anupam Roy <anupam.r@samsung.com>
---
 doc/adapter-api.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index 25e370d75..97af8e70b 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -356,3 +356,17 @@ Properties	string Address [readonly]
 				"LE2MRX"
 				"LECODEDTX"
 				"LECODEDRX"
+
+		array{string} PhyConfiguration [readwrite]
+
+			Set/Get the default PHYs to the controller.
+
+			These PHYs will be used for all the
+			subsequent scanning and connection initiation.
+
+			The value of this property is persistent. After
+			restart or unplugging of the adapter it will continue
+			with the last set PHYs.
+
+			Refer SupportedPhyConfiguration property
+			for the possible values.
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 3/7] adapter: Add support for get/set phy configuration property
       [not found]   ` <CGME20210722052740epcas5p1fce869a24434f376d111e9e5d4b208eb@epcas5p1.samsung.com>
@ 2021-07-22  5:26     ` Ayush Garg
  0 siblings, 0 replies; 11+ messages in thread
From: Ayush Garg @ 2021-07-22  5:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: anupam.r, nitin.j

This change introduces a new adapter property(PhyConfiguration)
which will be used to read and set controller's PHY.
This property is based upon MGMT_OP_GET_PHY_CONFIGURATION
and MGMT_OP_SET_PHY_CONFIGURATION operations.

Reviewed-by: Anupam Roy <anupam.r@samsung.com>
---
 src/adapter.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 282 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index 12e4ff5c0..fd4c654dc 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -285,6 +285,11 @@ struct btd_adapter {
 
 	bool le_simult_roles_supported;
 	bool quality_report_supported;
+
+	uint32_t supported_phys;
+	uint32_t configurable_phys;
+	uint32_t selected_phys;
+	uint32_t pending_phys;
 };
 
 typedef enum {
@@ -3245,6 +3250,237 @@ static gboolean property_get_roles(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static struct phys_config {
+	uint32_t flag;
+	const char *name;
+} phys_str[] = {
+	{ MGMT_PHY_BR_1M_1SLOT, "BR1M1SLOT" },
+	{ MGMT_PHY_BR_1M_3SLOT, "BR1M3SLOT" },
+	{ MGMT_PHY_BR_1M_5SLOT, "BR1M5SLOT" },
+	{ MGMT_PHY_EDR_2M_1SLOT, "EDR2M1SLOT" },
+	{ MGMT_PHY_EDR_2M_3SLOT, "EDR2M3SLOT" },
+	{ MGMT_PHY_EDR_2M_5SLOT, "EDR2M5SLOT" },
+	{ MGMT_PHY_EDR_3M_1SLOT, "EDR3M1SLOT" },
+	{ MGMT_PHY_EDR_3M_3SLOT, "EDR3M3SLOT" },
+	{ MGMT_PHY_EDR_3M_5SLOT, "EDR3M5SLOT" },
+	{ MGMT_PHY_LE_1M_TX, "LE1MTX" },
+	{ MGMT_PHY_LE_1M_RX, "LE1MRX" },
+	{ MGMT_PHY_LE_2M_TX, "LE2MTX" },
+	{ MGMT_PHY_LE_2M_RX, "LE2MRX" },
+	{ MGMT_PHY_LE_CODED_TX, "LECODEDTX" },
+	{ MGMT_PHY_LE_CODED_RX, "LECODEDRX" }
+};
+
+static void append_phys_str(DBusMessageIter *array, uint32_t phys)
+{
+	unsigned int i;
+
+	for (i = 0; i < NELEM(phys_str); i++) {
+		if (phys & phys_str[i].flag)
+			dbus_message_iter_append_basic(array, DBUS_TYPE_STRING,
+						&phys_str[i].name);
+	}
+}
+
+static bool parse_phys_str(DBusMessageIter *array, uint32_t *phys)
+{
+	const char *str;
+	unsigned int i;
+
+	*phys = 0;
+
+	do {
+		if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_STRING)
+			return false;
+
+		dbus_message_iter_get_basic(array, &str);
+
+		for (i = 0; i < NELEM(phys_str); i++) {
+			if (!strcmp(str, phys_str[i].name)) {
+				*phys |= phys_str[i].flag;
+				break;
+			}
+		}
+
+		if (i == NELEM(phys_str))
+			return false;
+	} while (dbus_message_iter_next(array));
+
+	return true;
+}
+
+static void set_default_phy_complete(uint8_t status, uint16_t length,
+					const void *param, void *user_data)
+{
+	struct property_set_data *data = user_data;
+	struct btd_adapter *adapter = data->adapter;
+
+	if (status != MGMT_STATUS_SUCCESS) {
+		adapter->pending_phys = 0;
+		btd_error(adapter->dev_id,
+				"Failed to set PHY configuration: %s (0x%02x)",
+						mgmt_errstr(status), status);
+		g_dbus_pending_property_error(data->id, ERROR_INTERFACE ".Failed",
+							mgmt_errstr(status));
+		return;
+	}
+
+	/* The default phys are successfully set. */
+	btd_info(adapter->dev_id, "PHY configuration successfully set");
+
+	adapter->selected_phys = adapter->pending_phys;
+	adapter->pending_phys = 0;
+
+	g_dbus_pending_property_success(data->id);
+
+	g_dbus_emit_property_changed(dbus_conn, adapter->path,
+					ADAPTER_INTERFACE, "PhyConfiguration");
+}
+
+static int set_default_phy(struct btd_adapter *adapter, uint32_t phys,
+			GDBusPendingPropertySet id)
+{
+	struct mgmt_cp_set_phy_confguration cp;
+	struct property_set_data *data;
+	uint32_t unconfigure_phys;
+
+	if (!phys) {
+		btd_error(adapter->dev_id,
+			"Set PHY configuration failed: No supplied phy(s)");
+		return -EINVAL;
+	}
+
+	if (phys & ~(adapter->supported_phys)) {
+		btd_error(adapter->dev_id,
+			"Set PHY configuration failed: supplied phy(s) is not supported");
+		return -EINVAL;
+	}
+
+	if (phys == adapter->selected_phys) {
+		DBG("PHYs are already set [0x%x]", phys);
+		g_dbus_pending_property_success(id);
+		return 0;
+	}
+
+	unconfigure_phys = adapter->supported_phys & ~(adapter->configurable_phys);
+
+	if ((phys & unconfigure_phys) != unconfigure_phys) {
+		btd_error(adapter->dev_id,
+			"Set PHY configuration failed: supplied phy(s) must include PHYs %u",
+			unconfigure_phys);
+		return -EINVAL;
+	}
+
+	if (adapter->pending_phys) {
+		btd_error(adapter->dev_id,
+			"Set PHY configuration failed: Operation in progress");
+		return -EINPROGRESS;
+	}
+
+	adapter->pending_phys = phys;
+
+	cp.selected_phys = cpu_to_le32(phys);
+
+	data = g_try_new0(struct property_set_data, 1);
+	if (!data)
+		goto failed;
+
+	data->adapter = adapter;
+	data->id = id;
+
+	DBG("sending set phy configuration command for index %u", adapter->dev_id);
+
+	if (mgmt_send(adapter->mgmt, MGMT_OP_SET_PHY_CONFIGURATION,
+				adapter->dev_id, sizeof(cp), &cp,
+				set_default_phy_complete, data, g_free) > 0)
+		return 0;
+
+	g_free(data);
+
+failed:
+	btd_error(adapter->dev_id, "Failed to set PHY configuration for index %u",
+							adapter->dev_id);
+	adapter->pending_phys = 0;
+
+	return -EIO;
+}
+
+static gboolean property_get_phy_configuration(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
+
+	append_phys_str(&array, adapter->selected_phys);
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static void property_set_phy_configuration(
+				const GDBusPropertyTable *property,
+				DBusMessageIter *iter,
+				GDBusPendingPropertySet id, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	DBusMessageIter array;
+	uint32_t phys;
+	int ret;
+
+	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY) {
+		ret = -EINVAL;
+		goto failed;
+	}
+
+	dbus_message_iter_recurse(iter, &array);
+
+	if (!(adapter->supported_settings & MGMT_SETTING_PHY_CONFIGURATION)) {
+		ret = -ENOTSUP;
+		goto failed;
+	}
+
+	if (!parse_phys_str(&array, &phys)) {
+		ret = -EINVAL;
+		goto failed;
+	}
+
+	ret = set_default_phy(adapter, phys, id);
+
+	/*
+	 * gdbus_pending_property_success event will be sent when success status
+	 * will be received from mgmt.
+	 */
+	if (!ret)
+		return;
+
+failed:
+	switch (-ret) {
+	case EINVAL:
+		g_dbus_pending_property_error(id,
+					ERROR_INTERFACE ".InvalidArguments",
+					"Invalid arguments in method call");
+		break;
+	case ENOTSUP:
+		g_dbus_pending_property_error(id,
+					ERROR_INTERFACE ".NotSupported",
+					"Operation is not supported");
+		break;
+	case EINPROGRESS:
+		g_dbus_pending_property_error(id,
+					ERROR_INTERFACE ".InProgress",
+					"Operation is in progress");
+		break;
+	default:
+		g_dbus_pending_property_error(id, ERROR_INTERFACE ".Failed",
+							strerror(-ret));
+		break;
+	}
+}
+
 static DBusMessage *remove_device(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
@@ -3517,6 +3753,8 @@ static const GDBusPropertyTable adapter_properties[] = {
 	{ "Modalias", "s", property_get_modalias, NULL,
 					property_exists_modalias },
 	{ "Roles", "as", property_get_roles },
+	{ "PhyConfiguration", "as", property_get_phy_configuration,
+					property_set_phy_configuration },
 	{ }
 };
 
@@ -9389,6 +9627,43 @@ static void read_exp_features(struct btd_adapter *adapter)
 	btd_error(adapter->dev_id, "Failed to read exp features info");
 }
 
+static void read_phy_configuration_resp(uint8_t status, uint16_t length,
+					const void *param, void *user_data)
+{
+	const struct mgmt_rp_get_phy_confguration *rp = param;
+	struct btd_adapter *adapter = user_data;
+
+	if (status != MGMT_STATUS_SUCCESS) {
+		btd_error(adapter->dev_id,
+				"Failed to get PHY configuration info: %s (0x%02x)",
+				mgmt_errstr(status), status);
+		return;
+	}
+
+	if (length < sizeof(*rp)) {
+		btd_error(adapter->dev_id, "Response too small");
+		return;
+	}
+
+	adapter->supported_phys = get_le32(&rp->supported_phys);
+	adapter->configurable_phys = get_le32(&rp->configurable_phys);
+	adapter->selected_phys = get_le32(&rp->selected_phys);
+
+	DBG("Supported phys: [0x%x]", adapter->supported_phys);
+	DBG("Configurable phys: [0x%x]", adapter->configurable_phys);
+	DBG("Selected phys: [0x%x]", adapter->selected_phys);
+}
+
+static void read_phy_configuration(struct btd_adapter *adapter)
+{
+	if (mgmt_send(adapter->mgmt, MGMT_OP_GET_PHY_CONFIGURATION,
+			adapter->dev_id, 0, NULL, read_phy_configuration_resp,
+			adapter, NULL) > 0)
+		return;
+
+	btd_error(adapter->dev_id, "Failed to read phy configuration info");
+}
+
 static void read_info_complete(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {
@@ -9498,6 +9773,13 @@ static void read_info_complete(uint8_t status, uint16_t length,
 			(missing_settings & MGMT_SETTING_FAST_CONNECTABLE))
 		set_mode(adapter, MGMT_OP_SET_FAST_CONNECTABLE, 0x01);
 
+	if (btd_opts.experimental &&
+			btd_has_kernel_features(KERNEL_EXP_FEATURES))
+		read_exp_features(adapter);
+
+	if (adapter->supported_settings & MGMT_SETTING_PHY_CONFIGURATION)
+		read_phy_configuration(adapter);
+
 	err = adapter_register(adapter);
 	if (err < 0) {
 		btd_error(adapter->dev_id, "Unable to register new adapter");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 4/7] adapter: Add support for the get supported phy property
       [not found]   ` <CGME20210722052741epcas5p2cb3cbad65a301b0ec0cf1780c0920103@epcas5p2.samsung.com>
@ 2021-07-22  5:26     ` Ayush Garg
  0 siblings, 0 replies; 11+ messages in thread
From: Ayush Garg @ 2021-07-22  5:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: anupam.r, nitin.j

This change introduces a new adapter property which
will allow user to get the adapter supported PHYs.

Reviewed-by: Anupam Roy <anupam.r@samsung.com>
---
 src/adapter.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index fd4c654dc..c64a5333d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3405,6 +3405,22 @@ failed:
 	return -EIO;
 }
 
+static gboolean property_get_supported_phy(
+					const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "s", &array);
+
+	append_phys_str(&array, adapter->supported_phys);
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
 static gboolean property_get_phy_configuration(
 					const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *user_data)
@@ -3753,6 +3769,8 @@ static const GDBusPropertyTable adapter_properties[] = {
 	{ "Modalias", "s", property_get_modalias, NULL,
 					property_exists_modalias },
 	{ "Roles", "as", property_get_roles },
+	{ "SupportedPhyConfiguration", "as", property_get_supported_phy,
+					NULL},
 	{ "PhyConfiguration", "as", property_get_phy_configuration,
 					property_set_phy_configuration },
 	{ }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 5/7] adapter: Add support for PHY Configuration Changed event
       [not found]   ` <CGME20210722052742epcas5p1b53db2cd68028170a86f291bd052bf14@epcas5p1.samsung.com>
@ 2021-07-22  5:26     ` Ayush Garg
  0 siblings, 0 replies; 11+ messages in thread
From: Ayush Garg @ 2021-07-22  5:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: anupam.r, nitin.j

This change will subscribe the MGTM PHY Configuration
Changed event. This event will come whenever the controller
PHYs changed. Upon receiving the event, it will notify the
user by emitting "PhyConfiguration" property changed event.

Reviewed-by: Anupam Roy <anupam.r@samsung.com>
---
 src/adapter.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/adapter.c b/src/adapter.c
index c64a5333d..84dd2e5bd 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -9682,6 +9682,28 @@ static void read_phy_configuration(struct btd_adapter *adapter)
 	btd_error(adapter->dev_id, "Failed to read phy configuration info");
 }
 
+static void phy_configuration_changed_callback(uint16_t index,
+					uint16_t length, const void *param,
+					void *user_data)
+{
+	const struct mgmt_ev_phy_configuration_changed *ev = param;
+	struct btd_adapter *adapter = user_data;
+
+	if (length < sizeof(*ev)) {
+		btd_error(adapter->dev_id,
+				"Too small PHY configuration changed event");
+		return;
+	}
+
+	adapter->selected_phys = get_le32(&ev->selected_phys);
+	info("PHYs changed, New PHYs [0x%x]", adapter->selected_phys);
+
+	adapter->pending_phys = 0;
+
+	g_dbus_emit_property_changed(dbus_conn, adapter->path,
+					ADAPTER_INTERFACE, "PhyConfiguration");
+}
+
 static void read_info_complete(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {
@@ -9917,6 +9939,11 @@ static void read_info_complete(uint8_t status, uint16_t length,
 						controller_resume_callback,
 						adapter, NULL);
 
+	mgmt_register(adapter->mgmt, MGMT_EV_PHY_CONFIGURATION_CHANGED,
+						adapter->dev_id,
+						phy_configuration_changed_callback,
+						adapter, NULL);
+
 	set_dev_class(adapter);
 
 	set_name(adapter, btd_adapter_get_name(adapter));
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 6/7] client: Add support for get/set PHY configuration in bluetoothctl
       [not found]   ` <CGME20210722052743epcas5p34af6f55c11db2e8909c2730fae7e1cac@epcas5p3.samsung.com>
@ 2021-07-22  5:26     ` Ayush Garg
  0 siblings, 0 replies; 11+ messages in thread
From: Ayush Garg @ 2021-07-22  5:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: anupam.r, nitin.j

Verification logs based on BT 5.0 controller
==============================================
[bluetooth] default-phy LE1MTX LE2MTX LE1MRX LE2MRX LECODEDTX LECODEDRX
[bluetooth]
Changing PHY Configuration succeeded
Controller D6:C9:16:E8:6A:D9 PhyConfiguration: LE1MTX
Controller D6:C9:16:E8:6A:D9 PhyConfiguration: LE1MRX
Controller D6:C9:16:E8:6A:D9 PhyConfiguration: LE2MTX
Controller D6:C9:16:E8:6A:D9 PhyConfiguration: LE2MRX
Controller D6:C9:16:E8:6A:D9 PhyConfiguration: LECODEDTX
Controller D6:C9:16:E8:6A:D9 PhyConfiguration: LECODEDRX

Verification logs based on BT 4.0 controller
=============================================
[bluetooth]# default-phy
	Supported phys: BR1M1SLOT
	Supported phys: BR1M3SLOT
	Supported phys: BR1M5SLOT
	Supported phys: EDR2M1SLOT
	Supported phys: EDR2M3SLOT
	Supported phys: EDR2M5SLOT
	Supported phys: EDR3M1SLOT
	Supported phys: EDR3M3SLOT
	Supported phys: EDR3M5SLOT
	Supported phys: LE1MTX
	Supported phys: LE1MRX
	Selected phys: BR1M1SLOT
	Selected phys: LE1MTX
	Selected phys: LE1MRX
[bluetooth]# default-phy BR1M1SLOT LE1MTX LE1MRX BR1M3SLOT
Changing PHY Configuration succeeded
[CHG] Controller 00:1A:7D:DA:71:14 PhyConfiguration: BR1M1SLOT
[CHG] Controller 00:1A:7D:DA:71:14 PhyConfiguration: BR1M3SLOT
[CHG] Controller 00:1A:7D:DA:71:14 PhyConfiguration: LE1MTX
[CHG] Controller 00:1A:7D:DA:71:14 PhyConfiguration: LE1MRX

Reviewed-by: Anupam Roy <anupam.r@samsung.com>
---
 client/main.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/client/main.c b/client/main.c
index c1a62edb7..ab925769b 100644
--- a/client/main.c
+++ b/client/main.c
@@ -934,6 +934,8 @@ static void cmd_show(int argc, char *argv[])
 	print_property(adapter->proxy, "Modalias");
 	print_property(adapter->proxy, "Discovering");
 	print_property(adapter->proxy, "Roles");
+	print_property(adapter->proxy, "SupportedPhyConfiguration");
+	print_property(adapter->proxy, "PhyConfiguration");
 
 	if (adapter->ad_proxy) {
 		bt_shell_printf("Advertising Features:\n");
@@ -2038,6 +2040,37 @@ static void cmd_disconn(int argc, char *argv[])
 						proxy_address(proxy));
 }
 
+static void get_phy_configuration(void)
+{
+	print_property_with_label(default_ctrl->proxy,
+					"SupportedPhyConfiguration", "Supported phys");
+
+	print_property_with_label(default_ctrl->proxy, "PhyConfiguration",
+					"Selected phys");
+}
+
+static void cmd_phy_configuration(int argc, char *argv[])
+{
+	char **phys = NULL;
+	size_t phys_len = 0;
+
+	if (check_default_ctrl() == FALSE)
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+
+	if (argc < 2 || !strlen(argv[1]))
+		return get_phy_configuration();
+
+	phys = g_strdupv(&argv[1]);
+	phys_len = g_strv_length(phys);
+
+	g_dbus_proxy_set_property_array(default_ctrl->proxy,
+					"PhyConfiguration", DBUS_TYPE_STRING, phys,
+					phys_len, generic_callback, "PHY Configuration",
+					NULL);
+
+	g_strfreev(phys);
+}
+
 static void cmd_list_attributes(int argc, char *argv[])
 {
 	GDBusProxy *proxy;
@@ -3033,6 +3066,12 @@ static const struct bt_shell_menu main_menu = {
 							dev_generator },
 	{ "disconnect",   "[dev]",    cmd_disconn, "Disconnect device",
 							dev_generator },
+	{ "default-phy",	"[LE1MTX] [LE1MRX] [LE2MTX] [LE2MRX] "
+				"[LECODEDTX] [LECODEDRX] "
+				"[BR1M1SLOT] [BR1M3SLOT] [BR1M5SLOT] "
+				"[EDR2M1SLOT] [EDR2M3SLOT] [EDR2M5SLOT] "
+				"[EDR3M1SLOT] [EDR3M3SLOT] [EDR3M5SLOT]",
+		cmd_phy_configuration,		"Get/Set PHY Configuration" },
 	{ } },
 };
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 7/7] adapter: Save PHY Configuration in storage and read it at init
       [not found]   ` <CGME20210722052744epcas5p46996d48d12500c39b4b63dfcecd30d37@epcas5p4.samsung.com>
@ 2021-07-22  5:26     ` Ayush Garg
  0 siblings, 0 replies; 11+ messages in thread
From: Ayush Garg @ 2021-07-22  5:26 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: anupam.r, nitin.j

This change will save the current selected PHY configuration
in the storage when PHY Configuration Changed Event is
received. It will also load and set the stored PHY configuration
when controller initialized. Thus it enables the user to continue
with the last selected PHYs on controller restart.

Reviewed-by: Anupam Roy <anupam.r@samsung.com>
---
 src/adapter.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/adapter.c b/src/adapter.c
index 84dd2e5bd..3861ade8a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -288,7 +288,8 @@ struct btd_adapter {
 
 	uint32_t supported_phys;
 	uint32_t configurable_phys;
-	uint32_t selected_phys;
+	uint32_t selected_phys;		/* Current selected PHYs */
+	uint32_t stored_phys;		/* Stored last selected PHYs */
 	uint32_t pending_phys;
 };
 
@@ -508,6 +509,10 @@ static void store_adapter_info(struct btd_adapter *adapter)
 		g_key_file_set_string(key_file, "General", "Alias",
 							adapter->stored_alias);
 
+	if (adapter->stored_phys)
+		g_key_file_set_integer(key_file, "General", "PhyConfiguration",
+					adapter->stored_phys);
+
 	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/settings",
 					btd_adapter_get_storage_dir(adapter));
 
@@ -3329,10 +3334,13 @@ static void set_default_phy_complete(uint8_t status, uint16_t length,
 	btd_info(adapter->dev_id, "PHY configuration successfully set");
 
 	adapter->selected_phys = adapter->pending_phys;
+	adapter->stored_phys = adapter->pending_phys;
 	adapter->pending_phys = 0;
 
 	g_dbus_pending_property_success(data->id);
 
+	store_adapter_info(adapter);
+
 	g_dbus_emit_property_changed(dbus_conn, adapter->path,
 					ADAPTER_INTERFACE, "PhyConfiguration");
 }
@@ -6548,6 +6556,15 @@ static void load_config(struct btd_adapter *adapter)
 		gerr = NULL;
 	}
 
+	/* Get PHY Configuration */
+	adapter->stored_phys = g_key_file_get_integer(key_file,
+				"General", "PhyConfiguration", &gerr);
+	if (gerr) {
+		adapter->stored_phys = 0;
+		g_error_free(gerr);
+		gerr = NULL;
+	}
+
 	g_key_file_free(key_file);
 }
 
@@ -9670,6 +9687,13 @@ static void read_phy_configuration_resp(uint8_t status, uint16_t length,
 	DBG("Supported phys: [0x%x]", adapter->supported_phys);
 	DBG("Configurable phys: [0x%x]", adapter->configurable_phys);
 	DBG("Selected phys: [0x%x]", adapter->selected_phys);
+
+	/*
+	 * Set the default PHYs to the last selected PHYs on which the
+	 * controller was operating before shutting down.
+	 */
+	if (adapter->supported_settings & MGMT_SETTING_PHY_CONFIGURATION)
+		set_default_phy(adapter, adapter->stored_phys, -1);
 }
 
 static void read_phy_configuration(struct btd_adapter *adapter)
@@ -9696,10 +9720,14 @@ static void phy_configuration_changed_callback(uint16_t index,
 	}
 
 	adapter->selected_phys = get_le32(&ev->selected_phys);
+	adapter->stored_phys = adapter->selected_phys;
+
 	info("PHYs changed, New PHYs [0x%x]", adapter->selected_phys);
 
 	adapter->pending_phys = 0;
 
+	store_adapter_info(adapter);
+
 	g_dbus_emit_property_changed(dbus_conn, adapter->path,
 					ADAPTER_INTERFACE, "PhyConfiguration");
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: Support for Adapter's Default PHY Configuration
  2021-07-22  5:26     ` [PATCH BlueZ 1/7] doc/adapter-api: Add SupportedPhyConfiguration property Ayush Garg
@ 2021-07-22  6:45       ` bluez.test.bot
  2021-07-22 14:46       ` [PATCH BlueZ 1/7] doc/adapter-api: Add SupportedPhyConfiguration property Marcel Holtmann
  1 sibling, 0 replies; 11+ messages in thread
From: bluez.test.bot @ 2021-07-22  6:45 UTC (permalink / raw)
  To: linux-bluetooth, ayush.garg

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

---Test result---

Test Summary:
CheckPatch                    FAIL      1.86 seconds
GitLint                       FAIL      0.84 seconds
Prep - Setup ELL              PASS      47.00 seconds
Build - Prep                  PASS      0.10 seconds
Build - Configure             PASS      8.33 seconds
Build - Make                  PASS      203.42 seconds
Make Check                    PASS      9.65 seconds
Make Distcheck                PASS      241.66 seconds
Build w/ext ELL - Configure   PASS      8.32 seconds
Build w/ext ELL - Make        PASS      191.62 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
adapter: Add support for get/set phy configuration property
WARNING:LONG_LINE_STRING: line length of 82 exceeds 80 columns
#103: FILE: src/adapter.c:3323:
+		g_dbus_pending_property_error(data->id, ERROR_INTERFACE ".Failed",

WARNING:LONG_LINE: line length of 83 exceeds 80 columns
#145: FILE: src/adapter.c:3365:
+	unconfigure_phys = adapter->supported_phys & ~(adapter->configurable_phys);

WARNING:LONG_LINE: line length of 83 exceeds 80 columns
#171: FILE: src/adapter.c:3391:
+	DBG("sending set phy configuration command for index %u", adapter->dev_id);

WARNING:LONG_LINE_STRING: line length of 82 exceeds 80 columns
#181: FILE: src/adapter.c:3401:
+	btd_error(adapter->dev_id, "Failed to set PHY configuration for index %u",

ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#189: FILE: src/adapter.c:3409:
+					const GDBusPropertyTable *property,
 					                         ^

ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#205: FILE: src/adapter.c:3425:
+				const GDBusPropertyTable *property,
 				                         ^

- total: 2 errors, 4 warnings, 312 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.

"[PATCH] adapter: Add support for get/set phy configuration property" 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.

adapter: Add support for the get supported phy property
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#20: FILE: src/adapter.c:3409:
+					const GDBusPropertyTable *property,
 					                         ^

- total: 1 errors, 0 warnings, 30 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.

"[PATCH] adapter: Add support for the get supported phy property" 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.

adapter: Add support for PHY Configuration Changed event
WARNING:LONG_LINE: line length of 83 exceeds 80 columns
#52: FILE: src/adapter.c:9944:
+						phy_configuration_changed_callback,

- total: 0 errors, 1 warnings, 39 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.

"[PATCH] adapter: Add support for PHY Configuration Changed event" 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.

client: Add support for get/set PHY configuration in bluetoothctl
WARNING:LONG_LINE_STRING: line length of 87 exceeds 80 columns
#65: FILE: client/main.c:2046:
+					"SupportedPhyConfiguration", "Supported phys");

WARNING:LONG_LINE: line length of 83 exceeds 80 columns
#86: FILE: client/main.c:2067:
+					"PhyConfiguration", DBUS_TYPE_STRING, phys,

WARNING:LONG_LINE_STRING: line length of 88 exceeds 80 columns
#87: FILE: client/main.c:2068:
+					phys_len, generic_callback, "PHY Configuration",

- total: 0 errors, 3 warnings, 57 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.

"[PATCH] client: Add support for get/set PHY configuration in" 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:
client: Add support for get/set PHY configuration in bluetoothctl
18: B3 Line contains hard tab characters (\t): "	Supported phys: BR1M1SLOT"
19: B3 Line contains hard tab characters (\t): "	Supported phys: BR1M3SLOT"
20: B3 Line contains hard tab characters (\t): "	Supported phys: BR1M5SLOT"
21: B3 Line contains hard tab characters (\t): "	Supported phys: EDR2M1SLOT"
22: B3 Line contains hard tab characters (\t): "	Supported phys: EDR2M3SLOT"
23: B3 Line contains hard tab characters (\t): "	Supported phys: EDR2M5SLOT"
24: B3 Line contains hard tab characters (\t): "	Supported phys: EDR3M1SLOT"
25: B3 Line contains hard tab characters (\t): "	Supported phys: EDR3M3SLOT"
26: B3 Line contains hard tab characters (\t): "	Supported phys: EDR3M5SLOT"
27: B3 Line contains hard tab characters (\t): "	Supported phys: LE1MTX"
28: B3 Line contains hard tab characters (\t): "	Supported phys: LE1MRX"
29: B3 Line contains hard tab characters (\t): "	Selected phys: BR1M1SLOT"
30: B3 Line contains hard tab characters (\t): "	Selected phys: LE1MTX"
31: B3 Line contains hard tab characters (\t): "	Selected phys: LE1MRX"


##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 1/7] doc/adapter-api: Add SupportedPhyConfiguration property
  2021-07-22  5:26     ` [PATCH BlueZ 1/7] doc/adapter-api: Add SupportedPhyConfiguration property Ayush Garg
  2021-07-22  6:45       ` Support for Adapter's Default PHY Configuration bluez.test.bot
@ 2021-07-22 14:46       ` Marcel Holtmann
  1 sibling, 0 replies; 11+ messages in thread
From: Marcel Holtmann @ 2021-07-22 14:46 UTC (permalink / raw)
  To: Ayush Garg; +Cc: linux-bluetooth, anupam.r, nitin.j

Hi Aysush,

> This change add a new property to retrieve the PHYs
> supported by the controller.
> 
> Reviewed-by: Anupam Roy <anupam.r@samsung.com>
> ---
> doc/adapter-api.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> 
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index 464434a81..25e370d75 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -335,3 +335,24 @@ Properties	string Address [readonly]
> 				"peripheral": Supports the peripheral role.
> 				"central-peripheral": Supports both roles
> 						      concurrently.
> +
> +		array{string} SupportedPhyConfiguration [readonly]
> +
> +			List of PHYs supported by the controller.
> +
> +			Possible values:
> +				"BR1M1SLOT"
> +				"BR1M3SLOT"
> +				"BR1M5SLOT"
> +				"EDR2M1SLOT"
> +				"EDR2M3SLOT"
> +				"EDR2M5SLOT"
> +				"EDR3M1SLOT"
> +				"EDR3M3SLOT"
> +				"EDR3M5SLOT"
> +				"LE1MTX"
> +				"LE1MRX"
> +				"LE2MTX"
> +				"LE2MRX"
> +				"LECODEDTX"
> +				"LECODEDRX"

we don’t do string constants like this in D-Bus API. They are lowercase and verbose “br-1m-1slot” etc. at least.

Regards

Marcel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 2/7] doc/adapter-api: Add PhyConfiguration property
  2021-07-22  5:26     ` [PATCH BlueZ 2/7] doc/adapter-api: Add PhyConfiguration property Ayush Garg
@ 2021-07-22 17:20       ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2021-07-22 17:20 UTC (permalink / raw)
  To: Ayush Garg; +Cc: linux-bluetooth, Anupam Roy, nitin.j

Hi Ayush,

On Wed, Jul 21, 2021 at 11:02 PM Ayush Garg <ayush.garg@samsung.com> wrote:
>
> This change add a new property which will allow the user
> to get the controller's current selected PHYs and to
> change it at any point of time.
>
> Reviewed-by: Anupam Roy <anupam.r@samsung.com>
> ---
>  doc/adapter-api.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index 25e370d75..97af8e70b 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -356,3 +356,17 @@ Properties string Address [readonly]
>                                 "LE2MRX"
>                                 "LECODEDTX"
>                                 "LECODEDRX"
> +
> +               array{string} PhyConfiguration [readwrite]
> +
> +                       Set/Get the default PHYs to the controller.
> +
> +                       These PHYs will be used for all the
> +                       subsequent scanning and connection initiation.
> +
> +                       The value of this property is persistent. After
> +                       restart or unplugging of the adapter it will continue
> +                       with the last set PHYs.
> +
> +                       Refer SupportedPhyConfiguration property
> +                       for the possible values.
> --
> 2.17.1

You should probably have create a github issue to discuss the API
first, anyway for the adapter I don't think we should be doing the PHY
configuration at runtime so it would probably have been better to have
such configuration as part of main.conf, and Ive done this with a
exclude logic so one can disable PHYs that it doesn't want the system
to use.


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-07-22 17:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210722052737epcas5p1682a7f66d42470c3cdb8dd04160912ab@epcas5p1.samsung.com>
2021-07-22  5:26 ` [PATCH BlueZ 0/7] Support for Adapter's Default PHY Configuration Ayush Garg
     [not found]   ` <CGME20210722052738epcas5p3b6074e49f088159e8282bba3457d56ba@epcas5p3.samsung.com>
2021-07-22  5:26     ` [PATCH BlueZ 1/7] doc/adapter-api: Add SupportedPhyConfiguration property Ayush Garg
2021-07-22  6:45       ` Support for Adapter's Default PHY Configuration bluez.test.bot
2021-07-22 14:46       ` [PATCH BlueZ 1/7] doc/adapter-api: Add SupportedPhyConfiguration property Marcel Holtmann
     [not found]   ` <CGME20210722052739epcas5p4412d38b394f51ab195c77c887f0a0aaf@epcas5p4.samsung.com>
2021-07-22  5:26     ` [PATCH BlueZ 2/7] doc/adapter-api: Add PhyConfiguration property Ayush Garg
2021-07-22 17:20       ` Luiz Augusto von Dentz
     [not found]   ` <CGME20210722052740epcas5p1fce869a24434f376d111e9e5d4b208eb@epcas5p1.samsung.com>
2021-07-22  5:26     ` [PATCH BlueZ 3/7] adapter: Add support for get/set phy configuration property Ayush Garg
     [not found]   ` <CGME20210722052741epcas5p2cb3cbad65a301b0ec0cf1780c0920103@epcas5p2.samsung.com>
2021-07-22  5:26     ` [PATCH BlueZ 4/7] adapter: Add support for the get supported phy property Ayush Garg
     [not found]   ` <CGME20210722052742epcas5p1b53db2cd68028170a86f291bd052bf14@epcas5p1.samsung.com>
2021-07-22  5:26     ` [PATCH BlueZ 5/7] adapter: Add support for PHY Configuration Changed event Ayush Garg
     [not found]   ` <CGME20210722052743epcas5p34af6f55c11db2e8909c2730fae7e1cac@epcas5p3.samsung.com>
2021-07-22  5:26     ` [PATCH BlueZ 6/7] client: Add support for get/set PHY configuration in bluetoothctl Ayush Garg
     [not found]   ` <CGME20210722052744epcas5p46996d48d12500c39b4b63dfcecd30d37@epcas5p4.samsung.com>
2021-07-22  5:26     ` [PATCH BlueZ 7/7] adapter: Save PHY Configuration in storage and read it at init Ayush Garg

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).