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

initial ksz8851snl driver files for Ethernet XPRO expansion #4212

Closed
wants to merge 12 commits into from

Conversation

poppe34
Copy link

@poppe34 poppe34 commented Nov 2, 2015

Ok... Here is the new fork from the previous fork that I made. This will allow for communication to the Atmel Ethernet Xpro on the SAMR21 Board. I do have plenty of bugs in this but I am hoping people can help me squash them. I will be working on correcting the big flaw that it currently has with somehow freezing the UART.

@msolters
Copy link

msolters commented Nov 2, 2015

@poppe34 Nice fix on NETOPT 26.

I think that jdebug (https://github.com/poppe34/RIOT/blob/6b1576b12422af0b24e2562d4a8f8f61d954d83b/boards/samr21-xpro/dist/samr21.jdebug) is just a file specific to your IDE?

As such it should be excluded from the PR, I think. You may find it convenient to add it as a .gitignore rule(?)

@poppe34
Copy link
Author

poppe34 commented Nov 2, 2015

I will take it out and I won't add it anymore. It is for my debugger with jlink.

@msolters
Copy link

msolters commented Nov 2, 2015

:) ok, I also have a copy of your PR here with a SAMR21 + Ethernet1 and I'd like to help you track down this mutex issue if I can.

@msolters
Copy link

msolters commented Nov 2, 2015

So here's something interesting. I tagged the mutex functions inside ksz8851_netdev.c with some debug output so I could see if there is ever a time that lock is called, with no corresponding unlock:

static void lock(ksz8851snl_t *dev) {
  DEBUG(SECT_NAME "Locking Mutex.\n");
  mutex_lock(&dev->mutex);
}

static void unlock(ksz8851snl_t *dev) {
  DEBUG(SECT_NAME "Unlocking Mutex.\n");
  mutex_unlock(&dev->mutex);
}

But I've never seen an unbalanced pair! Every lock is followed by an unlock, an yet the gnrc_ksz8851snl process still ends up in bl mutex.

@@ -0,0 +1,254 @@

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure for what you included this file? Also seems unrelated to this PR?!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I pointed that out as well, @poppe34 said it's from his debugger & he will be removing it.

@haukepetersen haukepetersen added Area: network Area: Networking State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Area: drivers Area: Device drivers labels Nov 2, 2015
@haukepetersen haukepetersen added this to the Release 2015.12 milestone Nov 2, 2015
@haukepetersen haukepetersen self-assigned this Nov 2, 2015

#define SECT_NAME "[KSZ8851 Driver]:"

static void lock(ksz8851snl_t *dev) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move the opening '{' to the next line -> RIOT coding conventions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also makes sense to inline this function, but might be that the compililer does this anyway...

@poppe34
Copy link
Author

poppe34 commented Nov 2, 2015

msolters I did the exact same thing. I even took out the locks all together and it still got locked in the mutex. I have been digging into the issue and the only area that I see that locks a mutex is in the pktbuf. So I am looking for an area that gets locked but doesn't get unlocked. I might try the same method we did in the driver, but I am running out of time today..

haukepetersen: I will roll your comments into my code. I did auto init just like the: auto_init_at86rf2xx.c just thought there could better way since the config file is with the driver and other config files tend to be in the board directory. So each board can have their own config, but there is other ways that could be done.

@msolters
Copy link

msolters commented Nov 2, 2015

In an effort to debug this further, I set ENABLE_DEBUG=1 in mutex.c. Unfortunately it seems I run out of stack space:

2015-11-02 16:43:20,131 - INFO # [KSZ8851 Driver]:Locking Mutex.
2015-11-02 16:43:20,134 - INFO # gnrc_ksz8851snl: trying to get mutex. val: 0
2015-11-02 16:43:20,138 - INFO # gnrc_ksz8851snl: trying to get mutex. val: 0
2015-11-02 16:43:20,142 - INFO # gnrc_ksz8851snl: trying to get mutex. val: 1
2015-11-02 16:43:20,145 - INFO # gnrc_ksz8851snl: Mutex in use. 1
2015-11-02 16:43:20,150 - INFO # gnrc_ksz8851snl: Adding node to mutex queue: prio: 3
2015-11-02 16:43:20,152 - INFO # mutex_unlock(): val: 1 pid: 6
2015-11-02 16:43:20,153 - INFO # 
2015-11-02 16:43:20,155 - INFO # Context before hardfault:
2015-11-02 16:43:20,156 - INFO #    r0: 0x200058c4
2015-11-02 16:43:20,158 - INFO #    r1: 0x0000e779
2015-11-02 16:43:20,159 - INFO #    r2: 0x00000400
2015-11-02 16:43:20,161 - INFO #    r3: 0x00000006
2015-11-02 16:43:20,163 - INFO #   r12: 0x0000001d
2015-11-02 16:43:20,164 - INFO #    lr: 0x000008bb
2015-11-02 16:43:20,166 - INFO #    pc: 0x000008dc
2015-11-02 16:43:20,167 - INFO #   psr: 0x21000014
2015-11-02 16:43:20,168 - INFO # 
2015-11-02 16:43:20,168 - INFO # Misc
2015-11-02 16:43:20,170 - INFO # EXC_RET: 0xfffffff1
2015-11-02 16:43:20,174 - INFO # Attempting to reconstruct state for debugging...
2015-11-02 16:43:20,174 - INFO # In GDB:
2015-11-02 16:43:20,176 - INFO #   set $pc=0x8dc
2015-11-02 16:43:20,177 - INFO #   frame 0
2015-11-02 16:43:20,177 - INFO #   bt
2015-11-02 16:43:20,177 - INFO # 
2015-11-02 16:43:20,181 - INFO # ISR stack overflowed by at least 104 bytes.

Unfortunately I don't fully understand where this ISR stack is being initialized; where would I be able to assign more stack space to this process to continue debugging?

@poppe34
Copy link
Author

poppe34 commented Nov 2, 2015

the stack space is initiated in the auto_init file inside sys/auto_init/netif/auto_init_ksz8851snl

ksz8851snl_t * dev = (ksz8851snl_t*)encdev;

lock(dev);
for (uint8_t x = 0; x < 6; x++)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nicer to read with s/6/ETHERNET_ADDR_LEN/

@OlegHahm OlegHahm modified the milestones: Release 2016.07, Release 2016.04 Mar 28, 2016
@OlegHahm
Copy link
Member

Any news?

@miri64
Copy link
Member

miri64 commented Jun 2, 2016

Needs rebase and adaption to #4871.

@kYc0o
Copy link
Contributor

kYc0o commented Jul 25, 2016

WIP -> postponed.

@kYc0o kYc0o modified the milestones: Release 2016.10, Release 2016.07 Jul 25, 2016
@miri64
Copy link
Member

miri64 commented Oct 31, 2016

Feature freeze -> postponed. But would be happy to finally have this merged come January 😃.

@miri64 miri64 modified the milestones: Release 2017.01, Release 2016.10 Oct 31, 2016
@OlegHahm
Copy link
Member

We really should try to get it in for the release. I would volunteer to be the shepherd if someone could give me the hardware.

@miri64
Copy link
Member

miri64 commented Jan 15, 2017

I think I have one of these in my desk drawer in the office.

@PeterKietzmann
Copy link
Member

@miri64 did you find one in your desk drawer?

@OlegHahm OlegHahm added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Jan 18, 2017
@miri64
Copy link
Member

miri64 commented Jan 18, 2017

Japp :-)

@miri64
Copy link
Member

miri64 commented Jan 18, 2017

(and delivered)

@PeterKietzmann
Copy link
Member

@OlegHahm do you still think this is realistic for the upcoming release?

@OlegHahm
Copy link
Member

Since I don't have time to test this today, unfortunately not.

@OlegHahm OlegHahm removed this from the Release 2017.01 milestone Jan 20, 2017
@aabadie aabadie modified the milestone: Release 2017.07 Jun 23, 2017
@miri64
Copy link
Member

miri64 commented Oct 5, 2017

Ping?

@miri64
Copy link
Member

miri64 commented Oct 5, 2017

Since there was no real activity on this PR for > 6 month, I would actually go ahead and close this as memo.

@miri64
Copy link
Member

miri64 commented Oct 5, 2017

Reopen if you disagree.

@miri64 miri64 closed this Oct 5, 2017
@miri64 miri64 added the State: archived State: The PR has been archived for possible future re-adaptation label Oct 5, 2017
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 Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Impact: major The PR changes a significant part of the code base. It should be reviewed carefully State: archived State: The PR has been archived for possible future re-adaptation State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.