>From: Dmitry Eremin-Solenikov [mailto:[email protected]]
>Hello,
>
>Thank you for your work!
>
>Hennerich, Michael wrote:
>> This patch adds support for MAC Hardware Accelerator functions such as
>Automated Acknowledgement (AACK) and CSMA-CA with Retransmission found on a
>few 802.15.4 Radio Transceivers.
>> In order to support such functionality the radio's address filtering needs
>to know it's Short, IEEE Address and PAN ID.
>> This patch makes sure that this information is pushed to the radio in case
>it provides a new ieee802154_ops callback function (ext_mac_ctrl).
>> All this is optional, and shouldn't influence the current implementation.
>
>Most of devices should support HW-ACK, otherwise they will be too
>constrained. See below.
>
>>
>> Current kernel-zigbee doesn't implement ACK transmissions, nor does it
>implement any kind of Retry.
>> (BTW it also doesn't do anything useful with the xmit return value)
>> I understand that this is going to be added in future, as part of the
>softmac implementation.
>> I also understand that this softmac allows for greater flexibility such
>indirect addressing or utilizing different sets of addresses on the same
>radio.
>
>There was once software ACK implementation. It was removed once, as it
>didn't provide any additional value due to time constraints in standard.
>
>>
>> From: Michael Hennerich <[email protected]>
>>
>> include/net/mac802154.h | 6 +++
>> include/net/wpan-phy.h | 8 ++++
>> net/mac802154/dev.c | 1 +
>> net/mac802154/mac_cmd.c | 3 ++
>> net/mac802154/mib.c | 89
>+++++++++++++++++++++++++++++++++++++++++++++++
>> net/mac802154/mib.h | 2 +
>> 6 files changed, 109 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/mac802154.h b/include/net/mac802154.h
>> index 3463996..081aa45 100644
>> --- a/include/net/mac802154.h
>> +++ b/include/net/mac802154.h
>> @@ -21,6 +21,10 @@
>> #ifndef NET_MAC802154_H
>> #define NET_MAC802154_H
>>
>> +#define IEEE802154_PHY_CMD_IEEE_ADDR 1
>> +#define IEEE802154_PHY_CMD_SHORT_ADDR 2
>> +#define IEEE802154_PHY_CMD_PAN_ID 3
>> +#define IEEE802154_PHY_CMD_PAN_COORD 4
>> struct ieee802154_dev {
>> /* filled by the driver */
>> int extra_tx_headroom; /* headroom to reserve for tx skb */
>> @@ -96,6 +100,8 @@ struct ieee802154_ops {
>> int (*ed)(struct ieee802154_dev *dev, u8 *level);
>> int (*set_channel)(struct ieee802154_dev *dev,
>> int channel);
>> + int (*ext_mac_ctrl)(struct ieee802154_dev *dev,
>> + int cmd, unsigned long
>val);
>> };
>>
>
>Hmmm. This sounds like an IOCTL. I'd prefer to have an optional function
>specially dedicated to hw-ack/filtering addressing fields. This function
>can take as an argument a pointer to a structure containing all
>addressing fields.
This is how the radio driver side could look like.
static int trx_ext_mac_ctrl(struct ieee802154_dev *dev,
int cmd, unsigned long val)
{
struct trx_local *lp = dev->priv;
int ret;
u8 reg;
DBG(2, "%s :Enter\n", __func__);
DBG(1, "%s :CMD=0x%X VAL=0x%X\n", __func__, cmd, (u32) val);
might_sleep();
switch(cmd) {
case IEEE802154_PHY_CMD_IEEE_ADDR:
trx_write_reg(lp, REG_IEEE_ADDR_0,
dev->phy->current_ieee_addr[7]);
trx_write_reg(lp, REG_IEEE_ADDR_1,
dev->phy->current_ieee_addr[6]);
trx_write_reg(lp, REG_IEEE_ADDR_2,
dev->phy->current_ieee_addr[5]);
trx_write_reg(lp, REG_IEEE_ADDR_3,
dev->phy->current_ieee_addr[4]);
trx_write_reg(lp, REG_IEEE_ADDR_4,
dev->phy->current_ieee_addr[3]);
trx_write_reg(lp, REG_IEEE_ADDR_5,
dev->phy->current_ieee_addr[2]);
trx_write_reg(lp, REG_IEEE_ADDR_6,
dev->phy->current_ieee_addr[1]);
trx_write_reg(lp, REG_IEEE_ADDR_7,
dev->phy->current_ieee_addr[0]);
break;
case IEEE802154_PHY_CMD_SHORT_ADDR:
trx_write_reg(lp, REG_SHORT_ADDR_0, val);
trx_write_reg(lp, REG_SHORT_ADDR_1, val >> 8);
dev->phy->current_short_addr = val;
break;
case IEEE802154_PHY_CMD_PAN_ID:
trx_write_reg(lp, REG_PAN_ID0, val);
trx_write_reg(lp, REG_PAN_ID1, val >> 8);
dev->phy->current_pan_id = val;
break;
case IEEE802154_PHY_CMD_PAN_COORD:
trx_read_reg(lp, REG_AUTO_CFG, ®);
if (val)
reg |= IS_PANCOORD;
else
reg &= ~IS_PANCOORD;
trx_write_reg(lp, REG_AUTO_CFG, reg);
dev->phy->current_pan_coord = val;
break;
default:
ret = -EFAULT;
}
DBG(2, "%s :Exit\n", __func__);
return 0;
}
However - you are right - this looks pretty much like an ioctl.
>
>> struct ieee802154_dev *ieee802154_alloc_device(size_t priv_size,
>> diff --git a/include/net/wpan-phy.h b/include/net/wpan-phy.h
>> index 8592623..802c555 100644
>> --- a/include/net/wpan-phy.h
>> +++ b/include/net/wpan-phy.h
>> @@ -23,6 +23,7 @@
>>
>> #include <linux/netdevice.h>
>> #include <linux/mutex.h>
>> +#include <net/af_ieee802154.h>
>>
>> struct wpan_phy {
>> struct mutex pib_lock;
>> @@ -38,6 +39,13 @@ struct wpan_phy {
>> u8 transmit_power;
>> u8 cca_mode;
>>
>> + /*
>> + * Extended MAC Control
>> + */
>> + u8 current_pan_coord;
>> + u16 current_pan_id;
>> + u16 current_short_addr;
>> + u8 current_ieee_addr[IEEE802154_ADDR_LEN];
>
>I'm not sure that these fields should be present in MAC. I think it may
>be better to move them directly to per-interface/per-device private
>data. We have (long-term) plans for one of our soft devices to support
>multiple-mac filtering and I'd really prefer to keep wpan-phy a phy-only
>instance (at least for now).
The reason I added those was to only queue work into the drivers 'ioctl' when
values need to be changed.
See above ext_mac_ctrl() example.
So if we decide to better go with a pointer to a structure containing all
addressing fields, I would prefer to branch off the work queue in the driver
only in case fields differ.
-Michael
>
>> struct device dev;
>> int idx;
>>
>
>[rest of the patch skipped]
>
>--
>With best wishes
>Dmitry
------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
_______________________________________________
Linux-zigbee-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel