From: Jakub Kicinski <kuba@kernel.org>
To: Andrew Lunn <andrew@lunn.ch>, Alexandra Winter <wintera@linux.ibm.com>
Cc: David Miller <davem@davemloft.net>,
netdev@vger.kernel.org, linux-s390@vger.kernel.org,
Heiko Carstens <hca@linux.ibm.com>,
Thorsten Winkler <twinkler@linux.ibm.com>
Subject: Re: [PATCH net 1/2] s390/qeth: update cached link_info for ethtool
Date: Thu, 4 Aug 2022 13:27:42 -0700 [thread overview]
Message-ID: <20220804132742.73f8bfda@kernel.org> (raw)
In-Reply-To: <YuvEu9/bzLGU2sTA@lunn.ch>
On Thu, 4 Aug 2022 15:08:11 +0200 Andrew Lunn wrote:
> On Thu, Aug 04, 2022 at 10:53:33AM +0200, Alexandra Winter wrote:
> > 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.
To make sure I understand - the code depends on the speed and duplex
being set to something specific when the device is _down_? Can this be
spelled out more clearly in the commit message?
> 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.
Then again this patch doesn't look like a regression fix (and does not
have a fixes tag). Channeling my inner Greg I'd say - fix this right and
then worry about backports later.
next prev parent reply other threads:[~2022-08-04 20:27 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-03 14:40 [PATCH net 0/2] s390/qeth: cache link_info for ethtool Alexandra Winter
2022-08-03 14:40 ` [PATCH net 1/2] s390/qeth: update cached " Alexandra Winter
2022-08-03 15:19 ` Andrew Lunn
2022-08-04 8:53 ` Alexandra Winter
2022-08-04 13:08 ` Andrew Lunn
2022-08-04 13:44 ` Alexandra Winter
2022-08-04 20:27 ` Jakub Kicinski [this message]
2022-08-05 7:05 ` Alexandra Winter
2022-08-05 21:29 ` Jakub Kicinski
2022-08-03 14:40 ` [PATCH net 2/2] s390/qeth: use " Alexandra Winter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220804132742.73f8bfda@kernel.org \
--to=kuba@kernel.org \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=hca@linux.ibm.com \
--cc=linux-s390@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=twinkler@linux.ibm.com \
--cc=wintera@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.