diff mbox series

[v3] Bluetooth: le_simult_central_peripheral experimental feature

Message ID 20200706154449.3828193-1-alainm@chromium.org (mailing list archive)
State Superseded
Headers show
Series [v3] Bluetooth: le_simult_central_peripheral experimental feature | expand

Commit Message

Alain Michaud July 6, 2020, 3:44 p.m. UTC
This patch adds an le_simult_central_peripheral features which allows a
clients to determine if the controller is able to support peripheral and
central connections separately and at the same time.

Signed-off-by: Alain Michaud <alainm@chromium.org>
---

Changes in v3:
 - Back to the original design of V1 and integrated Marcel's feedback.

Changes in v2:
 - Slight change of design based on offline feedback

 net/bluetooth/mgmt.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Comments

Alain Michaud July 7, 2020, 2:20 a.m. UTC | #1
Here is the userspace change associated with this change:
https://patchwork.kernel.org/project/bluetooth/list/?series=313771

On Mon, Jul 6, 2020 at 11:44 AM Alain Michaud <alainm@chromium.org> wrote:
>
> This patch adds an le_simult_central_peripheral features which allows a
> clients to determine if the controller is able to support peripheral and
> central connections separately and at the same time.
>
> Signed-off-by: Alain Michaud <alainm@chromium.org>
> ---
>
> Changes in v3:
>  - Back to the original design of V1 and integrated Marcel's feedback.
>
> Changes in v2:
>  - Slight change of design based on offline feedback
>
>  net/bluetooth/mgmt.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 5e9b9728eeac..b34e6f3c07ec 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3753,12 +3753,19 @@ static const u8 debug_uuid[16] = {
>  };
>  #endif
>
> +/* 671b10b5-42c0-4696-9227-eb28d1b049d6 */
> +static const u8 le_simult_central_peripheral[16] = {
> +       0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, 0x27, 0x92,
> +       0x96, 0x46, 0xc0, 0x42, 0xb5, 0x10, 0x1b, 0x67,
> +};
> +
>  static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
>                                   void *data, u16 data_len)
>  {
> -       char buf[42];
> +       char buf[44];
>         struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
>         u16 idx = 0;
> +       u32 flags;
>
>         bt_dev_dbg(hdev, "sock %p", sk);
>
> @@ -3766,7 +3773,7 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
>
>  #ifdef CONFIG_BT_FEATURE_DEBUG
>         if (!hdev) {
> -               u32 flags = bt_dbg_get() ? BIT(0) : 0;
> +               flags = bt_dbg_get() ? BIT(0) : 0;
>
>                 memcpy(rp->features[idx].uuid, debug_uuid, 16);
>                 rp->features[idx].flags = cpu_to_le32(flags);
> @@ -3774,6 +3781,19 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
>         }
>  #endif
>
> +       flags = 0;
> +       memcpy(rp->features[idx].uuid, le_simult_central_peripheral,
> +              sizeof(le_simult_central_peripheral));
> +
> +       if (hdev && test_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks) &&
> +           (hdev->le_states[4] & 0x08) &&      /* Central */
> +           (hdev->le_states[4] & 0x40) &&      /* Peripheral */
> +           (hdev->le_states[3] & 0x10)         /* Simultaneous */)
> +               flags |= BIT(0);
> +
> +       rp->features[idx].flags = cpu_to_le32(flags);
> +       idx++;
> +
>         rp->feature_count = cpu_to_le16(idx);
>
>         /* After reading the experimental features information, enable
> --
> 2.27.0.212.ge8ba1cc988-goog
>
Marcel Holtmann July 7, 2020, 6:43 a.m. UTC | #2
Hi Alain,

> This patch adds an le_simult_central_peripheral features which allows a
> clients to determine if the controller is able to support peripheral and
> central connections separately and at the same time.
> 
> Signed-off-by: Alain Michaud <alainm@chromium.org>
> ---
> 
> Changes in v3:
> - Back to the original design of V1 and integrated Marcel's feedback.
> 
> Changes in v2:
> - Slight change of design based on offline feedback
> 
> net/bluetooth/mgmt.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 5e9b9728eeac..b34e6f3c07ec 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3753,12 +3753,19 @@ static const u8 debug_uuid[16] = {
> };
> #endif
> 
> +/* 671b10b5-42c0-4696-9227-eb28d1b049d6 */
> +static const u8 le_simult_central_peripheral[16] = {
> +	0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, 0x27, 0x92,
> +	0x96, 0x46, 0xc0, 0x42, 0xb5, 0x10, 0x1b, 0x67,
> +};
> +
> static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> 				  void *data, u16 data_len)
> {
> -	char buf[42];
> +	char buf[44];
> 	struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
> 	u16 idx = 0;
> +	u32 flags;
> 
> 	bt_dev_dbg(hdev, "sock %p", sk);
> 
> @@ -3766,7 +3773,7 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> 
> #ifdef CONFIG_BT_FEATURE_DEBUG
> 	if (!hdev) {
> -		u32 flags = bt_dbg_get() ? BIT(0) : 0;
> +		flags = bt_dbg_get() ? BIT(0) : 0;

I was trying to contain the used variables in the scope here so that we don’t get funny kernel warnings about unused or uninitialized variables when options are not selected and we eventually add more here.

> 
> 		memcpy(rp->features[idx].uuid, debug_uuid, 16);
> 		rp->features[idx].flags = cpu_to_le32(flags);
> @@ -3774,6 +3781,19 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> 	}
> #endif
> 
> +	flags = 0;
> +	memcpy(rp->features[idx].uuid, le_simult_central_peripheral,
> +	       sizeof(le_simult_central_peripheral));
> +
> +	if (hdev && test_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks) &&
> +	    (hdev->le_states[4] & 0x08) &&	/* Central */
> +	    (hdev->le_states[4] & 0x40) &&	/* Peripheral */
> +	    (hdev->le_states[3] & 0x10)		/* Simultaneous */)
> +		flags |= BIT(0);
> +
> +	rp->features[idx].flags = cpu_to_le32(flags);
> +	idx++;
> +
> 	rp->feature_count = cpu_to_le16(idx);

