2022-08-11 10:10:48

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH 2/2] PCI: endpoint: Use link_up() callback in place of LINK_UP notifier

As a part of the transition towards callback mechanism for signalling the
events from EPC to EPF, let's use the link_up() callback in the place of
the LINK_UP notifier. This also removes the notifier support completely
from the PCI endpoint framework.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-test.c | 33 ++++++-------------
drivers/pci/endpoint/pci-epc-core.c | 8 +++--
include/linux/pci-epc.h | 8 -----
include/linux/pci-epf.h | 8 ++---
4 files changed, 18 insertions(+), 39 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 069943b57029..37454538c000 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -731,30 +731,21 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
return 0;
}

-static const struct pci_epc_event_ops pci_epf_test_event_ops = {
- .core_init = pci_epf_test_core_init,
-};
-
-static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
- void *data)
+int pci_epf_test_link_up(struct pci_epf *epf)
{
- struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
struct pci_epf_test *epf_test = epf_get_drvdata(epf);

- switch (val) {
- 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;
- }
+ queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
+ msecs_to_jiffies(1));

- return NOTIFY_OK;
+ return 0;
}

+static const struct pci_epc_event_ops pci_epf_test_event_ops = {
+ .core_init = pci_epf_test_core_init,
+ .link_up = pci_epf_test_link_up,
+};
+
static int pci_epf_test_alloc_space(struct pci_epf *epf)
{
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
@@ -881,12 +872,8 @@ static int pci_epf_test_bind(struct pci_epf *epf)
if (ret)
epf_test->dma_supported = false;

- if (linkup_notifier || core_init_notifier) {
- epf->nb.notifier_call = pci_epf_test_notifier;
- pci_epc_register_notifier(epc, &epf->nb);
- } else {
+ if (!linkup_notifier && !core_init_notifier)
queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
- }

return 0;
}
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 805f3d64f93b..a51ba9f158c3 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -690,10 +690,15 @@ EXPORT_SYMBOL_GPL(pci_epc_remove_epf);
*/
void pci_epc_linkup(struct pci_epc *epc)
{
+ struct pci_epf *epf;
+
if (!epc || IS_ERR(epc))
return;

- atomic_notifier_call_chain(&epc->notifier, LINK_UP, NULL);
+ list_for_each_entry(epf, &epc->pci_epf, list) {
+ if (epf->event_ops->link_up)
+ epf->event_ops->link_up(epf);
+ }
}
EXPORT_SYMBOL_GPL(pci_epc_linkup);

@@ -779,7 +784,6 @@ __pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,

mutex_init(&epc->lock);
INIT_LIST_HEAD(&epc->pci_epf);
- ATOMIC_INIT_NOTIFIER_HEAD(&epc->notifier);

