diff mbox series

[v22,14/16] PCI: dwc: rcar-gen4: Add R-Car Gen4 PCIe Endpoint support

Message ID 20230925072130.3901087-15-yoshihiro.shimoda.uh@renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
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
Add R-Car Gen4 PCIe controller for endpoint mode. This controller is based
on Synopsys DesignWare PCIe.

Link: https://lore.kernel.org/linux-pci/20230825093219.2685912-18-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/Kconfig          |  11 ++
 drivers/pci/controller/dwc/pcie-rcar-gen4.c | 136 +++++++++++++++++++-
 2 files changed, 144 insertions(+), 3 deletions(-)

Comments

Serge Semin Sept. 25, 2023, 10:53 a.m. UTC | #1
On Mon, Sep 25, 2023 at 04:21:28PM +0900, Yoshihiro Shimoda wrote:
> Add R-Car Gen4 PCIe controller for endpoint mode. This controller is based
> on Synopsys DesignWare PCIe.
> 
> Link: https://lore.kernel.org/linux-pci/20230825093219.2685912-18-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/Kconfig          |  11 ++
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c | 136 +++++++++++++++++++-
>  2 files changed, 144 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index bc69fcab2e2a..e7fd37717998 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -429,4 +429,15 @@ config PCIE_RCAR_GEN4_HOST
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called pcie-rcar-gen4.ko. This uses the DesignWare core.
>  
> +config PCIE_RCAR_GEN4_EP
> +	tristate "Renesas R-Car Gen4 PCIe controller (endpoint mode)"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on PCI_ENDPOINT
> +	select PCIE_DW_EP
> +	select PCIE_RCAR_GEN4
> +	help
> +	  Say Y here if you want PCIe controller (endpoint mode) on R-Car Gen4
> +	  SoCs. To compile this driver as a module, choose M here: the module
> +	  will be called pcie-rcar-gen4.ko. This uses the DesignWare core.
> +
>  endmenu
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index f6b3c3ef187c..d1b31ea909ba 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -45,6 +45,9 @@
>  #define RCAR_NUM_SPEED_CHANGE_RETRIES	10
>  #define RCAR_MAX_LINK_SPEED		4
>  
> +#define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET	0x1000
> +#define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET	0x800
> +
>  struct rcar_gen4_pcie {
>  	struct dw_pcie dw;
>  	void __iomem *base;
> @@ -53,6 +56,7 @@ struct rcar_gen4_pcie {
>  };
>  #define to_rcar_gen4_pcie(_dw)	container_of(_dw, struct rcar_gen4_pcie, dw)
>  
> +/* Common */
>  static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
>  					bool enable)
>  {
> @@ -311,6 +315,9 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
>  {
>  	struct dw_pcie_rp *pp = &rcar->dw.pp;
>  
> +	if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_HOST))
> +		return -ENODEV;
> +
>  	pp->num_vectors = MAX_MSI_IRQS;
>  	pp->ops = &rcar_gen4_pcie_host_ops;
>  	rcar->mode = DW_PCIE_RC_TYPE;
> @@ -323,8 +330,114 @@ static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
>  	dw_pcie_host_deinit(&rcar->dw.pp);
>  }
>  
> +/* Endpoind mode */
> +static void rcar_gen4_pcie_ep_pre_init(struct dw_pcie_ep *ep)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +	int ret;
> +
> +	ret = rcar_gen4_pcie_common_init(rcar);
> +	if (ret)
> +		return;
> +
> +	writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
> +}
> +
> +static void rcar_gen4_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	enum pci_barno bar;
> +
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
> +		dw_pcie_ep_reset_bar(pci, bar);
> +}
> +
> +static void rcar_gen4_pcie_ep_deinit(struct dw_pcie_ep *ep)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> +	writel(0, rcar->base + PCIEDMAINTSTSEN);
> +	rcar_gen4_pcie_common_deinit(rcar);
> +}
> +
> +static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> +				       unsigned int type, u16 interrupt_num)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> +
> +	switch (type) {
> +	case PCI_IRQ_LEGACY:
> +		return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> +	case PCI_IRQ_MSI:
> +		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> +	default:
> +		dev_err(dw->dev, "Unknown IRQ type\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
> +	.linkup_notifier = false,
> +	.msi_capable = true,
> +	.msix_capable = false,
> +	.reserved_bar = 1 << BAR_1 | 1 << BAR_3 | 1 << BAR_5,
> +	.align = SZ_1M,
> +};
> +
> +static const struct pci_epc_features*
> +rcar_gen4_pcie_ep_get_features(struct dw_pcie_ep *ep)
> +{
> +	return &rcar_gen4_pcie_epc_features;
> +}
> +
> +static unsigned int rcar_gen4_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> +						       u8 func_no)
> +{
> +	return func_no * RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET;
> +}
> +
> +static unsigned int rcar_gen4_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> +						      u8 func_no)
> +{
> +	return func_no * RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET;
> +}
> +
> +static const struct dw_pcie_ep_ops pcie_ep_ops = {
> +	.pre_init = rcar_gen4_pcie_ep_pre_init,
> +	.ep_init = rcar_gen4_pcie_ep_init,
> +	.deinit = rcar_gen4_pcie_ep_deinit,
> +	.raise_irq = rcar_gen4_pcie_ep_raise_irq,
> +	.get_features = rcar_gen4_pcie_ep_get_features,
> +	.func_conf_select = rcar_gen4_pcie_ep_func_conf_select,
> +	.get_dbi2_offset = rcar_gen4_pcie_ep_get_dbi2_offset,
> +};
> +
> +static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> +{
> +	struct dw_pcie_ep *ep = &rcar->dw.ep;
> +
> +	if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
> +		return -ENODEV;
> +
> +	rcar->mode = DW_PCIE_EP_TYPE;
> +	ep->ops = &pcie_ep_ops;
> +
> +	return dw_pcie_ep_init(ep);
> +}
> +
> +static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> +{
> +	dw_pcie_ep_exit(&rcar->dw.ep);
> +}
> +
> +/* Common */
>  static int rcar_gen4_pcie_probe(struct platform_device *pdev)
>  {
> +	enum dw_pcie_device_mode mode;
>  	struct rcar_gen4_pcie *rcar;
>  	int err;
>  
> @@ -340,7 +453,13 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  

> -	err = rcar_gen4_add_dw_pcie_rp(rcar);
> +	mode = (enum dw_pcie_device_mode)of_device_get_match_data(&pdev->dev);
> +
> +	if (mode == DW_PCIE_RC_TYPE)
> +		err = rcar_gen4_add_dw_pcie_rp(rcar);
> +	else if (mode == DW_PCIE_EP_TYPE)
> +		err = rcar_gen4_add_dw_pcie_ep(rcar);
> +

So now you have two places with the controller mode initialization:
1. rcar_gen4_pcie_of_match
2. rcar_gen4_add_dw_pcie_rp() and rcar_gen4_add_dw_pcie_ep()
It looks a bit clumsy and less maintainable than could be. What I
suggest is to create a new method which would do what is done above,
but also would initialize the rcar_gen4_pcie->mode field:

static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar)
{
	rcar->mode = device_get_match_data(&rcar->pdev->dev);
	switch (rcar->mode) {
	case DW_PCIE_RC_TYPE:
		return rcar_gen4_add_dw_pcie_rp(rcar);
	case DW_PCIE_EP_TYPE:
		return rcar_gen4_add_dw_pcie_ep(rcar);
	}

	return -EINVAL;
}

