| Message ID | 20220303182508.288136-8-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ieee802154: Better Tx error handling | expand |
Hi, On Thu, Mar 3, 2022 at 1:25 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Either the spi operation failed, or the device encountered an error. In > both case, we know more or less what happened thanks to the spi call > return code or the content of the TRAC register otherwise. Use them 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 12ee071057d2..5f19266b3045 100644 > --- a/drivers/net/ieee802154/at86rf230.c > +++ b/drivers/net/ieee802154/at86rf230.c > @@ -370,7 +370,27 @@ 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; > + case TRAC_INVALID: > + reason = IEEE802154_SYSTEM_ERROR; > + break; > + default: > + reason = rc; > + } > + Actually the rc value here is not a TRAC status register value... and it should not be one. The reason is because this function can also be called during a non-tx state change failure whereas the trac register is only valid when the transmission "is successfully offloaded to device" and delivers us an error of the transmission operation on the device. It is called during the tx case only if there was a "state transition error" and then it should report IEEE802154_SYSTEM_ERROR in at86rf230_async_error_recover_complete(). Whereas I think we can use IEEE802154_SYSTEM_ERROR as a non-specific 802.15.4 error code, because a bus error of a state transition is not 802.15.4 specific. - Alex
Hi Alexander, alex.aring@gmail.com wrote on Sun, 13 Mar 2022 16:16:45 -0400: > Hi, > > On Thu, Mar 3, 2022 at 1:25 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Either the spi operation failed, or the device encountered an error. In > > both case, we know more or less what happened thanks to the spi call > > return code or the content of the TRAC register otherwise. Use them 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 12ee071057d2..5f19266b3045 100644 > > --- a/drivers/net/ieee802154/at86rf230.c > > +++ b/drivers/net/ieee802154/at86rf230.c > > @@ -370,7 +370,27 @@ 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; > > + case TRAC_INVALID: > > + reason = IEEE802154_SYSTEM_ERROR; > > + break; > > + default: > > + reason = rc; > > + } > > + > > Actually the rc value here is not a TRAC status register value... and > it should not be one. > > The reason is because this function can also be called during a non-tx > state change failure whereas the trac register is only valid when the > transmission "is successfully offloaded to device" and delivers us an > error of the transmission operation on the device. It is called during > the tx case only if there was a "state transition error" and then it > should report IEEE802154_SYSTEM_ERROR in > at86rf230_async_error_recover_complete(). Whereas I think we can use > IEEE802154_SYSTEM_ERROR as a non-specific 802.15.4 error code, because > a bus error of a state transition is not 802.15.4 specific. Ok I'm totally fine using SYSTEM_ERROR as a generic placeholder in these cases. Thanks, Miquèl
diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c index 12ee071057d2..5f19266b3045 100644 --- a/drivers/net/ieee802154/at86rf230.c +++ b/drivers/net/ieee802154/at86rf230.c @@ -370,7 +370,27 @@ 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; + case TRAC_INVALID: + reason = IEEE802154_SYSTEM_ERROR; + break; + default: + reason = rc; + } + + if (reason < 0) + dev_err(&lp->spi->dev, "spi_async error %d\n", reason); + 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); @@ -693,6 +713,7 @@ at86rf230_tx_trac_check(void *context) goto failure; default: WARN_ONCE(1, "received tx trac status %d\n", trac); + trac = TRAC_INVALID; lp->trac.invalid++; goto failure; } @@ -702,7 +723,7 @@ at86rf230_tx_trac_check(void *context) return; failure: - at86rf230_async_error(lp, ctx, -EIO); + at86rf230_async_error(lp, ctx, trac); } static void
Either the spi operation failed, or the device encountered an error. In both case, we know more or less what happened thanks to the spi call return code or the content of the TRAC register otherwise. Use them 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(-)