[PATCH wpan-next v4 00/11] ieee802154: Better Tx error handling

[PATCH wpan-next v4 00/11] ieee802154: Better Tx error handling

[Miquel Raynal]

The idea here is to provide a fully synchronous Tx API and also be able
to be sure that a transfer as finished. This will be used later by
another series. However, while working on this task, it appeared
necessary to first rework the way MLME errors were (not) propagated to
the upper layers. This small series tries to tackle exactly that.

Changes in v4:
* Reverted the at86rf320 patch introducing trac values for debugfs
  purposes as suggested by Alex. Reintroduced some of its content in a
  subsequent patch to filter out offloaded transmission error cases.
* Used IEEE802154_SYSTEM_ERROR as a non specific error code.

Changes in v3:
* Split the series into two parts, this is the "error handling" halve.
* Reworked the error path to not handle the ifs_handling situation
  anymore.
* Enhanced the list of MLME status codes available.
* Improved the error handling by collecting the error codes, somethimes
  by changing device drivers directly to propagate these MLME
  statuses. Then, once in the core, save one global Tx status value so
  that in the case of synchronous transfers we can check the return
  value and eventually error out.
* Prevented the core to stop the device before the end of the last
  transmission to avoid deadlocks by just sync'ing the last Tx
  transfer.

Changes in v2:
* Adapted with the changes already merged/refused.

Miquel Raynal (11):
  net: ieee802154: Enhance/fix the names of the MLME return codes
  net: ieee802154: Fill the list of MLME return codes
  net: mac802154: Save a global error code on transmissions
  net: mac802154: Create a transmit error helper
  Revert "at86rf230: add debugfs support"
  net: ieee802154: at86rf230: Error out upon failed offloaded
    transmissions
  net: ieee802154: at86rf230: Provide meaningful error codes when
    possible
  net: ieee802154: at86rf230: Call _xmit_error() when a transmission
    fails
  net: ieee802154: atusb: Call _xmit_error() when a transmission fails
  net: ieee802154: ca8210: Use core return codes instead of hardcoding
    them
  net: ieee802154: ca8210: Call _xmit_error() when a transmission fails

 drivers/net/ieee802154/Kconfig     |   7 --
 drivers/net/ieee802154/at86rf230.c | 139 +++++-----------------
 drivers/net/ieee802154/atusb.c     |   5 +-
 drivers/net/ieee802154/ca8210.c    | 182 +++++++++++------------------
 include/linux/ieee802154.h         |  81 +++++++++++--
 include/net/mac802154.h            |  10 ++
 net/mac802154/ieee802154_i.h       |   2 +
 net/mac802154/util.c               |  16 ++-
 8 files changed, 202 insertions(+), 240 deletions(-)

-- 
2.27.0

[PATCH wpan-next v4 01/11] net: ieee802154: Enhance/fix the names of the MLME return codes

[Miquel Raynal]