Of course the rcar_gen4_pcie->mode field initialization should be
dropped from the rcar_gen4_add_dw_pcie_rp() and
rcar_gen4_add_dw_pcie_ep() methods.

and ...

>  	if (err)
>  		goto err_unprepare;
>  
> @@ -356,12 +475,23 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev)
>  {
>  	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
>  

> -	rcar_gen4_remove_dw_pcie_rp(rcar);
> +	if (rcar->mode == DW_PCIE_RC_TYPE)
> +		rcar_gen4_remove_dw_pcie_rp(rcar);
> +	else if (rcar->mode == DW_PCIE_EP_TYPE)
> +		rcar_gen4_remove_dw_pcie_ep(rcar);
> +

... similarly I would have added a respective antagonist:

static void rcar_gen4_remove_dw_pcie(struct rcar_gen4_pcie *rcar)
{
	switch (rcar->mode) {
	case DW_PCIE_RC_TYPE:
		rcar_gen4_remove_dw_pcie_rp(rcar);
		break;
	case DW_PCIE_EP_TYPE:
		rcar_gen4_remove_dw_pcie_ep(rcar);
		break;
	}
}

which would have been called from here instead of the open-coded
switch-case statement. Thus the driver design will be preserved (a set
of the init/deinit, add/remove, probe/remove and functional helper
methods) and the mode initialization will be localized in a single
place.

-Serge(y)

>  	rcar_gen4_pcie_unprepare(rcar);
>  }
>  
>  static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> -	{ .compatible = "renesas,rcar-gen4-pcie", },
> +	{
> +		.compatible = "renesas,rcar-gen4-pcie",
> +		.data = (void *)DW_PCIE_RC_TYPE,
> +	},
> +	{
> +		.compatible = "renesas,rcar-gen4-pcie-ep",
> +		.data = (void *)DW_PCIE_EP_TYPE,
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, rcar_gen4_pcie_of_match);
> -- 
> 2.25.1
>
Yoshihiro Shimoda Sept. 26, 2023, 11:34 a.m. UTC | #2
Hello Serge,

