[PATCH 0/3 v2] LE New PHYs kernel interfaces

[PATCH 0/3 v2] LE New PHYs kernel interfaces

[Jaganath Kanakkassery]

These patches enables user to get the supported phys by the
controller and set preferred phy to be used in subsequent
scanning and connection initiation.

Also added flags to support user selection for phy to be
advertised.

v1: Applied Marcel's comments
v2: Applied Eramoto's comments

Jaganath Kanakkassery (3):
  doc/mgmt-api: Add support for Get PHY information command
  doc/mgmt-api: Add support for Set default Phy command
  doc/mgmt-api: Add advertising phys support to flags

 doc/mgmt-api.txt | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

-- 
2.7.4

[PATCH 1/3 v2] doc/mgmt-api: Add support for Get PHY information command

[Jaganath Kanakkassery]

---
 doc/mgmt-api.txt | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 8e7de14..69e84cd 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -2871,6 +2871,7 @@ Read Extended Controller Information Command
 		13	Privacy
 		14	Controller Configuration
 		15	Static Address
+		16  	PHY configuration
 
 	The EIR_Data field contains information about class of device,
 	local name and other values. Not all of them might be present. For
@@ -2916,6 +2917,33 @@ Set Appearance Command
 				Invalid Parameters
 				Invalid Index
 
+Get PHY Information Command
+==========================
+
+	Command Code:		0x0043
+	Controller Index:	<controller id>
+	Command Parameters:
+	Return Parameters:	Supported_phys (1 Octet)
+				Selected_phys (1 Octet)
+
+	This command is used to retrieve the supported PHYs and currently
+	selected PHYs.
+
+	Supported_phys and Selected_phys is a bitmask with the following bits.
+		0	LE 1M TX
+		1	LE 1M RX
+		2	LE 2M TX
+		3	LE 2M RX
+		4	LE CODED TX
+		5	LE CODED RX
+
+	LE 1M TX and LE 1M RX would be supported by default.
+
+	This command is only available for LE capable controllers.
+	It will return Not Supported otherwise.
+
+	Possible errors:	Not Supported
+				Invalid Index
 
 Command Complete Event
 ======================
-- 
2.7.4

Re: [PATCH 1/3 v2] doc/mgmt-api: Add support for Get PHY information command

[Johan Hedberg]

Hi Jaganath,

On Mon, Nov 20, 2017, Jaganath Kanakkassery wrote:
> +Get PHY Information Command
> +==========================
> +
> +	Command Code:		0x0043
> +	Controller Index:	<controller id>
> +	Command Parameters:
> +	Return Parameters:	Supported_phys (1 Octet)
> +				Selected_phys (1 Octet)

Shouldn't the command have "Default" in its name, to be consistent with
your second patch? If it's not the default that this is returning, then
shouldn't there be a reference to which connection it applies to
(assuming this is intended to map to the HCI_LE_Set_PHY command)?

What about the PHY_options HCI parameter, where is that planned to be
specified?

Johan

Re: [PATCH 1/3 v2] doc/mgmt-api: Add support for Get PHY information command

[Jaganath K]

Hi Johan,

> Hi Jaganath,
>
> On Mon, Nov 20, 2017, Jaganath Kanakkassery wrote:
>> +Get PHY Information Command
>> +==========================
>> +
>> +     Command Code:           0x0043
>> +     Controller Index:       <controller id>
>> +     Command Parameters:
>> +     Return Parameters:      Supported_phys (1 Octet)
>> +                             Selected_phys (1 Octet)
>
> Shouldn't the command have "Default" in its name, to be consistent with
> your second patch?

I did not add "Default" since the API also returns the Supported PHYs by the
controller. Will "Current_default_phys" be more intuitive than "Selected_phys"?

> If it's not the default that this is returning, then
> shouldn't there be a reference to which connection it applies to
> (assuming this is intended to map to the HCI_LE_Set_PHY command)?
>
> What about the PHY_options HCI parameter, where is that planned to be
> specified?
>

