[PATCH 04/17] lpfc: NVME Initiator: Base modifications Part C

[James Smart]

On 1/18/2017 3:03 AM, Johannes Thumshirn wrote:
>
>> +	/* maximum number of xris available for nvme buffers */
>> +	els_xri_cnt = lpfc_sli4_get_els_iocb_cnt(phba);
>> +	phba->sli4_hba.nvme_xri_max = phba->sli4_hba.max_cfg_param.max_xri -
>> +				      els_xri_cnt;
>> +	phba->sli4_hba.nvme_xri_max -= phba->sli4_hba.scsi_xri_max;
> 	nvme_xri_max = phba->sli4_hba.max_cfg_param.max_xri - els_xri_cnt;
> 	nvme_xri_max -= phba->sli4_hba.scsi_xri_max;
> 	phba->sli4_hba.nvme_xri_max = nvme_xri_max;
>
> Low hanging anti line-break fruit.

ok - but I didn't think that a style change like this is a mandate.   As 
I'm addressing your other comments, I'll do so.


>>   		}
>> @@ -4273,13 +4489,13 @@ lpfc_sli4_async_sli_evt(struct lpfc_hba *phba, struct lpfc_acqe_sli *acqe_sli)
>>   			sprintf(message, "Unqualified optics - Replace with "
>>   				"Avago optics for Warranty and Technical "
> Is Avago still correct, or should it read Broadcom?

It's right - Avago.


>> @@ -4854,17 +5070,20 @@ static int
>>   lpfc_enable_pci_dev(struct lpfc_hba *phba)
>>   {
>>   	struct pci_dev *pdev;
>> +	int bars = 0;
>>   
>>   	/* Obtain PCI device reference */
>>   	if (!phba->pcidev)
>>   		goto out_error;
>>   	else
>>   		pdev = phba->pcidev;
>> +	/* Select PCI BARs */
>> +	bars = pci_select_bars(pdev, IORESOURCE_MEM);
>>   	/* Enable PCI device */
>>   	if (pci_enable_device_mem(pdev))
>>   		goto out_error;
>>   	/* Request PCI resource for the device */
>> -	if (pci_request_mem_regions(pdev, LPFC_DRIVER_NAME))
>> +	if (pci_request_selected_regions(pdev, bars, LPFC_DRIVER_NAME))
>>   		goto out_disable_device;
>>   	/* Set up device as PCI master and save state for EEH */
>>   	pci_set_master(pdev);
>> @@ -4881,7 +5100,7 @@ lpfc_enable_pci_dev(struct lpfc_hba *phba)
>>   	pci_disable_device(pdev);
>>   out_error:
>>   	lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
>> -			"1401 Failed to enable pci device\n");
>> +			"1401 Failed to enable pci device, bars:x%x\n", bars);
>>   	return -ENODEV;
>>   }
> I don't get this change. pci_request_mem_regions does
>
> pci_request_selected_regions(pdev,
> 		pci_select_bars(pdev, IORESOURCE_MEM), name);
>
> if you want to have the bars in the error log please do:
>    	lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
> 			"1401 Failed to enable pci device, bars:x%x\n",
> 			pci_select_regions(pdev, IORESOURCE_MEM));

I agree - this is weird.  I'll track why it was ever changed and address it.

Other comments are good. I'll address them.

-- james