On 04.08.22 15:08, Andrew Lunn wrote: > On Thu, Aug 04, 2022 at 10:53:33AM +0200, Alexandra Winter wrote: >> >> >> On 03.08.22 17:19, Andrew Lunn wrote: >>> On Wed, Aug 03, 2022 at 04:40:14PM +0200, Alexandra Winter wrote: >>>> Speed, duplex, port type and link mode can change, after the physical link >>>> goes down (STOPLAN) or the card goes offline >>> >>> If the link is down, speed, and duplex are meaningless. They should be >>> set to DUPLEX_UNKNOWN, SPEED_UNKNOWN. There is no PORT_UNKNOWN, but >>> generally, it does not change on link up, so you could set this >>> depending on the hardware type. >>> >>> Andrew >> >> Thank you Andrew for the review. I fully understand your point. >> I would like to propose that I put that on my ToDo list and fix >> that in a follow-on patch to net-next. >> >> The fields in the link_info control blocks are used today to generate >> other values (e.g. supported speed) which will not work with *_UNKNOWN, >> so the follow-on patch will be more than just 2 lines. > > So it sounds like your code is all backwards around. If you know what > the hardware is, you know the supported link modes are, assuming its > not an SFP and the SFP module is not plugged in. Those link modes > should be independent of if the link is up or not. speed/duplex is > only valid when the link is up and negotiation has finished. > > Since this is for net, than yes, maybe it would be best to go with a > minimal patch to make your backwards around code work. But for > net-next, you really should fix this properly. > > Andrew Thank you Andrew. I agree with your analysis.