| Message ID | 20240224-pci-dbi-rework-v8-10-64c7fd0cfe64@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | PCI: dwc: ep: Fix DBI access failure for drivers requiring refclk from host | expand |
On Sat, Feb 24, 2024 at 12:24:16PM +0530, Manivannan Sadhasivam wrote: > All of the APIs are missing the Kernel-doc comments. Hence, add them. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Reviewed-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 92 +++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index fed4c2936c78..cdcb33a279db 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -14,6 +14,11 @@ > #include <linux/pci-epc.h> > #include <linux/pci-epf.h> > > +/** > + * dw_pcie_ep_init_notify - Notify EPF drivers about EPC initialization > + * complete > + * @ep: DWC EP device > + */ > void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep) > { > struct pci_epc *epc = ep->epc; > @@ -22,6 +27,14 @@ void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep) > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify); > > +/** > + * dw_pcie_ep_get_func_from_ep - Get the struct dw_pcie_ep_func corresponding to > + * the endpoint function > + * @ep: DWC EP device > + * @func_no: Function number of the endpoint device > + * > + * Return: struct dw_pcie_ep_func if success, NULL otherwise. > + */ > struct dw_pcie_ep_func * > dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no) > { > @@ -52,6 +65,11 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no, > dw_pcie_dbi_ro_wr_dis(pci); > } > > +/** > + * dw_pcie_ep_reset_bar - Reset endpoint BAR > + * @pci: DWC PCI device > + * @bar: BAR number of the endpoint > + */ > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) > { > u8 func_no, funcs; > @@ -431,6 +449,13 @@ static const struct pci_epc_ops epc_ops = { > .get_features = dw_pcie_ep_get_features, > }; > > +/** > + * dw_pcie_ep_raise_intx_irq - Raise INTx IRQ to the host > + * @ep: DWC EP device > + * @func_no: Function number of the endpoint > + * > + * Return: 0 if success, errono otherwise. > + */ > int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no) > { > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > @@ -442,6 +467,14 @@ int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no) > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq); > > +/** > + * dw_pcie_ep_raise_msi_irq - Raise MSI IRQ to the host > + * @ep: DWC EP device > + * @func_no: Function number of the endpoint > + * @interrupt_num: Interrupt number to be raised > + * > + * Return: 0 if success, errono otherwise. > + */ > int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, > u8 interrupt_num) > { > @@ -490,6 +523,15 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_msi_irq); > > +/** > + * dw_pcie_ep_raise_msix_irq_doorbell - Raise MSIX to the host using Doorbell > + * method > + * @ep: DWC EP device > + * @func_no: Function number of the endpoint device > + * @interrupt_num: Interrupt number to be raised > + * > + * Return: 0 if success, errno otherwise. > + */ > int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, > u16 interrupt_num) > { > @@ -509,6 +551,14 @@ int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, > return 0; > } > > +/** > + * dw_pcie_ep_raise_msix_irq - Raise MSIX to the host > + * @ep: DWC EP device > + * @func_no: Function number of the endpoint device > + * @interrupt_num: Interrupt number to be raised > + * > + * Return: 0 if success, errno otherwise. > + */ > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > u16 interrupt_num) > { > @@ -556,6 +606,12 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > return 0; > } > > +/** > + * dw_pcie_ep_cleanup - Cleanup DWC EP resources > + * @ep: DWC EP device > + * > + * Cleans up the DWC EP specific resources like eDMA etc... > + */ > void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep) > { > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > @@ -564,6 +620,13 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep) > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup); > > +/** > + * dw_pcie_ep_deinit - Deinitialize the endpoint device > + * @ep: DWC EP device > + * > + * Deinitialize the endpoint device. EPC device is not destroyed since that will > + * taken care by Devres. > + */ > void dw_pcie_ep_deinit(struct dw_pcie_ep *ep) > { > struct pci_epc *epc = ep->epc; > @@ -635,6 +698,14 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) > dw_pcie_dbi_ro_wr_dis(pci); > } > > +/** > + * dw_pcie_ep_init_registers - Initialize DWC EP specific registers > + * @ep: DWC EP device > + * > + * Initialize the registers (CSRs) specific to DWC EP. This API should be called > + * only when the endpoint receives an active refclk (either from host or > + * generated locally). > + */ > int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep) > { > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > @@ -718,6 +789,10 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep) > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers); > > +/** > + * dw_pcie_ep_linkup - Notify EPF drivers about link up event > + * @ep: DWC EP device > + */ > void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > { > struct pci_epc *epc = ep->epc; > @@ -726,6 +801,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup); > > +/** > + * dw_pcie_ep_linkdown - Notify EPF drivers about link down event > + * @ep: DWC EP device > + * > + * Non-sticky registers are also initialized before sending the notification to > + * the EPF drivers. This is needed since the registers need to be initialized > + * before the link comes back again. > + */ > void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep) > { > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > @@ -743,6 +826,15 @@ void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep) > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown); > > +/** > + * dw_pcie_ep_init - Initialize the endpoint device > + * @ep: DWC EP device > + * > + * Initialize the endpoint device. Allocate resources and create the EPC > + * device with the endpoint framework. > + * > + * Return: 0 if success, errno otherwise. > + */ > int dw_pcie_ep_init(struct dw_pcie_ep *ep) > { > int ret; > > -- > 2.25.1 >
Hello Mani, On Sat, Feb 24, 2024 at 12:24:16PM +0530, Manivannan Sadhasivam wrote: > All of the APIs are missing the Kernel-doc comments. Hence, add them. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 92 +++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index fed4c2936c78..cdcb33a279db 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c (snip) > @@ -556,6 +606,12 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > return 0; > } > > +/** > + * dw_pcie_ep_cleanup - Cleanup DWC EP resources > + * @ep: DWC EP device > + * > + * Cleans up the DWC EP specific resources like eDMA etc... I think that you should mention that this is only for glue drivers that use PERST# handling, so that other glue drivers do no start using it :) > + */ > void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep) > { > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > @@ -564,6 +620,13 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep) > } > EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup); Kind regards, Niklas
On Thu, Feb 29, 2024 at 01:56:23PM +0100, Niklas Cassel wrote: > Hello Mani, > > On Sat, Feb 24, 2024 at 12:24:16PM +0530, Manivannan Sadhasivam wrote: > > All of the APIs are missing the Kernel-doc comments. Hence, add them. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/pci/controller/dwc/pcie-designware-ep.c | 92 +++++++++++++++++++++++++ > > 1 file changed, 92 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index fed4c2936c78..cdcb33a279db 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > (snip) > > > @@ -556,6 +606,12 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > > return 0; > > } > > > > +/** > > + * dw_pcie_ep_cleanup - Cleanup DWC EP resources > > + * @ep: DWC EP device > > + * > > + * Cleans up the DWC EP specific resources like eDMA etc... > > I think that you should mention that this is only for glue drivers that > use PERST# handling, so that other glue drivers do no start using it :) > You are right. Will add the wording. - Mani
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index fed4c2936c78..cdcb33a279db 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -14,6 +14,11 @@ #include <linux/pci-epc.h> #include <linux/pci-epf.h> +/** + * dw_pcie_ep_init_notify - Notify EPF drivers about EPC initialization + * complete + * @ep: DWC EP device + */ void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep) { struct pci_epc *epc = ep->epc; @@ -22,6 +27,14 @@ void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep) } EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify); +/** + * dw_pcie_ep_get_func_from_ep - Get the struct dw_pcie_ep_func corresponding to + * the endpoint function + * @ep: DWC EP device + * @func_no: Function number of the endpoint device + * + * Return: struct dw_pcie_ep_func if success, NULL otherwise. + */ struct dw_pcie_ep_func * dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no) { @@ -52,6 +65,11 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no, dw_pcie_dbi_ro_wr_dis(pci); } +/** + * dw_pcie_ep_reset_bar - Reset endpoint BAR + * @pci: DWC PCI device + * @bar: BAR number of the endpoint + */ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) { u8 func_no, funcs; @@ -431,6 +449,13 @@ static const struct pci_epc_ops epc_ops = { .get_features = dw_pcie_ep_get_features, }; +/** + * dw_pcie_ep_raise_intx_irq - Raise INTx IRQ to the host + * @ep: DWC EP device + * @func_no: Function number of the endpoint + * + * Return: 0 if success, errono otherwise. + */ int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no) { struct dw_pcie *pci = to_dw_pcie_from_ep(ep); @@ -442,6 +467,14 @@ int dw_pcie_ep_raise_intx_irq(struct dw_pcie_ep *ep, u8 func_no) } EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_intx_irq); +/** + * dw_pcie_ep_raise_msi_irq - Raise MSI IRQ to the host + * @ep: DWC EP device + * @func_no: Function number of the endpoint + * @interrupt_num: Interrupt number to be raised + * + * Return: 0 if success, errono otherwise. + */ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, u8 interrupt_num) { @@ -490,6 +523,15 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, } EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_msi_irq); +/** + * dw_pcie_ep_raise_msix_irq_doorbell - Raise MSIX to the host using Doorbell + * method + * @ep: DWC EP device + * @func_no: Function number of the endpoint device + * @interrupt_num: Interrupt number to be raised + * + * Return: 0 if success, errno otherwise. + */ int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, u16 interrupt_num) { @@ -509,6 +551,14 @@ int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, return 0; } +/** + * dw_pcie_ep_raise_msix_irq - Raise MSIX to the host + * @ep: DWC EP device + * @func_no: Function number of the endpoint device + * @interrupt_num: Interrupt number to be raised + * + * Return: 0 if success, errno otherwise. + */ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, u16 interrupt_num) { @@ -556,6 +606,12 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, return 0; } +/** + * dw_pcie_ep_cleanup - Cleanup DWC EP resources + * @ep: DWC EP device + * + * Cleans up the DWC EP specific resources like eDMA etc... + */ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep) { struct dw_pcie *pci = to_dw_pcie_from_ep(ep); @@ -564,6 +620,13 @@ void dw_pcie_ep_cleanup(struct dw_pcie_ep *ep) } EXPORT_SYMBOL_GPL(dw_pcie_ep_cleanup); +/** + * dw_pcie_ep_deinit - Deinitialize the endpoint device + * @ep: DWC EP device + * + * Deinitialize the endpoint device. EPC device is not destroyed since that will + * taken care by Devres. + */ void dw_pcie_ep_deinit(struct dw_pcie_ep *ep) { struct pci_epc *epc = ep->epc; @@ -635,6 +698,14 @@ static void dw_pcie_ep_init_non_sticky_registers(struct dw_pcie *pci) dw_pcie_dbi_ro_wr_dis(pci); } +/** + * dw_pcie_ep_init_registers - Initialize DWC EP specific registers + * @ep: DWC EP device + * + * Initialize the registers (CSRs) specific to DWC EP. This API should be called + * only when the endpoint receives an active refclk (either from host or + * generated locally). + */ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep) { struct dw_pcie *pci = to_dw_pcie_from_ep(ep); @@ -718,6 +789,10 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep) } EXPORT_SYMBOL_GPL(dw_pcie_ep_init_registers); +/** + * dw_pcie_ep_linkup - Notify EPF drivers about link up event + * @ep: DWC EP device + */ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) { struct pci_epc *epc = ep->epc; @@ -726,6 +801,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) } EXPORT_SYMBOL_GPL(dw_pcie_ep_linkup); +/** + * dw_pcie_ep_linkdown - Notify EPF drivers about link down event + * @ep: DWC EP device + * + * Non-sticky registers are also initialized before sending the notification to + * the EPF drivers. This is needed since the registers need to be initialized + * before the link comes back again. + */ void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep) { struct dw_pcie *pci = to_dw_pcie_from_ep(ep); @@ -743,6 +826,15 @@ void dw_pcie_ep_linkdown(struct dw_pcie_ep *ep) } EXPORT_SYMBOL_GPL(dw_pcie_ep_linkdown); +/** + * dw_pcie_ep_init - Initialize the endpoint device + * @ep: DWC EP device + * + * Initialize the endpoint device. Allocate resources and create the EPC + * device with the endpoint framework. + * + * Return: 0 if success, errno otherwise. + */ int dw_pcie_ep_init(struct dw_pcie_ep *ep) { int ret;
All of the APIs are missing the Kernel-doc comments. Hence, add them. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/pci/controller/dwc/pcie-designware-ep.c | 92 +++++++++++++++++++++++++ 1 file changed, 92 insertions(+)