Let's keep these definitions as close to the specification as possible
while they are not yet in use. The names get slightly longer, but we
gain the minor cost of being able to search the spec more easily.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/linux/ieee802154.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index 95c831162212..01d945c8b2e1 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -136,16 +136,16 @@ enum {
 	IEEE802154_SUCCESS = 0x0,
 
 	/* The beacon was lost following a synchronization request. */
-	IEEE802154_BEACON_LOSS = 0xe0,
+	IEEE802154_BEACON_LOST = 0xe0,
 	/*
 	 * A transmission could not take place due to activity on the
 	 * channel, i.e., the CSMA-CA mechanism has failed.
 	 */
-	IEEE802154_CHNL_ACCESS_FAIL = 0xe1,
+	IEEE802154_CHANNEL_ACCESS_FAILURE = 0xe1,
 	/* The GTS request has been denied by the PAN coordinator. */
-	IEEE802154_DENINED = 0xe2,
+	IEEE802154_DENIED = 0xe2,
 	/* The attempt to disable the transceiver has failed. */
-	IEEE802154_DISABLE_TRX_FAIL = 0xe3,
+	IEEE802154_DISABLE_TRX_FAILURE = 0xe3,
 	/*
 	 * The received frame induces a failed security check according to
 	 * the security suite.
@@ -185,9 +185,9 @@ enum {
 	 * A PAN identifier conflict has been detected and communicated to the
 	 * PAN coordinator.
 	 */
-	IEEE802154_PANID_CONFLICT = 0xee,
+	IEEE802154_PAN_ID_CONFLICT = 0xee,
 	/* A coordinator realignment command has been received. */
-	IEEE802154_REALIGMENT = 0xef,
+	IEEE802154_REALIGNMENT = 0xef,
 	/* The transaction has expired and its information discarded. */
 	IEEE802154_TRANSACTION_EXPIRED = 0xf0,
 	/* There is no capacity to store the transaction. */
@@ -203,7 +203,7 @@ enum {
 	 * A SET/GET request was issued with the identifier of a PIB attribute
 	 * that is not supported.
 	 */
-	IEEE802154_UNSUPPORTED_ATTR = 0xf4,
+	IEEE802154_UNSUPPORTED_ATTRIBUTE = 0xf4,
 	/*
 	 * A request to perform a scan operation failed because the MLME was
 	 * in the process of performing a previously initiated scan operation.
-- 
2.27.0

[PATCH wpan-next v4 02/11] net: ieee802154: Fill the list of MLME return codes

[Miquel Raynal]

There are more codes than already listed, let's be a bit more
exhaustive. This will allow to drop device drivers local definitions of
these codes.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/linux/ieee802154.h | 67 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index 01d945c8b2e1..f1f9412b6ac6 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -134,7 +134,35 @@ enum {
 	 * a successful transmission.
 	 */
 	IEEE802154_SUCCESS = 0x0,
-
+	/* The requested operation failed. */
+	IEEE802154_MAC_ERROR = 0x1,
+	/* The requested operation has been cancelled. */
+	IEEE802154_CANCELLED = 0x2,
+	/*
+	 * Device is ready to poll the coordinator for data in a non beacon
+	 * enabled PAN.
+	 */
+	IEEE802154_READY_FOR_POLL = 0x3,
+	/* Wrong frame counter. */
+	IEEE802154_COUNTER_ERROR = 0xdb,
+	/*
+	 * The frame does not conforms to the incoming key usage policy checking
+	 * procedure.
+	 */
+	IEEE802154_IMPROPER_KEY_TYPE = 0xdc,
+	/*
+	 * The frame does not conforms to the incoming security level usage
+	 * policy checking procedure.
+	 */
+	IEEE802154_IMPROPER_SECURITY_LEVEL = 0xdd,
+	/* Secured frame received with an empty Frame Version field. */
+	IEEE802154_UNSUPPORTED_LEGACY = 0xde,
+	/*
+	 * A secured frame is received or must be sent but security is not
+	 * enabled in the device. Or, the Auxiliary Security Header has security
+	 * level of zero in it.
+	 */
+	IEEE802154_UNSUPPORTED_SECURITY = 0xdf,
 	/* The beacon was lost following a synchronization request. */
 	IEEE802154_BEACON_LOST = 0xe0,
 	/*
@@ -204,11 +232,48 @@ enum {
 	 * that is not supported.
 	 */
 	IEEE802154_UNSUPPORTED_ATTRIBUTE = 0xf4,
+	/* Missing source or destination address or address mode. */
+	IEEE802154_INVALID_ADDRESS = 0xf5,
+	/*
+	 * MLME asked to turn the receiver on, but the on time duration is too
+	 * big compared to the macBeaconOrder.
+	 */
+	IEEE802154_ON_TIME_TOO_LONG = 0xf6,
+	/*
+	 * MLME asaked to turn the receiver on, but the request was delayed for
+	 * too long before getting processed.
+	 */
+	IEEE802154_PAST_TIME = 0xf7,
+	/*
+	 * The StartTime parameter is nonzero, and the MLME is not currently
+	 * tracking the beacon of the coordinator through which it is
+	 * associated.
+	 */
+	IEEE802154_TRACKING_OFF = 0xf8,
+	/*
+	 * The index inside the hierarchical values in PIBAttribute is out of
+	 * range.
+	 */
+	IEEE802154_INVALID_INDEX = 0xf9,
+	/*
+	 * The number of PAN descriptors discovered during a scan has been
+	 * reached.
+	 */
+	IEEE802154_LIMIT_REACHED = 0xfa,
+	/*
+	 * The PIBAttribute parameter specifies an attribute that is a read-only
+	 * attribute.
+	 */
+	IEEE802154_READ_ONLY = 0xfb,
 	/*
 	 * A request to perform a scan operation failed because the MLME was
 	 * in the process of performing a previously initiated scan operation.
 	 */
 	IEEE802154_SCAN_IN_PROGRESS = 0xfc,
+	/* The outgoing superframe overlaps the incoming superframe. */
+	IEEE802154_SUPERFRAME_OVERLAP = 0xfd,
+	/* Any other error situation. */
+	IEEE802154_SYSTEM_ERROR = 0xff,
 };
 
 /* frame control handling */
-- 
2.27.0

[PATCH wpan-next v4 03/11] net: mac802154: Save a global error code on transmissions

[Miquel Raynal]

So far no error is returned from a failing transmission. However it
might sometimes be useful, and particularly easy to use during sync
transfers (for certain MLME commands). Let's create an internal variable
for that, global to the device. Right now only success are registered,
which is rather useless, but soon we will have more situations filling
this field.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 net/mac802154/ieee802154_i.h | 2 ++
 net/mac802154/util.c         | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 702560acc8ce..1381e6a5e180 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -56,6 +56,8 @@ struct ieee802154_local {
 
 	struct sk_buff *tx_skb;
 	struct work_struct tx_work;
+	/* A negative Linux error code or a null/positive MLME error status */
+	int tx_result;
 };
 
 enum {
diff --git a/net/mac802154/util.c b/net/mac802154/util.c
index f2078238718b..0bf46f174de3 100644
--- a/net/mac802154/util.c
+++ b/net/mac802154/util.c
@@ -58,8 +58,11 @@ enum hrtimer_restart ieee802154_xmit_ifs_timer(struct hrtimer *timer)
 void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb,
 			      bool ifs_handling)
 {
+	struct ieee802154_local *local = hw_to_local(hw);
+
+	local->tx_result = IEEE802154_SUCCESS;
+
 	if (ifs_handling) {
-		struct ieee802154_local *local = hw_to_local(hw);
 		u8 max_sifs_size;
 
 		/* If transceiver sets CRC on his own we need to use lifs
-- 
2.27.0

[PATCH wpan-next v4 04/11] net: mac802154: Create a transmit error helper

[Miquel Raynal]

So far there is only a helper for successful transmission, which led
device drivers to implement their own handling in case of
error. Unfortunately, we really need all the drivers to give the hand
back to the core once they are done in order to be able to build a
proper synchronous API. So let's create a _xmit_error() helper and take
this opportunity to fill the new device-global field storing Tx
statuses.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/mac802154.h | 10 ++++++++++
 net/mac802154/util.c    | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/net/mac802154.h b/include/net/mac802154.h
index 2c3bbc6645ba..abbe88dc9df5 100644
--- a/include/net/mac802154.h
+++ b/include/net/mac802154.h
@@ -498,4 +498,14 @@ void ieee802154_stop_queue(struct ieee802154_hw *hw);
 void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb,
 			      bool ifs_handling);
 
+/**
+ * ieee802154_xmit_error - frame transmission failed
+ *
+ * @hw: pointer as obtained from ieee802154_alloc_hw().
+ * @skb: buffer for transmission
+ * @reason: error code
+ */
+void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb,
+			   int reason);
+
 #endif /* NET_MAC802154_H */
diff --git a/net/mac802154/util.c b/net/mac802154/util.c
index 0bf46f174de3..ec523335336c 100644
--- a/net/mac802154/util.c
+++ b/net/mac802154/util.c
@@ -91,6 +91,17 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb,
 }
 EXPORT_SYMBOL(ieee802154_xmit_complete);
 
+void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb,
+			   int reason)
+{
+	struct ieee802154_local *local = hw_to_local(hw);
+
+	local->tx_result = reason;
+	ieee802154_wake_queue(hw);
+	dev_kfree_skb_any(skb);
+}
+EXPORT_SYMBOL(ieee802154_xmit_error);
+
 void ieee802154_stop_device(struct ieee802154_local *local)
 {
 	flush_workqueue(local->workqueue);
-- 
2.27.0

[PATCH wpan-next v4 05/11] Revert "at86rf230: add debugfs support"

[Miquel Raynal]

This reverts commit 493bc90a96839ffde5e6216c62c025d2f9e6efc3.

This commit was introduced because of some testing capabilities
involving ack handling. Not we want to collect errors so let's first
revert this commit, then right after reintroduce the Tx trac handling
which is the only one that makes sense for now.

Suggested-by: Alexander Aring <alex.aring@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/Kconfig     |   7 --
 drivers/net/ieee802154/at86rf230.c | 108 +----------------------------
 drivers/net/ieee802154/at86rf230.h |   8 ---
 3 files changed, 1 insertion(+), 122 deletions(-)

diff --git a/drivers/net/ieee802154/Kconfig b/drivers/net/ieee802154/Kconfig
index 0f7c6dc2ed15..95da876c5613 100644
--- a/drivers/net/ieee802154/Kconfig
+++ b/drivers/net/ieee802154/Kconfig
@@ -33,13 +33,6 @@ config IEEE802154_AT86RF230
 	  This driver can also be built as a module. To do so, say M here.
 	  the module will be called 'at86rf230'.
 
-config IEEE802154_AT86RF230_DEBUGFS
-	depends on IEEE802154_AT86RF230
-	bool "AT86RF230 debugfs interface"
-	depends on DEBUG_FS
-	help
-	  This option compiles debugfs code for the at86rf230 driver.
-
 config IEEE802154_MRF24J40
 	tristate "Microchip MRF24J40 transceiver driver"
 	depends on IEEE802154_DRIVERS && MAC802154
diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 563031ce76f0..8a03722cd1f7 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -23,7 +23,6 @@
 #include <linux/skbuff.h>
 #include <linux/of_gpio.h>
 #include <linux/ieee802154.h>
-#include <linux/debugfs.h>
 
 #include <net/mac802154.h>
 #include <net/cfg802154.h>
@@ -76,15 +75,6 @@ struct at86rf230_state_change {
 	bool free;
 };
 
-struct at86rf230_trac {
-	u64 success;
-	u64 success_data_pending;
-	u64 success_wait_for_ack;
-	u64 channel_access_failure;
-	u64 no_ack;
-	u64 invalid;
-};
-
 struct at86rf230_local {
 	struct spi_device *spi;
 
@@ -104,8 +94,6 @@ struct at86rf230_local {
 	u8 tx_retry;
 	struct sk_buff *tx_skb;
 	struct at86rf230_state_change tx;
-
-	struct at86rf230_trac trac;
 };
 
 #define AT86RF2XX_NUMREGS 0x3F
@@ -673,31 +661,6 @@ at86rf230_tx_trac_check(void *context)
 	struct at86rf230_state_change *ctx = context;
 	struct at86rf230_local *lp = ctx->lp;
 
-	if (IS_ENABLED(CONFIG_IEEE802154_AT86RF230_DEBUGFS)) {
-		u8 trac = TRAC_MASK(ctx->buf[1]);
-
-		switch (trac) {
-		case TRAC_SUCCESS:
-			lp->trac.success++;
-			break;
-		case TRAC_SUCCESS_DATA_PENDING:
-			lp->trac.success_data_pending++;
-			break;
-		case TRAC_CHANNEL_ACCESS_FAILURE:
-			lp->trac.channel_access_failure++;
-			break;
-		case TRAC_NO_ACK:
-			lp->trac.no_ack++;
-			break;
-		case TRAC_INVALID:
-			lp->trac.invalid++;
-			break;
-		default:
-			WARN_ONCE(1, "received tx trac status %d\n", trac);
-			break;
-		}
-	}
-
 	at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
 }
 
@@ -737,25 +700,6 @@ at86rf230_rx_trac_check(void *context)
 	u8 *buf = ctx->buf;
 	int rc;
 
-	if (IS_ENABLED(CONFIG_IEEE802154_AT86RF230_DEBUGFS)) {
-		u8 trac = TRAC_MASK(buf[1]);
-
-		switch (trac) {
-		case TRAC_SUCCESS:
-			lp->trac.success++;
-			break;
-		case TRAC_SUCCESS_WAIT_FOR_ACK:
-			lp->trac.success_wait_for_ack++;
-			break;
-		case TRAC_INVALID:
-			lp->trac.invalid++;
-			break;
-		default:
-			WARN_ONCE(1, "received rx trac status %d\n", trac);
-			break;
-		}
-	}
-
 	buf[0] = CMD_FB;
 	ctx->trx.len = AT86RF2XX_MAX_BUF;
 	ctx->msg.complete = at86rf230_rx_read_frame_complete;
@@ -951,10 +895,6 @@ at86rf230_start(struct ieee802154_hw *hw)
 {
 	struct at86rf230_local *lp = hw->priv;
 
-	/* reset trac stats on start */
-	if (IS_ENABLED(CONFIG_IEEE802154_AT86RF230_DEBUGFS))
-		memset(&lp->trac, 0, sizeof(struct at86rf230_trac));
-
 	at86rf230_awake(lp);
 	enable_irq(lp->spi->irq);
 
@@ -1582,47 +1522,6 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 	return rc;
 }
 
-#ifdef CONFIG_IEEE802154_AT86RF230_DEBUGFS
-static struct dentry *at86rf230_debugfs_root;
-
-static int at86rf230_stats_show(struct seq_file *file, void *offset)
-{
-	struct at86rf230_local *lp = file->private;
-
-	seq_printf(file, "SUCCESS:\t\t%8llu\n", lp->trac.success);
-	seq_printf(file, "SUCCESS_DATA_PENDING:\t%8llu\n",
-		   lp->trac.success_data_pending);
-	seq_printf(file, "SUCCESS_WAIT_FOR_ACK:\t%8llu\n",
-		   lp->trac.success_wait_for_ack);
-	seq_printf(file, "CHANNEL_ACCESS_FAILURE:\t%8llu\n",
-		   lp->trac.channel_access_failure);
-	seq_printf(file, "NO_ACK:\t\t\t%8llu\n", lp->trac.no_ack);
-	seq_printf(file, "INVALID:\t\t%8llu\n", lp->trac.invalid);
-	return 0;
-}
-DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
-
-static void at86rf230_debugfs_init(struct at86rf230_local *lp)
-{
-	char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
-
-	strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
-
-	at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
-
-	debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
-			    &at86rf230_stats_fops);
-}
-
-static void at86rf230_debugfs_remove(void)
-{
-	debugfs_remove_recursive(at86rf230_debugfs_root);
-}
-#else
-static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
-static void at86rf230_debugfs_remove(void) { }
-#endif
-
 static int at86rf230_probe(struct spi_device *spi)
 {
 	struct ieee802154_hw *hw;
@@ -1719,16 +1618,12 @@ static int at86rf230_probe(struct spi_device *spi)
 	/* going into sleep by default */
 	at86rf230_sleep(lp);
 
-	at86rf230_debugfs_init(lp);
-
 	rc = ieee802154_register_hw(lp->hw);
 	if (rc)
-		goto free_debugfs;
+		goto free_dev;
 
 	return rc;
 
-free_debugfs:
-	at86rf230_debugfs_remove();
 free_dev:
 	ieee802154_free_hw(lp->hw);
 
@@ -1743,7 +1638,6 @@ static int at86rf230_remove(struct spi_device *spi)
 	at86rf230_write_subreg(lp, SR_IRQ_MASK, 0);
 	ieee802154_unregister_hw(lp->hw);
 	ieee802154_free_hw(lp->hw);
-	at86rf230_debugfs_remove();
 	dev_dbg(&spi->dev, "unregistered at86rf230\n");
 
 	return 0;
diff --git a/drivers/net/ieee802154/at86rf230.h b/drivers/net/ieee802154/at86rf230.h
index 042bb27287a3..fbdfb705c7a3 100644
--- a/drivers/net/ieee802154/at86rf230.h
+++ b/drivers/net/ieee802154/at86rf230.h
@@ -208,13 +208,5 @@
 #define STATE_TRANSITION_IN_PROGRESS 0x1F
 
 #define TRX_STATE_MASK		(0x1F)
-#define TRAC_MASK(x)		((x & 0xe0) >> 5)
-
-#define TRAC_SUCCESS			0
-#define TRAC_SUCCESS_DATA_PENDING	1
-#define TRAC_SUCCESS_WAIT_FOR_ACK	2
-#define TRAC_CHANNEL_ACCESS_FAILURE	3
-#define TRAC_NO_ACK			5
-#define TRAC_INVALID			7
 
 #endif /* !_AT86RF230_H */
-- 
2.27.0

Re: [PATCH wpan-next v4 05/11] Revert "at86rf230: add debugfs support"

[Alexander Aring]

Hi,

On Fri, Mar 18, 2022 at 2:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> This reverts commit 493bc90a96839ffde5e6216c62c025d2f9e6efc3.
>
> This commit was introduced because of some testing capabilities
> involving ack handling. Not we want to collect errors so let's first
> revert this commit, then right after reintroduce the Tx trac handling
> which is the only one that makes sense for now.
>
> Suggested-by: Alexander Aring <alex.aring@gmail.com>

I didn't want to have a revert of the commit or... yes I wanted but
not in such a way as `git revert...`, just that the whole debug
feature is dropped (which was introduced by a commit) because you
implemented a real first use case to make something with those values
now and do not leave a half of the debug feature upstream.

Nevertheless it's okay, this way will do it as well...

- Alex

[PATCH wpan-next v4 06/11] net: ieee802154: at86rf230: Error out upon failed offloaded transmissions

[Miquel Raynal]

Let's parse the TRAC register in the Tx path in order to know if the
offloaded transmission was successful. If not, let's return early with
an error code.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/at86rf230.c | 10 +++++++++-
 drivers/net/ieee802154/at86rf230.h |  8 ++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 8a03722cd1f7..d3cf6d23b57e 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -660,8 +660,16 @@ at86rf230_tx_trac_check(void *context)
 {
 	struct at86rf230_state_change *ctx = context;
 	struct at86rf230_local *lp = ctx->lp;
+	u8 trac = TRAC_MASK(ctx->buf[1]);
 
-	at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
+	switch (trac) {
+	case TRAC_SUCCESS:
+	case TRAC_SUCCESS_DATA_PENDING:
+		at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
+		break;
+	default:
+		at86rf230_async_error(lp, ctx, -EIO);
+	}
 }
 
 static void
diff --git a/drivers/net/ieee802154/at86rf230.h b/drivers/net/ieee802154/at86rf230.h
index fbdfb705c7a3..042bb27287a3 100644
--- a/drivers/net/ieee802154/at86rf230.h
+++ b/drivers/net/ieee802154/at86rf230.h
@@ -208,5 +208,13 @@
 #define STATE_TRANSITION_IN_PROGRESS 0x1F
 
 #define TRX_STATE_MASK		(0x1F)
+#define TRAC_MASK(x)		((x & 0xe0) >> 5)
+
+#define TRAC_SUCCESS			0
+#define TRAC_SUCCESS_DATA_PENDING	1
+#define TRAC_SUCCESS_WAIT_FOR_ACK	2
+#define TRAC_CHANNEL_ACCESS_FAILURE	3
+#define TRAC_NO_ACK			5
+#define TRAC_INVALID			7
 
 #endif /* !_AT86RF230_H */
-- 
2.27.0

[PATCH wpan-next v4 07/11] net: ieee802154: at86rf230: Provide meaningful error codes when possible

[Miquel Raynal]

Either the spi operation failed, or the offloaded transmit operation
failed and returned a TRAC value. Use this value when available or use
the default "SYSTEM_ERROR" otherwise, in order to propagate one step
above the error.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/at86rf230.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index d3cf6d23b57e..34d199f597c9 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -358,7 +358,23 @@ static inline void
 at86rf230_async_error(struct at86rf230_local *lp,
 		      struct at86rf230_state_change *ctx, int rc)
 {
-	dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
+	int reason;
+
+	switch (rc) {
+	case TRAC_CHANNEL_ACCESS_FAILURE:
+		reason = IEEE802154_CHANNEL_ACCESS_FAILURE;
+		break;
+	case TRAC_NO_ACK:
+		reason = IEEE802154_NO_ACK;
+		break;
+	default:
+		reason = IEEE802154_SYSTEM_ERROR;
+	}
+
+	if (rc < 0)
+		dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
+	else
+		dev_err(&lp->spi->dev, "xceiver error %d\n", reason);
 
 	at86rf230_async_state_change(lp, ctx, STATE_FORCE_TRX_OFF,
 				     at86rf230_async_error_recover);
@@ -666,10 +682,15 @@ at86rf230_tx_trac_check(void *context)
 	case TRAC_SUCCESS:
 	case TRAC_SUCCESS_DATA_PENDING:
 		at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
+		return;
+	case TRAC_CHANNEL_ACCESS_FAILURE:
+	case TRAC_NO_ACK:
 		break;
 	default:
-		at86rf230_async_error(lp, ctx, -EIO);
+		trac = TRAC_INVALID;
 	}
+
+	at86rf230_async_error(lp, ctx, trac);
 }
 
 static void
