kw2xrf: adaption to netdev2 by jfischer-no · Pull Request #5469 · RIOT-OS/RIOT

jfischer-no · 2016-05-27T22:20:30Z

This PR adapts the kw2xrf driver to netdev2 and adds new timer and test modes functionalities.
Also support for CR20A devices will be tested. WIP, not ready for review

depends on ~~#5485~~, ~~#5487~~

miri64 · 2016-05-31T18:53:32Z

Needs rebase.

miri64 · 2016-06-02T11:37:44Z

Needs rebase and adaption to #4871.

miri64 · 2016-08-05T13:13:56Z

Sorry, another adaption needed: #5495. :/

miri64 · 2016-08-30T18:38:21Z

cpu/cortexm_common/vectors_cortexm.c

-    uint32_t pc;
-    uint32_t* orig_sp;
+    uint32_t pc = 0;
+    uint32_t* orig_sp = NULL;


Seems like an unrelated fix.

It will/should be removed after rebase.

...when #5727 was merged

miri64 · 2016-08-30T18:42:49Z

There seems to be more than just moving to netdev2 here. Is it maybe possible to split this up in several PRs?

miri64 · 2016-08-30T18:43:06Z

(I know this is reflected in the description, but not in the title).

smlng · 2016-09-22T08:37:17Z

I'd like to test this one. Could you @jfischer-phytec-iot please rebase, and maybe adapt #5834.

Why is #5487 needed?

jfischer-no · 2016-09-22T08:59:39Z

Why is #5487 needed?

to display actual CCA-Mode ...

jfischer-no · 2016-09-22T09:44:07Z

@smlng rebased on #5487 and master, I have dropped #5834. The new driver may have some issues, e.g. state tracing, that appeared with the openthread package.

smlng · 2016-09-26T08:27:46Z

@jfischer-phytec-iot: I'm still testing this one. However, you somewhat changed the hardware adress generation, its basically reads backwards from what it was before. Any particular reason to do that? I labeled my nodes with their link-local address - changing the hardware address means changing the lladdr, too - hence, I would have to relabel all (~30) my nodes, which would be a little bit inconvenient 😬

smlng · 2016-09-26T08:45:30Z

I looked into the responsible code, but it looks the same as before. But still compiling with current master I get a different hwaddr (and lladdr) compared to this PR? Weird ...

jfischer-no · 2016-09-26T09:27:26Z

However, you somewhat changed the hardware adress generation, its basically reads backwards from what it was before. Any particular reason to do that?

Yes, i changed it 😄 , sorry. Now, least significant bytes of LL references to the the least significant bytes of CPUID.

smlng · 2016-09-26T09:46:14Z

So you want to keep it that way, and I have to relabel my nodes? Well, then NACK 😁 ... No, I'm fine with it - I just have to remember it to not get confused, when this one is merged.

smlng · 2016-09-26T09:47:51Z

Ah, and could you please fix this conflict in auto_init_kw2xr.c in the meantime.

smlng · 2016-09-26T12:00:43Z

I ran some tests on this PR, I tried to ping a samr21-xpro, both running latest RIOT and gnrc_networking example; and I also tried to ping a RPi with Openlabs transceiver. Additionally, I had a network sniffer running on another RPi, these are the results:

ping6 pba-d-01-kw2x (netdev2) -> samr21-xpro: I see the ping request of kw2x on the sniffer and a subsequent ACK. Afterwards I see 4 ping replies for each ping request (3 retrans) by the samr21-xpro, but no ACKs by kw2x though the ACKreq flag is set by samr21-xpro. kw2x reports 100% packet loss, though sniffer sees all packets.
ping6 pba-d-01-kw2x (netdev2) -> RPi: I only see ping requests no reply by the RPi, result is again 100% packet loss. This might be due to the fact that linux drops all packet with INTRA-PAN flag false even though src and dst PAN are equal.

@jfischer-phytec-iot please set INTRA-PAN flag true if src and dst PAN are equal, see also #5684 and #5685. The latter might fix this already.

smlng · 2016-09-26T12:23:08Z

I applied #5685, now UDP send and recv works in both directions with samr21-xpro, pings still don't work same behavior as described above. Same for UDP with RPi: works in both directions. Ping6 does not work, from kw2x to RPi I see requests and replies, but kw2x still reports 100% packet loss. Ping from RPi to kw2x I see only ping requests on the sniffer but no reply by kw2x.

smlng · 2016-11-30T17:58:48Z

@jfischer-phytec-iot can we get this merged soon? Might be easier to do if not depending on #5487, please adapt/rebase accordingly. If you don't have time for this right now, I'm willing to overtake this PR and make necessary changes, if you don't mind?!

jfischer-no · 2016-12-01T09:48:31Z

will be done today

smlng · 2016-12-13T17:27:57Z

