> +static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
> + void *buf, int size)
> +{
> + struct hwrm_nvm_get_variable_input req = {0};
> + dma_addr_t dest_data_dma_addr;
> + void *dest_data_addr = NULL;
> + int bytesize;
> + int rc;
> +
> + bytesize = (size + 7) / BITS_PER_BYTE;
roundup?
..
+static int bnxt_nvm_write(struct bnxt *bp, int nvm_param, int idx,
> + const void *buf, int size)
> +{
> + struct hwrm_nvm_set_variable_input req = {0};
> + dma_addr_t src_data_dma_addr;
> + void *src_data_addr = NULL;
> + int bytesize;
> + int rc;
> +
> + bytesize = (size + 7) / BITS_PER_BYTE;
Likewise
> +
> + src_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
> + &src_data_dma_addr,
> GFP_KERNEL);
> + if (!src_data_addr) {
> + netdev_err(bp->dev, "dma_alloc_coherent failure\n");
Won't you see an oom? Why do you need the print?
> +static int bnxt_dl_perm_config_set(struct devlink *devlink,
> + enum devlink_perm_config_param param,
> + u8 type, void *value, u8 *restart_reqd)
> +{
> + struct bnxt *bp = bnxt_get_bp_from_dl(devlink);
> + struct bnxt_drv_cfgparam *entry;
> + int idx = 0;
> + int ret = 0;
> + u32 bytesize;
> + u32 val32;
> + u16 val16;
> + u8 val8;
> + int i;
> +
> + *restart_reqd = 0;
> +
> + /* Find parameter in table */
> + for (i = 0; i < BNXT_NUM_DRV_CFGPARAM; i++) {
> + if (param == bnxt_drv_cfgparam_list[i].param) {
> + entry = &bnxt_drv_cfgparam_list[i];
> + break;
> + }
> + }
> +
> + /* Not found */
> + if (i == BNXT_NUM_DRV_CFGPARAM)
> + return -EINVAL;
> +
Looks cleaner to check whether entry is set instead
...
> + bytesize = (entry->bitlength + 7) / BITS_PER_BYTE;
Roundup?
...
> + if (bytesize == 1) {
> + val8 = val32;
Don't you need explicit castings for these kind of assignments
to prevent warnings?
> + ret = bnxt_nvm_write(bp, entry->nvm_param, idx,
> &val8,
> + entry->bitlength);
> + } else if (bytesize == 2) {
> + val16 = val32;
> + ret = bnxt_nvm_write(bp, entry->nvm_param, idx,
> &val16,
> + entry->bitlength);
> + } else {
> + ret = bnxt_nvm_write(bp, entry->nvm_param, idx,
> &val32,
> + entry->bitlength);
> + }
> + }
> +
> + /* Restart required for all nvm parameter writes */
> + *restart_reqd = 1;
> +
> + return ret;
> +}
> +
> +static int bnxt_dl_perm_config_get(struct devlink *devlink,
> + enum devlink_perm_config_param param,
> + u8 type, void *value)
> +{
Same comments as for the setter
...
> - if (!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV))
> - return 0;
> -
> - if (bp->hwrm_spec_code < 0x10800) {
> + if ((!pci_find_ext_capability(bp->pdev, PCI_EXT_CAP_ID_SRIOV)) ||
> + bp->hwrm_spec_code < 0x10800) {
> + /* eswitch switchdev mode not supported */
> + bnxt_dl_ops.eswitch_mode_set = NULL;
> + bnxt_dl_ops.eswitch_mode_get = NULL;
Why would you need to tie this interface to the presence of SRIOV in PCIe?
Also, Assuming the ability to disable sriov in #2 would cause this capability
not to be exposed after reboot, isn't this a one-way ticket?