-- 
2.27.0

Re: [PATCH wpan-next v4 07/11] net: ieee802154: at86rf230: Provide meaningful error codes when possible

[Alexander Aring]

Hi,

On Fri, Mar 18, 2022 at 2:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Either the spi operation failed, or the offloaded transmit operation
> failed and returned a TRAC value. Use this value when available or use
> the default "SYSTEM_ERROR" otherwise, in order to propagate one step
> above the error.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/net/ieee802154/at86rf230.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> index d3cf6d23b57e..34d199f597c9 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -358,7 +358,23 @@ static inline void
>  at86rf230_async_error(struct at86rf230_local *lp,
>                       struct at86rf230_state_change *ctx, int rc)
>  {
> -       dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> +       int reason;
> +
> +       switch (rc) {

I think there was a miscommunication last time, this rc variable is
not a trac register value, it is a linux errno. Also the error here
has nothing to do with a trac error. A trac error is the result of the
offloaded transmit functionality on the transceiver, here we dealing
about bus communication errors produced by the spi subsystem. What we
need is to report it to the softmac layer as "IEEE802154_SYSTEM_ERROR"
(as we decided that this is a user specific error and can be returned
by the transceiver for non 802.15.4 "error" return code.

> +       case TRAC_CHANNEL_ACCESS_FAILURE:
> +               reason = IEEE802154_CHANNEL_ACCESS_FAILURE;
> +               break;
> +       case TRAC_NO_ACK:
> +               reason = IEEE802154_NO_ACK;
> +               break;
> +       default:
> +               reason = IEEE802154_SYSTEM_ERROR;
> +       }
> +
> +       if (rc < 0)
> +               dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> +       else
> +               dev_err(&lp->spi->dev, "xceiver error %d\n", reason);
>
>         at86rf230_async_state_change(lp, ctx, STATE_FORCE_TRX_OFF,
>                                      at86rf230_async_error_recover);
> @@ -666,10 +682,15 @@ at86rf230_tx_trac_check(void *context)
>         case TRAC_SUCCESS:
>         case TRAC_SUCCESS_DATA_PENDING:
>                 at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
> +               return;
> +       case TRAC_CHANNEL_ACCESS_FAILURE:
> +       case TRAC_NO_ACK:
>                 break;
>         default:
> -               at86rf230_async_error(lp, ctx, -EIO);
> +               trac = TRAC_INVALID;
>         }
> +
> +       at86rf230_async_error(lp, ctx, trac);

That makes no sense, at86rf230_async_error() is not a trac error
handling, it is a bus error handling. As noted above. With this change
you mix bus errors and trac errors (which are not bus errors). If
there are no bus errors then trac should be evaluated and should
either deliver some 802.15.4 $SUCCESS_CODE or $ERROR_CODE to the
softmac stack, which is xmit_complete() or xmit_error().

- Alex

Re: [PATCH wpan-next v4 07/11] net: ieee802154: at86rf230: Provide meaningful error codes when possible

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Sun, 27 Mar 2022 11:46:12 -0400:

> Hi,
> 
> On Fri, Mar 18, 2022 at 2:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Either the spi operation failed, or the offloaded transmit operation
> > failed and returned a TRAC value. Use this value when available or use
> > the default "SYSTEM_ERROR" otherwise, in order to propagate one step
> > above the error.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/net/ieee802154/at86rf230.c | 25 +++++++++++++++++++++++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > index d3cf6d23b57e..34d199f597c9 100644
> > --- a/drivers/net/ieee802154/at86rf230.c
> > +++ b/drivers/net/ieee802154/at86rf230.c
> > @@ -358,7 +358,23 @@ static inline void
> >  at86rf230_async_error(struct at86rf230_local *lp,
> >                       struct at86rf230_state_change *ctx, int rc)
> >  {
> > -       dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> > +       int reason;
> > +
> > +       switch (rc) {  
> 
> I think there was a miscommunication last time, this rc variable is
> not a trac register value, it is a linux errno. Also the error here
> has nothing to do with a trac error. A trac error is the result of the
> offloaded transmit functionality on the transceiver, here we dealing
> about bus communication errors produced by the spi subsystem. What we
> need is to report it to the softmac layer as "IEEE802154_SYSTEM_ERROR"
> (as we decided that this is a user specific error and can be returned
> by the transceiver for non 802.15.4 "error" return code.
> 
> > +       case TRAC_CHANNEL_ACCESS_FAILURE:
> > +               reason = IEEE802154_CHANNEL_ACCESS_FAILURE;
> > +               break;
> > +       case TRAC_NO_ACK:
> > +               reason = IEEE802154_NO_ACK;
> > +               break;
> > +       default:
> > +               reason = IEEE802154_SYSTEM_ERROR;

I went for the solution: if it is a bus error, I return SYSTEM ERROR,
otherwise I return a trac error.

> > +       }
> > +
> > +       if (rc < 0)
> > +               dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> > +       else
> > +               dev_err(&lp->spi->dev, "xceiver error %d\n", reason);
> >
> >         at86rf230_async_state_change(lp, ctx, STATE_FORCE_TRX_OFF,
> >                                      at86rf230_async_error_recover);
> > @@ -666,10 +682,15 @@ at86rf230_tx_trac_check(void *context)
> >         case TRAC_SUCCESS:
> >         case TRAC_SUCCESS_DATA_PENDING:
> >                 at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
> > +               return;
> > +       case TRAC_CHANNEL_ACCESS_FAILURE:
> > +       case TRAC_NO_ACK:
> >                 break;
> >         default:
> > -               at86rf230_async_error(lp, ctx, -EIO);
> > +               trac = TRAC_INVALID;
> >         }
> > +
> > +       at86rf230_async_error(lp, ctx, trac);  
> 
> That makes no sense, at86rf230_async_error() is not a trac error
> handling, it is a bus error handling. As noted above. With this change
> you mix bus errors and trac errors (which are not bus errors). If
> there are no bus errors then trac should be evaluated and should
> either deliver some 802.15.4 $SUCCESS_CODE or $ERROR_CODE to the
> softmac stack, which is xmit_complete() or xmit_error().

There is no specific path for bus errors, everything is supposedly
asynchronous and all the function return void. In both cases I need to
free the skb. So I am questioning myself about the right solution (need
to think further...)

Thanks,
Miquèl

Re: [PATCH wpan-next v4 07/11] net: ieee802154: at86rf230: Provide meaningful error codes when possible

[Miquel Raynal]

Hi Alexander,

alex.aring@gmail.com wrote on Sun, 27 Mar 2022 11:46:12 -0400:

> Hi,
> 
> On Fri, Mar 18, 2022 at 2:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Either the spi operation failed, or the offloaded transmit operation
> > failed and returned a TRAC value. Use this value when available or use
> > the default "SYSTEM_ERROR" otherwise, in order to propagate one step
> > above the error.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/net/ieee802154/at86rf230.c | 25 +++++++++++++++++++++++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > index d3cf6d23b57e..34d199f597c9 100644
> > --- a/drivers/net/ieee802154/at86rf230.c
> > +++ b/drivers/net/ieee802154/at86rf230.c
> > @@ -358,7 +358,23 @@ static inline void
> >  at86rf230_async_error(struct at86rf230_local *lp,
> >                       struct at86rf230_state_change *ctx, int rc)
> >  {
> > -       dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> > +       int reason;
> > +
> > +       switch (rc) {  
> 
> I think there was a miscommunication last time, this rc variable is
> not a trac register value, it is a linux errno. Also the error here
> has nothing to do with a trac error. A trac error is the result of the
> offloaded transmit functionality on the transceiver, here we dealing
> about bus communication errors produced by the spi subsystem. What we
> need is to report it to the softmac layer as "IEEE802154_SYSTEM_ERROR"
> (as we decided that this is a user specific error and can be returned
> by the transceiver for non 802.15.4 "error" return code.

I think we definitely need to handle both, see below.

> 
> > +       case TRAC_CHANNEL_ACCESS_FAILURE:
> > +               reason = IEEE802154_CHANNEL_ACCESS_FAILURE;
> > +               break;
> > +       case TRAC_NO_ACK:
> > +               reason = IEEE802154_NO_ACK;
> > +               break;
> > +       default:
> > +               reason = IEEE802154_SYSTEM_ERROR;
> > +       }
> > +
> > +       if (rc < 0)
> > +               dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> > +       else
> > +               dev_err(&lp->spi->dev, "xceiver error %d\n", reason);
> >
> >         at86rf230_async_state_change(lp, ctx, STATE_FORCE_TRX_OFF,
> >                                      at86rf230_async_error_recover);
> > @@ -666,10 +682,15 @@ at86rf230_tx_trac_check(void *context)
> >         case TRAC_SUCCESS:
> >         case TRAC_SUCCESS_DATA_PENDING:
> >                 at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
> > +               return;
> > +       case TRAC_CHANNEL_ACCESS_FAILURE:
> > +       case TRAC_NO_ACK:
> >                 break;
> >         default:
> > -               at86rf230_async_error(lp, ctx, -EIO);
> > +               trac = TRAC_INVALID;
> >         }
> > +
> > +       at86rf230_async_error(lp, ctx, trac);  
> 
> That makes no sense, at86rf230_async_error() is not a trac error
> handling, it is a bus error handling.

Both will have to be handled asynchronously, which means we have to
tell the soft mac layer that something bad happened in each case.

> As noted above. With this change
> you mix bus errors and trac errors (which are not bus errors).

In the case of a SPI error, it will happen asynchronously, which means
the tx call is over and something bad happened. We are aware that
something bad happened and there was a bus error. We need to:
- Free the skb
- Restart the internal machinery
- Somehow tell the soft mac layer something bad happened and the packet
  will not be transmitted as expected (IOW, balance the "end" calls
  with the "start" calls, just because we did not return immediately
  when we got the transmit request).

In the case of a transmission error, this is a trac condition that is
reported to us by an IRQ. We know it is a trac error, we can look at a
buffer to find which trac error exactly happened. In this case we need
to go through exactly the same steps as above.

But you are right that a spi_async() error is not a trac error, hence
my choice in the switch statement to default to the
IEEE80154_SYSTEM_ERROR flag in this case.

Should I ignore spi bus errors? I don't think I can, so I don't really
see how to handle it differently.

Thanks,
Miquèl

Re: [PATCH wpan-next v4 07/11] net: ieee802154: at86rf230: Provide meaningful error codes when possible

[Miquel Raynal]

Hello,

miquel.raynal@bootlin.com wrote on Tue, 29 Mar 2022 18:35:06 +0200:

> Hi Alexander,
> 
> alex.aring@gmail.com wrote on Sun, 27 Mar 2022 11:46:12 -0400:
> 
> > Hi,
> > 
> > On Fri, Mar 18, 2022 at 2:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > >
> > > Either the spi operation failed, or the offloaded transmit operation
> > > failed and returned a TRAC value. Use this value when available or use
> > > the default "SYSTEM_ERROR" otherwise, in order to propagate one step
> > > above the error.
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/net/ieee802154/at86rf230.c | 25 +++++++++++++++++++++++--
> > >  1 file changed, 23 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > > index d3cf6d23b57e..34d199f597c9 100644
> > > --- a/drivers/net/ieee802154/at86rf230.c
> > > +++ b/drivers/net/ieee802154/at86rf230.c
> > > @@ -358,7 +358,23 @@ static inline void
> > >  at86rf230_async_error(struct at86rf230_local *lp,
> > >                       struct at86rf230_state_change *ctx, int rc)
> > >  {
> > > -       dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> > > +       int reason;
> > > +
> > > +       switch (rc) {    
> > 
> > I think there was a miscommunication last time, this rc variable is
> > not a trac register value, it is a linux errno. Also the error here
> > has nothing to do with a trac error. A trac error is the result of the
> > offloaded transmit functionality on the transceiver, here we dealing
> > about bus communication errors produced by the spi subsystem. What we
> > need is to report it to the softmac layer as "IEEE802154_SYSTEM_ERROR"
> > (as we decided that this is a user specific error and can be returned
> > by the transceiver for non 802.15.4 "error" return code.  
> 
> I think we definitely need to handle both, see below.
> 
> >   
> > > +       case TRAC_CHANNEL_ACCESS_FAILURE:
> > > +               reason = IEEE802154_CHANNEL_ACCESS_FAILURE;
> > > +               break;
> > > +       case TRAC_NO_ACK:
> > > +               reason = IEEE802154_NO_ACK;
> > > +               break;
> > > +       default:
> > > +               reason = IEEE802154_SYSTEM_ERROR;
> > > +       }
> > > +
> > > +       if (rc < 0)
> > > +               dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> > > +       else
> > > +               dev_err(&lp->spi->dev, "xceiver error %d\n", reason);
> > >
> > >         at86rf230_async_state_change(lp, ctx, STATE_FORCE_TRX_OFF,
> > >                                      at86rf230_async_error_recover);
> > > @@ -666,10 +682,15 @@ at86rf230_tx_trac_check(void *context)
> > >         case TRAC_SUCCESS:
> > >         case TRAC_SUCCESS_DATA_PENDING:
> > >                 at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
> > > +               return;
> > > +       case TRAC_CHANNEL_ACCESS_FAILURE:
> > > +       case TRAC_NO_ACK:
> > >                 break;
> > >         default:
> > > -               at86rf230_async_error(lp, ctx, -EIO);
> > > +               trac = TRAC_INVALID;
> > >         }
> > > +
> > > +       at86rf230_async_error(lp, ctx, trac);    
> > 
> > That makes no sense, at86rf230_async_error() is not a trac error
> > handling, it is a bus error handling.  
> 
> Both will have to be handled asynchronously, which means we have to
> tell the soft mac layer that something bad happened in each case.
> 
> > As noted above. With this change
> > you mix bus errors and trac errors (which are not bus errors).  
> 
> In the case of a SPI error, it will happen asynchronously, which means
> the tx call is over and something bad happened. We are aware that
> something bad happened and there was a bus error. We need to:
> - Free the skb
> - Restart the internal machinery
> - Somehow tell the soft mac layer something bad happened and the packet
>   will not be transmitted as expected (IOW, balance the "end" calls
>   with the "start" calls, just because we did not return immediately
>   when we got the transmit request).
> 
> In the case of a transmission error, this is a trac condition that is
> reported to us by an IRQ. We know it is a trac error, we can look at a
> buffer to find which trac error exactly happened. In this case we need
> to go through exactly the same steps as above.
> 
> But you are right that a spi_async() error is not a trac error, hence
> my choice in the switch statement to default to the
> IEEE80154_SYSTEM_ERROR flag in this case.
> 
> Should I ignore spi bus errors? I don't think I can, so I don't really
> see how to handle it differently.

Sorry to bother you again, but in the end, do you agree on returning
IEEE802154_SYSTEM_ERROR upon asynchronous bus errors?

Any other modification of the driver in favor of having two distinct
paths would be really costly in term of time spent and probability of
breaking something, so I would rather avoid it, unless I am missing
something simpler?

Cheers,
Miquèl

Re: [PATCH wpan-next v4 07/11] net: ieee802154: at86rf230: Provide meaningful error codes when possible

[Alexander Aring]

Hi,

On Mon, Apr 4, 2022 at 8:40 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hello,
>
> miquel.raynal@bootlin.com wrote on Tue, 29 Mar 2022 18:35:06 +0200:
>
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Sun, 27 Mar 2022 11:46:12 -0400:
> >
> > > Hi,
> > >
> > > On Fri, Mar 18, 2022 at 2:56 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > Either the spi operation failed, or the offloaded transmit operation
> > > > failed and returned a TRAC value. Use this value when available or use
> > > > the default "SYSTEM_ERROR" otherwise, in order to propagate one step
> > > > above the error.
> > > >
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/net/ieee802154/at86rf230.c | 25 +++++++++++++++++++++++--
> > > >  1 file changed, 23 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > > > index d3cf6d23b57e..34d199f597c9 100644
> > > > --- a/drivers/net/ieee802154/at86rf230.c
> > > > +++ b/drivers/net/ieee802154/at86rf230.c
> > > > @@ -358,7 +358,23 @@ static inline void
> > > >  at86rf230_async_error(struct at86rf230_local *lp,
> > > >                       struct at86rf230_state_change *ctx, int rc)
> > > >  {
> > > > -       dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> > > > +       int reason;
> > > > +
> > > > +       switch (rc) {
> > >
> > > I think there was a miscommunication last time, this rc variable is
> > > not a trac register value, it is a linux errno. Also the error here
> > > has nothing to do with a trac error. A trac error is the result of the
> > > offloaded transmit functionality on the transceiver, here we dealing
> > > about bus communication errors produced by the spi subsystem. What we
> > > need is to report it to the softmac layer as "IEEE802154_SYSTEM_ERROR"
> > > (as we decided that this is a user specific error and can be returned
> > > by the transceiver for non 802.15.4 "error" return code.
> >
> > I think we definitely need to handle both, see below.
> >
> > >
> > > > +       case TRAC_CHANNEL_ACCESS_FAILURE:
> > > > +               reason = IEEE802154_CHANNEL_ACCESS_FAILURE;
> > > > +               break;
> > > > +       case TRAC_NO_ACK:
> > > > +               reason = IEEE802154_NO_ACK;
> > > > +               break;
> > > > +       default:
> > > > +               reason = IEEE802154_SYSTEM_ERROR;
> > > > +       }
> > > > +
> > > > +       if (rc < 0)
> > > > +               dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
> > > > +       else
> > > > +               dev_err(&lp->spi->dev, "xceiver error %d\n", reason);
> > > >
> > > >         at86rf230_async_state_change(lp, ctx, STATE_FORCE_TRX_OFF,
> > > >                                      at86rf230_async_error_recover);
> > > > @@ -666,10 +682,15 @@ at86rf230_tx_trac_check(void *context)
> > > >         case TRAC_SUCCESS:
> > > >         case TRAC_SUCCESS_DATA_PENDING:
> > > >                 at86rf230_async_state_change(lp, ctx, STATE_TX_ON, at86rf230_tx_on);
> > > > +               return;
> > > > +       case TRAC_CHANNEL_ACCESS_FAILURE:
> > > > +       case TRAC_NO_ACK:
> > > >                 break;
> > > >         default:
> > > > -               at86rf230_async_error(lp, ctx, -EIO);
> > > > +               trac = TRAC_INVALID;
> > > >         }
> > > > +
> > > > +       at86rf230_async_error(lp, ctx, trac);
> > >
> > > That makes no sense, at86rf230_async_error() is not a trac error
> > > handling, it is a bus error handling.
> >
> > Both will have to be handled asynchronously, which means we have to
> > tell the soft mac layer that something bad happened in each case.
> >
> > > As noted above. With this change
> > > you mix bus errors and trac errors (which are not bus errors).
> >
> > In the case of a SPI error, it will happen asynchronously, which means
> > the tx call is over and something bad happened. We are aware that
> > something bad happened and there was a bus error. We need to:
> > - Free the skb
> > - Restart the internal machinery
> > - Somehow tell the soft mac layer something bad happened and the packet
> >   will not be transmitted as expected (IOW, balance the "end" calls
> >   with the "start" calls, just because we did not return immediately
> >   when we got the transmit request).
> >
> > In the case of a transmission error, this is a trac condition that is
> > reported to us by an IRQ. We know it is a trac error, we can look at a
> > buffer to find which trac error exactly happened. In this case we need
> > to go through exactly the same steps as above.
> >
> > But you are right that a spi_async() error is not a trac error, hence
> > my choice in the switch statement to default to the
> > IEEE80154_SYSTEM_ERROR flag in this case.
> >
> > Should I ignore spi bus errors? I don't think I can, so I don't really
> > see how to handle it differently.
>
> Sorry to bother you again, but in the end, do you agree on returning
> IEEE802154_SYSTEM_ERROR upon asynchronous bus errors?
>

yes, I said nothing different here. What I said is that bus errors and
trac status get mixed here and this patch breaks things.

Really this is just changing either call xmit_complete() when trac was
one of the successful codes or xmit_error($REASON) when trac was one
which failed. In case of bus error and it was "tx" then call
xmit_error(SYSTEM_ERROR) in at86rf230_async_error_recover_complete().
You might need to store the last trac register to decide what to call
at the current places of "xmit_complete()".

I also would like to see a helper here which statically sends
SYSTEM_ERROR in case of bus error because I am worried that somebody
is choosing any other 802.15.4 error to return which might be
interpreted differently by SoftMAC.

> Any other modification of the driver in favor of having two distinct
> paths would be really costly in term of time spent and probability of
> breaking something, so I would rather avoid it, unless I am missing
> something simpler?

If it's too much time, then just update the driver like any others and
don't use the new feature, somebody else will send patches for it to
update the driver then.

- Alex

[PATCH wpan-next v4 08/11] net: ieee802154: at86rf230: Call _xmit_error() when a transmission fails

[Miquel Raynal]

ieee802154_xmit_error() is the right helper to call when a transmission
has failed. Let's use it instead of open-coding it.

As the error helper also requires an error code, save the error from the
previous helper in order to give this value to the core when
opportunate.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/at86rf230.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 34d199f597c9..2e6d09b3372a 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -73,6 +73,7 @@ struct at86rf230_state_change {
 	u8 to_state;
 
 	bool free;
+	int reason;
 };
 
 struct at86rf230_local {
@@ -334,8 +335,7 @@ at86rf230_async_error_recover_complete(void *context)
 
 	if (lp->was_tx) {
 		lp->was_tx = 0;
-		dev_kfree_skb_any(lp->tx_skb);
-		ieee802154_wake_queue(lp->hw);
+		ieee802154_xmit_error(lp->hw, lp->tx_skb, ctx->reason);
 	}
 }
 
@@ -358,23 +358,21 @@ static inline void
 at86rf230_async_error(struct at86rf230_local *lp,
 		      struct at86rf230_state_change *ctx, int rc)
 {
-	int reason;
-
 	switch (rc) {
 	case TRAC_CHANNEL_ACCESS_FAILURE:
-		reason = IEEE802154_CHANNEL_ACCESS_FAILURE;
+		ctx->reason = IEEE802154_CHANNEL_ACCESS_FAILURE;
 		break;
 	case TRAC_NO_ACK:
-		reason = IEEE802154_NO_ACK;
+		ctx->reason = IEEE802154_NO_ACK;
 		break;
 	default:
-		reason = IEEE802154_SYSTEM_ERROR;
+		ctx->reason = IEEE802154_SYSTEM_ERROR;
 	}
 
 	if (rc < 0)
 		dev_err(&lp->spi->dev, "spi_async error %d\n", rc);
 	else
-		dev_err(&lp->spi->dev, "xceiver error %d\n", reason);
+		dev_err(&lp->spi->dev, "xceiver error %d\n", ctx->reason);
 
 	at86rf230_async_state_change(lp, ctx, STATE_FORCE_TRX_OFF,
 				     at86rf230_async_error_recover);
-- 
2.27.0

[PATCH wpan-next v4 09/11] net: ieee802154: atusb: Call _xmit_error() when a transmission fails

[Miquel Raynal]

ieee802154_xmit_error() is the right helper to call when a transmission
has failed. Let's use it instead of open-coding it.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/atusb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index f27a5f535808..d04db4d07a64 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -271,9 +271,8 @@ static void atusb_tx_done(struct atusb *atusb, u8 seq)
 		 * unlikely case now that seq == expect is then true, but can
 		 * happen and fail with a tx_skb = NULL;
 		 */
-		ieee802154_wake_queue(atusb->hw);
-		if (atusb->tx_skb)
-			dev_kfree_skb_irq(atusb->tx_skb);
+		ieee802154_xmit_error(atusb->hw, atusb->tx_skb,
+				      IEEE802154_SYSTEM_ERROR);
 	}
 }
 
-- 
2.27.0

[PATCH wpan-next v4 10/11] net: ieee802154: ca8210: Use core return codes instead of hardcoding them

[Miquel Raynal]

All the error codes defined in this driver are generic and already
defined in the ieee802154 main header. Let's just get rid of these extra
definition and switch to the core's values.

There is no functional change.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/ca8210.c | 178 ++++++++++++--------------------
 1 file changed, 68 insertions(+), 110 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index fc74fa0f1ddd..116aece191cd 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -89,48 +89,6 @@
 #define CA8210_TEST_INT_FILE_NAME "ca8210_test"
 #define CA8210_TEST_INT_FIFO_SIZE 256
 
-/* MAC status enumerations */
-#define MAC_SUCCESS                     (0x00)
-#define MAC_ERROR                       (0x01)
-#define MAC_CANCELLED                   (0x02)
-#define MAC_READY_FOR_POLL              (0x03)
-#define MAC_COUNTER_ERROR               (0xDB)
-#define MAC_IMPROPER_KEY_TYPE           (0xDC)
-#define MAC_IMPROPER_SECURITY_LEVEL     (0xDD)
-#define MAC_UNSUPPORTED_LEGACY          (0xDE)
-#define MAC_UNSUPPORTED_SECURITY        (0xDF)
-#define MAC_BEACON_LOST                 (0xE0)
-#define MAC_CHANNEL_ACCESS_FAILURE      (0xE1)
-#define MAC_DENIED                      (0xE2)
-#define MAC_DISABLE_TRX_FAILURE         (0xE3)
-#define MAC_SECURITY_ERROR              (0xE4)
-#define MAC_FRAME_TOO_LONG              (0xE5)
-#define MAC_INVALID_GTS                 (0xE6)
-#define MAC_INVALID_HANDLE              (0xE7)
-#define MAC_INVALID_PARAMETER           (0xE8)
-#define MAC_NO_ACK                      (0xE9)
-#define MAC_NO_BEACON                   (0xEA)
-#define MAC_NO_DATA                     (0xEB)
-#define MAC_NO_SHORT_ADDRESS            (0xEC)
-#define MAC_OUT_OF_CAP                  (0xED)
-#define MAC_PAN_ID_CONFLICT             (0xEE)
-#define MAC_REALIGNMENT                 (0xEF)
-#define MAC_TRANSACTION_EXPIRED         (0xF0)
-#define MAC_TRANSACTION_OVERFLOW        (0xF1)
-#define MAC_TX_ACTIVE                   (0xF2)
-#define MAC_UNAVAILABLE_KEY             (0xF3)
-#define MAC_UNSUPPORTED_ATTRIBUTE       (0xF4)
-#define MAC_INVALID_ADDRESS             (0xF5)
-#define MAC_ON_TIME_TOO_LONG            (0xF6)
-#define MAC_PAST_TIME                   (0xF7)
-#define MAC_TRACKING_OFF                (0xF8)
-#define MAC_INVALID_INDEX               (0xF9)
-#define MAC_LIMIT_REACHED               (0xFA)
-#define MAC_READ_ONLY                   (0xFB)
-#define MAC_SCAN_IN_PROGRESS            (0xFC)
-#define MAC_SUPERFRAME_OVERLAP          (0xFD)
-#define MAC_SYSTEM_ERROR                (0xFF)
-
 /* HWME attribute IDs */
 #define HWME_EDTHRESHOLD       (0x04)
 #define HWME_EDVALUE           (0x06)
@@ -551,58 +509,58 @@ static int link_to_linux_err(int link_status)
 		return link_status;
 	}
 	switch (link_status) {
-	case MAC_SUCCESS:
-	case MAC_REALIGNMENT:
+	case IEEE802154_SUCCESS:
+	case IEEE802154_REALIGNMENT:
 		return 0;
-	case MAC_IMPROPER_KEY_TYPE:
+	case IEEE802154_IMPROPER_KEY_TYPE:
 		return -EKEYREJECTED;
-	case MAC_IMPROPER_SECURITY_LEVEL:
-	case MAC_UNSUPPORTED_LEGACY:
-	case MAC_DENIED:
+	case IEEE802154_IMPROPER_SECURITY_LEVEL:
+	case IEEE802154_UNSUPPORTED_LEGACY:
+	case IEEE802154_DENIED:
 		return -EACCES;
-	case MAC_BEACON_LOST:
-	case MAC_NO_ACK:
-	case MAC_NO_BEACON:
+	case IEEE802154_BEACON_LOST:
+	case IEEE802154_NO_ACK:
+	case IEEE802154_NO_BEACON:
 		return -ENETUNREACH;
-	case MAC_CHANNEL_ACCESS_FAILURE:
-	case MAC_TX_ACTIVE:
-	case MAC_SCAN_IN_PROGRESS:
+	case IEEE802154_CHANNEL_ACCESS_FAILURE:
+	case IEEE802154_TX_ACTIVE:
+	case IEEE802154_SCAN_IN_PROGRESS:
 		return -EBUSY;
-	case MAC_DISABLE_TRX_FAILURE:
-	case MAC_OUT_OF_CAP:
+	case IEEE802154_DISABLE_TRX_FAILURE:
+	case IEEE802154_OUT_OF_CAP:
 		return -EAGAIN;
-	case MAC_FRAME_TOO_LONG:
+	case IEEE802154_FRAME_TOO_LONG:
 		return -EMSGSIZE;
-	case MAC_INVALID_GTS:
-	case MAC_PAST_TIME:
+	case IEEE802154_INVALID_GTS:
+	case IEEE802154_PAST_TIME:
 		return -EBADSLT;
-	case MAC_INVALID_HANDLE:
+	case IEEE802154_INVALID_HANDLE:
 		return -EBADMSG;
-	case MAC_INVALID_PARAMETER:
-	case MAC_UNSUPPORTED_ATTRIBUTE:
-	case MAC_ON_TIME_TOO_LONG:
-	case MAC_INVALID_INDEX:
+	case IEEE802154_INVALID_PARAMETER:
+	case IEEE802154_UNSUPPORTED_ATTRIBUTE:
+	case IEEE802154_ON_TIME_TOO_LONG:
+	case IEEE802154_INVALID_INDEX:
 		return -EINVAL;
-	case MAC_NO_DATA:
+	case IEEE802154_NO_DATA:
 		return -ENODATA;
-	case MAC_NO_SHORT_ADDRESS:
+	case IEEE802154_NO_SHORT_ADDRESS:
 		return -EFAULT;
-	case MAC_PAN_ID_CONFLICT:
+	case IEEE802154_PAN_ID_CONFLICT:
 		return -EADDRINUSE;
-	case MAC_TRANSACTION_EXPIRED:
+	case IEEE802154_TRANSACTION_EXPIRED:
 		return -ETIME;
-	case MAC_TRANSACTION_OVERFLOW:
+	case IEEE802154_TRANSACTION_OVERFLOW:
 		return -ENOBUFS;
-	case MAC_UNAVAILABLE_KEY:
+	case IEEE802154_UNAVAILABLE_KEY:
 		return -ENOKEY;
-	case MAC_INVALID_ADDRESS:
+	case IEEE802154_INVALID_ADDRESS:
 		return -ENXIO;
-	case MAC_TRACKING_OFF:
-	case MAC_SUPERFRAME_OVERLAP:
+	case IEEE802154_TRACKING_OFF:
+	case IEEE802154_SUPERFRAME_OVERLAP:
 		return -EREMOTEIO;
-	case MAC_LIMIT_REACHED:
+	case IEEE802154_LIMIT_REACHED:
 		return -EDQUOT;
-	case MAC_READ_ONLY:
+	case IEEE802154_READ_ONLY:
 		return -EROFS;
 	default:
 		return -EPROTO;
@@ -754,7 +712,7 @@ static void ca8210_rx_done(struct cas_control *cas_ctl)
 
 	ca8210_net_rx(priv->hw, buf, len);
 	if (buf[0] == SPI_MCPS_DATA_CONFIRM) {
-		if (buf[3] == MAC_TRANSACTION_OVERFLOW) {
+		if (buf[3] == IEEE802154_TRANSACTION_OVERFLOW) {
 			dev_info(
 				&priv->spi->dev,
 				"Waiting for transaction overflow to stabilise...\n");
@@ -1128,7 +1086,7 @@ static u8 tdme_setsfr_request_sync(
 	);
 	if (ret) {
 		dev_crit(&spi->dev, "cascoda_api_downstream returned %d", ret);
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 	}
 
 	if (response.command_id != SPI_TDME_SETSFR_CONFIRM) {
@@ -1137,7 +1095,7 @@ static u8 tdme_setsfr_request_sync(
 			"sync response to SPI_TDME_SETSFR_REQUEST was not SPI_TDME_SETSFR_CONFIRM, it was %d\n",
 			response.command_id
 		);
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 	}
 
 	return response.pdata.tdme_set_sfr_cnf.status;
@@ -1151,7 +1109,7 @@ static u8 tdme_setsfr_request_sync(
  */
 static u8 tdme_chipinit(void *device_ref)
 {
-	u8 status = MAC_SUCCESS;
+	u8 status = IEEE802154_SUCCESS;
 	u8 sfr_address;
 	struct spi_device *spi = device_ref;
 	struct preamble_cfg_sfr pre_cfg_value = {
@@ -1220,7 +1178,7 @@ static u8 tdme_chipinit(void *device_ref)
 		goto finish;
 
 finish:
-	if (status != MAC_SUCCESS) {
+	if (status != IEEE802154_SUCCESS) {
 		dev_err(
 			&spi->dev,
 			"failed to set sfr at %#03x, status = %#03x\n",
@@ -1287,7 +1245,7 @@ static u8 tdme_checkpibattribute(
 	const void   *pib_attribute_value
 )
 {
-	u8 status = MAC_SUCCESS;
+	u8 status = IEEE802154_SUCCESS;
 	u8 value;
 
 	value  = *((u8 *)pib_attribute_value);
@@ -1296,52 +1254,52 @@ static u8 tdme_checkpibattribute(
 	/* PHY */
 	case PHY_TRANSMIT_POWER:
 		if (value > 0x3F)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case PHY_CCA_MODE:
 		if (value > 0x03)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	/* MAC */
 	case MAC_BATT_LIFE_EXT_PERIODS:
 		if (value < 6 || value > 41)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_BEACON_PAYLOAD:
 		if (pib_attribute_length > MAX_BEACON_PAYLOAD_LENGTH)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_BEACON_PAYLOAD_LENGTH:
 		if (value > MAX_BEACON_PAYLOAD_LENGTH)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_BEACON_ORDER:
 		if (value > 15)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_MAX_BE:
 		if (value < 3 || value > 8)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_MAX_CSMA_BACKOFFS:
 		if (value > 5)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_MAX_FRAME_RETRIES:
 		if (value > 7)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_MIN_BE:
 		if (value > 8)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_RESPONSE_WAIT_TIME:
 		if (value < 2 || value > 64)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_SUPERFRAME_ORDER:
 		if (value > 15)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	/* boolean */
 	case MAC_ASSOCIATED_PAN_COORD:
@@ -1353,16 +1311,16 @@ static u8 tdme_checkpibattribute(
 	case MAC_RX_ON_WHEN_IDLE:
 	case MAC_SECURITY_ENABLED:
 		if (value > 1)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	/* MAC SEC */
 	case MAC_AUTO_REQUEST_SECURITY_LEVEL:
 		if (value > 7)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	case MAC_AUTO_REQUEST_KEY_ID_MODE:
 		if (value > 3)
-			status = MAC_INVALID_PARAMETER;
+			status = IEEE802154_INVALID_PARAMETER;
 		break;
 	default:
 		break;
@@ -1522,9 +1480,9 @@ static u8 mcps_data_request(
 
 	if (ca8210_spi_transfer(device_ref, &command.command_id,
 				command.length + 2))
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 
-	return MAC_SUCCESS;
+	return IEEE802154_SUCCESS;
 }
 
 /**
@@ -1553,11 +1511,11 @@ static u8 mlme_reset_request_sync(
 		&response.command_id,
 		device_ref)) {
 		dev_err(&spi->dev, "cascoda_api_downstream failed\n");
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 	}
 
 	if (response.command_id != SPI_MLME_RESET_CONFIRM)
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 
 	status = response.pdata.status;
 
@@ -1600,7 +1558,7 @@ static u8 mlme_set_request_sync(
 	 */
 	if (tdme_checkpibattribute(
 		pib_attribute, pib_attribute_length, pib_attribute_value)) {
-		return MAC_INVALID_PARAMETER;
+		return IEEE802154_INVALID_PARAMETER;
 	}
 
 	if (pib_attribute == PHY_CURRENT_CHANNEL) {
@@ -1636,11 +1594,11 @@ static u8 mlme_set_request_sync(
 		command.length + 2,
 		&response.command_id,
 		device_ref)) {
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 	}
 
 	if (response.command_id != SPI_MLME_SET_CONFIRM)
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 
 	return response.pdata.status;
 }
@@ -1678,11 +1636,11 @@ static u8 hwme_set_request_sync(
 		command.length + 2,
 		&response.command_id,
 		device_ref)) {
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 	}
 
 	if (response.command_id != SPI_HWME_SET_CONFIRM)
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 
 	return response.pdata.hwme_set_cnf.status;
 }
@@ -1714,13 +1672,13 @@ static u8 hwme_get_request_sync(
 		command.length + 2,
 		&response.command_id,
 		device_ref)) {
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 	}
 
 	if (response.command_id != SPI_HWME_GET_CONFIRM)
-		return MAC_SYSTEM_ERROR;
+		return IEEE802154_SYSTEM_ERROR;
 
-	if (response.pdata.hwme_get_cnf.status == MAC_SUCCESS) {
+	if (response.pdata.hwme_get_cnf.status == IEEE802154_SUCCESS) {
 		*hw_attribute_length =
 			response.pdata.hwme_get_cnf.hw_attribute_length;
 		memcpy(
@@ -1770,7 +1728,7 @@ static int ca8210_async_xmit_complete(
 			"Link transmission unsuccessful, status = %d\n",
 			status
 		);
-		if (status != MAC_TRANSACTION_OVERFLOW) {
+		if (status != IEEE802154_TRANSACTION_OVERFLOW) {
 			dev_kfree_skb_any(priv->tx_skb);
 			ieee802154_wake_queue(priv->hw);
 			return 0;
@@ -2436,7 +2394,7 @@ static int ca8210_test_check_upstream(u8 *buf, void *device_ref)
 		if (ret) {
 			response[0]  = SPI_MLME_SET_CONFIRM;
 			response[1] = 3;
-			response[2] = MAC_INVALID_PARAMETER;
+			response[2] = IEEE802154_INVALID_PARAMETER;
 			response[3] = buf[2];
 			response[4] = buf[3];
 			if (cascoda_api_upstream)
-- 
2.27.0

[PATCH wpan-next v4 11/11] net: ieee802154: ca8210: Call _xmit_error() when a transmission fails

[Miquel Raynal]

ieee802154_xmit_error() is the right helper to call when a transmission
has failed. Let's use it instead of open-coding it.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/ca8210.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 116aece191cd..b1eb74200f23 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -1729,11 +1729,11 @@ static int ca8210_async_xmit_complete(
 			status
 		);
 		if (status != IEEE802154_TRANSACTION_OVERFLOW) {
-			dev_kfree_skb_any(priv->tx_skb);
-			ieee802154_wake_queue(priv->hw);
+			ieee802154_xmit_error(priv->hw, priv->tx_skb, status);
 			return 0;
 		}
 	}
+
 	ieee802154_xmit_complete(priv->hw, priv->tx_skb, true);
 
 	return 0;
-- 
2.27.0