Hi,
On Sun, Sep 4, 2022 at 11:22 PM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
>
> 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
Really? for me it was just apt install avr-gcc libc-avr (or what the
debian packages name is). Then the dfu-util and be sure you invoke
dfu-util (as root because you might need some udev rules otherwise)
when the atusb is booting up.
> 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.
>
mhh, I am sorry. I am not sure if I understand what you mean? Does the
firmware patch have issues regarding data[2]?
> 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 :)
>
ok, no problem. Thanks for testing it. I am happy that if I could help
you here a little bit.
- Alex