This API is not for any connection (and so not mapped to
HCI_LE_Set_PHY command).
For that we have L2CAP socket option for which i have already raised a patch
"[PATCH] Bluetooth: Define l2cap sock option for LE PHYs"
As you mentioned we would need to add support for PHY_options in that.

Thanks,
Jaganath

Re: [PATCH 1/3 v2] doc/mgmt-api: Add support for Get PHY information command

[Marcel Holtmann]

Hi Jaganath,

> ---
> doc/mgmt-api.txt | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
> 
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index 8e7de14..69e84cd 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -2871,6 +2871,7 @@ Read Extended Controller Information Command
> 		13	Privacy
> 		14	Controller Configuration
> 		15	Static Address
> +		16  	PHY configuration
> 
> 	The EIR_Data field contains information about class of device,
> 	local name and other values. Not all of them might be present. For
> @@ -2916,6 +2917,33 @@ Set Appearance Command
> 				Invalid Parameters
> 				Invalid Index
> 
> +Get PHY Information Command
> +==========================
> +
> +	Command Code:		0x0043
> +	Controller Index:	<controller id>
> +	Command Parameters:
> +	Return Parameters:	Supported_phys (1 Octet)
> +				Selected_phys (1 Octet)
> +
> +	This command is used to retrieve the supported PHYs and currently
> +	selected PHYs.
> +
> +	Supported_phys and Selected_phys is a bitmask with the following bits.
> +		0	LE 1M TX
> +		1	LE 1M RX
> +		2	LE 2M TX
> +		3	LE 2M RX
> +		4	LE CODED TX
> +		5	LE CODED RX
> +
> +	LE 1M TX and LE 1M RX would be supported by default.
> +
> +	This command is only available for LE capable controllers.
> +	It will return Not Supported otherwise.
> +
> +	Possible errors:	Not Supported
> +				Invalid Index

I think that I want to extend this command to also include BR/EDR PHY selection so that we have one command to drive all of these. So maybe the packet type selection of BR/EDR should be worked into this as well. Anyway, that should not stop us from implementing this for LE since BR/EDR can be added later.

My only question would be if we have TX/RX in the same octet or split into individual octets. In case we leave it like this, then we should make this a 2 octet value.

Regards

Marcel

[PATCH 2/3 v2] doc/mgmt-api: Add support for Set default Phy command

[Jaganath Kanakkassery]

---
 doc/mgmt-api.txt | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 69e84cd..b59bf0c 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -2940,9 +2940,47 @@ Get PHY Information Command
 	LE 1M TX and LE 1M RX would be supported by default.
 
 	This command is only available for LE capable controllers.
+        It will return Not Supported otherwise.
+
+        Possible errors:        Not Supported
+				Invalid Index
+
+Set Default PHY Command
+==========================
+
+	Command Code:		0x0044
+	Controller Index:	<controller id>
+	Command Parameters: 	Default_phy (1 Octet)
+	Return Parameters:	Current_phys (1 Octet)
+
+	This command is used to set the default PHYs to the controller.
+
+	This will be stored and used for all the subsequent scanning
+	and connection initiation.
+
+	Prior to this, Get PHY information Command should be called
+	to retrieve the supported PHYs by the controller. If default_phy
+	has PHY not supported by the controller (not there in supported_phys)
+	then Invalid Parameters will be returned.
+
+	This can be called at any point to change the preferred PHYs.
+
+	Default_phy is a bitmask with the following bits.
+		0	LE 1M TX
+		1	LE 1M RX
+		2	LE 2M TX
+		3	LE 2M RX
+		4	LE CODED TX
+		5	LE CODED RX
+
+	This command generates a Command Complete event on success
+	or a Command Status event on failure.
+
+	This command is only available for LE capable controllers.
 	It will return Not Supported otherwise.
 
 	Possible errors:	Not Supported
+				Invalid Parameters
 				Invalid Index
 
 Command Complete Event
-- 
2.7.4

Re: [PATCH 2/3 v2] doc/mgmt-api: Add support for Set default Phy command

[Marcel Holtmann]

Hi Jaganath,

