netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Marangi <ansuelsmth@gmail.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
	Kees Cook <keescook@chromium.org>,
	Piergiorgio Beruto <piergiorgio.beruto@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [net-next PATCH 1/2] net: ethtool: add define for link speed mode number
Date: Wed, 13 Dec 2023 21:15:27 +0100	[thread overview]
Message-ID: <657a10e1.050a0220.22d18.b3cb@mx.google.com> (raw)
In-Reply-To: <ZXoPwmDsy7geZe6N@shell.armlinux.org.uk>

On Wed, Dec 13, 2023 at 08:10:42PM +0000, Russell King (Oracle) wrote:
> NAK.
> 
> You *clearly* didn't look before you leaped.
> 
> On Wed, Dec 13, 2023 at 07:15:53PM +0100, Christian Marangi wrote:
> > +enum ethtool_link_speeds {
> > +	SPEED_10 = 0,
> > +	SPEED_100,
> > +	SPEED_1000,
> ...
> 
> and from the context immediately below, included in your patch:
> >  #define SPEED_10		10
>            ^^^^^^^^
> >  #define SPEED_100		100
>            ^^^^^^^^^
> >  #define SPEED_1000		1000
>            ^^^^^^^^^^
> 
> Your enumerated values will be overridden by the preprocessor
> definitions.
> 
> Moreover, SPEED_xxx is an already taken namespace and part of the UAPI,
> and thus can _not_ be changed. Convention is that SPEED_x will be
> defined as the numeric speed.
>

Well yes that is the idea of having the enum to count them and then redefining
them to the correct value. (wasn't trying to introduce new define for
the speed and trying to assign incremental values)

Any idea how to handle this without the enum - redefine thing?

Was trying to find a more automated way than defining the raw number of
the current modes. (but maybe this is not that bad? since on adding more
modes, other values has to be changed so it would be just another value
to document in the comment)

-- 
	Ansuel

  reply	other threads:[~2023-12-13 20:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-13 18:15 [net-next PATCH 0/2] net: add define to describe link speed modes Christian Marangi
2023-12-13 18:15 ` [net-next PATCH 1/2] net: ethtool: add define for link speed mode number Christian Marangi
2023-12-13 20:10   ` Russell King (Oracle)
2023-12-13 20:15     ` Christian Marangi [this message]
2023-12-13 20:23       ` Russell King (Oracle)
2023-12-14  7:35   ` kernel test robot
2023-12-13 18:15 ` [net-next PATCH 2/2] net: phy: leds: use new define for link speed modes number Christian Marangi
2023-12-13 20:18   ` Russell King (Oracle)
2023-12-13 20:34     ` Christian Marangi
2023-12-13 23:05 ` [net-next PATCH 0/2] net: add define to describe link speed modes Andrew Lunn
2023-12-13 23:10   ` Christian Marangi

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=657a10e1.050a0220.22d18.b3cb@mx.google.com \
    --to=ansuelsmth@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=keescook@chromium.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mailhol.vincent@wanadoo.fr \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=piergiorgio.beruto@gmail.com \
    --cc=vladimir.oltean@nxp.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).