Is there consensus on this iwpan shell command stuff? If not, I would recommend to remove it; and even if so, I'd like to see this in a separate PR.

miri64 · 2016-12-14T07:29:26Z

@smlng: #5750

smlng · 2016-12-13T17:32:04Z

drivers/kw2xrf/include/kw2xrf_intern.h

+    KW2XRF_IDLE,
+    KW2XRF_AUTODOZE,
+}
+kw2xrf_powermode_t;


coding style: no newline required

smlng · 2016-12-13T17:33:08Z

drivers/kw2xrf/include/kw2xrf_intern.h

+ * @brief Returns Timestamp of the actual received packet
+ */
+uint32_t kw2xrf_get_timestamp(kw2xrf_t *dev);
+#ifdef __cplusplus


add newline here

smlng · 2016-12-13T17:33:42Z

drivers/kw2xrf/include/kw2xrf_reg.h

 #define MKW2XDM_IRQSTS3_TMR3IRQ                     (1 << 2)
 #define MKW2XDM_IRQSTS3_TMR2IRQ                     (1 << 1)
 #define MKW2XDM_IRQSTS3_TMR1IRQ                     (1 << 0)
+#define MKW2XDM_IRQSTS3_TMR_IRQ_MASK                0xfu


(maybe) put in parentheses, and below, too?

no, is not necessary

smlng · 2016-12-13T17:33:52Z

drivers/kw2xrf/include/kw2xrf_reg.h

 #define MKW2XDM_PHY_CTRL1_RXACKRQD                  (1 << 4)
 #define MKW2XDM_PHY_CTRL1_AUTOACK                   (1 << 3)
-#define MKW2XDM_PHY_CTRL1_XCVSEQ_MASK               0x03u
+#define MKW2XDM_PHY_CTRL1_XCVSEQ_MASK               0x07u


parentheses

smlng · 2016-12-14T08:53:18Z

@miri64 I still don't see that this PR must be based on (or wait for) #5750, I'd rather have this rebased without this dependency and merged quickly.

@jfischer-phytec-iot is it possible to rebase without #5750?

jfischer-no · 2016-12-14T09:24:06Z

@jfischer-phytec-iot is it possible to rebase without #5750?

I will try it.

smlng · 2016-12-20T16:52:30Z

ping

miri64 · 2017-01-10T10:14:12Z

ping

PeterKietzmann · 2017-01-14T11:22:17Z

@jfischer-phytec-iot we'd like to remove the netdev1 interface after the release. Any chance you prepare this PR until middle of next week so @smlng can take over the netdev2 adoption part of this pr?

smlng · 2017-01-16T17:08:48Z

drivers/kw2xrf/kw2xrf_netdev.c

+    return sizeof(netopt_state_t);
+}
+
+netopt_state_t _get_state(kw2xrf_t *dev)


should be static, otherwise name clash with other drivers, e.g. at86rf233, see Jenkins log here

PeterKietzmann · 2017-01-20T10:16:01Z

@jfischer-phytec-iot we would really much like to have this adoption a a part of the 2016.01 release so we can remove the obsolete netdev1 API. Is there any change that you squash all your commits until the afternoon???

jfischer-no · 2017-01-20T11:43:41Z

@PeterKietzmann I will not make as long as checks are red

smlng · 2017-01-20T12:16:26Z

needs rebase again, that's why CI does not build

jfischer-no · 2017-01-20T13:21:12Z

I give up, that was the last drop:
drivers/kw2xrf/kw2xrf_intern.c:35: style (unreadVariable): Variable 'reg' is assigned a value that is never used.

@PeterKietzmann @smlng If anyone makes PR against this branch, I can merge and squash it, but I do not have time for this today.

miri64 · 2017-01-20T13:23:45Z

drivers/kw2xrf/kw2xrf_intern.c

+    /* Clear and disable all interrupts */
+    kw2xrf_write_dreg(dev, MKW2XDM_PHY_CTRL2, 0xff);
+    int reg = kw2xrf_read_dreg(dev, MKW2XDM_PHY_CTRL3);
+    reg |= MKW2XDM_PHY_CTRL3_WAKE_MSK | MKW2XDM_PHY_CTRL3_PB_ERR_MSK;


The cppcheck actually is right. This variable is set, but never read again.

mhm, maybe a call is missing, in the current driver (kw2xrf.c) I found:

/* Mask all possible interrupts */ reg = kw2xrf_read_dreg(MKW2XDM_PHY_CTRL3); reg |= MKW2XDM_PHY_CTRL3_WAKE_MSK; kw2xrf_write_dreg(MKW2XDM_PHY_CTRL3, reg); kw2xrf_write_dreg(MKW2XDM_IRQSTS1, 0x7f); kw2xrf_write_dreg(MKW2XDM_IRQSTS2, 0x03); kw2xrf_write_dreg(MKW2XDM_IRQSTS3, 0xff);

