Messages in this thread | | | Subject | Re: [PATCH net-next v5 5/8] net: phylink: Adjust link settings based on rate adaptation | From | Sean Anderson <> | Date | Wed, 7 Sep 2022 13:01:49 -0400 |
| |
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
| |