> ---
> doc/mgmt-api.txt | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> 
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index 69e84cd..b59bf0c 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -2940,9 +2940,47 @@ Get PHY Information Command
> 	LE 1M TX and LE 1M RX would be supported by default.
> 
> 	This command is only available for LE capable controllers.
> +        It will return Not Supported otherwise.
> +
> +        Possible errors:        Not Supported
> +				Invalid Index
> +
> +Set Default PHY Command
> +==========================

I think the name “Set PHY Configuration” would be better.

> +
> +	Command Code:		0x0044
> +	Controller Index:	<controller id>
> +	Command Parameters: 	Default_phy (1 Octet)
> +	Return Parameters:	Current_phys (1 Octet)

Returning the current selected PHYs is rather useless here. It would be exactly the same value we put in. However what might be useful is an event that indicates the change of the PHY Configuration.

I would also name it Selected_PHYs to match up with the Get PHY Configuration command.

> +
> +	This command is used to set the default PHYs to the controller.
> +
> +	This will be stored and used for all the subsequent scanning
> +	and connection initiation.
> +
> +	Prior to this, Get PHY information Command should be called
> +	to retrieve the supported PHYs by the controller. If default_phy
> +	has PHY not supported by the controller (not there in supported_phys)
> +	then Invalid Parameters will be returned.

“The list of supported PHYs can be retrieved via the Get PHY Configuration command. Selecting unsupported PHYs will result in an Invalid Parameters error.”

> +
> +	This can be called at any point to change the preferred PHYs.
> +
> +	Default_phy is a bitmask with the following bits.
> +		0	LE 1M TX
> +		1	LE 1M RX
> +		2	LE 2M TX
> +		3	LE 2M RX
> +		4	LE CODED TX
> +		5	LE CODED RX
> +
> +	This command generates a Command Complete event on success
> +	or a Command Status event on failure.
> +
> +	This command is only available for LE capable controllers.
> 	It will return Not Supported otherwise.

Same comments from Get command applies. If we combine TX/RX into a single parameter, then we should use 2 octets. Also we might want to extend this to BR/EDR packet type selection.

Regards

Marcel

Re: [PATCH 2/3 v2] doc/mgmt-api: Add support for Set default Phy command

[Jaganath K]

Hi Marcel,

> Hi Jaganath,
>
>> ---
>> doc/mgmt-api.txt | 38 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>>
>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>> index 69e84cd..b59bf0c 100644
>> --- a/doc/mgmt-api.txt
>> +++ b/doc/mgmt-api.txt
>> @@ -2940,9 +2940,47 @@ Get PHY Information Command
>>       LE 1M TX and LE 1M RX would be supported by default.
>>
>>       This command is only available for LE capable controllers.
>> +        It will return Not Supported otherwise.
>> +
>> +        Possible errors:        Not Supported
>> +                             Invalid Index
>> +
>> +Set Default PHY Command
>> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
>
> I think the name =E2=80=9CSet PHY Configuration=E2=80=9D would be better.
>
>> +
>> +     Command Code:           0x0044
>> +     Controller Index:       <controller id>
>> +     Command Parameters:     Default_phy (1 Octet)
>> +     Return Parameters:      Current_phys (1 Octet)
>
> Returning the current selected PHYs is rather useless here. It would be e=
xactly the same value we put in. However what might be useful is an event t=
hat indicates the change of the PHY Configuration.
>

I will remove Current_phys. But do we really need PHY Configuration
Changed event since it
can be changed only by Set PHY Configuration command and user can
understand the currently
selected_phys based on whether command is succeeded or failed?

We might need PHY changed event on a particular connection which
should correspond to
PHY Update Complete HCI Event.

Thanks,
Jaganath

Re: [PATCH 2/3 v2] doc/mgmt-api: Add support for Set default Phy command

[Marcel Holtmann]

Hi Jaganath,