device_initialize(&epc->dev);
epc->dev.class = pci_epc_class;
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index a48778e1a4ee..05604fe0ede0 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -134,7 +134,6 @@ struct pci_epc_mem {
* @group: configfs group representing the PCI EPC device
* @lock: mutex to protect pci_epc ops
* @function_num_map: bitmap to manage physical function number
- * @notifier: used to notify EPF of any EPC events (like linkup)
*/
struct pci_epc {
struct device dev;
@@ -149,7 +148,6 @@ struct pci_epc {
/* mutex to protect against concurrent access of EP controller */
struct mutex lock;
unsigned long function_num_map;
- struct atomic_notifier_head notifier;
};

/**
@@ -192,12 +190,6 @@ static inline void *epc_get_drvdata(struct pci_epc *epc)
return dev_get_drvdata(&epc->dev);
}

-static inline int
-pci_epc_register_notifier(struct pci_epc *epc, struct notifier_block *nb)
-{
- return atomic_notifier_chain_register(&epc->notifier, nb);
-}
-
struct pci_epc *
__devm_pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
struct module *owner);
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index a06f3b4c8bee..bc613f0df7e3 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -17,10 +17,6 @@
struct pci_epf;
enum pci_epc_interface_type;

-enum pci_notify_event {
- LINK_UP,
-};
-
enum pci_barno {
NO_BAR = -1,
BAR_0,
@@ -74,9 +70,11 @@ struct pci_epf_ops {
/**
* struct pci_epf_event_ops - Callbacks for capturing the EPC events
* @core_init: Callback for the EPC initialization complete event
+ * @link_up: Callback for the EPC link up event
*/
struct pci_epc_event_ops {
int (*core_init)(struct pci_epf *epf);
+ int (*link_up)(struct pci_epf *epf);
};

/**
@@ -135,7 +133,6 @@ struct pci_epf_bar {
* @driver: the EPF driver to which this EPF device is bound
* @id: Pointer to the EPF device ID
* @list: to add pci_epf as a list of PCI endpoint functions to pci_epc
- * @nb: notifier block to notify EPF of any EPC events (like linkup)
* @lock: mutex to protect pci_epf_ops
* @sec_epc: the secondary EPC device to which this EPF device is bound
* @sec_epc_list: to add pci_epf as list of PCI endpoint functions to secondary
@@ -164,7 +161,6 @@ struct pci_epf {
struct pci_epf_driver *driver;
const struct pci_epf_device_id *id;
struct list_head list;
- struct notifier_block nb;
/* mutex to protect against concurrent access of pci_epf_ops */
struct mutex lock;

--
2.25.1


2022-08-25 10:25:28

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: endpoint: Use link_up() callback in place of LINK_UP notifier

Hi Mani,

On 11/08/22 15:12, Manivannan Sadhasivam wrote:
> As a part of the transition towards callback mechanism for signalling the
> events from EPC to EPF, let's use the link_up() callback in the place of
> the LINK_UP notifier. This also removes the notifier support completely
> from the PCI endpoint framework.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 33 ++++++-------------
> drivers/pci/endpoint/pci-epc-core.c | 8 +++--
> include/linux/pci-epc.h | 8 -----
> include/linux/pci-epf.h | 8 ++---
> 4 files changed, 18 insertions(+), 39 deletions(-)
>

.
.
<snip>
.
.
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 805f3d64f93b..a51ba9f158c3 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -690,10 +690,15 @@ EXPORT_SYMBOL_GPL(pci_epc_remove_epf);
> */
> void pci_epc_linkup(struct pci_epc *epc)
> {
> + struct pci_epf *epf;
> +
> if (!epc || IS_ERR(epc))
> return;
>
> - atomic_notifier_call_chain(&epc->notifier, LINK_UP, NULL);
> + list_for_each_entry(epf, &epc->pci_epf, list) {
> + if (epf->event_ops->link_up)
> + epf->event_ops->link_up(epf);
> + }

how do you ensure there is no race while invoking the call back
functions during add/remove of epf?

Thanks,
Kishon

2022-08-31 08:39:38

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: endpoint: Use link_up() callback in place of LINK_UP notifier

On Thu, Aug 25, 2022 at 03:47:10PM +0530, Kishon Vijay Abraham I wrote:
> Hi Mani,
>
> On 11/08/22 15:12, Manivannan Sadhasivam wrote:
> > As a part of the transition towards callback mechanism for signalling the
> > events from EPC to EPF, let's use the link_up() callback in the place of
> > the LINK_UP notifier. This also removes the notifier support completely
> > from the PCI endpoint framework.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/pci/endpoint/functions/pci-epf-test.c | 33 ++++++-------------
> > drivers/pci/endpoint/pci-epc-core.c | 8 +++--
> > include/linux/pci-epc.h | 8 -----
> > include/linux/pci-epf.h | 8 ++---
> > 4 files changed, 18 insertions(+), 39 deletions(-)
> >
>
> .
> .
> <snip>
> .
> .
> > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> > index 805f3d64f93b..a51ba9f158c3 100644
> > --- a/drivers/pci/endpoint/pci-epc-core.c
> > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > @@ -690,10 +690,15 @@ EXPORT_SYMBOL_GPL(pci_epc_remove_epf);
> > */
> > void pci_epc_linkup(struct pci_epc *epc)
> > {
> > + struct pci_epf *epf;
> > +
> > if (!epc || IS_ERR(epc))
> > return;
> >
> > - atomic_notifier_call_chain(&epc->notifier, LINK_UP, NULL);
> > + list_for_each_entry(epf, &epc->pci_epf, list) {
> > + if (epf->event_ops->link_up)
> > + epf->event_ops->link_up(epf);
> > + }
>
> how do you ensure there is no race while invoking the call back
> functions during add/remove of epf?
>

Good catch! These should be guarded by epf->lock.

Thanks,
Mani

> Thanks,
> Kishon

--
மணிவண்ணன் சதாசிவம்

2022-09-09 10:47:47

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: endpoint: Use link_up() callback in place of LINK_UP notifier

On Wed, Aug 31, 2022 at 01:50:25PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Aug 25, 2022 at 03:47:10PM +0530, Kishon Vijay Abraham I wrote:
> > Hi Mani,
> >
> > On 11/08/22 15:12, Manivannan Sadhasivam wrote:
> > > As a part of the transition towards callback mechanism for signalling the
> > > events from EPC to EPF, let's use the link_up() callback in the place of
> > > the LINK_UP notifier. This also removes the notifier support completely
> > > from the PCI endpoint framework.
> > >
> > > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > > ---
> > > drivers/pci/endpoint/functions/pci-epf-test.c | 33 ++++++-------------
> > > drivers/pci/endpoint/pci-epc-core.c | 8 +++--
> > > include/linux/pci-epc.h | 8 -----
> > > include/linux/pci-epf.h | 8 ++---
> > > 4 files changed, 18 insertions(+), 39 deletions(-)
> > >
> >
> > .
> > .
> > <snip>
> > .
> > .
> > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> > > index 805f3d64f93b..a51ba9f158c3 100644
> > > --- a/drivers/pci/endpoint/pci-epc-core.c
> > > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > > @@ -690,10 +690,15 @@ EXPORT_SYMBOL_GPL(pci_epc_remove_epf);
> > > */
> > > void pci_epc_linkup(struct pci_epc *epc)
> > > {
> > > + struct pci_epf *epf;
> > > +
> > > if (!epc || IS_ERR(epc))
> > > return;
> > >
> > > - atomic_notifier_call_chain(&epc->notifier, LINK_UP, NULL);
> > > + list_for_each_entry(epf, &epc->pci_epf, list) {
> > > + if (epf->event_ops->link_up)
> > > + epf->event_ops->link_up(epf);
> > > + }
> >
> > how do you ensure there is no race while invoking the call back
> > functions during add/remove of epf?
> >
>
> Good catch! These should be guarded by epf->lock.

I shall wait for v2 then and mark this one as "Changes requested".

Lorenzo