> From: Serge Semin, Sent: Monday, September 25, 2023 7:53 PM
> To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Cc: lpieralisi@kernel.org; kw@linux.com; robh@kernel.org; bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; jingoohan1@gmail.com; gustavo.pimentel@synopsys.com; mani@kernel.org;
> marek.vasut+renesas@gmail.com; linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH v22 14/16] PCI: dwc: rcar-gen4: Add R-Car Gen4 PCIe Endpoint support
> 
> On Mon, Sep 25, 2023 at 04:21:28PM +0900, Yoshihiro Shimoda wrote:
> > Add R-Car Gen4 PCIe controller for endpoint mode. This controller is based
> > on Synopsys DesignWare PCIe.
> >
> > Link:
<snip>
> > 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/Kconfig          |  11 ++
> >  drivers/pci/controller/dwc/pcie-rcar-gen4.c | 136 +++++++++++++++++++-
> >  2 files changed, 144 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index bc69fcab2e2a..e7fd37717998 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -429,4 +429,15 @@ config PCIE_RCAR_GEN4_HOST
> >  	  To compile this driver as a module, choose M here: the module will be
> >  	  called pcie-rcar-gen4.ko. This uses the DesignWare core.
> >
> > +config PCIE_RCAR_GEN4_EP
> > +	tristate "Renesas R-Car Gen4 PCIe controller (endpoint mode)"
> > +	depends on ARCH_RENESAS || COMPILE_TEST
> > +	depends on PCI_ENDPOINT
> > +	select PCIE_DW_EP
> > +	select PCIE_RCAR_GEN4
> > +	help
> > +	  Say Y here if you want PCIe controller (endpoint mode) on R-Car Gen4
> > +	  SoCs. To compile this driver as a module, choose M here: the module
> > +	  will be called pcie-rcar-gen4.ko. This uses the DesignWare core.
> > +
> >  endmenu
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > index f6b3c3ef187c..d1b31ea909ba 100644
> > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > @@ -45,6 +45,9 @@
> >  #define RCAR_NUM_SPEED_CHANGE_RETRIES	10
> >  #define RCAR_MAX_LINK_SPEED		4
> >
> > +#define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET	0x1000
> > +#define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET	0x800
> > +
> >  struct rcar_gen4_pcie {
> >  	struct dw_pcie dw;
> >  	void __iomem *base;
> > @@ -53,6 +56,7 @@ struct rcar_gen4_pcie {
> >  };
> >  #define to_rcar_gen4_pcie(_dw)	container_of(_dw, struct rcar_gen4_pcie, dw)
> >
> > +/* Common */
> >  static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> >  					bool enable)
> >  {
> > @@ -311,6 +315,9 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> >  {
> >  	struct dw_pcie_rp *pp = &rcar->dw.pp;
> >
> > +	if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_HOST))
> > +		return -ENODEV;
> > +
> >  	pp->num_vectors = MAX_MSI_IRQS;
> >  	pp->ops = &rcar_gen4_pcie_host_ops;
> >  	rcar->mode = DW_PCIE_RC_TYPE;
> > @@ -323,8 +330,114 @@ static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> >  	dw_pcie_host_deinit(&rcar->dw.pp);
> >  }
> >
> > +/* Endpoind mode */
> > +static void rcar_gen4_pcie_ep_pre_init(struct dw_pcie_ep *ep)
> > +{
> > +	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +	int ret;
> > +
> > +	ret = rcar_gen4_pcie_common_init(rcar);
> > +	if (ret)
> > +		return;
> > +
> > +	writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
> > +}
> > +
> > +static void rcar_gen4_pcie_ep_init(struct dw_pcie_ep *ep)
> > +{
> > +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +	enum pci_barno bar;
> > +
> > +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
> > +		dw_pcie_ep_reset_bar(pci, bar);
> > +}
> > +
> > +static void rcar_gen4_pcie_ep_deinit(struct dw_pcie_ep *ep)
> > +{
> > +	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > +	writel(0, rcar->base + PCIEDMAINTSTSEN);
> > +	rcar_gen4_pcie_common_deinit(rcar);
> > +}
> > +
> > +static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > +				       unsigned int type, u16 interrupt_num)
> > +{
> > +	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> > +
> > +	switch (type) {
> > +	case PCI_IRQ_LEGACY:
> > +		return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> > +	case PCI_IRQ_MSI:
> > +		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> > +	default:
> > +		dev_err(dw->dev, "Unknown IRQ type\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
> > +	.linkup_notifier = false,
> > +	.msi_capable = true,
> > +	.msix_capable = false,
> > +	.reserved_bar = 1 << BAR_1 | 1 << BAR_3 | 1 << BAR_5,
> > +	.align = SZ_1M,
> > +};
> > +
> > +static const struct pci_epc_features*
> > +rcar_gen4_pcie_ep_get_features(struct dw_pcie_ep *ep)
> > +{
> > +	return &rcar_gen4_pcie_epc_features;
> > +}
> > +
> > +static unsigned int rcar_gen4_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> > +						       u8 func_no)
> > +{
> > +	return func_no * RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET;
> > +}
> > +
> > +static unsigned int rcar_gen4_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> > +						      u8 func_no)
> > +{
> > +	return func_no * RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET;
> > +}
> > +
> > +static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > +	.pre_init = rcar_gen4_pcie_ep_pre_init,
> > +	.ep_init = rcar_gen4_pcie_ep_init,
> > +	.deinit = rcar_gen4_pcie_ep_deinit,
> > +	.raise_irq = rcar_gen4_pcie_ep_raise_irq,
> > +	.get_features = rcar_gen4_pcie_ep_get_features,
> > +	.func_conf_select = rcar_gen4_pcie_ep_func_conf_select,
> > +	.get_dbi2_offset = rcar_gen4_pcie_ep_get_dbi2_offset,
> > +};
> > +
> > +static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> > +{
> > +	struct dw_pcie_ep *ep = &rcar->dw.ep;
> > +
> > +	if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
> > +		return -ENODEV;
> > +
> > +	rcar->mode = DW_PCIE_EP_TYPE;
> > +	ep->ops = &pcie_ep_ops;
> > +
> > +	return dw_pcie_ep_init(ep);
> > +}
> > +
> > +static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> > +{
> > +	dw_pcie_ep_exit(&rcar->dw.ep);
> > +}
> > +
> > +/* Common */
> >  static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> >  {
> > +	enum dw_pcie_device_mode mode;
> >  	struct rcar_gen4_pcie *rcar;
> >  	int err;
> >
> > @@ -340,7 +453,13 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> >  	if (err)
> >  		return err;
> >
> 
> > -	err = rcar_gen4_add_dw_pcie_rp(rcar);
> > +	mode = (enum dw_pcie_device_mode)of_device_get_match_data(&pdev->dev);
> > +
> > +	if (mode == DW_PCIE_RC_TYPE)
> > +		err = rcar_gen4_add_dw_pcie_rp(rcar);
> > +	else if (mode == DW_PCIE_EP_TYPE)
> > +		err = rcar_gen4_add_dw_pcie_ep(rcar);
> > +
> 
> So now you have two places with the controller mode initialization:
> 1. rcar_gen4_pcie_of_match
> 2. rcar_gen4_add_dw_pcie_rp() and rcar_gen4_add_dw_pcie_ep()
> It looks a bit clumsy and less maintainable than could be. What I
> suggest is to create a new method which would do what is done above,
> but also would initialize the rcar_gen4_pcie->mode field:
> 
> static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar)
> {
> 	rcar->mode = device_get_match_data(&rcar->pdev->dev);
> 	switch (rcar->mode) {
> 	case DW_PCIE_RC_TYPE:
> 		return rcar_gen4_add_dw_pcie_rp(rcar);
> 	case DW_PCIE_EP_TYPE:
> 		return rcar_gen4_add_dw_pcie_ep(rcar);
> 	}
> 
> 	return -EINVAL;
> }

