kw2xrf: CSMA support missing in driver

[jnohlgard]

Originally posted by @PeterKietzmann in https://github.com/RIOT-OS/RIOT/pull/10355

Well, we're all short in time but we (HAW Hamburg) have certain interest in that radio so it might be us to do the follow-up. @gebart, @bergzand could you please open a feature request issue?

The kw2xrf driver does not support NETOPT_CSMA, but it does support NETOPT_AUTOCCA instead (and is currently the only driver which supports that flag)

The kw41z radio also does not support CSMA in hardware but only automatic CCA before TX (i.e. no automatic backoff and retry). The kw41zrf driver (#7107) implements CSMA exponential backoff and retry in software but utilizing the autocca hardware support. The same method could be used in the kw2xrf driver.

Whether the implementations can be shared between the two drivers or if it is better to reimplement the same behavior is left as an exercise to the reader.

[jnohlgard]

Some pointers towards the kw41zrf implementation:

TX initialization: https://github.com/gebart/RIOT/blob/pr/kw41zrf/drivers/kw41zrf/kw41zrf_netdev.c#L100-L174

CCA feedback using the CCAIRQ flag: https://github.com/gebart/RIOT/blob/pr/kw41zrf/drivers/kw41zrf/kw41zrf_netdev.c#L961-L1000

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[Ilmu011]

I am working on this right now and there is one thing I don't understand; https://github.com/gebart/RIOT/blob/pr/kw41zrf/drivers/kw41zrf/kw41zrf_netdev.c#L47-L51

#define KW41ZRF_CCA_TIME               8    //128 us  Ref Manual Page 2146
#define KW41ZRF_SHR_PHY_TIME          12    //192 us
#define KW41ZRF_PER_BYTE_TIME          2    //32 us
#define KW41ZRF_ACK_WAIT_TIME         54    //864 us
#define KW41ZRF_CSMA_UNIT_TIME        20    //320 us 

Where are these timing parameters coming from? I have perused the MKW41Z/31Z/21Z Reference Manual but were not able to find them except KW41ZRF_CCA_TIME, which is explicitly stated as 128 us. https://www.nxp.com/files-static/32bit/doc/ref_manual/MKW41Z512RM.pdf#page=2146

Furthermore, why are these timing parameters even a thing? These parameters are currently used to start a timer, which is supposed to indicate whether the acknowledge frame in a TR-sequence has timed out. The timer is set BEFORE issuing the operation to hardware, using these timing parameters to estimate, when the TX operation will be complete and adding KW41ZRF_ACK_WAIT_TIME on top of that. https://github.com/gebart/RIOT/blob/pr/kw41zrf/drivers/kw41zrf/kw41zrf_netdev.c#L125-L132

Why isn't the timer just set to KW41ZRF_ACK_WAIT_TIME AFTER the hardware has signaled us, that the TX operation is complete?

[PeterKietzmann]

Where are these timing parameters coming from?

These parameters result from the 802.15.4 specification. For example: 802.15.4 has a data rate of 250kbit/s. How long does it take to transmit one bit? 1 bit / 250kbit/s = 4 us and (1bit x 8) / 250kbps/s = 32us for one byte respectively. How does this relate to #define KW41ZRF_PER_BYTE_TIME 2 //32 us? I assmue the timer that is used within that driver runs at 62.5 kHz. Then, one tick takes16us and two ticks take 32us as mentioned next to the define.

[PeterKietzmann]

Furthermore, why are these timing parameters even a thing?

I don't understand. But in any way, please note that the driver that you are referring to is WIP and not part of the official RIOT master.

Why isn't the timer just set to KW41ZRF_ACK_WAIT_TIME AFTER the hardware has signaled us, that the TX operation is complete?

I'm not familiar with the implementation in that detail. Your concerns seem reasonable to me. Maybe @benemorius can shed light on the as he indicated to take over #7107.

[Ilmu011]

I assmue the timer that is used within that driver runs at 62.5 kHz. Then, one tick takes16us and two ticks take 32us as mentioned next to the define.

That's how it is. I checked it before.

https://github.com/gebart/RIOT/blob/pr/kw41zrf/drivers/kw41zrf/kw41zrf.c#L177

[Ilmu011]

Another thing; kw2xrf supports NETOPT_AUTOCCA and kw41zrf supports NETOPT_CSMA, despite both of them doing the exact same thing. Both use automatic CCA before TX operations, and in both cases that's exactly what the respective NETOPTs in both drivers enable. The only difference is, that kw41zrf additionally does exponential backoff.

CSMA emulation via usage of automatic CCA is exactly what NETOPT_AUTOCCA defines and should be supported here.

Perform channel clear assessment before transmitting

This may be a hardware feature of the given transceiver, or might be otherwise implemented in software. If the device supports CSMA this option will enable CSMA with a certain set of parameters to emulate the desired behaviour.

Actually, according to the documentation, it is even WRONG to support NETOPT_CSMA here;

If the device supports CSMA in hardware, this option enables it with default parameters. For further configuration refer to the other NETOPTCSMA* options.

Neither kw2xrf nor kw41zrf support CSMA in hardware. None should support NETOPT_CSMA, but should support NETOPT_AUTOCCA instead.

The advantage of supporting NETOPT_CSMA is that the driver can make use of the additional options for NETOPT_CSMA, like NETOPT_CSMA_RETRIES, NETOPT_CSMA_MAXBE or NETOPT_CSMA_MINBE, but from a specification standpoint, it's just wrong.

[Ilmu011]

For efficiency's sake, for now I'll just throw out NETOPT_AUTOCCA from kw2xrf and implement support for NETOPT_CSMA, similar to kw41zrf, but something should be done about this in the future.

[benemorius]

I was puzzled at first by the timer usage too. These radios have a tighter integration with software than others I've used. The use of the radio timer isn't just to fire callbacks in software. It's actually used to trigger events in the radio hardware like timing out an ACK wait for example.

In other words:

Why isn't the timer just set to KW41ZRF_ACK_WAIT_TIME AFTER the hardware has signaled us, that the TX operation is complete?

Because the hardware waits to signal us the outcome of the TX operation until it has received an ACK or not, and the timer is what causes it to stop waiting for a missed ACK and signal us. At least that's the case with kw41zrf. It looks like kw2xrf does as you say where the hardware signals TX complete before waiting for ACK and only then the driver starts an ACK timer. It's possible that both radios could support either paradigm and the difference in the two drivers is just developers' preference. You could save an otherwise unnecessary interrupt here.

My interpretation of NETOPT_AUTOCCA is that the clear channel assessment should be performed but NOT the retries. Meaning if CCA succeeds then the packet is transmitted but if it fails then the radio driver returns a medium-busy failure rather than retrying.

I interpret NETOPT_CSMA to mean NETOPT_AUTOCCA plus retries. And you're right the documentation specifically says that it must be hardware CSMA support which is interesting. Is that because we have (or intend to have (or used to indend to have)) software CSMA retries implemented in an upper layer? And maybe it's getting implemented in netdev drivers because the upper layer implementation is missing so far? I haven't looked in to that.

[benemorius]

By the way there's a bug in the #7107 CSMA implementation that prevents any retransmissions from actually happening if the initial CCA fails.

After _isr_event_seq_t_ccairq() calls kw41zrf_tx_exec() to retransmit the last frame, _isr_event_seq_tr() cancels the transmission by calling kw41zrf_abort_sequence() while handling ZLL_IRQSTS_SEQIRQ_MASK.

But with that fixed I think it works as intended.

[Ilmu011]

@PeterKietzmann I opened #12589 regarding this issue.

[miri64]

Should also be fixed by @jia200x lower-layer rework (see #13943)

[PeterKietzmann]

The kinetis hardware provides dedicated timers for CSMA. Although a SubMAC might solve the issue already using AutoCCA, a CSMA driver is missing which might reduce load.