Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Thu, 14 Dec 2023 00:05:52 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Christian Marangi <ansuelsmth@gmail.com>
Cc:     Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "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 0/2] net: add define to describe link speed modes
Message-ID: <a4661402-414a-4b0d-82e8-97031fa46230@lunn.ch>
References: <20231213181554.4741-1-ansuelsmth@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231213181554.4741-1-ansuelsmth@gmail.com>
Precedence: bulk

On Wed, Dec 13, 2023 at 07:15:52PM +0100, Christian Marangi wrote:
> This is a simple series to add define to describe link speed modes.
> 
> Hope the proposed way is acceptable with the enum and define.
> 
> This is also needed in the upcoming changes in the netdev trigger for LEDs
> where phy_speeds functions is used to declare a more compact array instead
> of using a "big enough" approach.

I'm trying to figure out the 'big picture' here.

The LED trigger will call ksetting_get to get a list of supported link
modes. You can then use the table struct phy_setting settings[] in
phy-core.c to translate the link mode to a speed. You are likely to
get a lot of duplicate speeds, but you can remove them. For each speed
you need to create a sysfs file. Why not just create a linked list,
rather than an array? Or just walk the table and find out how many
different speeds there are and allocate an array of that size. Its
currently 15, which is not that big. And then just use the is_visible
method to hide the ones which are not relevant.

I don't see any need for new enums or tables here, just a function to
look up a link mode in that table and return the speed.

       Andrew