Thank you for your suggestion. This code is better than my code.

> Of course the rcar_gen4_pcie->mode field initialization should be
> dropped from the rcar_gen4_add_dw_pcie_rp() and
> rcar_gen4_add_dw_pcie_ep() methods.
> 
> and ...
> 
> >  	if (err)
> >  		goto err_unprepare;
> >
> > @@ -356,12 +475,23 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev)
> >  {
> >  	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> >
> 
> > -	rcar_gen4_remove_dw_pcie_rp(rcar);
> > +	if (rcar->mode == DW_PCIE_RC_TYPE)
> > +		rcar_gen4_remove_dw_pcie_rp(rcar);
> > +	else if (rcar->mode == DW_PCIE_EP_TYPE)
> > +		rcar_gen4_remove_dw_pcie_ep(rcar);
> > +
> 
> ... similarly I would have added a respective antagonist:
> 
> static void rcar_gen4_remove_dw_pcie(struct rcar_gen4_pcie *rcar)
> {
> 	switch (rcar->mode) {
> 	case DW_PCIE_RC_TYPE:
> 		rcar_gen4_remove_dw_pcie_rp(rcar);
> 		break;
> 	case DW_PCIE_EP_TYPE:
> 		rcar_gen4_remove_dw_pcie_ep(rcar);
> 		break;
> 	}
> }
> 
> which would have been called from here instead of the open-coded
> switch-case statement. Thus the driver design will be preserved (a set
> of the init/deinit, add/remove, probe/remove and functional helper
> methods) and the mode initialization will be localized in a single
> place.

I understood it. I'll fix them on v23.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> >  	rcar_gen4_pcie_unprepare(rcar);
> >  }
> >
> >  static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > -	{ .compatible = "renesas,rcar-gen4-pcie", },
> > +	{
> > +		.compatible = "renesas,rcar-gen4-pcie",
> > +		.data = (void *)DW_PCIE_RC_TYPE,
> > +	},
> > +	{
> > +		.compatible = "renesas,rcar-gen4-pcie-ep",
> > +		.data = (void *)DW_PCIE_EP_TYPE,
> > +	},
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, rcar_gen4_pcie_of_match);
> > --
> > 2.25.1
> >
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index bc69fcab2e2a..e7fd37717998 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -429,4 +429,15 @@  config PCIE_RCAR_GEN4_HOST
 	  To compile this driver as a module, choose M here: the module will be
 	  called pcie-rcar-gen4.ko. This uses the DesignWare core.
 