so reg is written back to the register, that is missing here?!

ah the following line

kw2xrf_write_dreg(dev, MKW2XDM_PHY_CTRL3, 0x03);

here actually 0x03 should be reg because MKW2XDM_PHY_CTRL3_WAKE_MSK | MKW2XDM_PHY_CTRL3_PB_ERR_MSK = 0x03 according to the defines.

so fixing this by either remove reg or better as 0x03 is some magic number with

kw2xrf_write_dreg(dev, MKW2XDM_PHY_CTRL3, reg);

should do

will check squashed branch against backup

Adapt the kw2xrf driver for the netdev2 interface. This patch also adds overwrites.h, the header provides overwrite values for the kw2xrf PHY.

smlng · 2017-01-23T14:30:10Z

closing, superseded by #6453

miri64 · 2017-06-26T11:24:06Z

Wanted to try something

jfischer-no added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: drivers Area: Device drivers labels May 27, 2016

jfischer-no self-assigned this May 27, 2016

miri64 assigned miri64 and unassigned jfischer-no May 30, 2016

This was referenced May 31, 2016

add testmode and cca mode option to netdev #5485

Merged

at86rf2xx: lost interrupts #5486

Closed

miri64 mentioned this pull request Jun 1, 2016

RFC: shell: split WLAN/WPAN specific options into own command #5487

Closed

jfischer-no mentioned this pull request Jun 1, 2016

Implement NETOPT_ACK_REQ configuration option. #5297

Merged

jonas-rem mentioned this pull request Jul 25, 2016

OpenThread port #5552

Closed

jfischer-no mentioned this pull request Jul 28, 2016

kw2xrf: packet loss when packets get fragmented #4822

Closed

miri64 reviewed Aug 30, 2016
View reviewed changes

miri64 mentioned this pull request Sep 9, 2016

kw2xrf: set tx_power in gnrc_netdev_t #5834

Merged

jfischer-no mentioned this pull request Sep 22, 2016

kw2x: 15.4 duplicate transmits #5799

Closed

jfischer-no added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Dec 1, 2016

miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 14, 2016

smlng reviewed Dec 14, 2016

View reviewed changes

jfischer-no removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jan 16, 2017

smlng reviewed Jan 16, 2017

View reviewed changes

jfischer-no removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jan 20, 2017

miri64 reviewed Jan 20, 2017

View reviewed changes

jfischer-no added 2 commits January 20, 2017 14:47

drivers: kw2xrf: adapt for the netdev2 interface

1ea3bc7

Adapt the kw2xrf driver for the netdev2 interface. This patch also adds overwrites.h, the header provides overwrite values for the kw2xrf PHY.

tests/driver_kw2xrf: adapt for netdev2 and add testmodes

105770d

This was referenced Jan 23, 2017

drivers/kw2xrf: adapt to netdev2 #6453

Merged

drivers/cc110x: add xtimer as module dependency #6454

Merged

smlng closed this Jan 23, 2017

miri64 reopened this Jun 26, 2017

miri64 closed this Jun 26, 2017

Conversation

jfischer-no commented May 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miri64 commented May 31, 2016

Uh oh!

miri64 commented Jun 2, 2016

Uh oh!

miri64 commented Aug 5, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Aug 30, 2016

Uh oh!

miri64 commented Aug 30, 2016

Uh oh!

smlng commented Sep 22, 2016

Uh oh!

jfischer-no commented Sep 22, 2016

Uh oh!

jfischer-no commented Sep 22, 2016

Uh oh!

smlng commented Sep 26, 2016

Uh oh!

smlng commented Sep 26, 2016

Uh oh!

jfischer-no commented Sep 26, 2016

Uh oh!

smlng commented Sep 26, 2016

Uh oh!

smlng commented Sep 26, 2016

Uh oh!

smlng commented Sep 26, 2016

Uh oh!

smlng commented Sep 26, 2016

Uh oh!

smlng commented Nov 30, 2016

Uh oh!

jfischer-no commented Dec 1, 2016

Uh oh!

smlng commented Dec 13, 2016

Uh oh!

miri64 commented Dec 14, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smlng commented Dec 14, 2016

Uh oh!

jfischer-no commented Dec 14, 2016

Uh oh!

smlng commented Dec 20, 2016

Uh oh!

miri64 commented Jan 10, 2017

Uh oh!

PeterKietzmann commented Jan 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeterKietzmann commented Jan 20, 2017

Uh oh!

jfischer-no commented Jan 20, 2017

Uh oh!

smlng commented Jan 20, 2017

Uh oh!

jfischer-no commented Jan 20, 2017

Uh oh!

jfischer-no commented May 27, 2016 •

edited

Loading