Hi Dmitry,
>From: Dmitry Eremin-Solenikov [mailto:[email protected]]
>Hello,
>
>On Mon, Feb 1, 2010 at 7:32 PM, Hennerich, Michael
><[email protected]> wrote:
>>>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.
>
>Yeah. And also the name is too generic for the task. I'd really
>prefer not to have 'do-it-all-here' interfaces.
I remember you saying that you also started a interface between mac802154 and
such drivers.
What are your ideas?
How about call it set_hw_addr_filt(), since it's all about setting the hardware
address filtering?
>
>>>
>>>> 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.
>
>This is suitable. Or one can add 'changed' bitmask that will be used
>by the driver to
>push changes to the HW (see mac80211 for some examples of it).
We need to come up with a struct containing all members required for the
hardware address filtering mechanism.
mac802154 updates those fields as required - and always invokes the radio
drivers callback.
The radio driver memcmp()s the new pointer against a local cache and pushes to
required members to HW as appropriate?
(likely using a register cache)
>
>BTW: do you know if engenering samples/engenering evaluation boards
>for your tranceiver
>are available? We'd most probably consider getting one or two?
Boards and engineering samples are available to dedicated customers upon
request.
Please contact your ADI sales representative.
Alternatively you can try this:
https://form.analog.com/Form_Pages/sample_forms/sample_order.aspx?Generic=ADF7242&[email protected]
(it may connect you with the person having hands on boards faster)
>--
>With best wishes
>Dmitry
Best regards,
Michael
------------------------------------------------------------------------------
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
