diff mbox series

[1/3] PCI: dwc: ep: Add dw_pcie_ep_deinit_notify()

Message ID 20240528130035.1472871-6-cassel@kernel.org (mailing list archive)
State Superseded
Delegated to: Krzysztof WilczyƄski
Headers show
Series Make pci/endpoint branch build | expand

Commit Message

Niklas Cassel May 28, 2024, 1 p.m. UTC
Add a DWC specific wrapper function (dw_pcie_ep_deinit_notify()) around
pci_epc_deinit_notify(), similar to how we have a wrapper function
(dw_pcie_ep_init_notify()) around pci_epc_init_notify().

This will allow the DWC glue drivers to use the same API layer for init
and deinit notification.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 13 +++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h    |  5 +++++
 2 files changed, 18 insertions(+)

Comments

Bjorn Helgaas May 28, 2024, 3:55 p.m. UTC | #1
On Tue, May 28, 2024 at 03:00:37PM +0200, Niklas Cassel wrote:
> Add a DWC specific wrapper function (dw_pcie_ep_deinit_notify()) around
> pci_epc_deinit_notify(), similar to how we have a wrapper function
> (dw_pcie_ep_init_notify()) around pci_epc_init_notify().
> 
> This will allow the DWC glue drivers to use the same API layer for init
> and deinit notification.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 13 +++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h    |  5 +++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2063cf2049e5..3c9079651dff 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -39,6 +39,19 @@ void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify);
>  
> +/**
> + * dw_pcie_ep_deinit_notify - Notify EPF drivers about EPC deinitialization
> + *			      complete
> + * @ep: DWC EP device
> + */
> +void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep)
> +{
> +	struct pci_epc *epc = ep->epc;
> +
> +	pci_epc_deinit_notify(epc);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit_notify);

What is the value of this wrapper?  

I see that dw_pcie_ep_deinit_notify() would be parallel to
dw_pcie_ep_init_notify() and dw_pcie_ep_linkup(), but none of these
has any DWC-specific content other than the fact that
pcie-designware.h provides stubs for the non-CONFIG_PCIE_DW_EP case.

What if we added stubs to pci-epc.h pci_epc_init_notify(),
pci_epc_deinit_notify(), pci_epc_linkup(), and pci_epc_linkdown() for
the non-CONFIG_PCI_ENDPOINT case instead?  Then we might be able to
drop all these DWC-specific wrappers.

>  /**
>   * dw_pcie_ep_get_func_from_ep - Get the struct dw_pcie_ep_func corresponding to
>   *				 the endpoint function
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index f8e5431a207b..dc63f764b8ba 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -672,6 +672,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep);
>  int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
>  void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
>  void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep);
>  void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep);
>  int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
>  int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> @@ -706,6 +707,10 @@ static inline void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
>  {
>  }
>  
> +static inline void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep)
> +{
> +}
> +
>  static inline void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
>  {
>  }
> -- 
> 2.45.1
>
Niklas Cassel May 28, 2024, 7:17 p.m. UTC | #2
On Tue, May 28, 2024 at 10:55:34AM -0500, Bjorn Helgaas wrote:
> On Tue, May 28, 2024 at 03:00:37PM +0200, Niklas Cassel wrote:
> > Add a DWC specific wrapper function (dw_pcie_ep_deinit_notify()) around
> > pci_epc_deinit_notify(), similar to how we have a wrapper function
> > (dw_pcie_ep_init_notify()) around pci_epc_init_notify().
> > 
> > This will allow the DWC glue drivers to use the same API layer for init
> > and deinit notification.
> > 
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-ep.c | 13 +++++++++++++
> >  drivers/pci/controller/dwc/pcie-designware.h    |  5 +++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 2063cf2049e5..3c9079651dff 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -39,6 +39,19 @@ void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> >  }
> >  EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify);
> >  
> > +/**
> > + * dw_pcie_ep_deinit_notify - Notify EPF drivers about EPC deinitialization
> > + *			      complete
> > + * @ep: DWC EP device
> > + */
> > +void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep)
> > +{
> > +	struct pci_epc *epc = ep->epc;
> > +
> > +	pci_epc_deinit_notify(epc);
> > +}
> > +EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit_notify);
> 
> What is the value of this wrapper?  
> 
> I see that dw_pcie_ep_deinit_notify() would be parallel to
> dw_pcie_ep_init_notify() and dw_pcie_ep_linkup(), but none of these
> has any DWC-specific content other than the fact that
> pcie-designware.h provides stubs for the non-CONFIG_PCIE_DW_EP case.

Exactly what you are saying, consistency with the existing design.

To me, it seems a bit weird to use:
dw_pcie_ep_init_notify() to notify init completion, and then to use
pci_epc_deinit_notify() to notify deinit completion.

deinit notify callback should basically undo what the init notify callback
did, so it would make sense that the naming of the API calls are similar.


> 
> What if we added stubs to pci-epc.h pci_epc_init_notify(),
> pci_epc_deinit_notify(), pci_epc_linkup(), and pci_epc_linkdown() for
> the non-CONFIG_PCI_ENDPOINT case instead?  Then we might be able to
> drop all these DWC-specific wrappers.

The PCI endpoint subsystem currently does not provide any stubs at all,
so that would be a bigger change compared to this small patch.
(And considering that the pci/endpoint branch does not build, I opted
for the smaller patch.)

Your suggestion would of course work as well, but if we go that route,
then we should probably add stubs for all functions in both
include/linux/pci-epc.h and include/linux/pci-epf.h.
As long as the DWC glue drivers use the same "API layer" for init and
deinit notification, I'm happy :)


Kind regards,
Niklas
Bjorn Helgaas May 28, 2024, 7:55 p.m. UTC | #3
On Tue, May 28, 2024 at 09:17:40PM +0200, Niklas Cassel wrote:
> On Tue, May 28, 2024 at 10:55:34AM -0500, Bjorn Helgaas wrote:
> > On Tue, May 28, 2024 at 03:00:37PM +0200, Niklas Cassel wrote:
> > > Add a DWC specific wrapper function (dw_pcie_ep_deinit_notify()) around
> > > pci_epc_deinit_notify(), similar to how we have a wrapper function
> > > (dw_pcie_ep_init_notify()) around pci_epc_init_notify().
> > > 
> > > This will allow the DWC glue drivers to use the same API layer for init
> > > and deinit notification.
> > > 
> > > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware-ep.c | 13 +++++++++++++
> > >  drivers/pci/controller/dwc/pcie-designware.h    |  5 +++++
> > >  2 files changed, 18 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index 2063cf2049e5..3c9079651dff 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -39,6 +39,19 @@ void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> > >  }
> > >  EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify);
> > >  
> > > +/**
> > > + * dw_pcie_ep_deinit_notify - Notify EPF drivers about EPC deinitialization
> > > + *			      complete
> > > + * @ep: DWC EP device
> > > + */
> > > +void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep)
> > > +{
> > > +	struct pci_epc *epc = ep->epc;
> > > +
> > > +	pci_epc_deinit_notify(epc);
> > > +}
> > > +EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit_notify);
> > 
> > What is the value of this wrapper?  
> > 
> > I see that dw_pcie_ep_deinit_notify() would be parallel to
> > dw_pcie_ep_init_notify() and dw_pcie_ep_linkup(), but none of these
> > has any DWC-specific content other than the fact that
> > pcie-designware.h provides stubs for the non-CONFIG_PCIE_DW_EP case.
> 
> Exactly what you are saying, consistency with the existing design.
> 
> To me, it seems a bit weird to use:
> dw_pcie_ep_init_notify() to notify init completion, and then to use
> pci_epc_deinit_notify() to notify deinit completion.
> 
> deinit notify callback should basically undo what the init notify callback
> did, so it would make sense that the naming of the API calls are similar.
> 
> > What if we added stubs to pci-epc.h pci_epc_init_notify(),
> > pci_epc_deinit_notify(), pci_epc_linkup(), and pci_epc_linkdown() for
> > the non-CONFIG_PCI_ENDPOINT case instead?  Then we might be able to
> > drop all these DWC-specific wrappers.
> 
> The PCI endpoint subsystem currently does not provide any stubs at all,
> so that would be a bigger change compared to this small patch.
> (And considering that the pci/endpoint branch does not build, I opted
> for the smaller patch.)

> Your suggestion would of course work as well, but if we go that route,
> then we should probably add stubs for all functions in both
> include/linux/pci-epc.h and include/linux/pci-epf.h.
> As long as the DWC glue drivers use the same "API layer" for init and
> deinit notification, I'm happy :)

The cadence, rcar, and rockchip drivers use pci_epc_init_notify() with
no wrapper.

A bunch of DWC-based drivers (artpec6, dra7xx, imx6, keembay, ks, ls,
qcom, rcar_gen4, etc) use the dw_pcie_ep_init_notify() wrapper.

ls and qcom even use *both*: pci_epc_linkdown() but
dw_pcie_ep_linkup().

Personally I would drop the dw_*() wrappers.  It's a bigger patch but
not any more complicated, and the result is consistency across both
DWC and the non-DWC drivers.

I don't know if we need to add stubs for *all* the functions.  I'd
probably defer that until we trip over them.

Bjorn
Niklas Cassel May 29, 2024, 7:35 a.m. UTC | #4
On Tue, May 28, 2024 at 02:55:39PM -0500, Bjorn Helgaas wrote:
> On Tue, May 28, 2024 at 09:17:40PM +0200, Niklas Cassel wrote:
> > 
> > > What if we added stubs to pci-epc.h pci_epc_init_notify(),
> > > pci_epc_deinit_notify(), pci_epc_linkup(), and pci_epc_linkdown() for
> > > the non-CONFIG_PCI_ENDPOINT case instead?  Then we might be able to
> > > drop all these DWC-specific wrappers.
> > 
> > The PCI endpoint subsystem currently does not provide any stubs at all,
> > so that would be a bigger change compared to this small patch.
> > (And considering that the pci/endpoint branch does not build, I opted
> > for the smaller patch.)
> 
> > Your suggestion would of course work as well, but if we go that route,
> > then we should probably add stubs for all functions in both
> > include/linux/pci-epc.h and include/linux/pci-epf.h.
> > As long as the DWC glue drivers use the same "API layer" for init and
> > deinit notification, I'm happy :)
> 
> The cadence, rcar, and rockchip drivers use pci_epc_init_notify() with
> no wrapper.
> 
> A bunch of DWC-based drivers (artpec6, dra7xx, imx6, keembay, ks, ls,
> qcom, rcar_gen4, etc) use the dw_pcie_ep_init_notify() wrapper.
> 
> ls and qcom even use *both*: pci_epc_linkdown() but
> dw_pcie_ep_linkup().
> 
> Personally I would drop the dw_*() wrappers.  It's a bigger patch but
> not any more complicated, and the result is consistency across both
> DWC and the non-DWC drivers.
> 
> I don't know if we need to add stubs for *all* the functions.  I'd
> probably defer that until we trip over them.

Hello Mani,

considering that:

1) Bjorn dropped the commit:
"PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers"
which means that you will need resubmit your patch.

2) Any changes I would do would conflict with your patch.
(It probably makes most sense put your patch as the final patch in a series.)

3) You are the PCI endpoint maintainer, so you are most suited to decide
which functions to stub (if any).

4) Your patch only affects tegra and qcom, and I don't have the hardware
for either, so I wouldn't be able to test.

Thus, I do not intend to respin this series.
I hope that is okay with you.


Kind regards,
Niklas
Manivannan Sadhasivam May 29, 2024, 2:16 p.m. UTC | #5
Hi Niklas,

On Wed, May 29, 2024 at 09:35:44AM +0200, Niklas Cassel wrote:
> On Tue, May 28, 2024 at 02:55:39PM -0500, Bjorn Helgaas wrote:
> > On Tue, May 28, 2024 at 09:17:40PM +0200, Niklas Cassel wrote:
> > > 
> > > > What if we added stubs to pci-epc.h pci_epc_init_notify(),
> > > > pci_epc_deinit_notify(), pci_epc_linkup(), and pci_epc_linkdown() for
> > > > the non-CONFIG_PCI_ENDPOINT case instead?  Then we might be able to
> > > > drop all these DWC-specific wrappers.
> > > 
> > > The PCI endpoint subsystem currently does not provide any stubs at all,
> > > so that would be a bigger change compared to this small patch.
> > > (And considering that the pci/endpoint branch does not build, I opted
> > > for the smaller patch.)
> > 
> > > Your suggestion would of course work as well, but if we go that route,
> > > then we should probably add stubs for all functions in both
> > > include/linux/pci-epc.h and include/linux/pci-epf.h.
> > > As long as the DWC glue drivers use the same "API layer" for init and
> > > deinit notification, I'm happy :)
> > 
> > The cadence, rcar, and rockchip drivers use pci_epc_init_notify() with
> > no wrapper.
> > 
> > A bunch of DWC-based drivers (artpec6, dra7xx, imx6, keembay, ks, ls,
> > qcom, rcar_gen4, etc) use the dw_pcie_ep_init_notify() wrapper.
> > 
> > ls and qcom even use *both*: pci_epc_linkdown() but
> > dw_pcie_ep_linkup().
> > 
> > Personally I would drop the dw_*() wrappers.  It's a bigger patch but
> > not any more complicated, and the result is consistency across both
> > DWC and the non-DWC drivers.
> > 
> > I don't know if we need to add stubs for *all* the functions.  I'd
> > probably defer that until we trip over them.
> 
> Hello Mani,
> 
> considering that:
> 
> 1) Bjorn dropped the commit:
> "PCI: endpoint: Introduce 'epc_deinit' event and notify the EPF drivers"
> which means that you will need resubmit your patch.
> 
> 2) Any changes I would do would conflict with your patch.
> (It probably makes most sense put your patch as the final patch in a series.)
> 
> 3) You are the PCI endpoint maintainer, so you are most suited to decide
> which functions to stub (if any).
> 
> 4) Your patch only affects tegra and qcom, and I don't have the hardware
> for either, so I wouldn't be able to test.
> 
> Thus, I do not intend to respin this series.
> I hope that is okay with you.
> 

That's fine. Thanks a lot for stepping in to fix the build issue. I was on
vacation, so couldn't act on your query/series promptly.

Let us conclude the fix here itself as we have more than 1 threads going on.
I did consider adding the stubs to pci-epc.h, but only the deinit API requires
that. So I thought it will look odd to add stub for only one function, that too
for one of the two variants (init/deinit).

So I went ahead with the ugly (yes) conditional for the deinit_notify API.

Ideally, I would've expected both dwc and EP subsystem to provide stubs for the
APIs used by the common driver (host and EP). But since the controller drivers
were using the conditional check to differentiate between host and EP mode,
compilers were smart enough to spot the dead functions and removes them. So
there were no reports so far.

But in this case, the pci_epc_deinit_notify() is called in a separate helper and
hence the issue.

So to conclude, I think it is best if we can add stub just for
pci_epc_deinit_notify() in pci-epc.h and get rid of the dummy
dw_pcie_ep_init_notify() wrapper to make the init/deinit API usage consistent.

Also I do not want to remove the wrapper for dw_pcie_ep_linkup() since its
conterpart dw_pcie_ep_linkdown() is required.

Let me know what you think! I will submit a new series with the left over
patches.

- Mani
Niklas Cassel May 29, 2024, 2:54 p.m. UTC | #6
On Wed, May 29, 2024 at 07:46:14PM +0530, Manivannan Sadhasivam wrote:
> 
> That's fine. Thanks a lot for stepping in to fix the build issue. I was on
> vacation, so couldn't act on your query/series promptly.

Welcome back ;)


> 
> Let us conclude the fix here itself as we have more than 1 threads going on.
> I did consider adding the stubs to pci-epc.h, but only the deinit API requires
> that. So I thought it will look odd to add stub for only one function, that too
> for one of the two variants (init/deinit).
> 
> So I went ahead with the ugly (yes) conditional for the deinit_notify API.
> 
> Ideally, I would've expected both dwc and EP subsystem to provide stubs for the
> APIs used by the common driver (host and EP). But since the controller drivers
> were using the conditional check to differentiate between host and EP mode,
> compilers were smart enough to spot the dead functions and removes them. So
> there were no reports so far.
> 
> But in this case, the pci_epc_deinit_notify() is called in a separate helper and
> hence the issue.
> 
> So to conclude, I think it is best if we can add stub just for
> pci_epc_deinit_notify() in pci-epc.h and get rid of the dummy
> dw_pcie_ep_init_notify() wrapper to make the init/deinit API usage consistent.
> 
> Also I do not want to remove the wrapper for dw_pcie_ep_linkup() since its
> conterpart dw_pcie_ep_linkdown() is required.

I see, sounds good.

However, if we add a stub for pci_epc_deinit_notify(), it makes sense to also
add a stub for pci_epc_init_notify(). (I'm quite sure tegra will fail to link
if you change it from dw_pcie_ep_init_notify() to pci_epc_init_notify()
otherwise.)

We should probably also address Bjorn comment:
"ls and qcom even use *both*: pci_epc_linkdown() but dw_pcie_ep_linkup()."

As far as I can tell, it is only ls (not sure why Bjorn also mentioned qcom):
drivers/pci/controller/dwc/pci-layerscape-ep.c:         pci_epc_linkdown(pci->ep.epc);
But this should probably also be fixed to use dw_pcie_ep_linkdown().


Kind regards,
Niklas
Manivannan Sadhasivam May 29, 2024, 3:40 p.m. UTC | #7
On Wed, May 29, 2024 at 04:54:57PM +0200, Niklas Cassel wrote:
> On Wed, May 29, 2024 at 07:46:14PM +0530, Manivannan Sadhasivam wrote:
> > 
> > That's fine. Thanks a lot for stepping in to fix the build issue. I was on
> > vacation, so couldn't act on your query/series promptly.
> 
> Welcome back ;)
> 
> 
> > 
> > Let us conclude the fix here itself as we have more than 1 threads going on.
> > I did consider adding the stubs to pci-epc.h, but only the deinit API requires
> > that. So I thought it will look odd to add stub for only one function, that too
> > for one of the two variants (init/deinit).
> > 
> > So I went ahead with the ugly (yes) conditional for the deinit_notify API.
> > 
> > Ideally, I would've expected both dwc and EP subsystem to provide stubs for the
> > APIs used by the common driver (host and EP). But since the controller drivers
> > were using the conditional check to differentiate between host and EP mode,
> > compilers were smart enough to spot the dead functions and removes them. So
> > there were no reports so far.
> > 
> > But in this case, the pci_epc_deinit_notify() is called in a separate helper and
> > hence the issue.
> > 
> > So to conclude, I think it is best if we can add stub just for
> > pci_epc_deinit_notify() in pci-epc.h and get rid of the dummy
> > dw_pcie_ep_init_notify() wrapper to make the init/deinit API usage consistent.
> > 
> > Also I do not want to remove the wrapper for dw_pcie_ep_linkup() since its
> > conterpart dw_pcie_ep_linkdown() is required.
> 
> I see, sounds good.
> 
> However, if we add a stub for pci_epc_deinit_notify(), it makes sense to also
> add a stub for pci_epc_init_notify(). (I'm quite sure tegra will fail to link
> if you change it from dw_pcie_ep_init_notify() to pci_epc_init_notify()
> otherwise.)
> 