+config PCIE_RCAR_GEN4_EP
+	tristate "Renesas R-Car Gen4 PCIe controller (endpoint mode)"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on PCI_ENDPOINT
+	select PCIE_DW_EP
+	select PCIE_RCAR_GEN4
+	help
+	  Say Y here if you want PCIe controller (endpoint mode) on R-Car Gen4
+	  SoCs. To compile this driver as a module, choose M here: the module
+	  will be called pcie-rcar-gen4.ko. This uses the DesignWare core.
+
 endmenu
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index f6b3c3ef187c..d1b31ea909ba 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -45,6 +45,9 @@ 
 #define RCAR_NUM_SPEED_CHANGE_RETRIES	10
 #define RCAR_MAX_LINK_SPEED		4
 
+#define RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET	0x1000
+#define RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET	0x800
+
 struct rcar_gen4_pcie {
 	struct dw_pcie dw;
 	void __iomem *base;
@@ -53,6 +56,7 @@  struct rcar_gen4_pcie {
 };
 #define to_rcar_gen4_pcie(_dw)	container_of(_dw, struct rcar_gen4_pcie, dw)
 
+/* Common */
 static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
 					bool enable)
 {
@@ -311,6 +315,9 @@  static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
 {
 	struct dw_pcie_rp *pp = &rcar->dw.pp;
 
+	if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_HOST))
+		return -ENODEV;
+
 	pp->num_vectors = MAX_MSI_IRQS;
 	pp->ops = &rcar_gen4_pcie_host_ops;
 	rcar->mode = DW_PCIE_RC_TYPE;
@@ -323,8 +330,114 @@  static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
 	dw_pcie_host_deinit(&rcar->dw.pp);
 }
 
