Re: [PATCH v4 01/23] iidc: Introduce iidc.h

[Leon Romanovsky <leon@kernel.org>]

On Fri, Apr 09, 2021 at 01:38:37AM +0000, Saleem, Shiraz wrote:
> > Subject: Re: [PATCH v4 01/23] iidc: Introduce iidc.h
> > 
> > On Wed, Apr 07, 2021 at 07:43:24PM -0300, Jason Gunthorpe wrote:
> > > On Wed, Apr 07, 2021 at 08:58:49PM +0000, Saleem, Shiraz wrote:
> > > > > Subject: Re: [PATCH v4 01/23] iidc: Introduce iidc.h
> > > > >
> > > > > On Tue, Apr 06, 2021 at 04:01:03PM -0500, Shiraz Saleem wrote:
> > > > >
> > > > > > +/* Following APIs are implemented by core PCI driver */ struct
> > > > > > +iidc_core_ops {
> > > > > > +	/* APIs to allocate resources such as VEB, VSI, Doorbell queues,
> > > > > > +	 * completion queues, Tx/Rx queues, etc...
> > > > > > +	 */
> > > > > > +	int (*alloc_res)(struct iidc_core_dev_info *cdev_info,
> > > > > > +			 struct iidc_res *res,
> > > > > > +			 int partial_acceptable);
> > > > > > +	int (*free_res)(struct iidc_core_dev_info *cdev_info,
> > > > > > +			struct iidc_res *res);
> > > > > > +
> > > > > > +	int (*request_reset)(struct iidc_core_dev_info *cdev_info,
> > > > > > +			     enum iidc_reset_type reset_type);
> > > > > > +
> > > > > > +	int (*update_vport_filter)(struct iidc_core_dev_info *cdev_info,
> > > > > > +				   u16 vport_id, bool enable);
> > > > > > +	int (*vc_send)(struct iidc_core_dev_info *cdev_info, u32 vf_id, u8
> > *msg,
> > > > > > +		       u16 len);
> > > > > > +};
> > > > >
> > > > > What is this? There is only one implementation:
> > > > >
> > > > > static const struct iidc_core_ops ops = {
> > > > > 	.alloc_res			= ice_cdev_info_alloc_res,
> > > > > 	.free_res			= ice_cdev_info_free_res,
> > > > > 	.request_reset			= ice_cdev_info_request_reset,
> > > > > 	.update_vport_filter		= ice_cdev_info_update_vsi_filter,
> > > > > 	.vc_send			= ice_cdev_info_vc_send,
> > > > > };
> > > > >
> > > > > So export and call the functions directly.
> > > >
> > > > No. Then we end up requiring ice to be loaded even when just want to
> > > > use irdma with x722 [whose ethernet driver is "i40e"].
> > >
> > > So what? What does it matter to load a few extra kb of modules?
> > 
> > And if user cares about it, he will blacklist that module anyway.
> > 
>  blacklist ice when you just have an x722 card? How does that solve anything? You wont be able to load irdma then.

You will blacklist i40e if you want solely irdma functionality.

Thanks

> 
> Shiraz