>>> ---
>>> doc/mgmt-api.txt | 38 ++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 38 insertions(+)
>>> 
>>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>>> index 69e84cd..b59bf0c 100644
>>> --- a/doc/mgmt-api.txt
>>> +++ b/doc/mgmt-api.txt
>>> @@ -2940,9 +2940,47 @@ Get PHY Information Command
>>>      LE 1M TX and LE 1M RX would be supported by default.
>>> 
>>>      This command is only available for LE capable controllers.
>>> +        It will return Not Supported otherwise.
>>> +
>>> +        Possible errors:        Not Supported
>>> +                             Invalid Index
>>> +
>>> +Set Default PHY Command
>>> +==========================
>> 
>> I think the name “Set PHY Configuration” would be better.
>> 
>>> +
>>> +     Command Code:           0x0044
>>> +     Controller Index:       <controller id>
>>> +     Command Parameters:     Default_phy (1 Octet)
>>> +     Return Parameters:      Current_phys (1 Octet)
>> 
>> Returning the current selected PHYs is rather useless here. It would be exactly the same value we put in. However what might be useful is an event that indicates the change of the PHY Configuration.
>> 
> 
> I will remove Current_phys. But do we really need PHY Configuration
> Changed event since it
> can be changed only by Set PHY Configuration command and user can
> understand the currently
> selected_phys based on whether command is succeeded or failed?

and what about a 2nd mgmt user? We want to inform them that the configuration changed. They should be able to track selected PHYs without having to poll Get PHY Configuration.

> We might need PHY changed event on a particular connection which
> should correspond to
> PHY Update Complete HCI Event.

That is a different problem and I think that needs to be done via socket CMSG information.

Regards

Marcel

[PATCH 3/3 v2] doc/mgmt-api: Add advertising phys support to flags

[Jaganath Kanakkassery]

---
 doc/mgmt-api.txt | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index b59bf0c..c138f47 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -2504,6 +2504,9 @@ Read Advertising Features Command
 		4	Add TX Power field to Adv_Data
 		5	Add Appearance field to Scan_Rsp
 		6	Add Local Name in Scan_Rsp
+		7	Secondary Channel with LE 1M
+		8	Secondary Channel with LE 2M
+		9	Secondary Channel with LE Coded
 
 	The Flags bit 0 indicates support for connectable advertising
 	and for switching to connectable advertising independent of the
@@ -2553,6 +2556,15 @@ Read Advertising Features Command
 	space is limited a short version or no name information. The
 	Local Name will be added at the end of the scan response data.
 
+	The Flags bit 7 indicates support for advertising in secondary
+	channel in LE 1M PHY.
+
+	The Flags bit 8 indicates support for advertising in secondary
+	channel in LE 2M PHY. Primary channel would be on 1M.
+
+	The Flags bit 9 indicates support for advertising in secondary
+	channel in LE CODED PHY.
+
 	The valid range for Instance identifiers is 1-254. The value 0
 	is reserved for internal use and the value 255 is reserved for
 	future extensions. However the Max_Instances value for indicating
@@ -2613,6 +2625,9 @@ Add Advertising Command
 		4	Add TX Power field to Adv_Data
 		5	Add Appearance field to Scan_Rsp
 		6	Add Local Name in Scan_Rsp
+		7	Secondary Channel with LE 1M
+		8	Secondary Channel with LE 2M
+		9	Secondary Channel with LE Coded
 
 	When the connectable flag is set, then the controller will use
 	undirected connectable advertising. The value of the connectable
@@ -2640,6 +2655,13 @@ Add Advertising Command
 	supported to provide less air traffic for devices implementing
 	broadcaster role.
 
+	Secondary channel flags can be used to advertise in secondary
+	channel with the corresponding PHYs. In case of 2M, the primary
+	channel used would be 1M. If none of them are set then secondary
+	channel advertising would be disabled for this instance which is as
+	good as legacy advertising. Combining the sets would result in
+	multiple advertising sets being created.
+
 	The Duration parameter configures the length of an Instance. The
 	value is in seconds.
 
-- 
2.7.4

Re: [PATCH 3/3 v2] doc/mgmt-api: Add advertising phys support to flags

[Johan Hedberg]

