[PATCH bluetooth-next] nl802154: export supported commands

[PATCH bluetooth-next] nl802154: export supported commands

[Varka Bhadram]

This patch will export the supported commands by the devices
to the user. This is required because user should know supported
commands by the IEEE-802.15.4 devices. Drivers that are there in
the mainline are not supporting all the functionalities that the
core is providing to us.

Signed-off-by: Varka Bhadram <varkab@cdac.in>
---
 include/net/nl802154.h    |    2 ++
 net/ieee802154/nl802154.c |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index 0badebd..6fc231e 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -102,6 +102,8 @@ enum nl802154_attrs {
 
 	NL802154_ATTR_WPAN_PHY_CAPS,
 
+	NL802154_ATTR_SUPPORTED_COMMANDS,
+
 	/* add attributes here, update the policy in nl802154.c */
 
 	__NL802154_ATTR_AFTER_LAST,
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 7dbb1f4..29e1ecd 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -372,7 +372,9 @@ static int nl802154_send_wpan_phy(struct cfg802154_registered_device *rdev,
 				  struct sk_buff *msg, u32 portid, u32 seq,
 				  int flags)
 {
+	struct nlattr *nl_cmds;
 	void *hdr;
+	int i;
 
 	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
 	if (!hdr)
@@ -431,6 +433,37 @@ static int nl802154_send_wpan_phy(struct cfg802154_registered_device *rdev,
 	if (nl802154_put_capabilities(msg, rdev))
 		goto nla_put_failure;
 
+	nl_cmds = nla_nest_start(msg, NL802154_ATTR_SUPPORTED_COMMANDS);
+	if (!nl_cmds)
+		goto nla_put_failure;
+
+	i = 0;
+#define CMD(op, n)							\
+	do {								\
+		if (rdev->ops->op) {					\
+			i++;						\
+			if (nla_put_u32(msg, i, NL802154_CMD_ ## n))	\
+				goto nla_put_failure;			\
+		}							\
+	} while (0)
+
+	CMD(add_virtual_intf, NEW_INTERFACE);
+	CMD(del_virtual_intf, DEL_INTERFACE);
+	CMD(set_channel, SET_CHANNEL);
+	CMD(set_pan_id, SET_PAN_ID);
+	CMD(set_short_addr, SET_SHORT_ADDR);
+
+	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_TXPOWER)
+		CMD(set_tx_power, SET_TX_POWER);
+
+	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_CCA_ED_LEVEL)
+		CMD(set_cca_ed_level, SET_CCA_ED_LEVEL);
+
+	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_CCA_MODE)
+		CMD(set_cca_mode, SET_CCA_MODE);
+#undef CMD
+	nla_nest_end(msg, nl_cmds);
+
 finish:
 	genlmsg_end(msg, hdr);
 	return 0;
-- 
1.7.9.5

Re: [PATCH bluetooth-next] nl802154: export supported commands

[Varka Bhadram]

On 06/03/2015 09:39 AM, Varka Bhadram wrote:

> This patch will export the supported commands by the devices
> to the user. This is required because user should know supported
> commands by the IEEE-802.15.4 devices. Drivers that are there in
> the mainline are not supporting all the functionalities that the
> core is providing to us.
>
> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
>   include/net/nl802154.h    |    2 ++
>   net/ieee802154/nl802154.c |   33 +++++++++++++++++++++++++++++++++
>   2 files changed, 35 insertions(+)

Accidentally send this patch. Its not updated. Sorry for the noise.

I will send the updated one.

-- 
Varka Bhadram

[PATCH bluetooth-next] nl802154: export supported commands

[Varka Bhadram]

This patch will export the supported commands by the devices
to the user. This is required because user should know supported
commands by the IEEE-802.15.4 devices. Drivers that are there in
the mainline are not supporting all the functionalities that the
core is providing to us.

Signed-off-by: Varka Bhadram <varkab@cdac.in>
---
 include/net/nl802154.h    |    2 ++
 net/ieee802154/nl802154.c |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index 0badebd..6fc231e 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -102,6 +102,8 @@ enum nl802154_attrs {
 
 	NL802154_ATTR_WPAN_PHY_CAPS,
 
+	NL802154_ATTR_SUPPORTED_COMMANDS,
+
 	/* add attributes here, update the policy in nl802154.c */
 
 	__NL802154_ATTR_AFTER_LAST,
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 7dbb1f4..310a519 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -372,7 +372,9 @@ static int nl802154_send_wpan_phy(struct cfg802154_registered_device *rdev,
 				  struct sk_buff *msg, u32 portid, u32 seq,
 				  int flags)
 {
+	struct nlattr *nl_cmds;
 	void *hdr;
+	int i;
 
 	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
 	if (!hdr)
@@ -431,6 +433,40 @@ static int nl802154_send_wpan_phy(struct cfg802154_registered_device *rdev,
 	if (nl802154_put_capabilities(msg, rdev))
 		goto nla_put_failure;
 
+	nl_cmds = nla_nest_start(msg, NL802154_ATTR_SUPPORTED_COMMANDS);
+	if (!nl_cmds)
+		goto nla_put_failure;
+
+	i = 0;
+#define CMD(op, n)							\
+	do {								\
+		if (rdev->ops->op) {					\
+			i++;						\
+			if (nla_put_u32(msg, i, NL802154_CMD_ ## n))	\
+				goto nla_put_failure;			\
+		}							\
+	} while (0)
+
+	CMD(add_virtual_intf, NEW_INTERFACE);
+	CMD(del_virtual_intf, DEL_INTERFACE);
+	CMD(set_channel, SET_CHANNEL);
+	CMD(set_pan_id, SET_PAN_ID);
+	CMD(set_short_addr, SET_SHORT_ADDR);
+	CMD(set_backoff_exponent, SET_BACKOFF_EXPONENT);
+	CMD(set_max_csma_backoffs, SET_MAX_CSMA_BACKOFFS);
+	CMD(set_max_frame_retries, SET_MAX_FRAME_RETRIES);
+
+	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_TXPOWER)
+		CMD(set_tx_power, SET_TX_POWER);
+
+	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_CCA_ED_LEVEL)
+		CMD(set_cca_ed_level, SET_CCA_ED_LEVEL);
+
+	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_CCA_MODE)
+		CMD(set_cca_mode, SET_CCA_MODE);
+#undef CMD
+	nla_nest_end(msg, nl_cmds);
+
 finish:
 	genlmsg_end(msg, hdr);
 	return 0;
-- 
1.7.9.5

Re: [PATCH bluetooth-next] nl802154: export supported commands

[Alexander Aring]

Hi,

On Wed, Jun 03, 2015 at 09:44:01AM +0530, Varka Bhadram wrote:
> This patch will export the supported commands by the devices
> to the user. This is required because user should know supported
> commands by the IEEE-802.15.4 devices. Drivers that are there in
> the mainline are not supporting all the functionalities that the
> core is providing to us.
> 

I think this is not the reason why this was implemented. You check here
cfg802154_ops which ends in mac802154 (SoftMAC) OR _possible_ HardMAC drivers.

If you want to make sure that a driver implement it _and_ really sure
you need to check on ieee802154_ops.

I suppose the real reason why wireless has this implementation, is
because to check if HardMAC drivers can support a specific command.
This is because every HardMAC driver should implement then the full
cfg802154_ops callback structure. It's simple wrong here to say "to
check if the driver support the functionalities".

You skip the mac802154 layer here, these calls are direct driver-layer
calls in case of HardMAC driver only.


Currently that's why I think such feature has _at_ _the_ _moment_ no
really big use case, because we don't have any HardMAC driver currently.
Anyway this prepares to handle HardMAC drivers. After fixing the commit
message you can add a:

Acked-by: Alexander Aring <alex.aring@gmail.com>

> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
>  include/net/nl802154.h    |    2 ++
>  net/ieee802154/nl802154.c |   36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> index 0badebd..6fc231e 100644
> --- a/include/net/nl802154.h
> +++ b/include/net/nl802154.h
> @@ -102,6 +102,8 @@ enum nl802154_attrs {
>  
>  	NL802154_ATTR_WPAN_PHY_CAPS,
>  
> +	NL802154_ATTR_SUPPORTED_COMMANDS,
> +
>  	/* add attributes here, update the policy in nl802154.c */
>  
>  	__NL802154_ATTR_AFTER_LAST,
> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> index 7dbb1f4..310a519 100644
> --- a/net/ieee802154/nl802154.c
> +++ b/net/ieee802154/nl802154.c
> @@ -372,7 +372,9 @@ static int nl802154_send_wpan_phy(struct cfg802154_registered_device *rdev,
>  				  struct sk_buff *msg, u32 portid, u32 seq,
>  				  int flags)
>  {
> +	struct nlattr *nl_cmds;
>  	void *hdr;
> +	int i;
>  
>  	hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
>  	if (!hdr)
> @@ -431,6 +433,40 @@ static int nl802154_send_wpan_phy(struct cfg802154_registered_device *rdev,
>  	if (nl802154_put_capabilities(msg, rdev))
>  		goto nla_put_failure;
>  
> +	nl_cmds = nla_nest_start(msg, NL802154_ATTR_SUPPORTED_COMMANDS);
> +	if (!nl_cmds)
> +		goto nla_put_failure;
> +
> +	i = 0;
> +#define CMD(op, n)							\
> +	do {								\
> +		if (rdev->ops->op) {					\
> +			i++;						\
> +			if (nla_put_u32(msg, i, NL802154_CMD_ ## n))	\
> +				goto nla_put_failure;			\
> +		}							\
> +	} while (0)
> +
> +	CMD(add_virtual_intf, NEW_INTERFACE);
> +	CMD(del_virtual_intf, DEL_INTERFACE);
> +	CMD(set_channel, SET_CHANNEL);
> +	CMD(set_pan_id, SET_PAN_ID);
> +	CMD(set_short_addr, SET_SHORT_ADDR);
> +	CMD(set_backoff_exponent, SET_BACKOFF_EXPONENT);
> +	CMD(set_max_csma_backoffs, SET_MAX_CSMA_BACKOFFS);
> +	CMD(set_max_frame_retries, SET_MAX_FRAME_RETRIES);
> +
> +	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_TXPOWER)
> +		CMD(set_tx_power, SET_TX_POWER);
> +
> +	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_CCA_ED_LEVEL)
> +		CMD(set_cca_ed_level, SET_CCA_ED_LEVEL);
> +
> +	if (rdev->wpan_phy.flags & WPAN_PHY_FLAG_CCA_MODE)
> +		CMD(set_cca_mode, SET_CCA_MODE);
> +#undef CMD
> +	nla_nest_end(msg, nl_cmds);
> +
>  finish:
>  	genlmsg_end(msg, hdr);
>  	return 0;
> -- 
> 1.7.9.5
> 

Re: [PATCH bluetooth-next] nl802154: export supported commands

[Varka Bhadram]

On 06/03/2015 12:19 PM, Alexander Aring wrote:

> Hi,
>
> On Wed, Jun 03, 2015 at 09:44:01AM +0530, Varka Bhadram wrote:
>> This patch will export the supported commands by the devices
>> to the user. This is required because user should know supported
>> commands by the IEEE-802.15.4 devices. Drivers that are there in
>> the mainline are not supporting all the functionalities that the
>> core is providing to us.
>>
> I think this is not the reason why this was implemented. You check here
> cfg802154_ops which ends in mac802154 (SoftMAC) OR _possible_ HardMAC drivers.
>
> If you want to make sure that a driver implement it _and_ really sure
> you need to check on ieee802154_ops.
>
> I suppose the real reason why wireless has this implementation, is
> because to check if HardMAC drivers can support a specific command.
> This is because every HardMAC driver should implement then the full
> cfg802154_ops callback structure. It's simple wrong here to say "to
> check if the driver support the functionalities".

I found this thread for the reason used in wireless for implementation.

http://www.spinics.net/lists/linux-wireless/msg29923.html

> You skip the mac802154 layer here, these calls are direct driver-layer
> calls in case of HardMAC driver only.
>
>
> Currently that's why I think such feature has _at_ _the_ _moment_ no
> really big use case, because we don't have any HardMAC driver currently.
> Anyway this prepares to handle HardMAC drivers. After fixing the commit
> message you can add a:
>
> Acked-by: Alexander Aring <alex.aring@gmail.com>

Thanks

-- 
Varka Bhadram

Re: [PATCH bluetooth-next] nl802154: export supported commands

[Alexander Aring]

On Wed, Jun 03, 2015 at 12:43:17PM +0530, Varka Bhadram wrote:
> On 06/03/2015 12:19 PM, Alexander Aring wrote:
> 
> >Hi,
> >
> >On Wed, Jun 03, 2015 at 09:44:01AM +0530, Varka Bhadram wrote:
> >>This patch will export the supported commands by the devices
> >>to the user. This is required because user should know supported
> >>commands by the IEEE-802.15.4 devices. Drivers that are there in
> >>the mainline are not supporting all the functionalities that the
> >>core is providing to us.
> >>
> >I think this is not the reason why this was implemented. You check here
> >cfg802154_ops which ends in mac802154 (SoftMAC) OR _possible_ HardMAC drivers.
> >
> >If you want to make sure that a driver implement it _and_ really sure
> >you need to check on ieee802154_ops.
> >
> >I suppose the real reason why wireless has this implementation, is
> >because to check if HardMAC drivers can support a specific command.
> >This is because every HardMAC driver should implement then the full
> >cfg802154_ops callback structure. It's simple wrong here to say "to
> >check if the driver support the functionalities".
> 
> I found this thread for the reason used in wireless for implementation.
> 
> http://www.spinics.net/lists/linux-wireless/msg29923.html
> 

Yea, but in this case he means really HardMAC drivers only which
implements some cfg802154_ops which others doesn't implement.

I would split our use cases in two categories: setting phy or mac cmds.


for phy setttings cmds:

You can check if the phy supports the specific CMD. E.g. tx_power,
before calling the specific cmd. The cmd will not set because you check the
phy flags.


for mac setttings cmds:

This currently makes no sense, because these are all supported by the
mac802154 layer and we have currently no HardMAC drivers. Each HardMAC
driver implements the cfg802154_ops and then it could make sense to
check if the HardMAC driver supports a specific command.

At the moment for mac settings (in case of SoftMAC) it should display
always the same.



Nevertheless, I would ack this. This would be useful if somebody wants
to implement a HardMAC driver.

- Alex

Re: [PATCH bluetooth-next] nl802154: export supported commands

[Alexander Aring]

On Wed, Jun 03, 2015 at 09:44:01AM +0530, Varka Bhadram wrote:
> This patch will export the supported commands by the devices
> to the user. This is required because user should know supported
> commands by the IEEE-802.15.4 devices. Drivers that are there in
> the mainline are not supporting all the functionalities that the
> core is providing to us.
> 
> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
>  include/net/nl802154.h    |    2 ++
>  net/ieee802154/nl802154.c |   36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> index 0badebd..6fc231e 100644
> --- a/include/net/nl802154.h
> +++ b/include/net/nl802154.h
> @@ -102,6 +102,8 @@ enum nl802154_attrs {
>  
>  	NL802154_ATTR_WPAN_PHY_CAPS,
>  
> +	NL802154_ATTR_SUPPORTED_COMMANDS,
> +
>  	/* add attributes here, update the policy in nl802154.c */
>  

for your v2, please also note the comment here "update the policy in
nl802154.c".

This means you need to add:

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 310a519..a6ae29d 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -228,6 +228,8 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
        [NL802154_ATTR_LBT_MODE] = { .type = NLA_U8, },
 
        [NL802154_ATTR_WPAN_PHY_CAPS] = { .type = NLA_NESTED },
+
+       [NL802154_ATTR_SUPPORTED_COMMANDS] = { .type = NLA_NESTED },
 };
 
 /* message building helper */


- Alex