RE: [PATCH net-next 2/5] liquidio CN23XX: sriov enable
From: Yuval Mintz <hidden>
Date: 2016-09-11 12:53:36
- dev_dbg(&oct->pci_dev->dev, "%s[%llx] : 0x%llx\n", - "CN23XX_WIN_WR_MASK_REG",
....
+ pr_devel("%s[%llx] : 0x%llx\n",
+ "CN23XX_WIN_WR_MASK_REG",It looks like at least half of this patch [and I think it's also true for other patches in this series] merely change debug prints. Why not extract all of those to a single patch?
+static unsigned int num_vfs[2] = { 0, 0 }; module_param_array(num_vfs,
+uint, NULL, 0444); MODULE_PARM_DESC(num_vfs, "two comma-separated
+unsigned integers that specify number of VFs for PF0 (left of the
+comma) and PF1 (right of the comma); for 23xx only");I believe we're way past the days where it's acceptable to enable IOV Via module parameters; you have sysfs to dynamically enable VFs. BTW, I glanced at pci-iov-howto.txt and noticed it still lists having a module-parameter as control node for activating this feature; But I believe it's been years since this has been considered a valid Method for new drivers [I recall this was forbidden when we've added bnx2x IOV support, and that was more than 3.5 years ago]. Perhaps it would be better to rephrase it so it would be obvious this is a legacy sort of configuration [and not merely 'less preferable']?
+static unsigned int num_queues_per_pf[2] = { 0, 0 };
+module_param_array(num_queues_per_pf, uint, NULL, 0444);
+MODULE_PARM_DESC(num_queues_per_pf, "two comma-separated unsigned
+integers that specify number of queues per PF0 (left of the comma) and
+PF1 (right of the comma); for 23xx only");
+
+static unsigned int num_queues_per_vf[2] = { 0, 0 };
+module_param_array(num_queues_per_vf, uint, NULL, 0444);
+MODULE_PARM_DESC(num_queues_per_vf, "two comma-separated unsigned
+integers that specify number of queues per VFs for PF0 (left of the
+comma) and PF1 (right of the comma); for 23xx only");I don't believe this is a suitable solution as this is a generic problem - how to split resources between PFs and their various VFs. I don't believe there's good infrastructure for it today, though. [Besides, you're introducing new module parameters...]