On 03/13/2013 03:45 AM, Wolf-Bastian Pöttner wrote:
> Implement the filter function to update short address, pan id and ieee
> address on change. Allowing for hardware address filtering needed for
> auto ACK. Switch state machine to use auto ACK states for rx.
>
> This will handle CSMA/CA for us as well as re-sending frames when no ack comes
> in timely.
>
> The register names have been wrong since the beginning but it only showed up
> now
> as they are actualy used for the upcoming auto ACK support.
>
> Signed-off-by: Stefan Schmidt <[email protected]>
> ---
> drivers/ieee802154/at86rf230.c | 108 ++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 105 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ieee802154/at86rf230.c b/drivers/ieee802154/at86rf230.c
> index 4d32ac4..761343b 100644
> --- a/drivers/ieee802154/at86rf230.c
> +++ b/drivers/ieee802154/at86rf230.c
> @@ -38,6 +38,9 @@
> long unsigned int bytes_tx;
> long unsigned int bytes_rx;
>
> +#define ENABLE_AACK
> +#define ENABLE_ARET
> +
#ifdef stuff isn't going to fly in the mainline kernel. Just a heads-up.
Ack support is something that's been on my radar to add, but haven't had
time. Maybe you have time to do this properly, by making a new netlink
item to turn acking on and off and pushing it through iz.
> struct at86rf230_local {
> struct spi_device *spi;
> int rstn, slp_tr, dig2;
> @@ -236,8 +239,8 @@ struct at86rf230_local {
> #define STATE_SLEEP 0x0F
> #define STATE_BUSY_RX_AACK 0x11
> #define STATE_BUSY_TX_ARET 0x12
> -#define STATE_BUSY_RX_AACK_ON 0x16
> -#define STATE_BUSY_TX_ARET_ON 0x19
> +#define STATE_RX_AACK_ON 0x16
> +#define STATE_TX_ARET_ON 0x19
> #define STATE_RX_ON_NOCLK 0x1C
> #define STATE_RX_AACK_ON_NOCLK 0x1D
> #define STATE_BUSY_RX_AACK_NOCLK 0x1E
> @@ -462,7 +465,11 @@ at86rf230_state(struct ieee802154_dev *dev, int state)
> might_sleep();
>
> if (state == STATE_FORCE_TX_ON)
> +#ifdef ENABLE_ARET
> + desired_status = STATE_TX_ARET_ON;
> +#else
> desired_status = STATE_TX_ON;
> +#endif
> else if (state == STATE_FORCE_TRX_OFF)
> desired_status = STATE_TRX_OFF;
> else
> @@ -488,10 +495,43 @@ at86rf230_state(struct ieee802154_dev *dev, int state)
> goto err;
> } while (val == STATE_TRANSITION_IN_PROGRESS);
>
> +#ifdef ENABLE_ARET
> + /* Make sure we go to TX_ON before we go to STATE_TX_ARET_ON */
> + if (desired_status == STATE_TX_ARET_ON) {
> + rc = at86rf230_write_subreg(lp, SR_TRX_CMD, STATE_TX_ON);
> + if (rc)
> + goto err;
> +
> + do {
> + rc = at86rf230_read_subreg(lp, SR_TRX_STATUS, &val);
> + if (rc)
> + goto err;
> + pr_debug("%s val3 = %x\n", __func__, val);
> + } while (val == STATE_TRANSITION_IN_PROGRESS);
> +
> + rc = at86rf230_write_subreg(lp, SR_TRX_CMD, desired_status);
> + if (rc)
> + goto err;
> +
> + do {
> + rc = at86rf230_read_subreg(lp, SR_TRX_STATUS, &val);
> + if (rc)
> + goto err;
> + pr_debug("%s val4 = %x\n", __func__, val);
> + } while (val == STATE_TRANSITION_IN_PROGRESS);
> + }
> +#endif
>
> if (val == desired_status)
> return 0;
>
> +#ifdef ENABLE_AACK
> + if (state == STATE_RX_AACK_ON && val == STATE_BUSY_RX_AACK)
> +#else
> + if (state == STATE_RX_ON && val == STATE_BUSY_RX)
> +#endif
> + return 0;
> +
> pr_err("unexpected state change: %d, asked for %d\n", val, state);
> return -EBUSY;
>
> @@ -510,7 +550,11 @@ at86rf230_start(struct ieee802154_dev *dev)
> if (rc)
> return rc;
>
> +#ifdef ENABLE_AACK
> + return at86rf230_state(dev, STATE_RX_AACK_ON);
> +#else
> return at86rf230_state(dev, STATE_RX_ON);
> +#endif
> }
>
> static void
> @@ -625,6 +669,54 @@ err:
> return -EINVAL;
> }
>
> +#ifdef ENABLE_AACK
> +static int
> +at86rf230_set_hw_addr_filt(struct ieee802154_dev *dev,
> + struct ieee802154_hw_addr_filt
> *filt,
> + unsigned long changed)
> +{
> + struct at86rf230_local *lp = dev->priv;
> +
> + might_sleep();
> +
> + at86rf230_stop(dev);
> +
> + msleep(10);
Why this msleep()? is this from the datasheet, or is it a hack? Also,
you might get pushback on using a magic number (10) there, and not a
#define.
> +
> + if (changed & IEEE802515_AFILT_SADDR_CHANGED) {
> + __at86rf230_write(lp, RG_SHORT_ADDR_0, filt->short_addr &
> 0xff); /* LSB */
> + __at86rf230_write(lp, RG_SHORT_ADDR_1, (filt->short_addr >> 8)
> & 0xff); /* MSB */
> + }
> +
> + if (changed & IEEE802515_AFILT_PANID_CHANGED) {
> + __at86rf230_write(lp, RG_PAN_ID_0, filt->pan_id & 0xff); /* LSB
> */
> + __at86rf230_write(lp, RG_PAN_ID_1, (filt->pan_id >> 8) & 0xff);
> /* MSB */
> + }
> +
> + if (changed & IEEE802515_AFILT_IEEEADDR_CHANGED) {
> + // Make sure order MSB to LSB is correct
> + at86rf230_write_subreg(lp, SR_IEEE_ADDR_0, filt->ieee_addr[7]);
> + at86rf230_write_subreg(lp, SR_IEEE_ADDR_1, filt->ieee_addr[6]);
> + at86rf230_write_subreg(lp, SR_IEEE_ADDR_2, filt->ieee_addr[5]);
> + at86rf230_write_subreg(lp, SR_IEEE_ADDR_3, filt->ieee_addr[4]);
> + at86rf230_write_subreg(lp, SR_IEEE_ADDR_4, filt->ieee_addr[3]);
> + at86rf230_write_subreg(lp, SR_IEEE_ADDR_5, filt->ieee_addr[2]);
> + at86rf230_write_subreg(lp, SR_IEEE_ADDR_6, filt->ieee_addr[1]);
> + at86rf230_write_subreg(lp, SR_IEEE_ADDR_7, filt->ieee_addr[0]);
> + }
> +
> + if (changed & IEEE802515_AFILT_PANC_CHANGED) {
> + if (filt->pan_coord)
> + at86rf230_write_subreg(lp, SR_AACK_I_AM_COORD, 1);
> + else
> + at86rf230_write_subreg(lp, SR_AACK_I_AM_COORD, 0);
> + }
> +
> + at86rf230_start(dev);
> +
> + return 0;
> +}
> +#endif
We need filter stuff regardless of acking, so split this off into a
separate patch.
I do question the need to _start() and _stop() though. What makes you
think this is necessary?
> static struct ieee802154_ops at86rf230_ops = {
> .owner = THIS_MODULE,
> .xmit = at86rf230_xmit,
> @@ -632,6 +724,9 @@ static struct ieee802154_ops at86rf230_ops = {
> .set_channel = at86rf230_channel,
> .start = at86rf230_start,
> .stop = at86rf230_stop,
> +#ifdef ENABLE_AACK
> + .set_hw_addr_filt = at86rf230_set_hw_addr_filt,
> +#endif
> };
>
> static void at86rf230_irqwork(struct work_struct *work)
> @@ -680,7 +775,6 @@ static irqreturn_t at86rf230_isr(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -
^^^ Stray whitespace change
> static int at86rf230_hw_init(struct at86rf230_local *lp)
> {
> u8 status;
> @@ -718,7 +812,11 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
> /* Wait the next SLEEP cycle */
> msleep(100);
>
> +#ifdef ENABLE_ARET
> + rc = at86rf230_write_subreg(lp, SR_TRX_CMD, STATE_TX_ARET_ON);
> +#else
> rc = at86rf230_write_subreg(lp, SR_TRX_CMD, STATE_TX_ON);
> +#endif
> if (rc)
> return rc;
> msleep(1);
> @@ -802,7 +900,11 @@ static int __devinit at86rf230_probe(struct spi_device
> *spi)
> dev->extra_tx_headroom = 0;
> /* We do support only 2.4 Ghz */
> dev->phy->channels_supported[0] = 0x7FFF800;
> +#ifdef ENABLE_AACK
> + dev->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK;
> +#else
> dev->flags = IEEE802154_HW_OMIT_CKSUM;
> +#endif
>
> mutex_init(&lp->bmux);
> INIT_WORK(&lp->irqwork, at86rf230_irqwork);
It looks like you've put some work into this. Good job. I think a little
bit more and it'll be first class. :)
Alan.
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
Linux-zigbee-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
