Re: [PATCH net-next v2 2/6] pds_core: add support for identity version 2

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
pds_core: add support for identity version 2

This patch adds a new capabilities field to struct pds_core_dev_identity
and bumps the identify command version to PDS_CORE_IDENTITY_VERSION_2,
falling back to version 1 when version 2 negotiation fails.

> diff --git a/drivers/net/ethernet/amd/pds_core/dev.c b/drivers/net/ethernet/amd/pds_core/dev.c
> index 5b86d6cd0ac3..c9abea9b2eb1 100644
> --- a/drivers/net/ethernet/amd/pds_core/dev.c
> +++ b/drivers/net/ethernet/amd/pds_core/dev.c

[ ... ]

> @@ -327,6 +334,21 @@ static int pdsc_identify(struct pdsc *pdsc)
>  	return 0;
>  }
>
> +static int pdsc_identify(struct pdsc *pdsc)
> +{
> +	int err;
> +
> +	/* Older firmware rejects anything but PDS_CORE_IDENTIFY_VERSION_1
> +	 * instead of returning the max supported identify version, so retry if
> +	 * firmware doesn't support PDS_CORE_IDENTIFY_VERSION_2
> +	 */

[Low]
The macro names referenced in this comment are PDS_CORE_IDENTIFY_VERSION_1
and PDS_CORE_IDENTIFY_VERSION_2, but the macros defined by this patch in
include/linux/pds/pds_core_if.h are spelled PDS_CORE_IDENTITY_VERSION_1 and
PDS_CORE_IDENTITY_VERSION_2 (IDENTITY, not IDENTIFY).

Should the comment use the actual macro names so that grep finds them?

> +	err = pdsc_identify_ver(pdsc, PDS_CORE_IDENTITY_VERSION_2);
> +	if (err)
> +		err = pdsc_identify_ver(pdsc, PDS_CORE_IDENTITY_VERSION_1);
> +
> +	return err;
> +}
> +
-- 
pw-bot: cr