Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cc110x: not working since SPI driver interface rework #6857

Closed
patgrosse opened this issue Apr 4, 2017 · 16 comments
Closed

cc110x: not working since SPI driver interface rework #6857

patgrosse opened this issue Apr 4, 2017 · 16 comments
Assignees
Labels
Area: drivers Area: Device drivers Area: network Area: Networking Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@patgrosse
Copy link
Contributor

Since #4780 I cannot use networking on the msba2 board. I used the gnrc_networking example on two msba2 boards and I can't even access the shell anymore because the code in the main() is not reached. Possibly this is an issue/task for #6437.

Used example: gnrc_networking

With 513b20f:

2017-04-04 14:18:45,213 - INFO # Connect to serial port /dev/ttyUSB0
Welcome to pyterm!
Type '/exit' to exit.
2017-04-04 14:18:46,216 - INFO # cc110x: initialized with address=34 and channel=0
[no more console output, shell is not started]

With b101c70 (commit right before):

2017-04-04 14:20:29,366 - INFO # Connect to serial port /dev/ttyUSB0
Welcome to pyterm!
Type '/exit' to exit.
2017-04-04 14:20:30,368 - INFO # cc110x: initialized with address=34 and channel=0
2017-04-04 14:20:30,369 - INFO # main(): This is RIOT! (Version: 2017.01-devel-824-gb101c-patrick-ubuntu-HEAD)
2017-04-04 14:20:30,369 - INFO # RIOT network stack example application
2017-04-04 14:20:30,369 - INFO # All up, running the shell now
> 
[shell is started correctly, networking seems good to me]
@haukepetersen
Copy link
Contributor

haukepetersen commented May 15, 2017

Hi, sorry for the late reply! Just did a little digging into the driver: when I introduced some delays into the cc110x_get_reg_robust() function (through DEBUG output in my case) the initialization and the shell work fine. So my guess is, that we have some timing issue in the driver.

@kaspar030: would you mind to have a quick look?

@haukepetersen haukepetersen added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: drivers Area: Device drivers Area: network Area: Networking labels May 15, 2017
@haukepetersen haukepetersen added this to the Release 2017.07 milestone May 15, 2017
@haukepetersen
Copy link
Contributor

to go more specific, inserting a simple DEBUG("foo"); between cpsr = irq_disable(); and cc110x_cs(dev); in cc110x_get_reg_robust(), then it works fine, taking out the DEBUG it while compaing res1 and res2...

@maribu
Copy link
Member

maribu commented Nov 3, 2017

Hi,

the loop

do {
        res1 = spi_transfer_reg(dev->params.spi, SPI_CS_UNDEF,
                                (addr | CC110X_READ_BURST), CC110X_NOBYTE);
        res2 = spi_transfer_reg(dev->params.spi, SPI_CS_UNDEF,
                                (addr | CC110X_READ_BURST), CC110X_NOBYTE);
} while (res1 != res2);

in cc110x_get_reg_robust in https://github.com/RIOT-OS/RIOT/blob/master/drivers/cc110x/cc110x-spi.c is never terminating. This causes the hang.

@maribu
Copy link
Member

maribu commented Nov 3, 2017

printf("res1 = %u, res2 = %u\n", (unsigned int)res1, (unsigned int)res2); in this loop prints:

res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
res1 = 60, res2 = 240
res1 = 195, res2 = 15
...

@maribu
Copy link
Member

maribu commented Nov 3, 2017

Let me summarize what I figured out so far:

  • The function cc110x_get_reg_robust is used to read out a register twice until both reads return the same value.
  • The address read in the loop is 0xfa, so addr | CC110X_READ_BURST which is 0xfa | 0xc0 which is 0xfa points to the register "TXBYTES" in read burst mode.
  • The corresponding call is in static void _tx_continue(cc110x_t *dev) in file https://github.com/RIOT-OS/RIOT/blob/master/drivers/cc110x/cc110x-rxtx.c#L180
    • Should the call int fifo = 64 - cc110x_get_reg_robust(dev, 0xfa); not be int fifo = 64 - cc110x_get_reg_robust(dev, 0x3a);? This way (addr | CC110X_READ_BURST) would yield still 0xfa and the logical or does make sense?
  • The data sheet refers to the errata in this document http://www.ti.com/lit/er/swrz020e/swrz020e.pdf on page 10, which states that the TXBYTES register may return wrong results under specific conditions
    • Reading it twice seems to be the chosen workaround for that problem
  • The data sheet of the CC1101 recommands to not read the TXBYTES register at all. Instead the value should be extracted from the status byte read of MISO "each time a header byte, status byte or command strobe is sent on the SPI bus." (page 42 of the sheet). The same is true for the RXBYTES. The command strobe SNOP has no side effect and only reads the status byte. I suggest to use this approach to read RXBYTES and TXBYTES and drop cc110x_get_reg_robust altogether. Is this possible, or am I missing something?

Update: Wrong data sheet. The chip is the CC1100, not CC1101. Reading the register twice until getting the same value twice is the suggested workarround.

@maribu
Copy link
Member

maribu commented Nov 6, 2017

Hi again,

this little patch will get the device stop from hanging by using the status byte instead of reading the register:

diff --git a/drivers/cc110x/cc110x-rxtx.c b/drivers/cc110x/cc110x-rxtx.c
index bbe216d..2dc1a44 100644
--- a/drivers/cc110x/cc110x-rxtx.c
+++ b/drivers/cc110x/cc110x-rxtx.c
@@ -177,9 +177,10 @@ static void _tx_continue(cc110x_t *dev)
         return;
     }
 
-    int fifo = 64 - cc110x_get_reg_robust(dev, 0xfa);
+    uint8_t status =  cc110x_strobe(dev, CC110X_SNOP);
+    int fifo = status & 0x0f;
 
-    if (fifo & 0x80) {
+    if ((status & 0x70) == 0x70) {
         DEBUG("%s:%s:%u tx underflow!\n", RIOT_FILE_RELATIVE, __func__, __LINE__);
         _tx_abort(dev);
         return;

However, this approach is suboptimal: The status bytes contains the number of free bytes in case this value is less than 15. If 15 or more bytes are available, the value 15 will be given. If reading the TXBYTES from the status register would not result such headaches, the efficiency would be greatly increased. (The above patch has never been tested. And at the very least using the SNOP approach should be implemented in a loop which write 15 Bytes chunks until the packet is written or the FIFO is full, which is not done.)

I'll have a look into it again sometime this week.

Kind regards,
Marian

@miri64 miri64 changed the title board: msba2: networking is not working since SPI driver interface rework cc110x: not working since SPI driver interface rework Nov 14, 2017
@miri64
Copy link
Member

miri64 commented Nov 14, 2017

since MSBA2 + Networking means cc110x and all the later discussion in this thread is about cc110x I adapted the issue title accordingly.

@maribu
Copy link
Member

maribu commented Nov 14, 2017

Btw: In the TI forum it is suggested to verify the SPI communication. I had no time to do that so far, but a wrong SPI configuration could easily explain the issue at hand.

@miri64
Copy link
Member

miri64 commented Nov 14, 2017

TI doesn't know about us :( how very hurtful ^^

@maribu
Copy link
Member

maribu commented Nov 14, 2017

But Nordic does ;-)

@miri64
Copy link
Member

miri64 commented Nov 14, 2017

I know :-)

@miri64
Copy link
Member

miri64 commented Nov 14, 2017

(well that Tutorial was written by @kaspar030, but regardless: we know that they know about us ;-))

@maribu
Copy link
Member

maribu commented Nov 14, 2017

The documentation says:

In RX mode, data is set up on the falling edge by CC1100 when GDOx_INV=0.
In TX mode, data is sampled by CC1100 on the rising edge of the serial clock when GDOx_INV=0.

I don't fully understand that. But lets assume that when changing to RX mode, the CC1100 expects data on falling edge, otherwise on rising edge. However, the driver expects SPI to work on rising edge only. So when reading the TX_BYTES register, reading always fails.

Lets look into the DEBUG output:

2017-11-14 20:11:52,344 - INFO # random: NO SEED AVAILABLE!
2017-11-14 20:11:52,347 - INFO # netdev_cc110x_setup()
2017-11-14 20:11:52,348 - INFO # drivers/cc110x/cc110x.c:cc110x_setup:49
2017-11-14 20:11:52,349 - INFO # drivers/cc110x/cc110x.c:_power_up_reset:212
2017-11-14 20:11:52,350 - INFO # drivers/cc110x/cc110x.c:_reset:203
2017-11-14 20:11:52,351 - INFO # cc110x_set_base_freq_raw(): setting base frequency to 867930228Hz
2017-11-14 20:11:52,351 - INFO # drivers/cc110x/cc110x.c:cc110x_set_channel:187
2017-11-14 20:11:52,351 - INFO # drivers/cc110x/cc110x.c:cc110x_set_address:98 setting address 34
2017-11-14 20:11:52,352 - INFO # cc110x: initialized with address=34 and channel=0
2017-11-14 20:11:52,352 - INFO # _init:189
2017-11-14 20:11:52,352 - INFO # drivers/cc110x/cc110x.c:cc110x_rd_set_mode:239
2017-11-14 20:11:52,352 - INFO # drivers/cc110x/cc110x.c:cc110x_setup_rx_mode:131
2017-11-14 20:11:52,353 - INFO # drivers/cc110x/cc110x.c:cc110x_switch_to_rx:140
2017-11-14 20:11:52,353 - INFO # _send:41
2017-11-14 20:11:52,353 - INFO # cc110x: snd pkt to 0 payload_length=20

So a switch_to_rx is called just before _send is called...

I'll try to find some time to dig deeper into it. But some help would be nice. Maybe someone can explain me in which cases the Chip will switch to falling edge, as I don't understand the documentation in this point?

Kind regards,
Marian

@miri64
Copy link
Member

miri64 commented Nov 14, 2017

Maybe, if they find time @haukepetersen or @OlegHahm can help you with that?

@maribu
Copy link
Member

maribu commented May 18, 2018

Fixed in PR #9154

@aabadie
Copy link
Contributor

aabadie commented May 21, 2018

#9154 has been merged. Let's close this one.

@aabadie aabadie closed this as completed May 21, 2018
@cladmi cladmi added this to the Release 2018.07 milestone Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: network Area: Networking Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

No branches or pull requests

6 participants