RE: [PATCH net-next 2/5] liquidio CN23XX: sriov enable

[Yuval Mintz]

> - dev_dbg(>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...]

[PATCH net-next 2/5] liquidio CN23XX: sriov enable

[Raghu Vatsavayi]

Adds support for enabling sriov on CN23XX cards.

Signed-off-by: Derek Chickles 
Signed-off-by: Satanand Burla 
Signed-off-by: Felix Manlunas 
Signed-off-by: Raghu Vatsavayi 
---
 .../ethernet/cavium/liquidio/cn23xx_pf_device.c| 257 +++--
 drivers/net/ethernet/cavium/liquidio/lio_main.c| 147 
 .../net/ethernet/cavium/liquidio/octeon_config.h   |   3 +
 .../net/ethernet/cavium/liquidio/octeon_device.h   |   5 +
 4 files changed, 241 insertions(+), 171 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c 
b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
index a2953d5..deec869 100644
--- a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
@@ -19,7 +19,7 @@
 * This file may also be available under a different license from Cavium.
 * Contact Cavium, Inc. for more information
 **/
-
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include 
 #include 
 #include 
@@ -52,174 +52,174 @@ void cn23xx_dump_pf_initialized_regs(struct octeon_device 
*oct)
struct octeon_cn23xx_pf *cn23xx = (struct octeon_cn23xx_pf *)oct->chip;
 
/*In cn23xx_soft_reset*/
-   dev_dbg(>pci_dev->dev, "%s[%llx] : 0x%llx\n",
-   "CN23XX_WIN_WR_MASK_REG", CVM_CAST64(CN23XX_WIN_WR_MASK_REG),
-   CVM_CAST64(octeon_read_csr64(oct, CN23XX_WIN_WR_MASK_REG)));
-   dev_dbg(>pci_dev->dev, "%s[%llx] : 0x%016llx\n",
-   "CN23XX_SLI_SCRATCH1", CVM_CAST64(CN23XX_SLI_SCRATCH1),
-   CVM_CAST64(octeon_read_csr64(oct, CN23XX_SLI_SCRATCH1)));
-   dev_dbg(>pci_dev->dev, "%s[%llx] : 0x%016llx\n",
-   "CN23XX_RST_SOFT_RST", CN23XX_RST_SOFT_RST,
-   lio_pci_readq(oct, CN23XX_RST_SOFT_RST));
+   pr_devel("%s[%llx] : 0x%llx\n",
+"CN23XX_WIN_WR_MASK_REG", CVM_CAST64(CN23XX_WIN_WR_MASK_REG),
+CVM_CAST64(octeon_read_csr64(oct, CN23XX_WIN_WR_MASK_REG)));
+   pr_devel("%s[%llx] : 0x%016llx\n",
+"CN23XX_SLI_SCRATCH1", CVM_CAST64(CN23XX_SLI_SCRATCH1),
+CVM_CAST64(octeon_read_csr64(oct, CN23XX_SLI_SCRATCH1)));
+   pr_devel("%s[%llx] : 0x%016llx\n",
+"CN23XX_RST_SOFT_RST", CN23XX_RST_SOFT_RST,
+lio_pci_readq(oct, CN23XX_RST_SOFT_RST));
 
/*In cn23xx_set_dpi_regs*/
-   dev_dbg(>pci_dev->dev, "%s[%llx] : 0x%016llx\n",
-   "CN23XX_DPI_DMA_CONTROL", CN23XX_DPI_DMA_CONTROL,
-   lio_pci_readq(oct, CN23XX_DPI_DMA_CONTROL));
+   pr_devel("%s[%llx] : 0x%016llx\n",
+"CN23XX_DPI_DMA_CONTROL", CN23XX_DPI_DMA_CONTROL,
+lio_pci_readq(oct, CN23XX_DPI_DMA_CONTROL));
 
for (i = 0; i < 6; i++) {
-   dev_dbg(>pci_dev->dev, "%s(%d)[%llx] : 0x%016llx\n",
-   "CN23XX_DPI_DMA_ENG_ENB", i,
-   CN23XX_DPI_DMA_ENG_ENB(i),
-   lio_pci_readq(oct, CN23XX_DPI_DMA_ENG_ENB(i)));
-   dev_dbg(>pci_dev->dev, "%s(%d)[%llx] : 0x%016llx\n",
-   "CN23XX_DPI_DMA_ENG_BUF", i,
-   CN23XX_DPI_DMA_ENG_BUF(i),
-   lio_pci_readq(oct, CN23XX_DPI_DMA_ENG_BUF(i)));
+   pr_devel("%s(%d)[%llx] : 0x%016llx\n",
+"CN23XX_DPI_DMA_ENG_ENB", i,
+CN23XX_DPI_DMA_ENG_ENB(i),
+lio_pci_readq(oct, CN23XX_DPI_DMA_ENG_ENB(i)));
+   pr_devel("%s(%d)[%llx] : 0x%016llx\n",
+"CN23XX_DPI_DMA_ENG_BUF", i,
+CN23XX_DPI_DMA_ENG_BUF(i),
+lio_pci_readq(oct, CN23XX_DPI_DMA_ENG_BUF(i)));
}
 
-   dev_dbg(>pci_dev->dev, "%s[%llx] : 0x%016llx\n", "CN23XX_DPI_CTL",
-   CN23XX_DPI_CTL, lio_pci_readq(oct, CN23XX_DPI_CTL));
+   pr_devel("%s[%llx] : 0x%016llx\n", "CN23XX_DPI_CTL",
+CN23XX_DPI_CTL, lio_pci_readq(oct, CN23XX_DPI_CTL));
 
/*In cn23xx_setup_pcie_mps and cn23xx_setup_pcie_mrrs */
pci_read_config_dword(oct->pci_dev, CN23XX_CONFIG_PCIE_DEVCTL, );
-   dev_dbg(>pci_dev->dev, "%s[%llx] : 0x%016llx\n",
-   "CN23XX_CONFIG_PCIE_DEVCTL",
-   CVM_CAST64(CN23XX_CONFIG_PCIE_DEVCTL), CVM_CAST64(regval));
+   pr_devel("%s[%llx] : 0x%016llx\n",
+"CN23XX_CONFIG_PCIE_DEVCTL",
+CVM_CAST64(CN23XX_CONFIG_PCIE_DEVCTL), CVM_CAST64(regval));
 
-   dev_dbg(>pci_dev->dev, "%s(%d)[%llx] : 0x%016llx\n",
-   "CN23XX_DPI_SLI_PRTX_CFG", oct->pcie_port,
-   CN23XX_DPI_SLI_PRTX_CFG(oct->pcie_port),
-   lio_pci_readq(oct, CN23XX_DPI_SLI_PRTX_CFG(oct->pcie_port)));
+