From: mans@mansr.com (Måns Rullgård)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
Date: Fri, 28 Jul 2017 19:56:17 +0100 [thread overview]
Message-ID: <yw1xfudgl7am.fsf@mansr.com> (raw)
In-Reply-To: <c1bf54ee-8634-23a1-abda-55bc3fafe2a8@sigmadesigns.com> (Marc Gonzalez's message of "Fri, 28 Jul 2017 18:43:42 +0200")
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes:
> On 28/07/2017 18:17, M?ns Rullg?rd wrote:
>
>> Marc Gonzalez wrote:
>>
>>> ndo_stop breaks RX in a way that ndo_open is unable to undo.
>>
>> Please elaborate. Why can't it be fixed in a less heavy-handed way?
>
> I'm not sure what "elaborate" means. After we've been through
> ndo_stop once, the board can send packets, but it doesn't see
> any replies from remote systems. RX is wedged.
So you say, but you have not explained why this happens. Until we know
why, we can't decide on the proper fix.
> I think ndo_stop is rare enough an event that doing a full
> reset is not an issue, in terms of performance.
Performance isn't the issue. Doing the right thing is.
> Also I will need this infrastructure anyway for suspend/resume
> support.
> It might also make sense to put the HW in reset at close
> (to save power). I will try measuring the power savings,
> if any.
>
>>> Work around the issue by resetting the HW in ndo_open.
>>> This will provide the basis for suspend/resume support.
>>>
>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>> ---
>>> drivers/net/ethernet/aurora/nb8800.c | 40 +++++++++++++++++-------------------
>>> drivers/net/ethernet/aurora/nb8800.h | 1 +
>>> 2 files changed, 20 insertions(+), 21 deletions(-)
>>
>> I'm pretty sure this doesn't preserve everything it should.
>
> Hmmm, we're supposed to start fresh ("full reset").
> What could there be to preserve?
> You mentioned flow control and multicast elsewhere.
> I will take a closer look. Thanks for the heads up.
Yes, those settings are definitely lost with your patch. Now I'm not
sure whether the networking core expects these to survive a stop/start
cycle, so please check that. There might also be other less obvious
things that need to be preserved.
--
M?ns Rullg?rd
WARNING: multiple messages have this Message-ID (diff)
From: "Måns Rullgård" <mans@mansr.com>
To: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
netdev <netdev@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Mason <slash.tmp@free.fr>
Subject: Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
Date: Fri, 28 Jul 2017 19:56:17 +0100 [thread overview]
Message-ID: <yw1xfudgl7am.fsf@mansr.com> (raw)
In-Reply-To: <c1bf54ee-8634-23a1-abda-55bc3fafe2a8@sigmadesigns.com> (Marc Gonzalez's message of "Fri, 28 Jul 2017 18:43:42 +0200")
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes:
> On 28/07/2017 18:17, Måns Rullgård wrote:
>
>> Marc Gonzalez wrote:
>>
>>> ndo_stop breaks RX in a way that ndo_open is unable to undo.
>>
>> Please elaborate. Why can't it be fixed in a less heavy-handed way?
>
> I'm not sure what "elaborate" means. After we've been through
> ndo_stop once, the board can send packets, but it doesn't see
> any replies from remote systems. RX is wedged.
So you say, but you have not explained why this happens. Until we know
why, we can't decide on the proper fix.
> I think ndo_stop is rare enough an event that doing a full
> reset is not an issue, in terms of performance.
Performance isn't the issue. Doing the right thing is.
> Also I will need this infrastructure anyway for suspend/resume
> support.
> It might also make sense to put the HW in reset at close
> (to save power). I will try measuring the power savings,
> if any.
>
>>> Work around the issue by resetting the HW in ndo_open.
>>> This will provide the basis for suspend/resume support.
>>>
>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>> ---
>>> drivers/net/ethernet/aurora/nb8800.c | 40 +++++++++++++++++-------------------
>>> drivers/net/ethernet/aurora/nb8800.h | 1 +
>>> 2 files changed, 20 insertions(+), 21 deletions(-)
>>
>> I'm pretty sure this doesn't preserve everything it should.
>
> Hmmm, we're supposed to start fresh ("full reset").
> What could there be to preserve?
> You mentioned flow control and multicast elsewhere.
> I will take a closer look. Thanks for the heads up.
Yes, those settings are definitely lost with your patch. Now I'm not
sure whether the networking core expects these to survive a stop/start
cycle, so please check that. There might also be other less obvious
things that need to be preserved.
--
Måns Rullgård
next prev parent reply other threads:[~2017-07-28 18:56 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-28 16:13 [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open Marc Gonzalez
2017-07-28 16:13 ` Marc Gonzalez
2017-07-28 16:17 ` Måns Rullgård
2017-07-28 16:17 ` Måns Rullgård
2017-07-28 16:43 ` Marc Gonzalez
2017-07-28 16:43 ` Marc Gonzalez
2017-07-28 18:56 ` Måns Rullgård [this message]
2017-07-28 18:56 ` Måns Rullgård
2017-07-28 21:53 ` Mason
2017-07-28 21:53 ` Mason
2017-07-29 11:24 ` Måns Rullgård
2017-07-29 11:24 ` Måns Rullgård
2017-07-29 12:02 ` Mason
2017-07-29 12:02 ` Mason
2017-07-29 12:05 ` Måns Rullgård
2017-07-29 12:05 ` Måns Rullgård
2017-07-29 12:44 ` Mason
2017-07-29 12:44 ` Mason
2017-07-29 12:51 ` Måns Rullgård
2017-07-29 12:51 ` Måns Rullgård
2017-07-29 20:15 ` Florian Fainelli
2017-07-29 20:15 ` Florian Fainelli
2017-07-29 22:48 ` Mason
2017-07-29 22:48 ` Mason
2017-07-29 15:18 ` Florian Fainelli
2017-07-29 15:18 ` Florian Fainelli
2017-07-31 11:49 ` Mason
2017-07-31 11:49 ` Mason
2017-07-31 11:59 ` Måns Rullgård
2017-07-31 11:59 ` Måns Rullgård
2017-07-31 14:08 ` Mason
2017-07-31 14:08 ` Mason
2017-07-31 15:18 ` Mason
2017-07-31 15:18 ` Mason
2017-07-31 15:28 ` Måns Rullgård
2017-07-31 15:28 ` Måns Rullgård
2017-07-31 15:18 ` Måns Rullgård
2017-07-31 15:18 ` Måns Rullgård
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=yw1xfudgl7am.fsf@mansr.com \
--to=mans@mansr.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.