Message ID | CAPzmaigy7TitxRGzML7Ck-ZKQF8EDnNGmRYOg-4WiGgT-s9WkA@mail.gmail.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
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 --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;