You do actually need to wrap this in

	if (hdev) {
	}

so that a) this looks cleanly and b) this is only reported when the <controller-id> is provided.

The experimental feature command is special since it can be used per hdev or globally.

> 
> 	/* After reading the experimental features information, enable
> -- 
> 2.27.0.212.ge8ba1cc988-goog
> 

Regards

Marcel
diff mbox series

Patch

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5e9b9728eeac..b34e6f3c07ec 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3753,12 +3753,19 @@  static const u8 debug_uuid[16] = {
 };
 #endif
 
+/* 671b10b5-42c0-4696-9227-eb28d1b049d6 */
+static const u8 le_simult_central_peripheral[16] = {
+	0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, 0x27, 0x92,
+	0x96, 0x46, 0xc0, 0x42, 0xb5, 0x10, 0x1b, 0x67,
+};
+
 static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
 				  void *data, u16 data_len)
 {
-	char buf[42];
+	char buf[44];
 	struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
 	u16 idx = 0;
+	u32 flags;
 
 	bt_dev_dbg(hdev, "sock %p", sk);
 
@@ -3766,7 +3773,7 @@  static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
 
 #ifdef CONFIG_BT_FEATURE_DEBUG
 	if (!hdev) {
-		u32 flags = bt_dbg_get() ? BIT(0) : 0;
+		flags = bt_dbg_get() ? BIT(0) : 0;
 
 		memcpy(rp->features[idx].uuid, debug_uuid, 16);
 		rp->features[idx].flags = cpu_to_le32(flags);
@@ -3774,6 +3781,19 @@  static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
 	}
 #endif
 
+	flags = 0;
+	memcpy(rp->features[idx].uuid, le_simult_central_peripheral,
+	       sizeof(le_simult_central_peripheral));
+
+	if (hdev && test_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks) &&
+	    (hdev->le_states[4] & 0x08) &&	/* Central */
+	    (hdev->le_states[4] & 0x40) &&	/* Peripheral */
+	    (hdev->le_states[3] & 0x10)		/* Simultaneous */)
+		flags |= BIT(0);
+
+	rp->features[idx].flags = cpu_to_le32(flags);
+	idx++;
+
 	rp->feature_count = cpu_to_le16(idx);
 
 	/* After reading the experimental features information, enable