diff mbox series

[v22,08/16] PCI: dwc: endpoint: Introduce .pre_init() and .deinit()

Message ID 20230925072130.3901087-9-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Superseded
Delegated to: Krzysztof Wilczyński
Headers show
Series PCI: dwc: rcar-gen4: Add R-Car Gen4 PCIe support | expand

Commit Message

Yoshihiro Shimoda Sept. 25, 2023, 7:21 a.m. UTC 
Renesas R-Car Gen4 PCIe controllers require vendor-specific
initialization before .init().

To use dw->dbi and dw->num-lanes in the initialization code,
introduce .pre_init() into struct dw_pcie_ep_ops. While at it,
also introduce .deinit() to disable the controller by using
vendor-specific de-initialization.

Note that the ep_init in the struct dw_pcie_ep_ops should be
renamed to init later.

[kwilczynski: commit log]
Link: https://lore.kernel.org/linux-pci/20230825093219.2685912-13-yoshihiro.shimoda.uh@renesas.com
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++++++++-
 drivers/pci/controller/dwc/pcie-designware.h    |  2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Serge Semin Sept. 25, 2023, 10:08 a.m. UTC | #1 
On Mon, Sep 25, 2023 at 04:21:22PM +0900, Yoshihiro Shimoda wrote:
> Renesas R-Car Gen4 PCIe controllers require vendor-specific
> initialization before .init().
> 
> To use dw->dbi and dw->num-lanes in the initialization code,
> introduce .pre_init() into struct dw_pcie_ep_ops. While at it,
> also introduce .deinit() to disable the controller by using
> vendor-specific de-initialization.
> 
> Note that the ep_init in the struct dw_pcie_ep_ops should be
> renamed to init later.
> 
> [kwilczynski: commit log]
> Link: https://lore.kernel.org/linux-pci/20230825093219.2685912-13-yoshihiro.shimoda.uh@renesas.com
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++++++++-
>  drivers/pci/controller/dwc/pcie-designware.h    |  2 ++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index a8bcbc57ef86..d34a5e87ad18 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -637,6 +637,9 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  			      epc->mem->window.page_size);
>  
>  	pci_epc_mem_exit(epc);
> +
> +	if (ep->ops->deinit)
> +		ep->ops->deinit(ep);
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
>  
> @@ -740,6 +743,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  	ep->phys_base = res->start;
>  	ep->addr_size = resource_size(res);
>  
> +	if (ep->ops->pre_init)
> +		ep->ops->pre_init(ep);
> +
>  	dw_pcie_version_detect(pci);
>  
>  	dw_pcie_iatu_detect(pci);
> @@ -794,7 +800,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  			       ep->page_size);
>  	if (ret < 0) {
>  		dev_err(dev, "Failed to initialize address space\n");
> -		return ret;
> +		goto err_ep_deinit;
>  	}
>  
>  	ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
> @@ -831,6 +837,10 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  err_exit_epc_mem:
>  	pci_epc_mem_exit(epc);
>  
> +err_ep_deinit:
> +	if (ep->ops->deinit)
> +		ep->ops->deinit(ep);
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_ep_init);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index e10f7e18b13a..8c637392ab08 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -330,7 +330,9 @@ struct dw_pcie_rp {
>  };
>  
>  struct dw_pcie_ep_ops {

> +	void	(*pre_init)(struct dw_pcie_ep *ep);
>  	void	(*ep_init)(struct dw_pcie_ep *ep);
> +	void	(*deinit)(struct dw_pcie_ep *ep);

Please, note you promised to release sometime in future a cleanup
patch which drops the "ep_" prefix from here and from the
dw_pcie_host_ops structure.)

https://lore.kernel.org/linux-pci/20230802104049.GB57374@thinkpad/

-Serge(y)

>  	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
>  			     enum pci_epc_irq_type type, u16 interrupt_num);
>  	const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> -- 
> 2.25.1
>
Yoshihiro Shimoda Sept. 25, 2023, 10:24 a.m. UTC | #2 
Hi Serge,

