On 8/16/2022 7:45 PM, Lorenzo Pieralisi wrote:
External email: Use caution opening links or attachments
On Tue, Aug 02, 2022 at 07:37:38PM +0530, Manivannan Sadhasivam wrote:
On Tue, Aug 02, 2022 at 12:54:37PM +0530, Manivannan Sadhasivam wrote:
On Mon, Aug 01, 2022 at 02:27:14PM -0600, Rob Herring wrote:
On Sat, Jul 30, 2022 at 8:50 AM Manivannan Sadhasivam
<manivannan.sadhasivam@xxxxxxxxxx> wrote:
On Fri, Jul 29, 2022 at 05:44:04PM -0500, Bjorn Helgaas wrote:
[+cc Xiaowei (author of 6bfc9c3a2c70), Hou (author of 8bcca2658558)]
On Thu, Jul 28, 2022 at 05:56:28PM +0530, Vidya Sagar wrote:
On 7/28/2022 3:44 AM, Bjorn Helgaas wrote:
On Wed, Jun 22, 2022 at 09:31:33AM +0530, Vidya Sagar wrote:
Platforms that cannot support their core initialization without the
reference clock from the host, implement the feature 'core_init_notifier'
to indicate the DesignWare sub-system about when their core is getting
initialized. Any accesses to the core (Ex:- DBI) would result in system
hang in such systems (Ex:- tegra194). This patch moves any access to the
core to dw_pcie_ep_init_complete() API which is effectively called only
after the core initialization.
6) What's going on with the CORE_INIT and LINK_UP notifiers?
dw_pcie_ep_init_notify() is only called by qcom and tegra.
dw_pcie_ep_linkup() is only called by dra7xx, qcom, and tegra.
As far as I can tell, nobody at all registers to handle those
events except a test. I think it's pointless to have that code
if nobody uses it.
I have submitted an actual driver that makes use of these notifiers:
https://lore.kernel.org/lkml/20220502060611.58987-9-manivannan.sadhasivam@xxxxxxxxxx/
Notifiers aren't the best interface in the kernel. I think they are
best used if there's no real linkage between the sender and receiver.
For an EPC and EPF that's a fixed interface, so define a proper
interface.
Fair point! The use of notifiers also suffer from an issue where the notifier
chain in EPC is atomic but the EPF calls some of the functions like
pci_epc_write_header() could potentially sleep.
I'll try to come up with an interface.
I thought about using a new set of callbacks that define the EPC events and
have the EPF drivers populate them during probe time. Like below,
```
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index e03c57129ed5..45247802d6f7 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -74,6 +74,20 @@ struct pci_epf_ops {
struct config_group *group);
};
+/**
+ * struct pci_epf_events - Callbacks for capturing the EPC events
+ * @init_complete: Callback for the EPC initialization complete event
+ * @link_up: Callback for the EPC link up event
+ */
+struct pci_epc_events {
+ void (*init_complete)(struct pci_epf *epf);
+ void (*link_up)(struct pci_epf *epf);
+};
+
/**
* struct pci_epf_driver - represents the PCI EPF driver
* @probe: ops to perform when a new EPF device has been bound to the EPF driver
@@ -172,6 +186,7 @@ struct pci_epf {
unsigned int is_vf;
unsigned long vfunction_num_map;
struct list_head pci_vepf;
+ struct pci_epc_events *events;
};
/**
```
When each of the event is received by the EPC driver, it will use the EPC API
to call the relevant event callback for _each_ EPF. Like below:
```
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 6ad9b38b63a9..4b0b30b91403 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -724,10 +724,15 @@ EXPORT_SYMBOL_GPL(pci_epc_linkdown);
*/
void pci_epc_init_notify(struct pci_epc *epc)
{
+ struct pci_epf *epf;
+
if (!epc || IS_ERR(epc))
return;
- blocking_notifier_call_chain(&epc->notifier, CORE_INIT, NULL);
+ list_for_each_entry(epf, &epc->pci_epf, list) {
+ if (epf->events->init_complete)
+ epf->events->init_complete(epf);
+ }
}
EXPORT_SYMBOL_GPL(pci_epc_init_notify);
```
Does this look good to you? I can spin up an RFC series, but wanted to check the
interface design beforehand.
I am resuming patch reviews, have you posted a follow up ?
Just to understand where we are with this thread and start reviewing
from there, I will update patchwork accordingly (you should add
a Link: to this thread anyway in the new series).
Manivannan posted a new patch set at
https://patchwork.kernel.org/project/linux-pci/list/?series=666818 to
address concerns with the notifier mechanism.
I would be sending a follow-up patch for the current patch soon.
Thanks,
Vidya Sagar
Thanks,
Lorenzo
Thanks,
Mani
Thanks,
Mani
Rob
--
மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்