Re: [PATCH v2 7/9] PCI: endpoint: Add the function number as argument to EPC ops

[Cyrille Pitchen <cyrille.pitchen@free-electrons.com>]

Hi Kishon,

Le 29/12/2017 à 10:23, Kishon Vijay Abraham I a écrit :
> Hi,
> 
> On Monday 18 December 2017 11:46 PM, Cyrille Pitchen wrote:
>> This patch updates the prototype of most handlers from 'struct
>> pci_epc_ops' so the EPC library can now support multi-function devices.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
>> ---
>>  drivers/pci/dwc/pcie-designware-ep.c          | 20 +++++----
>>  drivers/pci/endpoint/functions/pci-epf-test.c | 41 ++++++++++--------
>>  drivers/pci/endpoint/pci-epc-core.c           | 62 ++++++++++++++++-----------
>>  include/linux/pci-epc.h                       | 43 +++++++++++--------
>>  4 files changed, 96 insertions(+), 70 deletions(-)
>>
[...]
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index cd7d4788b94d..77420364a728 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -141,25 +141,26 @@ EXPORT_SYMBOL_GPL(pci_epc_start);
>>  /**
>>   * pci_epc_raise_irq() - interrupt the host system
>>   * @epc: the EPC device which has to interrupt the host
>> + * @func_no: the endpoint function number in the EPC device
>>   * @type: specify the type of interrupt; legacy or MSI
>>   * @interrupt_num: the MSI interrupt number
>>   *
>>   * Invoke to raise an MSI or legacy interrupt
>>   */
>> -int pci_epc_raise_irq(struct pci_epc *epc, enum pci_epc_irq_type type,
>> -		      u8 interrupt_num)
>> +int pci_epc_raise_irq(struct pci_epc *epc, u8 func_no,
>> +		      enum pci_epc_irq_type type, u8 interrupt_num)
>>  {
>>  	int ret;
>>  	unsigned long flags;
>>  
>> -	if (IS_ERR(epc))
>> +	if (IS_ERR(epc) || func_no > BAR_5)
> 
> why is function number compared with BAR? here and everywhere below..
>

Oops! sorry for that. Actually it's a silly mistake, thanks for
catching it!

I plan to fix it replacing with something like this:

s/func_no > BAR_5/func_no >= epc->max_functions/

Then I guess if we already test with IS_ERR(), I should now replace
IS_ERR() by IS_ERR_OR_NULL() before dereferencing epc when reading
max_functions.

Best regards,

Cyrille
 
> Thanks
> Kishon
> 


-- 
Cyrille Pitchen, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com