Hi Alexander,
aahringo@xxxxxxxxxx wrote on Sun, 4 Sep 2022 20:55:44 -0400:
> This patch adds support for reading the trac register if atusb firmware
> reports tx done. There is currently a feature to compare a sequence
> number, if the payload is 1 it tells the driver only the sequence number
> is available if it's two there is additional the trac status register as
> payload.
>
> Currently the atusb_in_good() function determines if it's a tx done or
> rx done if according the payload length. This patch is doing the same
> and assumes this behaviour.
>
> Signed-off-by: Alexander Aring <aahringo@xxxxxxxxxx>
> ---
>
> Just an RFC, need another weekend to test it.
>
> drivers/net/ieee802154/atusb.c | 33 ++++++++++++++++++++++++++++-----
> 1 file changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> index 2c338783893d..95a4a3cdc8a4 100644
> --- a/drivers/net/ieee802154/atusb.c
> +++ b/drivers/net/ieee802154/atusb.c
> @@ -191,7 +191,7 @@ static void atusb_work_urbs(struct work_struct *work)
>
> /* ----- Asynchronous USB -------------------------------------------------- */
>
> -static void atusb_tx_done(struct atusb *atusb, u8 seq)
> +static void atusb_tx_done(struct atusb *atusb, u8 seq, int reason)
> {
> struct usb_device *usb_dev = atusb->usb_dev;
> u8 expect = atusb->tx_ack_seq;
> @@ -199,7 +199,10 @@ static void atusb_tx_done(struct atusb *atusb, u8 seq)
> dev_dbg(&usb_dev->dev, "%s (0x%02x/0x%02x)\n", __func__, seq, expect);
> if (seq == expect) {
> /* TODO check for ifs handling in firmware */
> - ieee802154_xmit_complete(atusb->hw, atusb->tx_skb, false);
> + if (reason == IEEE802154_SUCCESS)
> + ieee802154_xmit_complete(atusb->hw, atusb->tx_skb, false);
> + else
> + ieee802154_xmit_error(atusb->hw, atusb->tx_skb, reason);
> } else {
> /* TODO I experience this case when atusb has a tx complete
> * irq before probing, we should fix the firmware it's an
> @@ -215,7 +218,8 @@ static void atusb_in_good(struct urb *urb)
> struct usb_device *usb_dev = urb->dev;
> struct sk_buff *skb = urb->context;
> struct atusb *atusb = SKB_ATUSB(skb);
> - u8 len, lqi;
> + int result = IEEE802154_SUCCESS;
> + u8 len, lqi, trac;
>
> if (!urb->actual_length) {
> dev_dbg(&usb_dev->dev, "atusb_in: zero-sized URB ?\n");
> @@ -224,8 +228,27 @@ static void atusb_in_good(struct urb *urb)
>
> len = *skb->data;
>
> - if (urb->actual_length == 1) {
> - atusb_tx_done(atusb, len);
> + switch (urb->actual_length) {
> + case 2:
> + trac = TRAC_MASK(*(skb->data + 1));
I've been fighting all night thinking the issues were on the atusb side
(it was horribly difficult to get the atusb toolchain up and running,
I'll send a patch to update the instructions), in particular because of
the data[2] definition which needed to be declared static outside of
the functions (see the other mail) and, I guess, because of this
beginner error: I was using skb->data[1] but of course it can't work.
Anyway, this patch works, I've tested it by:
- associating a device (an Arduino Nano running Zephyr, btw)
- having the atusb disassociating the nano successfully (trac = 0)
- reassociating the nano
- powering off the nano
- trying to disassociate
The disassociation notification transmission fails with a TRAC status
NO_ACK. So:
Tested-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
I still have the filtering thing to improve, I'm not sure I'll have
time to do that this week. I need some time to prepare the slides now
that the demo works :)
> + switch (trac) {
> + case TRAC_SUCCESS:
> + case TRAC_SUCCESS_DATA_PENDING:
> + /* already IEEE802154_SUCCESS */
> + break;
> + case TRAC_CHANNEL_ACCESS_FAILURE:
> + result = IEEE802154_CHANNEL_ACCESS_FAILURE;
> + break;
> + case TRAC_NO_ACK:
> + result = IEEE802154_NO_ACK;
> + break;
> + default:
> + result = IEEE802154_SYSTEM_ERROR;
> + }
> +
> + fallthrough;
> + case 1:
> + atusb_tx_done(atusb, len, result);
> return;
> }
>
Thanks,
Miquèl
