Re: [Intel-wired-lan] [PATCH net-next v1] idpf: add support for IDPF PCI programming interface

[Simon Horman <horms@kernel.org>]

On Tue, Aug 26, 2025 at 10:28:45AM -0700, Pavan Kumar Linga wrote:
> At present IDPF supports only 0x1452 and 0x145C as PF and VF device IDs
> on our current generation hardware. Future hardware exposes a new set of
> device IDs for each generation. To avoid adding a new device ID for each
> generation and to make the driver forward and backward compatible,
> make use of the IDPF PCI programming interface to load the driver.
> 
> Write and read the VF_ARQBAL mailbox register to find if the current
> device is a PF or a VF.
> 
> PCI SIG allocated a new programming interface for the IDPF compliant
> ethernet network controller devices. It can be found at:
> https://members.pcisig.com/wg/PCI-SIG/document/20113
> with the document titled as 'PCI Code and ID Assignment Revision 1.16'
> or any latest revisions.
> 
> Reviewed-by: Madhu Chittim <madhu.chittim@intel.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Signed-off-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>

...

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
> index 8c46481d2e1f..b161715e1168 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> @@ -7,11 +7,57 @@
>  
>  #define DRV_SUMMARY	"Intel(R) Infrastructure Data Path Function Linux Driver"
>  
> +#define IDPF_NETWORK_ETHERNET_PROGIF				0x01
> +#define IDPF_CLASS_NETWORK_ETHERNET_PROGIF			\
> +	(PCI_CLASS_NETWORK_ETHERNET << 8 | IDPF_NETWORK_ETHERNET_PROGIF)
> +
>  MODULE_DESCRIPTION(DRV_SUMMARY);
>  MODULE_IMPORT_NS("LIBETH");
>  MODULE_IMPORT_NS("LIBETH_XDP");
>  MODULE_LICENSE("GPL");
>  
> +/**
> + * idpf_dev_init - Initialize device specific parameters
> + * @adapter: adapter to initialize
> + * @ent: entry in idpf_pci_tbl
> + *
> + * Return: %0 on success, -%errno on failure.
> + */
> +static int idpf_dev_init(struct idpf_adapter *adapter,
> +			 const struct pci_device_id *ent)
> +{
> +	u8 is_vf = 0;
> +	int err;
> +
> +	switch (ent->device) {
> +	case IDPF_DEV_ID_PF:
> +		goto dev_ops_init;
> +	case IDPF_DEV_ID_VF:
> +		is_vf = 1;
> +		goto dev_ops_init;
> +	default:
> +		if (ent->class == IDPF_CLASS_NETWORK_ETHERNET_PROGIF)
> +			goto check_vf;
> +
> +		return -ENODEV;
> +	}
> +
> +check_vf:
> +	err = idpf_is_vf_device(adapter->pdev, &is_vf);
> +	if (err)
> +		return err;
> +
> +dev_ops_init:
> +	if (is_vf) {
> +		idpf_vf_dev_ops_init(adapter);
> +		adapter->crc_enable = true;
> +	} else {
> +		idpf_dev_ops_init(adapter);
> +	}
> +
> +	return 0;
> +}

Hi Pavan,

I think that in Kernel Networking code the usual use cases
of goto labels are: for error handling; and, optimisation,
f.e. in the datapath.

I don't think this code falls into either category.
So I suggest implementing it without gotos.

Thanks!

...