Hi Jaganath,

On Mon, Nov 20, 2017, Jaganath Kanakkassery wrote:
> @@ -2613,6 +2625,9 @@ Add Advertising Command
>  		4	Add TX Power field to Adv_Data
>  		5	Add Appearance field to Scan_Rsp
>  		6	Add Local Name in Scan_Rsp
> +		7	Secondary Channel with LE 1M
> +		8	Secondary Channel with LE 2M
> +		9	Secondary Channel with LE Coded
>  
>  	When the connectable flag is set, then the controller will use
>  	undirected connectable advertising. The value of the connectable
> @@ -2640,6 +2655,13 @@ Add Advertising Command
>  	supported to provide less air traffic for devices implementing
>  	broadcaster role.
>  
> +	Secondary channel flags can be used to advertise in secondary
> +	channel with the corresponding PHYs. In case of 2M, the primary
> +	channel used would be 1M. If none of them are set then secondary
> +	channel advertising would be disabled for this instance which is as
> +	good as legacy advertising. Combining the sets would result in
> +	multiple advertising sets being created.

What kind of combination of parameters would we select LE Coded for the
primary channel? Don't we need a flag for it? It should either way be
documented when LE Coded gets used for the primary PHY.

Johan

Re: [PATCH 3/3 v2] doc/mgmt-api: Add advertising phys support to flags

[Jaganath K]

Hi Johan,


> Hi Jaganath,
>
> On Mon, Nov 20, 2017, Jaganath Kanakkassery wrote:
>> @@ -2613,6 +2625,9 @@ Add Advertising Command
>>               4       Add TX Power field to Adv_Data
>>               5       Add Appearance field to Scan_Rsp
>>               6       Add Local Name in Scan_Rsp
>> +             7       Secondary Channel with LE 1M
>> +             8       Secondary Channel with LE 2M
>> +             9       Secondary Channel with LE Coded
>>
>>       When the connectable flag is set, then the controller will use
>>       undirected connectable advertising. The value of the connectable
>> @@ -2640,6 +2655,13 @@ Add Advertising Command
>>       supported to provide less air traffic for devices implementing
>>       broadcaster role.
>>
>> +     Secondary channel flags can be used to advertise in secondary
>> +     channel with the corresponding PHYs. In case of 2M, the primary
>> +     channel used would be 1M. If none of them are set then secondary
>> +     channel advertising would be disabled for this instance which is as
>> +     good as legacy advertising. Combining the sets would result in
>> +     multiple advertising sets being created.
>
> What kind of combination of parameters would we select LE Coded for the
> primary channel? Don't we need a flag for it? It should either way be
> documented when LE Coded gets used for the primary PHY.
>

Primary channel can be both 1M or CODED and secondary channel can be
either 1M, 2M or CODED. As per spec all the permutation and combination of
the above values is possible with Primary and secondary channel.

But in the current interface all the combinations are not supported
right now since
atleast i dont find the use case for all of them, for example if we
use CODED for
any of primary or secondary channel then the scanner is intended to be
in Long range
and using any other PHY on the other does not make sense.

So basically if "Secondary Channel with LE Coded" is selected then both primary
and secondary will be set as LE Coded.

I will raise a patch to document it if there is no objection to this assumption.

Thanks,
Jaganath

Re: [PATCH 3/3 v2] doc/mgmt-api: Add advertising phys support to flags

[Marcel Holtmann]

Hi Jaganath,

