Re: [net-next PATCH 1/2] net: ethtool: add define for link speed mode number

From: Christian Marangi
Date: Wed Dec 13 2023 - 15:15:37 EST


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