-----Original Message-----
From: Vladimir Oltean <olteanv@gmail.com>
Sent: Wednesday, January 29, 2020 12:19 PM
To: Madalin Bucur (OSS) <redacted>
Cc: Andrew Lunn <andrew@lunn.ch>; David S. Miller <davem@davemloft.net>;
Florian Fainelli [off-list ref]; Heiner Kallweit
[off-list ref]; netdev [off-list ref]; ykaukab@suse.de
Subject: Re: [PATCH v2 2/2] dpaa_eth: support all modes with rate
adapting PHYs

Hi Madalin,

On Wed, 29 Jan 2020 at 11:09, Madalin Bucur (OSS)
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Vladimir Oltean <olteanv@gmail.com>
Sent: Tuesday, January 28, 2020 5:42 PM
To: Andrew Lunn <andrew@lunn.ch>
Cc: Madalin Bucur (OSS) <redacted>; David S. Miller
[off-list ref]; Florian Fainelli [off-list ref];

Heiner

quoted

quoted

Kallweit [off-list ref]; netdev [off-list ref];
ykaukab@suse.de
Subject: Re: [PATCH v2 2/2] dpaa_eth: support all modes with rate
adapting PHYs

Hi Andrew,

On Mon, 27 Jan 2020 at 18:04, Andrew Lunn [off-list ref] wrote:

quoted

quoted

Is this sufficient?
I suppose this works because you have flow control enabled by

default?

quoted

quoted

What would happen if the user would disable flow control with

ethtool?

quoted

It will still work. Network protocols expect packets to be dropped,
there are bottlenecks on the network, and those bottlenecks change
dynamically. TCP will still be able to determine how much traffic

it

quoted

quoted

quoted

can send without too much packet loss, independent of if the
bottleneck is here between the MAC and the PHY, or later when it

hits

quoted

quoted

quoted

an RFC 1149 link.

Following this logic, this patch isn't needed at all, right? The PHY
will drop frames that it can't hold in its small FIFOs when adapting

a

quoted

quoted

link speed to another, and higher-level protocols will cope. And flow
control at large isn't needed.

I'm afraid you missed the patch description that explains there will be
no link with a 1G partner without this change:

So why not just remove that linkmode_and() at all, then?
What is it trying to accomplish, anyway? Avoiding the user from
shooting themselves in the foot maybe?

If you would take the time to read the patch set, I think it would be clear
that no, I do not intend to remove that altogether, but only when the PHY
can make the different modes work by performing rate adaptation.

quoted

<< After this
commit, the modes removed by the dpaa_eth driver were no longer
advertised thus autonegotiation with 1G link partners failed.>>

quoted

What I was trying to see Madalin's opinion on was whether in fact we
want to keep the RX flow control as 'fixed on' if the MAC supports it
and the PHY needs it, _as a function of the current phy_mode and

maybe

quoted

quoted

link speed_ (the underlined part is important IMO).

That's a separate concern, by default all is fine, should the user want

to

quoted

shoot himself in the foot, we may need to allow him to do it.

quoted

quoted

quoted

    Andrew

Thanks,
-Vladimir

-Vladimir