> ---
> doc/mgmt-api.txt | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
> 
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index b59bf0c..c138f47 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -2504,6 +2504,9 @@ Read Advertising Features Command
> 		4	Add TX Power field to Adv_Data
> 		5	Add Appearance field to Scan_Rsp
> 		6	Add Local Name in Scan_Rsp
> +		7	Secondary Channel with LE 1M
> +		8	Secondary Channel with LE 2M
> +		9	Secondary Channel with LE Coded
> 
> 	The Flags bit 0 indicates support for connectable advertising
> 	and for switching to connectable advertising independent of the
> @@ -2553,6 +2556,15 @@ Read Advertising Features Command
> 	space is limited a short version or no name information. The
> 	Local Name will be added at the end of the scan response data.
> 
> +	The Flags bit 7 indicates support for advertising in secondary
> +	channel in LE 1M PHY.
> +
> +	The Flags bit 8 indicates support for advertising in secondary
> +	channel in LE 2M PHY. Primary channel would be on 1M.
> +
> +	The Flags bit 9 indicates support for advertising in secondary
> +	channel in LE CODED PHY.
> +
> 	The valid range for Instance identifiers is 1-254. The value 0
> 	is reserved for internal use and the value 255 is reserved for
> 	future extensions. However the Max_Instances value for indicating
> @@ -2613,6 +2625,9 @@ Add Advertising Command
> 		4	Add TX Power field to Adv_Data
> 		5	Add Appearance field to Scan_Rsp
> 		6	Add Local Name in Scan_Rsp
> +		7	Secondary Channel with LE 1M
> +		8	Secondary Channel with LE 2M
> +		9	Secondary Channel with LE Coded
> 
> 	When the connectable flag is set, then the controller will use
> 	undirected connectable advertising. The value of the connectable
> @@ -2640,6 +2655,13 @@ Add Advertising Command
> 	supported to provide less air traffic for devices implementing
> 	broadcaster role.
> 
> +	Secondary channel flags can be used to advertise in secondary
> +	channel with the corresponding PHYs. In case of 2M, the primary
> +	channel used would be 1M. If none of them are set then secondary
> +	channel advertising would be disabled for this instance which is as
> +	good as legacy advertising. Combining the sets would result in
> +	multiple advertising sets being created.
> +

I changed my mind here. The behind the scenes creating multiple advertising sets is a bad idea. That is something the application should handle and not the kernel do in the background.

So we should mark these bits as mutually exclusive and note that specifying more than one will result in an invalid params error. In addition we need to note that choosing LE 1M and LE 2M will result in using extended advertising with the primary channel being 1M. And choosing LE Coded will result in extended advertising with both primary and secondary channel in coded. This should be in the text description. Choosing none of these flags will result in legacy advertising.

This just needs a bit of wordsmithing and then it should be fine.

Regards

Marcel

Re: [PATCH 3/3 v2] doc/mgmt-api: Add advertising phys support to flags

[Jaganath K]

Hi Marcel,

> Hi Jaganath,
>
>> ---
>> doc/mgmt-api.txt | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>> index b59bf0c..c138f47 100644
>> --- a/doc/mgmt-api.txt
>> +++ b/doc/mgmt-api.txt
>> @@ -2504,6 +2504,9 @@ Read Advertising Features Command
>>               4       Add TX Power field to Adv_Data
>>               5       Add Appearance field to Scan_Rsp
>>               6       Add Local Name in Scan_Rsp
>> +             7       Secondary Channel with LE 1M
>> +             8       Secondary Channel with LE 2M
>> +             9       Secondary Channel with LE Coded
>>
>>       The Flags bit 0 indicates support for connectable advertising
>>       and for switching to connectable advertising independent of the
>> @@ -2553,6 +2556,15 @@ Read Advertising Features Command
>>       space is limited a short version or no name information. The
>>       Local Name will be added at the end of the scan response data.
>>
>> +     The Flags bit 7 indicates support for advertising in secondary
>> +     channel in LE 1M PHY.
>> +
>> +     The Flags bit 8 indicates support for advertising in secondary
>> +     channel in LE 2M PHY. Primary channel would be on 1M.
>> +
>> +     The Flags bit 9 indicates support for advertising in secondary
>> +     channel in LE CODED PHY.
>> +
>>       The valid range for Instance identifiers is 1-254. The value 0
>>       is reserved for internal use and the value 255 is reserved for
>>       future extensions. However the Max_Instances value for indicating
>> @@ -2613,6 +2625,9 @@ Add Advertising Command
>>               4       Add TX Power field to Adv_Data
>>               5       Add Appearance field to Scan_Rsp
>>               6       Add Local Name in Scan_Rsp
>> +             7       Secondary Channel with LE 1M
>> +             8       Secondary Channel with LE 2M
>> +             9       Secondary Channel with LE Coded
>>
>>       When the connectable flag is set, then the controller will use
>>       undirected connectable advertising. The value of the connectable
>> @@ -2640,6 +2655,13 @@ Add Advertising Command
>>       supported to provide less air traffic for devices implementing
>>       broadcaster role.
>>
>> +     Secondary channel flags can be used to advertise in secondary
>> +     channel with the corresponding PHYs. In case of 2M, the primary
>> +     channel used would be 1M. If none of them are set then secondary
>> +     channel advertising would be disabled for this instance which is a=
s
>> +     good as legacy advertising. Combining the sets would result in
>> +     multiple advertising sets being created.
>> +
>
> I changed my mind here. The behind the scenes creating multiple advertisi=
ng sets is a bad idea. That is something the application should handle and =
not the kernel do in the background.
>
> So we should mark these bits as mutually exclusive and note that specifyi=
ng more than one will result in an invalid params error. In addition we nee=
d to note that choosing LE 1M and LE 2M will result in using extended adver=
tising with the primary channel being 1M. And choosing LE Coded will result=
 in extended advertising with both primary and secondary channel in coded. =