No it doesn't. Reason is, the EP IRQ handler itself gets optimized out due to
the CONFIG_PCIE_TEGRA194_EP check in tegra_pcie_dw_probe().

> We should probably also address Bjorn comment:
> "ls and qcom even use *both*: pci_epc_linkdown() but dw_pcie_ep_linkup()."
> 
> As far as I can tell, it is only ls (not sure why Bjorn also mentioned qcom):
> drivers/pci/controller/dwc/pci-layerscape-ep.c:         pci_epc_linkdown(pci->ep.epc);
> But this should probably also be fixed to use dw_pcie_ep_linkdown().
> 

Agree. I will fix that also.

- Mani
Bjorn Helgaas May 29, 2024, 5:25 p.m. UTC | #8
On Wed, May 29, 2024 at 04:54:57PM +0200, Niklas Cassel wrote:
> ...

> We should probably also address Bjorn comment:
> "ls and qcom even use *both*: pci_epc_linkdown() but dw_pcie_ep_linkup()."
> 
> As far as I can tell, it is only ls (not sure why Bjorn also mentioned qcom):
> drivers/pci/controller/dwc/pci-layerscape-ep.c:         pci_epc_linkdown(pci->ep.epc);
> But this should probably also be fixed to use dw_pcie_ep_linkdown().

qcom_pcie_ep_global_irq_thread() calls both pci_epc_linkdown() and
dw_pcie_ep_linkup():

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-qcom-ep.c?id=v6.10-rc1#n628
Niklas Cassel May 29, 2024, 5:48 p.m. UTC | #9
On Wed, May 29, 2024 at 12:25:15PM -0500, Bjorn Helgaas wrote:
> On Wed, May 29, 2024 at 04:54:57PM +0200, Niklas Cassel wrote:
> > ...
> 
> > We should probably also address Bjorn comment:
> > "ls and qcom even use *both*: pci_epc_linkdown() but dw_pcie_ep_linkup()."
> > 
> > As far as I can tell, it is only ls (not sure why Bjorn also mentioned qcom):
> > drivers/pci/controller/dwc/pci-layerscape-ep.c:         pci_epc_linkdown(pci->ep.epc);
> > But this should probably also be fixed to use dw_pcie_ep_linkdown().
> 
> qcom_pcie_ep_global_irq_thread() calls both pci_epc_linkdown() and
> dw_pcie_ep_linkup():
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pcie-qcom-ep.c?id=v6.10-rc1#n628

I see, you were looking at old code (Linus' tree) ;)
In pci/next there is just ls :)

qcom-ep was fixed in:
813c83de4ac0 ("PCI: qcom-ep: Use the generic dw_pcie_ep_linkdown() API to handle Link Down event")

Anyway, since Mani wanted to keep the wrapper, both qcom and ls should use
the wrapper after his series.


Kind regards,
Niklas
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 2063cf2049e5..3c9079651dff 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -39,6 +39,19 @@  void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
 }
 EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify);
 
+/**
+ * dw_pcie_ep_deinit_notify - Notify EPF drivers about EPC deinitialization
+ *			      complete
+ * @ep: DWC EP device
+ */
+void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep)
+{
+	struct pci_epc *epc = ep->epc;
+
+	pci_epc_deinit_notify(epc);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit_notify);
+
 /**
  * dw_pcie_ep_get_func_from_ep - Get the struct dw_pcie_ep_func corresponding to
  *				 the endpoint function
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index f8e5431a207b..dc63f764b8ba 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -672,6 +672,7 @@  int dw_pcie_ep_init(struct dw_pcie_ep *ep);
 int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep);
 void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
 void dw_pcie_ep_deinit(struct dw_pcie_ep *ep);
+void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep);
 void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep);
 int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no);
 int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
@@ -706,6 +707,10 @@  static inline void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
 {
 }
 
+static inline void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep)
+{
+}
+
 static inline void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep)
 {
 }