Re: [PATCH v2 00/13] ftgmac100: Rework batch 1 - Link & Interrupts

[Florian Fainelli <f.fainelli@gmail.com>]

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