Re: [PATCH ethtool v2 3/3] ethtool: pse-pd: Add PSE event monitoring support

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

On Wed, Aug 13, 2025 at 10:57:52AM +0200, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
.... 
> diff --git a/netlink/pse-pd.c b/netlink/pse-pd.c
> index 5bde176..3fb0616 100644
> --- a/netlink/pse-pd.c
> +++ b/netlink/pse-pd.c
> @@ -475,6 +475,64 @@ int nl_gpse(struct cmd_context *ctx)
>  	return ret;
>  }
>  
> +static const char *pse_events_name(u64 val)
> +{
> +	switch (val) {
> +	case ETHTOOL_PSE_EVENT_OVER_CURRENT:
> +		return "over-current";
> +	case ETHTOOL_PSE_EVENT_OVER_TEMP:
> +		return "over-temperature";
> +	case ETHTOOL_C33_PSE_EVENT_DETECTION:
> +		return "detection";
> +	case ETHTOOL_C33_PSE_EVENT_CLASSIFICATION:
> +		return "classification";
> +	case ETHTOOL_C33_PSE_EVENT_DISCONNECTION:
> +		return "disconnection";
> +	case ETHTOOL_PSE_EVENT_OVER_BUDGET:
> +		return "over-budget";
> +	case ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR:
> +		return "software power control error";
> +	default:
> +		return "unknown";
> +	}
> +}
> +
> +int pse_ntf_cb(const struct nlmsghdr *nlhdr, void *data)
> +{
> +	const struct nlattr *tb[ETHTOOL_A_PSE_MAX + 1] = {};

s/ETHTOOL_A_PSE_MAX/ETHTOOL_A_PSE_NTF_MAX ?

> +	struct nl_context *nlctx = data;
> +	DECLARE_ATTR_TB_INFO(tb);
> +	u64 val;
> +	int ret, i;
> +
> +	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
> +	if (ret < 0)
> +		return MNL_CB_OK;
> +
> +	if (!tb[ETHTOOL_A_PSE_NTF_EVENTS])
> +		return MNL_CB_OK;
> +
> +	nlctx->devname = get_dev_name(tb[ETHTOOL_A_PSE_HEADER]);

s/ETHTOOL_A_PSE_HEADER/ETHTOOL_A_PSE_NTF_HEADER ?

> +	if (!dev_ok(nlctx))
> +		return MNL_CB_OK;
> +
> +	open_json_object(NULL);
> +	print_string(PRINT_ANY, "ifname", "PSE event for %s:\n",
> +		     nlctx->devname);
> +	open_json_array("events", "Events:");
> +	val = attr_get_uint(tb[ETHTOOL_A_PSE_NTF_EVENTS]);

we have here uint but val is u64, is it as expected?

> +	for (i = 0; 1 << i <= ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR; i++)
> +		if (val & 1 << i)
> +			print_string(PRINT_ANY, NULL, " %s",
> +				     pse_events_name(val & 1 << i));

Hm, may be it is better to not limit to ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR
and report unknow numeric value. It will keep even old ethtool at least
partially usable.
-- 
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 |