diff mbox

RX problems with at86rf230 driver

Message ID CAPzmaigy7TitxRGzML7Ck-ZKQF8EDnNGmRYOg-4WiGgT-s9WkA@mail.gmail.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Luca Zulberti June 13, 2016, 9:57 a.m. UTC
Hello Alexander,

I'm trying to use the AT86RF233 radio transceiver for 6lowpan purposes.
Now I'm using an ieee802154 compatible transmitter to test the monitor
functionality of the 6lowpan node.

I've put the following code into the
at86rf230_rx_read_frame_complete() of the driver at86rf230.c into
kernel 4.7-rc2:


I've noticed some incompatible data read from the spi.

printk output:

INIZIO RX - spi_len: 13
0x00 0x0b 0x61 0x08 0x0f 0x04 0x16 0x04 0x16 0x01 0x00 0xe3 0xcf
FINE RX

INIZIO RX - spi_len: 248
0x20 0xf6 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0xfd 0xff 0x24 0xf0 0x62 0xff 0x24 0xf0 0x77 0xff 0x24 0xf0
0x41 0xff 0x24 0xf0 0x93 0xff 0x24 0xf0 0x72 0xff 0x24 0xf0 0xd4 0xff 0x24 0xf0
0x11 0xff 0x24 0xf0 0x8e 0xff 0x24 0xf0 0x8a 0xff 0x24 0xf0 0x0f 0xff 0x24 0xf0
0x49 0xff 0x24 0xf0 0xcb 0xff 0x24 0xf0 0x57 0xff 0x24 0xf0 0x30 0xff 0x24 0xf0
0xfb 0xff 0x24 0xf0 0x92 0xff 0x24 0xf0 0xce 0xff 0x24 0xf0 0x8d 0xff 0x24 0xf0
0x15 0xff 0x24 0xf0 0xa5 0xff 0x24 0xf0 0x4e 0xff 0x24 0xf0 0x57 0xff 0x24 0xf0
0x4d 0xff 0x24 0xf0 0x38 0xff 0x24 0xf0 0xf5 0xff 0x24 0xf0 0x41 0xff 0x24 0xf0
0xf9 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
FINE RX

INIZIO RX - spi_len: 248
0x00 0x0b 0x61 0x08 0x10 0x04 0x16 0x04 0x16 0x01 0x00 0xde 0x63 0xff 0x20 0xf0
0x43 0xff 0x20 0xf0 0xfd 0xff 0x20 0xf0 0x62 0xff 0x20 0xf0 0x77 0xff 0x20 0xf0
0x41 0xff 0x20 0xf0 0x93 0xff 0x20 0xf0 0x72 0xff 0x20 0xf0 0xd4 0xff 0x20 0xf0
0x11 0xff 0x20 0xf0 0x8e 0xff 0x20 0xf0 0x8a 0xff 0x20 0xf0 0x0f 0xff 0x20 0xf0
0x49 0xff 0x20 0xf0 0xcb 0xff 0x20 0xf0 0x57 0xff 0x20 0xf0 0x30 0xff 0x20 0xf0
0xfb 0xff 0x20 0xf0 0x92 0xff 0x20 0xf0 0xce 0xff 0x20 0xf0 0x8d 0xff 0x20 0xf0
0x15 0xff 0x20 0xf0 0xa5 0xff 0x20 0xf0 0x4e 0xff 0x20 0xf0 0x57 0xff 0x20 0xf0
0x4d 0xff 0x20 0xf0 0x38 0xff 0x20 0xf0 0xf5 0xff 0x20 0xf0 0x41 0xff 0x20 0xf0
0xf9 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
FINE RX

The transmitter sends 12 bytes (10 byte of payload and 2 of FCS) as
shown in the first output but the driver often reads 248 bytes from
the spi buffer which is wrong.
In the third output 'spi_len' is 248 even if the second byte of the
stream (the PHR) is 11!!!

With a logic analyzer I've checked the data from the at86rf233 and
they are correct, however when the driver reads them something goes
wrong and invalid data are returned!

It seems to me that the driver returns the previously written data
instead of the current read ones maybe due a race condition...

Below is the DTS settings for my board:

spi1: spi@f0004000 {
   status = "okay";
   cs-gpios = <&pioA 29 GPIO_ACTIVE_LOW>, <0>, <0>, <0>;

   at86rf233@0 {
           compatible = "atmel,at86rf233";
           reg = <0>;
           interrupt-parent = <&pioA>;
           interrupts = <27 IRQ_TYPE_EDGE_RISING 7>;
           pinctrl-names = "default";
           pinctrl-0 = <&pinctrl_at86rf233_irq>;

           reset-gpio = <&pioA 24 GPIO_ACTIVE_LOW>;
           sleep-gpio = <&pioA 25 GPIO_ACTIVE_HIGH>;
           spi-max-frequency = <3000000>;
           xtal-trim = /bits/ 8 <0x0F>;
};

pinctrl@fffff400 {
   board {
      pinctrl_at86rf233_irq: at86rf233_irq {
         atmel,pins = <AT91_PIOA 27 AT91_PERIPH_GPIO AT91_PINCTRL_DEGLITCH>;
      };
   };
};

Thanks in advance for your help!

Regards,

Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Alexander Aring June 13, 2016, 2:15 p.m. UTC | #1
Hi,

On 06/13/2016 11:57 AM, Luca Zulberti wrote:
> Hello Alexander,
> 
> I'm trying to use the AT86RF233 radio transceiver for 6lowpan purposes.
> Now I'm using an ieee802154 compatible transmitter to test the monitor
> functionality of the 6lowpan node.
> 
> I've put the following code into the
> at86rf230_rx_read_frame_complete() of the driver at86rf230.c into
> kernel 4.7-rc2:
> 
> diff --git a/drivers/net/ieee802154/at86rf230.c
> b/drivers/net/ieee802154/at86rf230.c
> index 9f10da6..4d616c8 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -711,6 +711,18 @@ at86rf230_rx_read_frame_complete(void *context)
>         u8 len, lqi;
> 
>         len = buf[1];
> +
> +       {
> +               int i;
> +               printk("\nSTART RX - spi_len: %d\n", len + 2);
> +               for (i = 0; i < len + 2; i++) {
> +                       printk("0x%02x ", buf[i]);
> +                       if (i % 16 == 15)
> +                               printk("\n");
> +               }
> +               printk("\nEND RX\n");
> +       }
> +
>         if (!ieee802154_is_valid_psdu_len(len)) {
>                 dev_vdbg(&lp->spi->dev, "corrupted frame received\n");
>                 len = IEEE802154_MTU;
> @@ -878,6 +890,18 @@ at86rf230_write_frame(void *context)
>         memcpy(buf + 2, skb->data, skb->len);
>         ctx->trx.len = skb->len + 2;
>         ctx->msg.complete = at86rf230_write_frame_complete;
> +
> +       {
> +                int i, len = buf[1] + 2;

should be len = buf[1];

The CRC will be added by transceiver automatically, you don't see it.

> +                printk("\nSTART TX - spi_len: %d\n", len);
> +                for (i = 0; i < len; i++) {
> +                        printk("0x%02x ", buf[i]);
> +                        if (i % 16 == 15)
> +                                printk("\n");
> +               }
> +                printk("\nEND TX\n");
> +        }
> +
>         rc = spi_async(lp->spi, &ctx->msg);
>         if (rc) {
>                 ctx->trx.len = 2;
> 
> I've noticed some incompatible data read from the spi.
> 
> printk output:
> 
> INIZIO RX - spi_len: 13
> 0x00 0x0b 0x61 0x08 0x0f 0x04 0x16 0x04 0x16 0x01 0x00 0xe3 0xcf
> FINE RX
> 
> INIZIO RX - spi_len: 248
> 0x20 0xf6 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0xfd 0xff 0x24 0xf0 0x62 0xff 0x24 0xf0 0x77 0xff 0x24 0xf0
> 0x41 0xff 0x24 0xf0 0x93 0xff 0x24 0xf0 0x72 0xff 0x24 0xf0 0xd4 0xff 0x24 0xf0
> 0x11 0xff 0x24 0xf0 0x8e 0xff 0x24 0xf0 0x8a 0xff 0x24 0xf0 0x0f 0xff 0x24 0xf0
> 0x49 0xff 0x24 0xf0 0xcb 0xff 0x24 0xf0 0x57 0xff 0x24 0xf0 0x30 0xff 0x24 0xf0
> 0xfb 0xff 0x24 0xf0 0x92 0xff 0x24 0xf0 0xce 0xff 0x24 0xf0 0x8d 0xff 0x24 0xf0
> 0x15 0xff 0x24 0xf0 0xa5 0xff 0x24 0xf0 0x4e 0xff 0x24 0xf0 0x57 0xff 0x24 0xf0
> 0x4d 0xff 0x24 0xf0 0x38 0xff 0x24 0xf0 0xf5 0xff 0x24 0xf0 0x41 0xff 0x24 0xf0
> 0xf9 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> FINE RX
> 
> INIZIO RX - spi_len: 248
> 0x00 0x0b 0x61 0x08 0x10 0x04 0x16 0x04 0x16 0x01 0x00 0xde 0x63 0xff 0x20 0xf0
> 0x43 0xff 0x20 0xf0 0xfd 0xff 0x20 0xf0 0x62 0xff 0x20 0xf0 0x77 0xff 0x20 0xf0
> 0x41 0xff 0x20 0xf0 0x93 0xff 0x20 0xf0 0x72 0xff 0x20 0xf0 0xd4 0xff 0x20 0xf0
> 0x11 0xff 0x20 0xf0 0x8e 0xff 0x20 0xf0 0x8a 0xff 0x20 0xf0 0x0f 0xff 0x20 0xf0
> 0x49 0xff 0x20 0xf0 0xcb 0xff 0x20 0xf0 0x57 0xff 0x20 0xf0 0x30 0xff 0x20 0xf0
> 0xfb 0xff 0x20 0xf0 0x92 0xff 0x20 0xf0 0xce 0xff 0x20 0xf0 0x8d 0xff 0x20 0xf0
> 0x15 0xff 0x20 0xf0 0xa5 0xff 0x20 0xf0 0x4e 0xff 0x20 0xf0 0x57 0xff 0x20 0xf0
> 0x4d 0xff 0x20 0xf0 0x38 0xff 0x20 0xf0 0xf5 0xff 0x20 0xf0 0x41 0xff 0x20 0xf0
> 0xf9 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00
> FINE RX
> 
> The transmitter sends 12 bytes (10 byte of payload and 2 of FCS) as
> shown in the first output but the driver often reads 248 bytes from
> the spi buffer which is wrong.
> In the third output 'spi_len' is 248 even if the second byte of the
> stream (the PHR) is 11!!!
> 

Who send this frame? We never send frames above PHR len 127 which is invalid.

> With a logic analyzer I've checked the data from the at86rf233 and
> they are correct, however when the driver reads them something goes
> wrong and invalid data are returned!
> 

mhh, how rx works:

 - IRQ occured
   - alloc some context space, on heap not on stack. So we don't use
     here multiple resources for reading framebuffer, etc.
 - read irq status
   - this will clear the irq flag and new irq's can occur, but this is
     for RX side safe because RX_SAFE_MODE should be "1".
     Maybe check better if this bit is really set with regmap, somewhere in
     "/sys/kernel/debug/regmap/$spi_bus/registers".
     The RX_SAFE_MODE bit will prevent overwriting framebuffer for new
     frames until the chipselect will be lost from framebuffer read
     command.
 - if we are no in TX states (is_tx), then it is a receive and we read
   the full framebuffer, because RX_SAFE_MODE.
 - check if length is valid 802.15.4 frame and fill and deliver skb.


btw:
That's all, also during this operation should no xmit_async occured. But
that should be fine, otherwise we should have issues on other boards.

The spi_async stuff is mostly not preemptable, no sleeps, etc.
Also the spi subsystem will not handle other messages until complete
handler of spi_async will be returned, otherwise spi_async will return
-EBUSY which should be displayed by [0].

So I think this can't happen. :-)

> It seems to me that the driver returns the previously written data
> instead of the current read ones maybe due a race condition...
> 
> Below is the DTS settings for my board:
> 
> spi1: spi@f0004000 {
>    status = "okay";
>    cs-gpios = <&pioA 29 GPIO_ACTIVE_LOW>, <0>, <0>, <0>;
> 
>    at86rf233@0 {
>            compatible = "atmel,at86rf233";
>            reg = <0>;
>            interrupt-parent = <&pioA>;
>            interrupts = <27 IRQ_TYPE_EDGE_RISING 7>;

maybe try IRQ_ACTIVE_HIGH here, I don't know your wiring maybe you have some
lossy connection on irq line. If it's a loosy connection and you will
get multiple irq's it could be that the framebuffer will be readed
twice, maybe HIGH_LEVEL is better here.

I don't know why you having such issues with at86rf230 driver, the
IRQ_ACTIVE_HIGH is just a try to change something and looks if works
better afterwards. :-/

You also said that the data is correct on the logic analyzer but wrong
in your hexdump, or? Then it looks more like some spi controller issue,
maybe ask on linux-spi mailinglist [1] and also cc maintainers?

btw:
I also got reports from RPi2/RPi3 users which has issues with
at86rf230 driver. But this was with RPi-kernel. I don't get any issues
when using mainline kernel (on RPi2). One users switched from RPi to
mainline kernel and the issue with corrupted spi data was gone.

- Alex

[0] http://lxr.free-electrons.com/source/drivers/net/ieee802154/at86rf230.c#L372
[1] http://vger.kernel.org/vger-lists.html#linux-spi
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ieee802154/at86rf230.c
b/drivers/net/ieee802154/at86rf230.c
index 9f10da6..4d616c8 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -711,6 +711,18 @@  at86rf230_rx_read_frame_complete(void *context)
        u8 len, lqi;

        len = buf[1];
+
+       {
+               int i;
+               printk("\nSTART RX - spi_len: %d\n", len + 2);
+               for (i = 0; i < len + 2; i++) {
+                       printk("0x%02x ", buf[i]);
+                       if (i % 16 == 15)
+                               printk("\n");
+               }
+               printk("\nEND RX\n");
+       }
+
        if (!ieee802154_is_valid_psdu_len(len)) {
                dev_vdbg(&lp->spi->dev, "corrupted frame received\n");
                len = IEEE802154_MTU;
@@ -878,6 +890,18 @@  at86rf230_write_frame(void *context)
        memcpy(buf + 2, skb->data, skb->len);
        ctx->trx.len = skb->len + 2;
        ctx->msg.complete = at86rf230_write_frame_complete;
+
+       {
+                int i, len = buf[1] + 2;
+                printk("\nSTART TX - spi_len: %d\n", len);
+                for (i = 0; i < len; i++) {
+                        printk("0x%02x ", buf[i]);
+                        if (i % 16 == 15)
+                                printk("\n");
+               }
+                printk("\nEND TX\n");
+        }
+
        rc = spi_async(lp->spi, &ctx->msg);
        if (rc) {
                ctx->trx.len = 2;