lkml.org 
[lkml]   [2022]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net-next v5 5/8] net: phylink: Adjust link settings based on rate adaptation
From
Date
On 9/7/22 6:10 AM, Russell King (Oracle) wrote:
> On Tue, Sep 06, 2022 at 12:18:49PM -0400, Sean Anderson wrote:
>> @@ -1015,19 +1086,45 @@ static void phylink_link_up(struct phylink *pl,
>> struct phylink_link_state link_state)
>> {
>> struct net_device *ndev = pl->netdev;
>> + int speed, duplex;
>> + bool rx_pause;
>> +
>> + speed = link_state.speed;
>> + duplex = link_state.duplex;
>> + rx_pause = !!(link_state.pause & MLO_PAUSE_RX);
>> +
>> + switch (link_state.rate_adaptation) {
>> + case RATE_ADAPT_PAUSE:
>> + /* The PHY is doing rate adaption from the media rate (in
>> + * the link_state) to the interface speed, and will send
>> + * pause frames to the MAC to limit its transmission speed.
>> + */
>> + speed = phylink_interface_max_speed(link_state.interface);
>> + duplex = DUPLEX_FULL;
>> + rx_pause = true;
>> + break;
>> +
>> + case RATE_ADAPT_CRS:
>> + /* The PHY is doing rate adaption from the media rate (in
>> + * the link_state) to the interface speed, and will cause
>> + * collisions to the MAC to limit its transmission speed.
>> + */
>> + speed = phylink_interface_max_speed(link_state.interface);
>> + duplex = DUPLEX_HALF;
>> + break;
>> + }
>>
>> pl->cur_interface = link_state.interface;
>> + if (link_state.rate_adaptation == RATE_ADAPT_PAUSE)
>> + link_state.pause |= MLO_PAUSE_RX;
>
> I specifically omitted this from my patch because I don't think we
> should tell the user that "Link is Up - ... - flow control rx" if
> we have rate adaption, but the media link is not using flow control.
>
> The "Link is Up" message tells the user what was negotiated on the
> media, not what is going on inside their network device, so the
> fact we're using rate adaption which has turned on RX pause on the
> MAC is neither here nor there.

OK, I will leave this out then.

>>
>> if (pl->pcs && pl->pcs->ops->pcs_link_up)
>> pl->pcs->ops->pcs_link_up(pl->pcs, pl->cur_link_an_mode,
>> - pl->cur_interface,
>> - link_state.speed, link_state.duplex);
>> + pl->cur_interface, speed, duplex);
>
> It seems you have one extra unnecessary space here - not sure how
> that occurred as it isn't in my original patch.
>

This corrects a spacing issue in the existing code.

--Sean

\
 
 \ /
  Last update: 2022-09-07 19:02    [W:0.078 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site