+/* Endpoind mode */
+static void rcar_gen4_pcie_ep_pre_init(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+	int ret;
+
+	ret = rcar_gen4_pcie_common_init(rcar);
+	if (ret)
+		return;
+
+	writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
+}
+
+static void rcar_gen4_pcie_ep_init(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	enum pci_barno bar;
+
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
+		dw_pcie_ep_reset_bar(pci, bar);
+}
+
+static void rcar_gen4_pcie_ep_deinit(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+
+	writel(0, rcar->base + PCIEDMAINTSTSEN);
+	rcar_gen4_pcie_common_deinit(rcar);
+}
+
+static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+				       unsigned int type, u16 interrupt_num)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
+
+	switch (type) {
+	case PCI_IRQ_LEGACY:
+		return dw_pcie_ep_raise_legacy_irq(ep, func_no);
+	case PCI_IRQ_MSI:
+		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+	default:
+		dev_err(dw->dev, "Unknown IRQ type\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
+	.linkup_notifier = false,
+	.msi_capable = true,
+	.msix_capable = false,
+	.reserved_bar = 1 << BAR_1 | 1 << BAR_3 | 1 << BAR_5,
+	.align = SZ_1M,
+};
+
+static const struct pci_epc_features*
+rcar_gen4_pcie_ep_get_features(struct dw_pcie_ep *ep)
+{
+	return &rcar_gen4_pcie_epc_features;
+}
+
+static unsigned int rcar_gen4_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
+						       u8 func_no)
+{
+	return func_no * RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET;
+}
+
+static unsigned int rcar_gen4_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
+						      u8 func_no)
+{
+	return func_no * RCAR_GEN4_PCIE_EP_FUNC_DBI2_OFFSET;
+}
+
+static const struct dw_pcie_ep_ops pcie_ep_ops = {
+	.pre_init = rcar_gen4_pcie_ep_pre_init,
+	.ep_init = rcar_gen4_pcie_ep_init,
+	.deinit = rcar_gen4_pcie_ep_deinit,
+	.raise_irq = rcar_gen4_pcie_ep_raise_irq,
+	.get_features = rcar_gen4_pcie_ep_get_features,
+	.func_conf_select = rcar_gen4_pcie_ep_func_conf_select,
+	.get_dbi2_offset = rcar_gen4_pcie_ep_get_dbi2_offset,
+};
+
+static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
+{
+	struct dw_pcie_ep *ep = &rcar->dw.ep;
+
+	if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
+		return -ENODEV;
+
+	rcar->mode = DW_PCIE_EP_TYPE;
+	ep->ops = &pcie_ep_ops;
+
+	return dw_pcie_ep_init(ep);
+}
+
+static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
+{
+	dw_pcie_ep_exit(&rcar->dw.ep);
+}
+
+/* Common */
 static int rcar_gen4_pcie_probe(struct platform_device *pdev)
 {
+	enum dw_pcie_device_mode mode;
 	struct rcar_gen4_pcie *rcar;
 	int err;
 
@@ -340,7 +453,13 @@  static int rcar_gen4_pcie_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	err = rcar_gen4_add_dw_pcie_rp(rcar);
+	mode = (enum dw_pcie_device_mode)of_device_get_match_data(&pdev->dev);
+
+	if (mode == DW_PCIE_RC_TYPE)
+		err = rcar_gen4_add_dw_pcie_rp(rcar);
+	else if (mode == DW_PCIE_EP_TYPE)
+		err = rcar_gen4_add_dw_pcie_ep(rcar);
+
 	if (err)
 		goto err_unprepare;
 
@@ -356,12 +475,23 @@  static void rcar_gen4_pcie_remove(struct platform_device *pdev)
 {
 	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
 
-	rcar_gen4_remove_dw_pcie_rp(rcar);
+	if (rcar->mode == DW_PCIE_RC_TYPE)
+		rcar_gen4_remove_dw_pcie_rp(rcar);
+	else if (rcar->mode == DW_PCIE_EP_TYPE)
+		rcar_gen4_remove_dw_pcie_ep(rcar);
+
 	rcar_gen4_pcie_unprepare(rcar);
 }
 
 static const struct of_device_id rcar_gen4_pcie_of_match[] = {
-	{ .compatible = "renesas,rcar-gen4-pcie", },
+	{
+		.compatible = "renesas,rcar-gen4-pcie",
+		.data = (void *)DW_PCIE_RC_TYPE,
+	},
+	{
+		.compatible = "renesas,rcar-gen4-pcie-ep",
+		.data = (void *)DW_PCIE_EP_TYPE,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, rcar_gen4_pcie_of_match);