Re: [PATCH wpan/next v3 9/9] ieee802154: atusb: add support for trac feature

[Stefan Schmidt <stefan@datenfreihafen.org>]

Hello Miquel, Alexander.

On 05.09.22 22:34, Miquel Raynal wrote:
> From: Alexander Aring <aahringo@redhat.com>
> 
> 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@redhat.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>   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));
> +		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;
>   	}
>   

There have been various RFC patches from either of you two on this. From 
what I can see this is the last one.

I am glad that this is done in a backwards compatible way, so it can 
keep working with the older v.03 firmware. See my comments on the 
firmware part of this in the other thread.

This patch has been applied to the wpan-next tree and will be
part of the next pull request to net-next. Thanks!

regards
Stefan Schmidt