This should be in the text description. Choosing none of these flags will r=
esult in legacy advertising.
>
> This just needs a bit of wordsmithing and then it should be fine.
>

I will make the suggested documentation in the next patch

I think we have left out two points related to extended advertising
with respect to current mgmt API

First thing is, Extended advertising HCI command does not have
duration parameter of mgmt API.
(It does have a duration param but it actually corresponds to timeout
parameter of mgmt API).
So shall we document that in case of extended advertising "duration"
parameter of mgmt API will
be ignored and controller will take care of scheduling?

Second point is current mgmt API adv_len and scan_rsp_len is only 1
octet. So basically
we cannot support user to send data more that 255 bytes length and
then kernel fragment it
based on the controllers supported data length.

We have two options here.

We can have a new Ext Advertising mgmt API which does not have
duration parameter and adv / scan_rsp len as 2 octets. So we can
remove PHYs also in the
legacy adv mgmt API.

Second option is to let user does the fragmentation and add extra
flags for that.
But in this case also we might need new mechanism to inform user about
maximum extended
data length since in "Read Advertising Features" Max_Adv_Data_Len is
also 1 octet

Please let us know your opinion on the above points.

Thanks,
Jaganath

Re: [PATCH 3/3 v2] doc/mgmt-api: Add advertising phys support to flags

[Marcel Holtmann]

Hi Jaganath,

>>> ---
>>> doc/mgmt-api.txt | 22 ++++++++++++++++++++++
>>> 1 file changed, 22 insertions(+)
>>> 
>>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>>> index b59bf0c..c138f47 100644
>>> --- a/doc/mgmt-api.txt
>>> +++ b/doc/mgmt-api.txt
>>> @@ -2504,6 +2504,9 @@ Read Advertising Features Command
>>>              4       Add TX Power field to Adv_Data
>>>              5       Add Appearance field to Scan_Rsp
>>>              6       Add Local Name in Scan_Rsp
>>> +             7       Secondary Channel with LE 1M
>>> +             8       Secondary Channel with LE 2M
>>> +             9       Secondary Channel with LE Coded
>>> 
>>>      The Flags bit 0 indicates support for connectable advertising
>>>      and for switching to connectable advertising independent of the
>>> @@ -2553,6 +2556,15 @@ Read Advertising Features Command
>>>      space is limited a short version or no name information. The
>>>      Local Name will be added at the end of the scan response data.
>>> 
>>> +     The Flags bit 7 indicates support for advertising in secondary
>>> +     channel in LE 1M PHY.
>>> +
>>> +     The Flags bit 8 indicates support for advertising in secondary
>>> +     channel in LE 2M PHY. Primary channel would be on 1M.
>>> +
>>> +     The Flags bit 9 indicates support for advertising in secondary
>>> +     channel in LE CODED PHY.
>>> +
>>>      The valid range for Instance identifiers is 1-254. The value 0
>>>      is reserved for internal use and the value 255 is reserved for
>>>      future extensions. However the Max_Instances value for indicating
>>> @@ -2613,6 +2625,9 @@ Add Advertising Command
>>>              4       Add TX Power field to Adv_Data
>>>              5       Add Appearance field to Scan_Rsp
>>>              6       Add Local Name in Scan_Rsp
>>> +             7       Secondary Channel with LE 1M
>>> +             8       Secondary Channel with LE 2M
>>> +             9       Secondary Channel with LE Coded
>>> 
>>>      When the connectable flag is set, then the controller will use
>>>      undirected connectable advertising. The value of the connectable
>>> @@ -2640,6 +2655,13 @@ Add Advertising Command
>>>      supported to provide less air traffic for devices implementing
>>>      broadcaster role.
>>> 
>>> +     Secondary channel flags can be used to advertise in secondary
>>> +     channel with the corresponding PHYs. In case of 2M, the primary
>>> +     channel used would be 1M. If none of them are set then secondary
>>> +     channel advertising would be disabled for this instance which is as
>>> +     good as legacy advertising. Combining the sets would result in
>>> +     multiple advertising sets being created.
>>> +
>> 
>> I changed my mind here. The behind the scenes creating multiple advertising sets is a bad idea. That is something the application should handle and not the kernel do in the background.
>> 
>> So we should mark these bits as mutually exclusive and note that specifying more than one will result in an invalid params error. In addition we need to note that choosing LE 1M and LE 2M will result in using extended advertising with the primary channel being 1M. And choosing LE Coded will result in extended advertising with both primary and secondary channel in coded. This should be in the text description. Choosing none of these flags will result in legacy advertising.
>> 
>> This just needs a bit of wordsmithing and then it should be fine.
>> 
> 
> I will make the suggested documentation in the next patch
> 
> I think we have left out two points related to extended advertising
> with respect to current mgmt API
> 
> First thing is, Extended advertising HCI command does not have
> duration parameter of mgmt API.
> (It does have a duration param but it actually corresponds to timeout
> parameter of mgmt API).
> So shall we document that in case of extended advertising "duration"
> parameter of mgmt API will
> be ignored and controller will take care of scheduling?
> 
> Second point is current mgmt API adv_len and scan_rsp_len is only 1
> octet. So basically
> we cannot support user to send data more that 255 bytes length and
> then kernel fragment it
> based on the controllers supported data length.
> 
> We have two options here.
> 
> We can have a new Ext Advertising mgmt API which does not have
> duration parameter and adv / scan_rsp len as 2 octets. So we can
> remove PHYs also in the
> legacy adv mgmt API.
> 
> Second option is to let user does the fragmentation and add extra
> flags for that.
> But in this case also we might need new mechanism to inform user about
> maximum extended
> data length since in "Read Advertising Features" Max_Adv_Data_Len is
> also 1 octet
> 
> Please let us know your opinion on the above points.

I would currently limit it to 255 octets in length and see how that goes. Frankly any longer advertising is costing the receiver and large advertising should be avoided if not really needed. At this point, the proposed API might be enough to get started and refine it later. Adding an extra mgmt command later is not really a problem and can be done easily.

Regards

Marcel

Re: [PATCH 0/3 v2] LE New PHYs kernel interfaces

[Jaganath K]

Hi,

Ping

> These patches enables user to get the supported phys by the
> controller and set preferred phy to be used in subsequent
> scanning and connection initiation.
>
> Also added flags to support user selection for phy to be
> advertised.
>
> v1: Applied Marcel's comments
> v2: Applied Eramoto's comments
>
> Jaganath Kanakkassery (3):
>   doc/mgmt-api: Add support for Get PHY information command
>   doc/mgmt-api: Add support for Set default Phy command
>   doc/mgmt-api: Add advertising phys support to flags
>
>  doc/mgmt-api.txt | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
>

Thanks,
Jaganath