[4/4] PCI: pci-epf-test: Add support to defer core initialization
diff mbox series

Message ID 20191113090851.26345-5-vidyas@nvidia.com
State Superseded
Delegated to: Lorenzo Pieralisi
Headers show
Series
  • Add support to defer core initialization
Related show

Commit Message

Vidya Sagar Nov. 13, 2019, 9:08 a.m. UTC
Add support to defer core initialization and to receive a notifier
when core is ready to accommodate platforms where core is not for
initialization untile reference clock from host is available.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
 1 file changed, 77 insertions(+), 37 deletions(-)

Comments

Kishon Vijay Abraham I Nov. 27, 2019, 9:20 a.m. UTC | #1
Hi,

On 13/11/19 2:38 PM, Vidya Sagar wrote:
> Add support to defer core initialization and to receive a notifier
> when core is ready to accommodate platforms where core is not for
> initialization untile reference clock from host is available.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>  1 file changed, 77 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index bddff15052cc..068024fab544 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
>  			   msecs_to_jiffies(1));
>  }
>  
> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
> -				 void *data)
> -{
> -	struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
> -	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> -
> -	queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
> -			   msecs_to_jiffies(1));
> -
> -	return NOTIFY_OK;
> -}
> -
>  static void pci_epf_test_unbind(struct pci_epf *epf)
>  {
>  	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>  	return 0;
>  }
>  
> +static int pci_epf_test_core_init(struct pci_epf *epf)
> +{
> +	struct pci_epf_header *header = epf->header;
> +	const struct pci_epc_features *epc_features;
> +	struct pci_epc *epc = epf->epc;
> +	struct device *dev = &epf->dev;
> +	bool msix_capable = false;
> +	bool msi_capable = true;
> +	int ret;
> +
> +	epc_features = pci_epc_get_features(epc, epf->func_no);
> +	if (epc_features) {
> +		msix_capable = epc_features->msix_capable;
> +		msi_capable = epc_features->msi_capable;
> +	}
> +
> +	ret = pci_epc_write_header(epc, epf->func_no, header);
> +	if (ret) {
> +		dev_err(dev, "Configuration header write failed\n");
> +		return ret;
> +	}
> +
> +	ret = pci_epf_test_set_bar(epf);
> +	if (ret)
> +		return ret;
> +
> +	if (msi_capable) {
> +		ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
> +		if (ret) {
> +			dev_err(dev, "MSI configuration failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	if (msix_capable) {
> +		ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
> +		if (ret) {
> +			dev_err(dev, "MSI-X configuration failed\n");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
> +				 void *data)
> +{
> +	struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
> +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> +	int ret;
> +
> +	switch (val) {
> +	case CORE_INIT:
> +		ret = pci_epf_test_core_init(epf);
> +		if (ret)
> +			return NOTIFY_BAD;
> +		break;
> +
> +	case LINK_UP:
> +		queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
> +				   msecs_to_jiffies(1));
> +		break;
> +
> +	default:
> +		dev_err(&epf->dev, "Invalid EPF test notifier event\n");
> +		return NOTIFY_BAD;
> +	}
> +
> +	return NOTIFY_OK;
> +}
> +
>  static int pci_epf_test_alloc_space(struct pci_epf *epf)
>  {
>  	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> @@ -496,12 +556,11 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  {
>  	int ret;
>  	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> -	struct pci_epf_header *header = epf->header;
>  	const struct pci_epc_features *epc_features;
>  	enum pci_barno test_reg_bar = BAR_0;
>  	struct pci_epc *epc = epf->epc;
> -	struct device *dev = &epf->dev;
>  	bool linkup_notifier = false;
> +	bool skip_core_init = false;
>  	bool msix_capable = false;
>  	bool msi_capable = true;
>  
> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  	epc_features = pci_epc_get_features(epc, epf->func_no);
>  	if (epc_features) {
>  		linkup_notifier = epc_features->linkup_notifier;
> +		skip_core_init = epc_features->skip_core_init;
>  		msix_capable = epc_features->msix_capable;
>  		msi_capable = epc_features->msi_capable;

Are these used anywhere in this function?
>  		test_reg_bar = pci_epc_get_first_free_bar(epc_features);
> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  	epf_test->test_reg_bar = test_reg_bar;
>  	epf_test->epc_features = epc_features;
>  
> -	ret = pci_epc_write_header(epc, epf->func_no, header);
> -	if (ret) {
> -		dev_err(dev, "Configuration header write failed\n");
> -		return ret;
> -	}
> -
>  	ret = pci_epf_test_alloc_space(epf);
>  	if (ret)
>  		return ret;
>  
> -	ret = pci_epf_test_set_bar(epf);
> -	if (ret)
> -		return ret;
> -
> -	if (msi_capable) {
> -		ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
> -		if (ret) {
> -			dev_err(dev, "MSI configuration failed\n");
> -			return ret;
> -		}
> -	}
> -
> -	if (msix_capable) {
> -		ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
> -		if (ret) {
> -			dev_err(dev, "MSI-X configuration failed\n");
> +	if (!skip_core_init) {
> +		ret = pci_epf_test_core_init(epf);
> +		if (ret)
>  			return ret;
> -		}
>  	}
>  
>  	if (linkup_notifier) {

This could as well be moved to pci_epf_test_core_init().

Thanks
Kishon
Vidya Sagar Dec. 1, 2019, 2:29 p.m. UTC | #2
On 11/27/2019 2:50 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>> Add support to defer core initialization and to receive a notifier
>> when core is ready to accommodate platforms where core is not for
>> initialization untile reference clock from host is available.
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>>   drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>>   1 file changed, 77 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
>> index bddff15052cc..068024fab544 100644
>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
>>   			   msecs_to_jiffies(1));
>>   }
>>   
>> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
>> -				 void *data)
>> -{
>> -	struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>> -	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> -
>> -	queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>> -			   msecs_to_jiffies(1));
>> -
>> -	return NOTIFY_OK;
>> -}
>> -
>>   static void pci_epf_test_unbind(struct pci_epf *epf)
>>   {
>>   	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>>   	return 0;
>>   }
>>   
>> +static int pci_epf_test_core_init(struct pci_epf *epf)
>> +{
>> +	struct pci_epf_header *header = epf->header;
>> +	const struct pci_epc_features *epc_features;
>> +	struct pci_epc *epc = epf->epc;
>> +	struct device *dev = &epf->dev;
>> +	bool msix_capable = false;
>> +	bool msi_capable = true;
>> +	int ret;
>> +
>> +	epc_features = pci_epc_get_features(epc, epf->func_no);
>> +	if (epc_features) {
>> +		msix_capable = epc_features->msix_capable;
>> +		msi_capable = epc_features->msi_capable;
>> +	}
>> +
>> +	ret = pci_epc_write_header(epc, epf->func_no, header);
>> +	if (ret) {
>> +		dev_err(dev, "Configuration header write failed\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = pci_epf_test_set_bar(epf);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (msi_capable) {
>> +		ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
>> +		if (ret) {
>> +			dev_err(dev, "MSI configuration failed\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	if (msix_capable) {
>> +		ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
>> +		if (ret) {
>> +			dev_err(dev, "MSI-X configuration failed\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
>> +				 void *data)
>> +{
>> +	struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>> +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> +	int ret;
>> +
>> +	switch (val) {
>> +	case CORE_INIT:
>> +		ret = pci_epf_test_core_init(epf);
>> +		if (ret)
>> +			return NOTIFY_BAD;
>> +		break;
>> +
>> +	case LINK_UP:
>> +		queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>> +				   msecs_to_jiffies(1));
>> +		break;
>> +
>> +	default:
>> +		dev_err(&epf->dev, "Invalid EPF test notifier event\n");
>> +		return NOTIFY_BAD;
>> +	}
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>>   static int pci_epf_test_alloc_space(struct pci_epf *epf)
>>   {
>>   	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> @@ -496,12 +556,11 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>   {
>>   	int ret;
>>   	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> -	struct pci_epf_header *header = epf->header;
>>   	const struct pci_epc_features *epc_features;
>>   	enum pci_barno test_reg_bar = BAR_0;
>>   	struct pci_epc *epc = epf->epc;
>> -	struct device *dev = &epf->dev;
>>   	bool linkup_notifier = false;
>> +	bool skip_core_init = false;
>>   	bool msix_capable = false;
>>   	bool msi_capable = true;
>>   
>> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>   	epc_features = pci_epc_get_features(epc, epf->func_no);
>>   	if (epc_features) {
>>   		linkup_notifier = epc_features->linkup_notifier;
>> +		skip_core_init = epc_features->skip_core_init;
>>   		msix_capable = epc_features->msix_capable;
>>   		msi_capable = epc_features->msi_capable;
> 
> Are these used anywhere in this function?
Nope. I'll remove them.

>>   		test_reg_bar = pci_epc_get_first_free_bar(epc_features);
>> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>   	epf_test->test_reg_bar = test_reg_bar;
>>   	epf_test->epc_features = epc_features;
>>   
>> -	ret = pci_epc_write_header(epc, epf->func_no, header);
>> -	if (ret) {
>> -		dev_err(dev, "Configuration header write failed\n");
>> -		return ret;
>> -	}
>> -
>>   	ret = pci_epf_test_alloc_space(epf);
>>   	if (ret)
>>   		return ret;
>>   
>> -	ret = pci_epf_test_set_bar(epf);
>> -	if (ret)
>> -		return ret;
>> -
>> -	if (msi_capable) {
>> -		ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
>> -		if (ret) {
>> -			dev_err(dev, "MSI configuration failed\n");
>> -			return ret;
>> -		}
>> -	}
>> -
>> -	if (msix_capable) {
>> -		ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
>> -		if (ret) {
>> -			dev_err(dev, "MSI-X configuration failed\n");
>> +	if (!skip_core_init) {
>> +		ret = pci_epf_test_core_init(epf);
>> +		if (ret)
>>   			return ret;
>> -		}
>>   	}
>>   
>>   	if (linkup_notifier) {
> 
> This could as well be moved to pci_epf_test_core_init().
Yes, but I would like to keep only the code that touches hardware in pci_epf_test_core_init()
to minimize the time it takes to execute it. Is there any strong reason to move it? if not,
I would prefer to leave it here in this function itself.

> 
> Thanks
> Kishon
>
Kishon Vijay Abraham I Dec. 5, 2019, 11:22 a.m. UTC | #3
Hi,

On 01/12/19 7:59 pm, Vidya Sagar wrote:
> On 11/27/2019 2:50 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>>> Add support to defer core initialization and to receive a notifier
>>> when core is ready to accommodate platforms where core is not for
>>> initialization untile reference clock from host is available.
>>>
>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>> ---
>>>   drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>>>   1 file changed, 77 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c 
>>> b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> index bddff15052cc..068024fab544 100644
>>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct 
>>> work_struct *work)
>>>                  msecs_to_jiffies(1));
>>>   }
>>> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned 
>>> long val,
>>> -                 void *data)
>>> -{
>>> -    struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>> -    struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> -
>>> -    queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>> -               msecs_to_jiffies(1));
>>> -
>>> -    return NOTIFY_OK;
>>> -}
>>> -
>>>   static void pci_epf_test_unbind(struct pci_epf *epf)
>>>   {
>>>       struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf 
>>> *epf)
>>>       return 0;
>>>   }
>>> +static int pci_epf_test_core_init(struct pci_epf *epf)
>>> +{
>>> +    struct pci_epf_header *header = epf->header;
>>> +    const struct pci_epc_features *epc_features;
>>> +    struct pci_epc *epc = epf->epc;
>>> +    struct device *dev = &epf->dev;
>>> +    bool msix_capable = false;
>>> +    bool msi_capable = true;
>>> +    int ret;
>>> +
>>> +    epc_features = pci_epc_get_features(epc, epf->func_no);
>>> +    if (epc_features) {
>>> +        msix_capable = epc_features->msix_capable;
>>> +        msi_capable = epc_features->msi_capable;
>>> +    }
>>> +
>>> +    ret = pci_epc_write_header(epc, epf->func_no, header);
>>> +    if (ret) {
>>> +        dev_err(dev, "Configuration header write failed\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    ret = pci_epf_test_set_bar(epf);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    if (msi_capable) {
>>> +        ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
>>> +        if (ret) {
>>> +            dev_err(dev, "MSI configuration failed\n");
>>> +            return ret;
>>> +        }
>>> +    }
>>> +
>>> +    if (msix_capable) {
>>> +        ret = pci_epc_set_msix(epc, epf->func_no, 
>>> epf->msix_interrupts);
>>> +        if (ret) {
>>> +            dev_err(dev, "MSI-X configuration failed\n");
>>> +            return ret;
>>> +        }
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned 
>>> long val,
>>> +                 void *data)
>>> +{
>>> +    struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>> +    struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> +    int ret;
>>> +
>>> +    switch (val) {
>>> +    case CORE_INIT:
>>> +        ret = pci_epf_test_core_init(epf);
>>> +        if (ret)
>>> +            return NOTIFY_BAD;
>>> +        break;
>>> +
>>> +    case LINK_UP:
>>> +        queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>> +                   msecs_to_jiffies(1));
>>> +        break;
>>> +
>>> +    default:
>>> +        dev_err(&epf->dev, "Invalid EPF test notifier event\n");
>>> +        return NOTIFY_BAD;
>>> +    }
>>> +
>>> +    return NOTIFY_OK;
>>> +}
>>> +
>>>   static int pci_epf_test_alloc_space(struct pci_epf *epf)
>>>   {
>>>       struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> @@ -496,12 +556,11 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>>   {
>>>       int ret;
>>>       struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> -    struct pci_epf_header *header = epf->header;
>>>       const struct pci_epc_features *epc_features;
>>>       enum pci_barno test_reg_bar = BAR_0;
>>>       struct pci_epc *epc = epf->epc;
>>> -    struct device *dev = &epf->dev;
>>>       bool linkup_notifier = false;
>>> +    bool skip_core_init = false;
>>>       bool msix_capable = false;
>>>       bool msi_capable = true;
>>> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>>       epc_features = pci_epc_get_features(epc, epf->func_no);
>>>       if (epc_features) {
>>>           linkup_notifier = epc_features->linkup_notifier;
>>> +        skip_core_init = epc_features->skip_core_init;
>>>           msix_capable = epc_features->msix_capable;
>>>           msi_capable = epc_features->msi_capable;
>>
>> Are these used anywhere in this function?
> Nope. I'll remove them.
> 
>>>           test_reg_bar = pci_epc_get_first_free_bar(epc_features);
>>> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>>       epf_test->test_reg_bar = test_reg_bar;
>>>       epf_test->epc_features = epc_features;
>>> -    ret = pci_epc_write_header(epc, epf->func_no, header);
>>> -    if (ret) {
>>> -        dev_err(dev, "Configuration header write failed\n");
>>> -        return ret;
>>> -    }
>>> -
>>>       ret = pci_epf_test_alloc_space(epf);
>>>       if (ret)
>>>           return ret;
>>> -    ret = pci_epf_test_set_bar(epf);
>>> -    if (ret)
>>> -        return ret;
>>> -
>>> -    if (msi_capable) {
>>> -        ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
>>> -        if (ret) {
>>> -            dev_err(dev, "MSI configuration failed\n");
>>> -            return ret;
>>> -        }
>>> -    }
>>> -
>>> -    if (msix_capable) {
>>> -        ret = pci_epc_set_msix(epc, epf->func_no, 
>>> epf->msix_interrupts);
>>> -        if (ret) {
>>> -            dev_err(dev, "MSI-X configuration failed\n");
>>> +    if (!skip_core_init) {
>>> +        ret = pci_epf_test_core_init(epf);
>>> +        if (ret)
>>>               return ret;
>>> -        }
>>>       }
>>>       if (linkup_notifier) {
>>
>> This could as well be moved to pci_epf_test_core_init().
> Yes, but I would like to keep only the code that touches hardware in 
> pci_epf_test_core_init()
> to minimize the time it takes to execute it. Is there any strong reason 
> to move it? if not,
> I would prefer to leave it here in this function itself.

There is no point in scheduling a work to check for commands from host 
when the EP itself is not initialized.

Thanks
Kishon
Vidya Sagar Jan. 3, 2020, 9:40 a.m. UTC | #4
On 12/5/2019 4:52 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 01/12/19 7:59 pm, Vidya Sagar wrote:
>> On 11/27/2019 2:50 PM, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>>>> Add support to defer core initialization and to receive a notifier
>>>> when core is ready to accommodate platforms where core is not for
>>>> initialization untile reference clock from host is available.
>>>>
>>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>>> ---
>>>>   drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>>>>   1 file changed, 77 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
>>>> index bddff15052cc..068024fab544 100644
>>>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>>>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>>>> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
>>>>                  msecs_to_jiffies(1));
>>>>   }
>>>> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
>>>> -                 void *data)
>>>> -{
>>>> -    struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>>> -    struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> -
>>>> -    queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>>> -               msecs_to_jiffies(1));
>>>> -
>>>> -    return NOTIFY_OK;
>>>> -}
>>>> -
>>>>   static void pci_epf_test_unbind(struct pci_epf *epf)
>>>>   {
>>>>       struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>>>>       return 0;
>>>>   }
>>>> +static int pci_epf_test_core_init(struct pci_epf *epf)
>>>> +{
>>>> +    struct pci_epf_header *header = epf->header;
>>>> +    const struct pci_epc_features *epc_features;
>>>> +    struct pci_epc *epc = epf->epc;
>>>> +    struct device *dev = &epf->dev;
>>>> +    bool msix_capable = false;
>>>> +    bool msi_capable = true;
>>>> +    int ret;
>>>> +
>>>> +    epc_features = pci_epc_get_features(epc, epf->func_no);
>>>> +    if (epc_features) {
>>>> +        msix_capable = epc_features->msix_capable;
>>>> +        msi_capable = epc_features->msi_capable;
>>>> +    }
>>>> +
>>>> +    ret = pci_epc_write_header(epc, epf->func_no, header);
>>>> +    if (ret) {
>>>> +        dev_err(dev, "Configuration header write failed\n");
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    ret = pci_epf_test_set_bar(epf);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    if (msi_capable) {
>>>> +        ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
>>>> +        if (ret) {
>>>> +            dev_err(dev, "MSI configuration failed\n");
>>>> +            return ret;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    if (msix_capable) {
>>>> +        ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
>>>> +        if (ret) {
>>>> +            dev_err(dev, "MSI-X configuration failed\n");
>>>> +            return ret;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
>>>> +                 void *data)
>>>> +{
>>>> +    struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>>> +    struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> +    int ret;
>>>> +
>>>> +    switch (val) {
>>>> +    case CORE_INIT:
>>>> +        ret = pci_epf_test_core_init(epf);
>>>> +        if (ret)
>>>> +            return NOTIFY_BAD;
>>>> +        break;
>>>> +
>>>> +    case LINK_UP:
>>>> +        queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>>> +                   msecs_to_jiffies(1));
>>>> +        break;
>>>> +
>>>> +    default:
>>>> +        dev_err(&epf->dev, "Invalid EPF test notifier event\n");
>>>> +        return NOTIFY_BAD;
>>>> +    }
>>>> +
>>>> +    return NOTIFY_OK;
>>>> +}
>>>> +
>>>>   static int pci_epf_test_alloc_space(struct pci_epf *epf)
>>>>   {
>>>>       struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> @@ -496,12 +556,11 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>>>   {
>>>>       int ret;
>>>>       struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> -    struct pci_epf_header *header = epf->header;
>>>>       const struct pci_epc_features *epc_features;
>>>>       enum pci_barno test_reg_bar = BAR_0;
>>>>       struct pci_epc *epc = epf->epc;
>>>> -    struct device *dev = &epf->dev;
>>>>       bool linkup_notifier = false;
>>>> +    bool skip_core_init = false;
>>>>       bool msix_capable = false;
>>>>       bool msi_capable = true;
>>>> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>>>       epc_features = pci_epc_get_features(epc, epf->func_no);
>>>>       if (epc_features) {
>>>>           linkup_notifier = epc_features->linkup_notifier;
>>>> +        skip_core_init = epc_features->skip_core_init;
>>>>           msix_capable = epc_features->msix_capable;
>>>>           msi_capable = epc_features->msi_capable;
>>>
>>> Are these used anywhere in this function?
>> Nope. I'll remove them.
>>
>>>>           test_reg_bar = pci_epc_get_first_free_bar(epc_features);
>>>> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>>>       epf_test->test_reg_bar = test_reg_bar;
>>>>       epf_test->epc_features = epc_features;
>>>> -    ret = pci_epc_write_header(epc, epf->func_no, header);
>>>> -    if (ret) {
>>>> -        dev_err(dev, "Configuration header write failed\n");
>>>> -        return ret;
>>>> -    }
>>>> -
>>>>       ret = pci_epf_test_alloc_space(epf);
>>>>       if (ret)
>>>>           return ret;
>>>> -    ret = pci_epf_test_set_bar(epf);
>>>> -    if (ret)
>>>> -        return ret;
>>>> -
>>>> -    if (msi_capable) {
>>>> -        ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
>>>> -        if (ret) {
>>>> -            dev_err(dev, "MSI configuration failed\n");
>>>> -            return ret;
>>>> -        }
>>>> -    }
>>>> -
>>>> -    if (msix_capable) {
>>>> -        ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
>>>> -        if (ret) {
>>>> -            dev_err(dev, "MSI-X configuration failed\n");
>>>> +    if (!skip_core_init) {
>>>> +        ret = pci_epf_test_core_init(epf);
>>>> +        if (ret)
>>>>               return ret;
>>>> -        }
>>>>       }
>>>>       if (linkup_notifier) {
>>>
>>> This could as well be moved to pci_epf_test_core_init().
>> Yes, but I would like to keep only the code that touches hardware in pci_epf_test_core_init()
>> to minimize the time it takes to execute it. Is there any strong reason to move it? if not,
>> I would prefer to leave it here in this function itself.
> 
> There is no point in scheduling a work to check for commands from host when the EP itself is not initialized.
True. But, since this is more of preparatory work, I thought we should just have it done here itself.
Main reason being, once PERST is perceived, endpoint can't take too much initializing its core. So, I want to
keep that part as minimalistic as possible.

- Vidya Sagar

> 
> Thanks
> Kishon

Patch
diff mbox series

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index bddff15052cc..068024fab544 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -360,18 +360,6 @@  static void pci_epf_test_cmd_handler(struct work_struct *work)
 			   msecs_to_jiffies(1));
 }
 
-static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
-				 void *data)
-{
-	struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
-	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
-
-	queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
-			   msecs_to_jiffies(1));
-
-	return NOTIFY_OK;
-}
-
 static void pci_epf_test_unbind(struct pci_epf *epf)
 {
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
@@ -428,6 +416,78 @@  static int pci_epf_test_set_bar(struct pci_epf *epf)
 	return 0;
 }
 
+static int pci_epf_test_core_init(struct pci_epf *epf)
+{
+	struct pci_epf_header *header = epf->header;
+	const struct pci_epc_features *epc_features;
+	struct pci_epc *epc = epf->epc;
+	struct device *dev = &epf->dev;
+	bool msix_capable = false;
+	bool msi_capable = true;
+	int ret;
+
+	epc_features = pci_epc_get_features(epc, epf->func_no);
+	if (epc_features) {
+		msix_capable = epc_features->msix_capable;
+		msi_capable = epc_features->msi_capable;
+	}
+
+	ret = pci_epc_write_header(epc, epf->func_no, header);
+	if (ret) {
+		dev_err(dev, "Configuration header write failed\n");
+		return ret;
+	}
+
+	ret = pci_epf_test_set_bar(epf);
+	if (ret)
+		return ret;
+
+	if (msi_capable) {
+		ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
+		if (ret) {
+			dev_err(dev, "MSI configuration failed\n");
+			return ret;
+		}
+	}
+
+	if (msix_capable) {
+		ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
+		if (ret) {
+			dev_err(dev, "MSI-X configuration failed\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
+				 void *data)
+{
+	struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
+	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
+	int ret;
+
+	switch (val) {
+	case CORE_INIT:
+		ret = pci_epf_test_core_init(epf);
+		if (ret)
+			return NOTIFY_BAD;
+		break;
+
+	case LINK_UP:
+		queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
+				   msecs_to_jiffies(1));
+		break;
+
+	default:
+		dev_err(&epf->dev, "Invalid EPF test notifier event\n");
+		return NOTIFY_BAD;
+	}
+
+	return NOTIFY_OK;
+}
+
 static int pci_epf_test_alloc_space(struct pci_epf *epf)
 {
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
@@ -496,12 +556,11 @@  static int pci_epf_test_bind(struct pci_epf *epf)
 {
 	int ret;
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
-	struct pci_epf_header *header = epf->header;
 	const struct pci_epc_features *epc_features;
 	enum pci_barno test_reg_bar = BAR_0;
 	struct pci_epc *epc = epf->epc;
-	struct device *dev = &epf->dev;
 	bool linkup_notifier = false;
+	bool skip_core_init = false;
 	bool msix_capable = false;
 	bool msi_capable = true;
 
@@ -511,6 +570,7 @@  static int pci_epf_test_bind(struct pci_epf *epf)
 	epc_features = pci_epc_get_features(epc, epf->func_no);
 	if (epc_features) {
 		linkup_notifier = epc_features->linkup_notifier;
+		skip_core_init = epc_features->skip_core_init;
 		msix_capable = epc_features->msix_capable;
 		msi_capable = epc_features->msi_capable;
 		test_reg_bar = pci_epc_get_first_free_bar(epc_features);
@@ -520,34 +580,14 @@  static int pci_epf_test_bind(struct pci_epf *epf)
 	epf_test->test_reg_bar = test_reg_bar;
 	epf_test->epc_features = epc_features;
 
-	ret = pci_epc_write_header(epc, epf->func_no, header);
-	if (ret) {
-		dev_err(dev, "Configuration header write failed\n");
-		return ret;
-	}
-
 	ret = pci_epf_test_alloc_space(epf);
 	if (ret)
 		return ret;
 
-	ret = pci_epf_test_set_bar(epf);
-	if (ret)
-		return ret;
-
-	if (msi_capable) {
-		ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
-		if (ret) {
-			dev_err(dev, "MSI configuration failed\n");
-			return ret;
-		}
-	}
-
-	if (msix_capable) {
-		ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
-		if (ret) {
-			dev_err(dev, "MSI-X configuration failed\n");
+	if (!skip_core_init) {
+		ret = pci_epf_test_core_init(epf);
+		if (ret)
 			return ret;
-		}
 	}
 
 	if (linkup_notifier) {