From: Florian Fainelli <f.fainelli@gmail•com>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>,
netdev@vger•kernel.org
Subject: Re: [PATCH v2 00/13] ftgmac100: Rework batch 1 - Link & Interrupts
Date: Tue, 4 Apr 2017 23:02:38 -0700 [thread overview]
Message-ID: <b59025eb-e653-8481-2e58-a780777df4ee@gmail.com> (raw)
In-Reply-To: <1491371595.4166.77.camel@kernel.crashing.org>
On 04/04/2017 10:53 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-04 at 21:21 -0700, Florian Fainelli wrote:
>>
>> This looks pretty good to me, two minor things:
>>
>> - most drivers keep track of the old status/duplex/pause/link variables
>> instead of the current one which is already available within struct
>> phy_device, any particular reason for not doing like the other drivers?
>
> We don't necessarily have a phydev attached when using NC-SI, so it was
> easier to have the core code path not have to go fishing for those
> settings in different places based on whether we're using NC-SI or not.
Oh right, I missed that part. Is there a reason why NC-SI does not have
a PHY device attached? If not, could you somehow model the link using a
fixed PHY (which appears to Linux as a normal phy_device) just to keep
things simple.
>
>> - the need to reset the HW during link changes is just ... well too bad
>
> Yup but there's little choice. The HW wants it. I don't see any real
> point in optimizing that path mind you. Losing a few packets around
> a link change isn't going to hurt and it keeps the code a lot simpler
> by having a single "re-init" path.
I was just merely trying to say nicely: what a nicely broken piece of HW
(there were other adjectives coming to mind), and I do understand the pain.
>
>> With that:
>>
>>> Reviewed-by: Florian Fainelli <f.fainelli@gmail•com>
>
> Thanks !
>
> I'll post batch 2 in the next couple of days which tackles the RX path.
Cool, looking forward to that!
--
Florian
next prev parent reply other threads:[~2017-04-05 6:02 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-05 2:28 [PATCH v2 00/13] ftgmac100: Rework batch 1 - Link & Interrupts Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 01/13] ftgmac100: Use netdev->irq instead of private copy Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 02/13] ftgmac100: Remove "banner" comments Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 03/13] ftgmac100: Reorder struct fields and comment Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 04/13] ftgmac100: Remove "enabled" flags Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 05/13] ftgmac100: Cleanup speed/duplex tracking and fix duplex config Benjamin Herrenschmidt
2017-04-05 14:58 ` Andrew Lunn
2017-04-05 2:28 ` [PATCH v2 06/13] ftgmac100: Split ring alloc, init and rx buffer alloc Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 07/13] ftgmac100: Move napi_add/del to open/close Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 08/13] ftgmac100: Request the interrupt only after HW is reset Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 09/13] ftgmac100: Move the bulk of inits to a separate function Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 10/13] ftgmac100: Add a reset task and use it for link changes Benjamin Herrenschmidt
2017-04-05 15:00 ` Andrew Lunn
2017-04-05 2:28 ` [PATCH v2 11/13] ftgmac100: Rework MAC reset and init Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 12/13] ftgmac100: Remove useless tests in interrupt handler Benjamin Herrenschmidt
2017-04-05 2:28 ` [PATCH v2 13/13] ftgmac100: Rework NAPI & interrupts handling Benjamin Herrenschmidt
2017-04-05 4:21 ` [PATCH v2 00/13] ftgmac100: Rework batch 1 - Link & Interrupts Florian Fainelli
2017-04-05 5:53 ` Benjamin Herrenschmidt
2017-04-05 6:02 ` Florian Fainelli [this message]
2017-04-05 6:31 ` Benjamin Herrenschmidt
2017-04-06 19:46 ` Florian Fainelli
2017-04-06 21:46 ` Benjamin Herrenschmidt
2017-04-06 19:38 ` David Miller
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=b59025eb-e653-8481-2e58-a780777df4ee@gmail.com \
--to=f.fainelli@gmail$(echo .)com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox