Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <657a10e1.050a0220.22d18.b3cb@mx.google.com>
Date:   Wed, 13 Dec 2023 21:15:27 +0100
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
References: <20231213181554.4741-1-ansuelsmth@gmail.com>
 <20231213181554.4741-2-ansuelsmth@gmail.com>
 <ZXoPwmDsy7geZe6N@shell.armlinux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZXoPwmDsy7geZe6N@shell.armlinux.org.uk>
Precedence: bulk

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