> From: Serge Semin, Sent: Monday, September 25, 2023 7:08 PM
> 
> On Mon, Sep 25, 2023 at 04:21:22PM +0900, Yoshihiro Shimoda wrote:
> > Renesas R-Car Gen4 PCIe controllers require vendor-specific
> > initialization before .init().
> >
> > To use dw->dbi and dw->num-lanes in the initialization code,
> > introduce .pre_init() into struct dw_pcie_ep_ops. While at it,
> > also introduce .deinit() to disable the controller by using
> > vendor-specific de-initialization.
> >
> > Note that the ep_init in the struct dw_pcie_ep_ops should be
> > renamed to init later.
> >
> > [kwilczynski: commit log]
> > Link:
> > https://lore.kernel.org/linux-pci/20230825093219.2685912-13-yoshihiro.shimoda.uh@renesas.com/
> > 
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
> > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++++++++-
> >  drivers/pci/controller/dwc/pcie-designware.h    |  2 ++
> >  2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index a8bcbc57ef86..d34a5e87ad18 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -637,6 +637,9 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> >  			      epc->mem->window.page_size);
> >
> >  	pci_epc_mem_exit(epc);
> > +
> > +	if (ep->ops->deinit)
> > +		ep->ops->deinit(ep);
> >  }
> >  EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
> >
> > @@ -740,6 +743,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> >  	ep->phys_base = res->start;
> >  	ep->addr_size = resource_size(res);
> >
> > +	if (ep->ops->pre_init)
> > +		ep->ops->pre_init(ep);
> > +
> >  	dw_pcie_version_detect(pci);
> >
> >  	dw_pcie_iatu_detect(pci);
> > @@ -794,7 +800,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> >  			       ep->page_size);
> >  	if (ret < 0) {
> >  		dev_err(dev, "Failed to initialize address space\n");
> > -		return ret;
> > +		goto err_ep_deinit;
> >  	}
> >
> >  	ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
> > @@ -831,6 +837,10 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> >  err_exit_epc_mem:
> >  	pci_epc_mem_exit(epc);
> >
> > +err_ep_deinit:
> > +	if (ep->ops->deinit)
> > +		ep->ops->deinit(ep);
> > +
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(dw_pcie_ep_init);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index e10f7e18b13a..8c637392ab08 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -330,7 +330,9 @@ struct dw_pcie_rp {
> >  };
> >
> >  struct dw_pcie_ep_ops {
> 
> > +	void	(*pre_init)(struct dw_pcie_ep *ep);
> >  	void	(*ep_init)(struct dw_pcie_ep *ep);
> > +	void	(*deinit)(struct dw_pcie_ep *ep);
> 
> Please, note you promised to release sometime in future a cleanup
> patch which drops the "ep_" prefix from here and from the
> dw_pcie_host_ops structure.)
> 
> https://lore.kernel.org/linux-pci/20230802104049.GB57374@thinkpad/

Since this patch series is not merged into the pci.git / next branch yet,
I should not include any cleanup patches, I think. In other words, this patch
series still seems under review now.

Best regards,
Yoshihiro Shimoda

> 
> -Serge(y)
> 
> >  	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
> >  			     enum pci_epc_irq_type type, u16 interrupt_num);
> >  	const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> > --
> > 2.25.1
> >
Serge Semin Sept. 25, 2023, 11:01 a.m. UTC | #3 
Hi Yoshihiro

On Mon, Sep 25, 2023 at 10:24:07AM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
> 
> > From: Serge Semin, Sent: Monday, September 25, 2023 7:08 PM
> > 
> > On Mon, Sep 25, 2023 at 04:21:22PM +0900, Yoshihiro Shimoda wrote:
> > > Renesas R-Car Gen4 PCIe controllers require vendor-specific
> > > initialization before .init().
> > >
> > > To use dw->dbi and dw->num-lanes in the initialization code,
> > > introduce .pre_init() into struct dw_pcie_ep_ops. While at it,
> > > also introduce .deinit() to disable the controller by using
> > > vendor-specific de-initialization.
> > >
> > > Note that the ep_init in the struct dw_pcie_ep_ops should be
> > > renamed to init later.
> > >
> > > [kwilczynski: commit log]
> > > Link:
> > > https://lore.kernel.org/linux-pci/20230825093219.2685912-13-yoshihiro.shimoda.uh@renesas.com/
> > > 
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
> > > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware-ep.c | 12 +++++++++++-
> > >  drivers/pci/controller/dwc/pcie-designware.h    |  2 ++
> > >  2 files changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index a8bcbc57ef86..d34a5e87ad18 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -637,6 +637,9 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> > >  			      epc->mem->window.page_size);
> > >
> > >  	pci_epc_mem_exit(epc);
> > > +
> > > +	if (ep->ops->deinit)
> > > +		ep->ops->deinit(ep);
> > >  }
> > >  EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
> > >
> > > @@ -740,6 +743,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > >  	ep->phys_base = res->start;
> > >  	ep->addr_size = resource_size(res);
> > >
> > > +	if (ep->ops->pre_init)
> > > +		ep->ops->pre_init(ep);
> > > +
> > >  	dw_pcie_version_detect(pci);
> > >
> > >  	dw_pcie_iatu_detect(pci);
> > > @@ -794,7 +800,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > >  			       ep->page_size);
> > >  	if (ret < 0) {
> > >  		dev_err(dev, "Failed to initialize address space\n");
> > > -		return ret;
> > > +		goto err_ep_deinit;
> > >  	}
> > >
> > >  	ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
> > > @@ -831,6 +837,10 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > >  err_exit_epc_mem:
> > >  	pci_epc_mem_exit(epc);
> > >
> > > +err_ep_deinit:
> > > +	if (ep->ops->deinit)
> > > +		ep->ops->deinit(ep);
> > > +
> > >  	return ret;
> > >  }
> > >  EXPORT_SYMBOL_GPL(dw_pcie_ep_init);
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > index e10f7e18b13a..8c637392ab08 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -330,7 +330,9 @@ struct dw_pcie_rp {
> > >  };
> > >
> > >  struct dw_pcie_ep_ops {
> > 
> > > +	void	(*pre_init)(struct dw_pcie_ep *ep);
> > >  	void	(*ep_init)(struct dw_pcie_ep *ep);
> > > +	void	(*deinit)(struct dw_pcie_ep *ep);
> > 
> > Please, note you promised to release sometime in future a cleanup
> > patch which drops the "ep_" prefix from here and from the
> > dw_pcie_host_ops structure.)
> > 
> > https://lore.kernel.org/linux-pci/20230802104049.GB57374@thinkpad/
> 

> Since this patch series is not merged into the pci.git / next branch yet,
> I should not include any cleanup patches, I think. In other words, this patch
> series still seems under review now.

Yeah. BTW sorry to hear that. Get new comments after you have already
got your series merged in. It must have been frustrating for you.
Sometimes it happens though...(

But my message was relevant to another series, which you hopefully
will submit sometimes later. It was just a reminder in addition to the
notes you placed in the commit log to the patch:
[PATCH v22 01/16] PCI: dwc: endpoint: Add multiple PFs support for dbi2

-Serge(y)

> 
> Best regards,
> Yoshihiro Shimoda
> 
> > 
> > -Serge(y)
> > 
> > >  	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
> > >  			     enum pci_epc_irq_type type, u16 interrupt_num);
> > >  	const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> > > --
> > > 2.25.1
> > >
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 a8bcbc57ef86..d34a5e87ad18 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -637,6 +637,9 @@  void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 			      epc->mem->window.page_size);
 
 	pci_epc_mem_exit(epc);
+
+	if (ep->ops->deinit)
+		ep->ops->deinit(ep);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
 
@@ -740,6 +743,9 @@  int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 	ep->phys_base = res->start;
 	ep->addr_size = resource_size(res);
 
+	if (ep->ops->pre_init)
+		ep->ops->pre_init(ep);
+
 	dw_pcie_version_detect(pci);
 
 	dw_pcie_iatu_detect(pci);
@@ -794,7 +800,7 @@  int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 			       ep->page_size);
 	if (ret < 0) {
 		dev_err(dev, "Failed to initialize address space\n");
-		return ret;
+		goto err_ep_deinit;
 	}
 
 	ep->msi_mem = pci_epc_mem_alloc_addr(epc, &ep->msi_mem_phys,
@@ -831,6 +837,10 @@  int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 err_exit_epc_mem:
 	pci_epc_mem_exit(epc);
 
+err_ep_deinit:
+	if (ep->ops->deinit)
+		ep->ops->deinit(ep);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dw_pcie_ep_init);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index e10f7e18b13a..8c637392ab08 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -330,7 +330,9 @@  struct dw_pcie_rp {
 };
 
 struct dw_pcie_ep_ops {
+	void	(*pre_init)(struct dw_pcie_ep *ep);
 	void	(*ep_init)(struct dw_pcie_ep *ep);
+	void	(*deinit)(struct dw_pcie_ep *ep);
 	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
 			     enum pci_epc_irq_type type, u16 interrupt_num);
 	const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);