Re: [PATCH net-next v7 2/5] ethtool: netlink: add ETHTOOL_MSG_MSE_GET and wire up PHY MSE access

[Oleksij Rempel <o.rempel@pengutronix.de>]

Hi Jakub,

On Fri, Oct 24, 2025 at 04:12:13PM -0700, Jakub Kicinski wrote:
> On Fri, 24 Oct 2025 15:18:04 +0200 Oleksij Rempel wrote:
> > Hi Jakub,
> > 
> > On Thu, Oct 23, 2025 at 06:13:43PM -0700, Jakub Kicinski wrote:
> > > On Mon, 20 Oct 2025 12:31:44 +0200 Oleksij Rempel wrote:  
> > > > +      -
> > > > +        name: supported-caps
> > > > +        type: nest
> > > > +        nested-attributes: bitset
> > > > +        enum: phy-mse-capability  
> > > 
> > > This is read only, does it really have to be a bitset?  
> > 
> > It describes the capabilities of the driver/hardware. You can get always
> > everything... Hm... I think we continue without capabilities for now and
> > also remove the specific channel request.
> 
> That's not what I'm saying. I'm just saying that it could be a basic
> uint with appropriate enum rather than bitset? At least with YNL its
> much easier to deal with. The main advantage of bitset is that you
> can modify individual bits, but that doesn't apply to read-only fields.
> 
> Sorry if I'm confused.

after discussing this with Marc Kleine-Budde, I realized that the current
MSE interface is not fully thought through.

Right now the interface lets user space select a specific channel to
poll, but that's not the only relevant selector. Each channel can expose
multiple metrics, and if we ever want to reduce the amount of data for
faster polling, we'll need a different, flag-based selector to describe
which parameters to fetch.

At the moment, however, the kernel simply returns all information that
the PHY can provide. In this situation, the capability flags are mostly
useful inside the kernel, but redundant for user space - we already
provide the values themselves, so there's no need for an extra
"supported-caps" flag set. They duplicate what the user already sees in
the reply (we have a value, and a flag telling that we have this value).

Still, user space needs part of the capabilities structure - the scale
and timing information (max-MSE ranges, refresh rate, number of symbols)
- to interpret the results and choose an appropriate polling rate.

So for the next revision I plan to:
- drop the user-space channel selector completely (the kernel will always
  return all available channels),
- remove the capability bitset from the UAPI,
- keep only the scale/timing fields in the capabilities nest,
- retain capability flags internally in the PHY API for kernel use.

Thanks,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |