[openib-general][RFC][PATCH] mthca: HCA initialization parameters

[Roland Dreier]

This looks OK, but:

 > +module_param_named(num_mcg, default_profile.num_mcg, int, 0444);
 > +MODULE_PARM_DESC(num_mcg, "Maximum number of Multicast groups per HCA");
 > +
 > +module_param_named(num_mpt, default_profile.num_mpt, int, 0444);
 > +MODULE_PARM_DESC(num_mpt,
 > +		 "Maximum number of Memory Protection Table entries per HCA");
 > +
 > +module_param_named(num_mtt, default_profile.num_mtt, int, 0444);
 > +MODULE_PARM_DESC(num_mtt,
 > +		 "Maximum number of Memory Translation table segments per HCA");

very inconsistent capitalization in the help texts...

 > +{
 > +

extra blank line here

 > +	if(default_profile.num_qp & (default_profile.num_qp-1)) {

There should be a space in "if(" -- it should be "if (".  Also there
should be spaces around the "-" like "num_qp - 1".

 > +err_inval:
 > +	mthca_err(mdev, "This parameter must be power of two.\n");

this all seems rather unfriendly -- maybe just round things up to a
power of 2 and print a warning about it?

 >  	printk(KERN_INFO PFX "Initializing %s\n",
 >  	       pci_name(pdev));
 >  
 > +
 >  	if (id->driver_data >= ARRAY_SIZE(mthca_hca_table)) {
 >  		printk(KERN_ERR PFX "%s has invalid driver data %lx\n",
 >  		       pci_name(pdev), id->driver_data);

strange whitespace change here...

 > +	err = mthca_validate_profile(mdev, &default_profile);
 > +	if (err)
 > +		goto err_profile;
 > +
 >  	err = mthca_init_hca(mdev);

why check the profile for every individual HCA?  wouldn't it make more
sense to do it once in the module_init function and refuse to load if
the profile isn't valid?

 - R.