Messages in this thread | | | Subject | Re: [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization | From | Vidya Sagar <> | Date | Fri, 3 Jan 2020 15:10:39 +0530 |
| |
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
| |