[PATCH v11 00/13] PCI: rcar-gen4: Add R-Car Gen4 PCIe support

[PATCH v11 00/13] PCI: rcar-gen4: Add R-Car Gen4 PCIe support

[Yoshihiro Shimoda]

Add R-Car S4-8 (R-Car Gen4) PCIe Host and Endpoint support.
To support them, modify PCIe DesignWare common codes.

Patch [1/12] and [2/12] are bug fix patches.

Changes from v10:
https://patchwork.kernel.org/project/linux-pci/cover/20230308082352.491561-1-yoshihiro.shimoda.uh@renesas.com/
 - Fix dt-bindings doc for endpoint (reported by Rob's bot).
 - Add reg and reg-names to the dt-bindings doc of host.
 - Fix examples in the dt-bindings docs of both host and endpoint.
 - Add R-Car S4-8 device ID into the pci_test_endpoint driver.

Changes from v9:
https://lore.kernel.org/linux-pci/20230210134917.2909314-1-yoshihiro.shimoda.uh@renesas.com/
 - Based on next-20230306
 - Add bug fix patches into this patch series.
   https://lore.kernel.org/linux-pci/20230216092012.3256440-1-yoshihiro.shimoda.uh@renesas.com/
   https://lore.kernel.org/linux-pci/20230222015327.3585691-1-yoshihiro.shimoda.uh@renesas.com/
 - Add maximum for max-link-speed and num-lanes to dt-bindings of both host and endpoint.
 - Add max-functions to dt-bindings of endpoint.
 - Use reg-names "app" on endpoint.
 - Remove unnecessary linkup and wait process in rcar_gen4_pcie_host_init().
 - Remove unnecessary macros in pcie-rcar-gen4.h.
 - Use dbi2 to write BAR mask registers.
 - Remove no_msix and intx_by_atu flags.
 - Reduce __dw_pcie_prog_outbound_atu() arguments.
 - Add dw_pcie_num_lanes_setup() to setup num_lanes.
 - Refactor dw_pcie_setup() to avoid PCIE_PORT_LINK_CONTROL writing twice.

Yoshihiro Shimoda (13):
  PCI: dwc: Fix writing wrong value if snps,enable-cdm-check
  PCI: endpoint: functions/pci-epf-test: Fix dma_chan direction
  dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host
  dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint
  PCI: dwc: Refactor PCIE_PORT_LINK_CONTROL handling
  PCI: Add PCI_EXP_LNKCAP_MLW macros
  PCI: designware-ep: Expose dw_pcie_ep_exit() to module
  PCI: dwc: Add dw_pcie_num_lanes_setup()
  PCI: dwc: Add support for triggering legacy IRQs
  PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support
  PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support
  MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4
  misc: pci_endpoint_test: Add Device ID for R-Car S4-8 PCIe controller

 .../bindings/pci/rcar-gen4-pci-ep.yaml        |  97 ++++++++++
 .../bindings/pci/rcar-gen4-pci-host.yaml      | 108 +++++++++++
 MAINTAINERS                                   |   1 +
 drivers/misc/pci_endpoint_test.c              |   4 +
 drivers/pci/controller/dwc/Kconfig            |  18 ++
 drivers/pci/controller/dwc/Makefile           |   4 +
 .../pci/controller/dwc/pcie-designware-ep.c   |  70 +++++++-
 drivers/pci/controller/dwc/pcie-designware.c  | 130 +++++++++-----
 drivers/pci/controller/dwc/pcie-designware.h  |  18 +-
 .../pci/controller/dwc/pcie-rcar-gen4-ep.c    | 170 ++++++++++++++++++
 .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 134 ++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 156 ++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  56 ++++++
 drivers/pci/controller/dwc/pcie-tegra194.c    |   5 +-
 drivers/pci/endpoint/functions/pci-epf-test.c |   2 +-
 include/uapi/linux/pci_regs.h                 |   6 +
 16 files changed, 919 insertions(+), 60 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h

-- 
2.25.1

[PATCH v11 01/13] PCI: dwc: Fix writing wrong value if snps,enable-cdm-check

[Yoshihiro Shimoda]

The "val" of PCIE_PORT_LINK_CONTROL will be reused on the
"Set the number of lanes". But, if snps,enable-cdm-check" exists,
the "val" will be set to PCIE_PL_CHK_REG_CONTROL_STATUS.
Therefore, unexpected register value is possible to be used
to PCIE_PORT_LINK_CONTROL register if snps,enable-cdm-check" exists.
So, change reading timing of PCIE_PORT_LINK_CONTROL register to fix
the issue.

Fixes: ec7b952f453c ("PCI: dwc: Always enable CDM check if "snps,enable-cdm-check" exists")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 53a16b8b6ac2..8e33e6e59e68 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -1001,11 +1001,6 @@ void dw_pcie_setup(struct dw_pcie *pci)
 		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
 	}
 
-	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
-	val &= ~PORT_LINK_FAST_LINK_MODE;
-	val |= PORT_LINK_DLL_LINK_EN;
-	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
-
 	if (dw_pcie_cap_is(pci, CDM_CHECK)) {
 		val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS);
 		val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS |
@@ -1013,6 +1008,11 @@ void dw_pcie_setup(struct dw_pcie *pci)
 		dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, val);
 	}
 
+	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
+	val &= ~PORT_LINK_FAST_LINK_MODE;
+	val |= PORT_LINK_DLL_LINK_EN;
+	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
+
 	if (!pci->num_lanes) {
 		dev_dbg(pci->dev, "Using h/w default number of lanes\n");
 		return;
-- 
2.25.1

Re: [PATCH v11 01/13] PCI: dwc: Fix writing wrong value if snps,enable-cdm-check

[Serge Semin]

On Fri, Mar 10, 2023 at 09:34:58PM +0900, Yoshihiro Shimoda wrote:
> The "val" of PCIE_PORT_LINK_CONTROL will be reused on the
> "Set the number of lanes". But, if snps,enable-cdm-check" exists,
> the "val" will be set to PCIE_PL_CHK_REG_CONTROL_STATUS.
> Therefore, unexpected register value is possible to be used
> to PCIE_PORT_LINK_CONTROL register if snps,enable-cdm-check" exists.
> So, change reading timing of PCIE_PORT_LINK_CONTROL register to fix
> the issue.

My version of the commit log:
< If CDM_CHECK capability is set then the local variable 'val' will be
< overwritten in the dw_pcie_setup() method in the PL_CHK register
< initialization procedure. Thus further variable usage in the framework of
< the PCIE_PORT_LINK_CONTROL register initialization at the very least must
< imply the variable re-initialization. Alas it hasn't been taken into
< account in the commit ec7b952f453c ("PCI: dwc: Always enable CDM check if
< "snps,enable-cdm-check" exists"). Due to that the PCIE_PORT_LINK_CONTROL
< register will be written with an improper value in case if the CDM-check
< is enabled. Let's fix this by moving the PCIE_PORT_LINK_CONTROL CSR
< updated to be fully performed after the PL_CHK register
< initialization.

> 
> Fixes: ec7b952f453c ("PCI: dwc: Always enable CDM check if "snps,enable-cdm-check" exists")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Looks good. Thanks.
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

@Bjorn, if it's possible could you please take this patch to a
fixes(-ish) branch of your tree and merge it in the next rc-cycle?
It definitely fixes a bug in the DW PCIe core driver.

-Serge(y)

> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 53a16b8b6ac2..8e33e6e59e68 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -1001,11 +1001,6 @@ void dw_pcie_setup(struct dw_pcie *pci)
>  		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>  	}
>  
> -	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> -	val &= ~PORT_LINK_FAST_LINK_MODE;
> -	val |= PORT_LINK_DLL_LINK_EN;
> -	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
> -
>  	if (dw_pcie_cap_is(pci, CDM_CHECK)) {
>  		val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS);
>  		val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS |
> @@ -1013,6 +1008,11 @@ void dw_pcie_setup(struct dw_pcie *pci)
>  		dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, val);
>  	}
>  
> +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> +	val &= ~PORT_LINK_FAST_LINK_MODE;
> +	val |= PORT_LINK_DLL_LINK_EN;
> +	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
> +
>  	if (!pci->num_lanes) {
>  		dev_dbg(pci->dev, "Using h/w default number of lanes\n");
>  		return;
> -- 
> 2.25.1
> 
> 

Re: [PATCH v11 01/13] PCI: dwc: Fix writing wrong value if snps,enable-cdm-check

[Bjorn Helgaas]

On Tue, Mar 21, 2023 at 12:02:55PM +0300, Serge Semin wrote:
> On Fri, Mar 10, 2023 at 09:34:58PM +0900, Yoshihiro Shimoda wrote:
> > The "val" of PCIE_PORT_LINK_CONTROL will be reused on the
> > "Set the number of lanes". But, if snps,enable-cdm-check" exists,
> > the "val" will be set to PCIE_PL_CHK_REG_CONTROL_STATUS.
> > Therefore, unexpected register value is possible to be used
> > to PCIE_PORT_LINK_CONTROL register if snps,enable-cdm-check" exists.
> > So, change reading timing of PCIE_PORT_LINK_CONTROL register to fix
> > the issue.
> 
> My version of the commit log:
> < If CDM_CHECK capability is set then the local variable 'val' will be
> < overwritten in the dw_pcie_setup() method in the PL_CHK register
> < initialization procedure. Thus further variable usage in the framework of
> < the PCIE_PORT_LINK_CONTROL register initialization at the very least must
> < imply the variable re-initialization. Alas it hasn't been taken into
> < account in the commit ec7b952f453c ("PCI: dwc: Always enable CDM check if
> < "snps,enable-cdm-check" exists"). Due to that the PCIE_PORT_LINK_CONTROL
> < register will be written with an improper value in case if the CDM-check
> < is enabled. Let's fix this by moving the PCIE_PORT_LINK_CONTROL CSR
> < updated to be fully performed after the PL_CHK register
> < initialization.
> 
> > 
> > Fixes: ec7b952f453c ("PCI: dwc: Always enable CDM check if "snps,enable-cdm-check" exists")
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> Looks good. Thanks.
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> 
> @Bjorn, if it's possible could you please take this patch to a
> fixes(-ish) branch of your tree and merge it in the next rc-cycle?
> It definitely fixes a bug in the DW PCIe core driver.

I applied this patch only to for-linus for v6.3.  I adapted the commit
message as follows, let me know if you spot a mistake:

  PCI: dwc: Fix PORT_LINK_CONTROL update when CDM check enabled
  
  If CDM_CHECK is enabled (by the DT "snps,enable-cdm-check" property), 'val'
  is overwritten by PCIE_PL_CHK_REG_CONTROL_STATUS initialization.  Commit
  ec7b952f453c ("PCI: dwc: Always enable CDM check if "snps,enable-cdm-check"
  exists") did not account for further usage of 'val', so we wrote improper
  values to PCIE_PORT_LINK_CONTROL when the CDM check is enabled.
  
  Move the PCIE_PORT_LINK_CONTROL update to be completely after the
  PCIE_PL_CHK_REG_CONTROL_STATUS register initialization.
  
  [bhelgaas: commit log adapted from Serge's version]

> > ---
> >  drivers/pci/controller/dwc/pcie-designware.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 53a16b8b6ac2..8e33e6e59e68 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -1001,11 +1001,6 @@ void dw_pcie_setup(struct dw_pcie *pci)
> >  		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> >  	}
> >  
> > -	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> > -	val &= ~PORT_LINK_FAST_LINK_MODE;
> > -	val |= PORT_LINK_DLL_LINK_EN;
> > -	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
> > -
> >  	if (dw_pcie_cap_is(pci, CDM_CHECK)) {
> >  		val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS);
> >  		val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS |
> > @@ -1013,6 +1008,11 @@ void dw_pcie_setup(struct dw_pcie *pci)
> >  		dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, val);
> >  	}
> >  
> > +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> > +	val &= ~PORT_LINK_FAST_LINK_MODE;
> > +	val |= PORT_LINK_DLL_LINK_EN;
> > +	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
> > +
> >  	if (!pci->num_lanes) {
> >  		dev_dbg(pci->dev, "Using h/w default number of lanes\n");
> >  		return;
> > -- 
> > 2.25.1
> > 
> > 

Re: [PATCH v11 01/13] PCI: dwc: Fix writing wrong value if snps,enable-cdm-check

[Serge Semin]

On Tue, Mar 21, 2023 at 01:52:28PM -0500, Bjorn Helgaas wrote:
> On Tue, Mar 21, 2023 at 12:02:55PM +0300, Serge Semin wrote:
> > On Fri, Mar 10, 2023 at 09:34:58PM +0900, Yoshihiro Shimoda wrote:
> > > The "val" of PCIE_PORT_LINK_CONTROL will be reused on the
> > > "Set the number of lanes". But, if snps,enable-cdm-check" exists,
> > > the "val" will be set to PCIE_PL_CHK_REG_CONTROL_STATUS.
> > > Therefore, unexpected register value is possible to be used
> > > to PCIE_PORT_LINK_CONTROL register if snps,enable-cdm-check" exists.
> > > So, change reading timing of PCIE_PORT_LINK_CONTROL register to fix
> > > the issue.
> > 
> > My version of the commit log:
> > < If CDM_CHECK capability is set then the local variable 'val' will be
> > < overwritten in the dw_pcie_setup() method in the PL_CHK register
> > < initialization procedure. Thus further variable usage in the framework of
> > < the PCIE_PORT_LINK_CONTROL register initialization at the very least must
> > < imply the variable re-initialization. Alas it hasn't been taken into
> > < account in the commit ec7b952f453c ("PCI: dwc: Always enable CDM check if
> > < "snps,enable-cdm-check" exists"). Due to that the PCIE_PORT_LINK_CONTROL
> > < register will be written with an improper value in case if the CDM-check
> > < is enabled. Let's fix this by moving the PCIE_PORT_LINK_CONTROL CSR
> > < updated to be fully performed after the PL_CHK register
> > < initialization.
> > 
> > > 
> > > Fixes: ec7b952f453c ("PCI: dwc: Always enable CDM check if "snps,enable-cdm-check" exists")
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > 
> > Looks good. Thanks.
> > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> > 
> > @Bjorn, if it's possible could you please take this patch to a
> > fixes(-ish) branch of your tree and merge it in the next rc-cycle?
> > It definitely fixes a bug in the DW PCIe core driver.
> 

> I applied this patch only to for-linus for v6.3.  I adapted the commit
> message as follows, let me know if you spot a mistake:
> 
>   PCI: dwc: Fix PORT_LINK_CONTROL update when CDM check enabled
>   
>   If CDM_CHECK is enabled (by the DT "snps,enable-cdm-check" property), 'val'
>   is overwritten by PCIE_PL_CHK_REG_CONTROL_STATUS initialization.  Commit
>   ec7b952f453c ("PCI: dwc: Always enable CDM check if "snps,enable-cdm-check"
>   exists") did not account for further usage of 'val', so we wrote improper
>   values to PCIE_PORT_LINK_CONTROL when the CDM check is enabled.
>   
>   Move the PCIE_PORT_LINK_CONTROL update to be completely after the
>   PCIE_PL_CHK_REG_CONTROL_STATUS register initialization.
>   
>   [bhelgaas: commit log adapted from Serge's version]

Sounds good. Thanks!

* I'll keep reviewing the rest of the patches in the series. There are
several aspects which I think is possible to improve/optimize.

-Serge(y)

> 
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > > index 53a16b8b6ac2..8e33e6e59e68 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > @@ -1001,11 +1001,6 @@ void dw_pcie_setup(struct dw_pcie *pci)
> > >  		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> > >  	}
> > >  
> > > -	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> > > -	val &= ~PORT_LINK_FAST_LINK_MODE;
> > > -	val |= PORT_LINK_DLL_LINK_EN;
> > > -	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
> > > -
> > >  	if (dw_pcie_cap_is(pci, CDM_CHECK)) {
> > >  		val = dw_pcie_readl_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS);
> > >  		val |= PCIE_PL_CHK_REG_CHK_REG_CONTINUOUS |
> > > @@ -1013,6 +1008,11 @@ void dw_pcie_setup(struct dw_pcie *pci)
> > >  		dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, val);
> > >  	}
> > >  
> > > +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
> > > +	val &= ~PORT_LINK_FAST_LINK_MODE;
> > > +	val |= PORT_LINK_DLL_LINK_EN;
> > > +	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
> > > +
> > >  	if (!pci->num_lanes) {
> > >  		dev_dbg(pci->dev, "Using h/w default number of lanes\n");
> > >  		return;
> > > -- 
> > > 2.25.1
> > > 
> > > 

[PATCH v11 02/13] PCI: endpoint: functions/pci-epf-test: Fix dma_chan direction

[Yoshihiro Shimoda]

In the pci_epf_test_init_dma_chan(), epf_test->dma_chan_rx
is assigned from dma_request_channel() with DMA_DEV_TO_MEM as
filter.dma_mask. However, in the pci_epf_test_data_transfer(),
if the dir is DMA_DEV_TO_MEM, it should use epf->dma_chan_rx,
but it used epf_test->dma_chan_tx. So, fix it. Otherwise,
results of pcitest with enabled DMA will be "NOT OKAY" on eDMA
environment.

Fixes: 8353813c88ef ("PCI: endpoint: Enable DMA tests for endpoints with DMA capabilities")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 0f9d2ec822ac..172e5ac0bd96 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -112,7 +112,7 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
 				      size_t len, dma_addr_t dma_remote,
 				      enum dma_transfer_direction dir)
 {
-	struct dma_chan *chan = (dir == DMA_DEV_TO_MEM) ?
+	struct dma_chan *chan = (dir == DMA_MEM_TO_DEV) ?
 				 epf_test->dma_chan_tx : epf_test->dma_chan_rx;
 	dma_addr_t dma_local = (dir == DMA_MEM_TO_DEV) ? dma_src : dma_dst;
 	enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
-- 
2.25.1

Re: [PATCH v11 02/13] PCI: endpoint: functions/pci-epf-test: Fix dma_chan direction

[Kunihiko Hayashi]

Hi Shimoda-san,

On 2023/03/10 21:34, Yoshihiro Shimoda wrote:
> In the pci_epf_test_init_dma_chan(), epf_test->dma_chan_rx
> is assigned from dma_request_channel() with DMA_DEV_TO_MEM as
> filter.dma_mask. However, in the pci_epf_test_data_transfer(),
> if the dir is DMA_DEV_TO_MEM, it should use epf->dma_chan_rx,
> but it used epf_test->dma_chan_tx. So, fix it. Otherwise,
> results of pcitest with enabled DMA will be "NOT OKAY" on eDMA
> environment.

I also encounted this issue and found this patch before sending my fixes patch
for the same diff as this one.
And I confirmed the issue is fixed using pcitest with eDMA on UniPhier SoCs.

For 02/13 patch if not already merged:

Tested-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Thank you,

> 
> Fixes: 8353813c88ef ("PCI: endpoint: Enable DMA tests for endpoints with
> DMA capabilities")
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>   drivers/pci/endpoint/functions/pci-epf-test.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
> b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 0f9d2ec822ac..172e5ac0bd96 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -112,7 +112,7 @@ static int pci_epf_test_data_transfer(struct
> pci_epf_test *epf_test,
>   				      size_t len, dma_addr_t dma_remote,
>   				      enum dma_transfer_direction dir)
>   {
> -	struct dma_chan *chan = (dir == DMA_DEV_TO_MEM) ?
> +	struct dma_chan *chan = (dir == DMA_MEM_TO_DEV) ?
>   				 epf_test->dma_chan_tx :
> epf_test->dma_chan_rx;
>   	dma_addr_t dma_local = (dir == DMA_MEM_TO_DEV) ? dma_src :
> dma_dst;
>   	enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;

---
Best Regards
Kunihiko Hayashi

RE: [PATCH v11 02/13] PCI: endpoint: functions/pci-epf-test: Fix dma_chan direction

[Yoshihiro Shimoda]

Hi Hayashi-san,

> From: Kunihiko Hayashi, Sent: Wednesday, April 12, 2023 1:23 PM
> 
> Hi Shimoda-san,
> 
> On 2023/03/10 21:34, Yoshihiro Shimoda wrote:
> > In the pci_epf_test_init_dma_chan(), epf_test->dma_chan_rx
> > is assigned from dma_request_channel() with DMA_DEV_TO_MEM as
> > filter.dma_mask. However, in the pci_epf_test_data_transfer(),
> > if the dir is DMA_DEV_TO_MEM, it should use epf->dma_chan_rx,
> > but it used epf_test->dma_chan_tx. So, fix it. Otherwise,
> > results of pcitest with enabled DMA will be "NOT OKAY" on eDMA
> > environment.
> 
> I also encounted this issue and found this patch before sending my fixes patch
> for the same diff as this one.
> And I confirmed the issue is fixed using pcitest with eDMA on UniPhier SoCs.
> 
> For 02/13 patch if not already merged:
> 
> Tested-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Thank you very much for your Tested-by!
Since I need more time to fix v12 patch series for some reasons,
I'll send this patch independently.

Best regards,
Yoshihiro Shimoda

> Thank you,
> 
> >
> > Fixes: 8353813c88ef ("PCI: endpoint: Enable DMA tests for endpoints with
> > DMA capabilities")
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >   drivers/pci/endpoint/functions/pci-epf-test.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
> > b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 0f9d2ec822ac..172e5ac0bd96 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -112,7 +112,7 @@ static int pci_epf_test_data_transfer(struct
> > pci_epf_test *epf_test,
> >   				      size_t len, dma_addr_t dma_remote,
> >   				      enum dma_transfer_direction dir)
> >   {
> > -	struct dma_chan *chan = (dir == DMA_DEV_TO_MEM) ?
> > +	struct dma_chan *chan = (dir == DMA_MEM_TO_DEV) ?
> >   				 epf_test->dma_chan_tx :
> > epf_test->dma_chan_rx;
> >   	dma_addr_t dma_local = (dir == DMA_MEM_TO_DEV) ? dma_src :
> > dma_dst;
> >   	enum dma_ctrl_flags flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
> 
> ---
> Best Regards
> Kunihiko Hayashi

[PATCH v11 03/13] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host

[Yoshihiro Shimoda]

Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
PCIe host module.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/pci/rcar-gen4-pci-host.yaml      | 108 ++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml

diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
new file mode 100644
index 000000000000..d838d7c50410
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-host.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car Gen4 PCIe Host
+
+maintainers:
+  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
+
+allOf:
+  - $ref: snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: renesas,r8a779f0-pcie   # R-Car S4-8
+      - const: renesas,rcar-gen4-pcie  # R-Car Gen4
+
+  reg:
+    maxItems: 5
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: atu
+      - const: dma
+      - const: app
+      - const: config
+
+  interrupts:
+    maxItems: 4
+
+  interrupt-names:
+    items:
+      - const: msi
+      - const: dma
+      - const: sft_ce
+      - const: app
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  max-link-speed:
+    maximum: 4
+
+  num-lanes:
+    maximum: 4
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - power-domains
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/r8a779f0-sysc.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie: pcie@e65d0000 {
+            compatible = "renesas,r8a779f0-pcie", "renesas,rcar-gen4-pcie";
+            reg = <0 0xe65d0000 0 0x3000>, <0 0xe65d3000 0 0x2000>,
+                  <0 0xe65d5000 0 0x1200>, <0 0xe65d6200 0 0x0e00>,
+                  <0 0xfe000000 0 0x400000>;
+            reg-names = "dbi", "atu", "dma", "app", "config";
+            #address-cells = <3>;
+            #size-cells = <2>;
+            bus-range = <0x00 0xff>;
+            device_type = "pci";
+            ranges = <0x81000000 0 0x00000000 0 0xfe000000 0 0x00400000
+                      0x82000000 0 0x30000000 0 0x30000000 0 0x10000000>;
+            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000>;
+            interrupts = <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "msi", "dma", "sft_ce", "app";
+            #interrupt-cells = <1>;
+            interrupt-map-mask = <0 0 0 7>;
+            interrupt-map = <0 0 0 1 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+                             0 0 0 2 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+                             0 0 0 3 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
+                             0 0 0 4 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cpg CPG_MOD 624>;
+            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
+            resets = <&cpg 624>;
+            num-lanes = <2>;
+            snps,enable-cdm-check;
+            max-link-speed = <2>;
+        };
+    };
-- 
2.25.1

Re: [PATCH v11 03/13] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host

[Serge Semin]

On Fri, Mar 10, 2023 at 09:35:00PM +0900, Yoshihiro Shimoda wrote:
> Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
> PCIe host module.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Looks good.
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

> ---
>  .../bindings/pci/rcar-gen4-pci-host.yaml      | 108 ++++++++++++++++++
>  1 file changed, 108 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> new file mode 100644
> index 000000000000..d838d7c50410
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> @@ -0,0 +1,108 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-host.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas R-Car Gen4 PCIe Host
> +
> +maintainers:
> +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> +
> +allOf:
> +  - $ref: snps,dw-pcie.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: renesas,r8a779f0-pcie   # R-Car S4-8
> +      - const: renesas,rcar-gen4-pcie  # R-Car Gen4
> +
> +  reg:
> +    maxItems: 5
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: atu
> +      - const: dma
> +      - const: app
> +      - const: config
> +
> +  interrupts:
> +    maxItems: 4
> +
> +  interrupt-names:
> +    items:
> +      - const: msi
> +      - const: dma
> +      - const: sft_ce

> +      - const: app

Just curious what is this IRQ used for?

-Serge(y)

> +
> +  clocks:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  max-link-speed:
> +    maximum: 4
> +
> +  num-lanes:
> +    maximum: 4
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - power-domains
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/r8a779f0-sysc.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcie: pcie@e65d0000 {
> +            compatible = "renesas,r8a779f0-pcie", "renesas,rcar-gen4-pcie";
> +            reg = <0 0xe65d0000 0 0x3000>, <0 0xe65d3000 0 0x2000>,
> +                  <0 0xe65d5000 0 0x1200>, <0 0xe65d6200 0 0x0e00>,
> +                  <0 0xfe000000 0 0x400000>;
> +            reg-names = "dbi", "atu", "dma", "app", "config";
> +            #address-cells = <3>;
> +            #size-cells = <2>;
> +            bus-range = <0x00 0xff>;
> +            device_type = "pci";
> +            ranges = <0x81000000 0 0x00000000 0 0xfe000000 0 0x00400000
> +                      0x82000000 0 0x30000000 0 0x30000000 0 0x10000000>;
> +            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000>;
> +            interrupts = <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "msi", "dma", "sft_ce", "app";
> +            #interrupt-cells = <1>;
> +            interrupt-map-mask = <0 0 0 7>;
> +            interrupt-map = <0 0 0 1 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> +                             0 0 0 2 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> +                             0 0 0 3 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> +                             0 0 0 4 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&cpg CPG_MOD 624>;
> +            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> +            resets = <&cpg 624>;
> +            num-lanes = <2>;
> +            snps,enable-cdm-check;
> +            max-link-speed = <2>;
> +        };
> +    };
> -- 
> 2.25.1
> 
> 

RE: [PATCH v11 03/13] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host

[Yoshihiro Shimoda]

Hi Serge,

> From: Serge Semin, Sent: Tuesday, March 21, 2023 8:36 PM
> 
> On Fri, Mar 10, 2023 at 09:35:00PM +0900, Yoshihiro Shimoda wrote:
> > Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
> > PCIe host module.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> 
> Looks good.
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

Thank you for your review!

> > ---
> >  .../bindings/pci/rcar-gen4-pci-host.yaml      | 108 ++++++++++++++++++
> >  1 file changed, 108 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> > new file mode 100644
> > index 000000000000..d838d7c50410
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> > @@ -0,0 +1,108 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2022 Renesas Electronics Corp.
> > +%YAML 1.2
> > +---
> > +$id:
<snip>
> > +$schema:
<snip>
> > +
> > +title: Renesas R-Car Gen4 PCIe Host
> > +
> > +maintainers:
> > +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > +
> > +allOf:
> > +  - $ref: snps,dw-pcie.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: renesas,r8a779f0-pcie   # R-Car S4-8
> > +      - const: renesas,rcar-gen4-pcie  # R-Car Gen4
> > +
> > +  reg:
> > +    maxItems: 5
> > +
> > +  reg-names:
> > +    items:
> > +      - const: dbi
> > +      - const: atu
> > +      - const: dma
> > +      - const: app
> > +      - const: config
> > +
> > +  interrupts:
> > +    maxItems: 4
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: msi
> > +      - const: dma
> > +      - const: sft_ce
> 
> > +      - const: app
> 
> Just curious what is this IRQ used for?

The pcie-rcar-gen4 drivers (host and endpoint) doesn't use this IRQ,
But the hardware issues the IRQ when it receives a vendor defined message
from remote device.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  max-link-speed:
> > +    maximum: 4
> > +
> > +  num-lanes:
> > +    maximum: 4
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - power-domains
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/power/r8a779f0-sysc.h>
> > +
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        pcie: pcie@e65d0000 {
> > +            compatible = "renesas,r8a779f0-pcie", "renesas,rcar-gen4-pcie";
> > +            reg = <0 0xe65d0000 0 0x3000>, <0 0xe65d3000 0 0x2000>,
> > +                  <0 0xe65d5000 0 0x1200>, <0 0xe65d6200 0 0x0e00>,
> > +                  <0 0xfe000000 0 0x400000>;
> > +            reg-names = "dbi", "atu", "dma", "app", "config";
> > +            #address-cells = <3>;
> > +            #size-cells = <2>;
> > +            bus-range = <0x00 0xff>;
> > +            device_type = "pci";
> > +            ranges = <0x81000000 0 0x00000000 0 0xfe000000 0 0x00400000
> > +                      0x82000000 0 0x30000000 0 0x30000000 0 0x10000000>;
> > +            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000>;
> > +            interrupts = <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> > +                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-names = "msi", "dma", "sft_ce", "app";
> > +            #interrupt-cells = <1>;
> > +            interrupt-map-mask = <0 0 0 7>;
> > +            interrupt-map = <0 0 0 1 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> > +                             0 0 0 2 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> > +                             0 0 0 3 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> > +                             0 0 0 4 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>;
> > +            clocks = <&cpg CPG_MOD 624>;
> > +            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> > +            resets = <&cpg 624>;
> > +            num-lanes = <2>;
> > +            snps,enable-cdm-check;
> > +            max-link-speed = <2>;
> > +        };
> > +    };
> > --
> > 2.25.1
> >
> >

Re: [PATCH v11 03/13] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Host

[Serge Semin]

On Wed, Mar 22, 2023 at 12:35:19AM +0000, Yoshihiro Shimoda wrote:
> Hi Serge,
> 
> > From: Serge Semin, Sent: Tuesday, March 21, 2023 8:36 PM
> > 
> > On Fri, Mar 10, 2023 at 09:35:00PM +0900, Yoshihiro Shimoda wrote:
> > > Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
> > > PCIe host module.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > 
> > Looks good.
> > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> 
> Thank you for your review!
> 
> > > ---
> > >  .../bindings/pci/rcar-gen4-pci-host.yaml      | 108 ++++++++++++++++++
> > >  1 file changed, 108 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> > b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> > > new file mode 100644
> > > index 000000000000..d838d7c50410
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-host.yaml
> > > @@ -0,0 +1,108 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +# Copyright (C) 2022 Renesas Electronics Corp.
> > > +%YAML 1.2
> > > +---
> > > +$id:
> <snip>
> > > +$schema:
> <snip>
> > > +
> > > +title: Renesas R-Car Gen4 PCIe Host
> > > +
> > > +maintainers:
> > > +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > +
> > > +allOf:
> > > +  - $ref: snps,dw-pcie.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - const: renesas,r8a779f0-pcie   # R-Car S4-8
> > > +      - const: renesas,rcar-gen4-pcie  # R-Car Gen4
> > > +
> > > +  reg:
> > > +    maxItems: 5
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: dbi
> > > +      - const: atu
> > > +      - const: dma
> > > +      - const: app
> > > +      - const: config
> > > +
> > > +  interrupts:
> > > +    maxItems: 4
> > > +
> > > +  interrupt-names:
> > > +    items:
> > > +      - const: msi
> > > +      - const: dma
> > > +      - const: sft_ce
> > 
> > > +      - const: app
> > 
> > Just curious what is this IRQ used for?
> 
> The pcie-rcar-gen4 drivers (host and endpoint) doesn't use this IRQ,
> But the hardware issues the IRQ when it receives a vendor defined message
> from remote device.

Ok. Thanks for clarification.

-Serge(y)

> 
> Best regards,
> Yoshihiro Shimoda
> 
> > -Serge(y)
> > 
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  power-domains:
> > > +    maxItems: 1
> > > +
> > > +  resets:
> > > +    maxItems: 1
> > > +
> > > +  max-link-speed:
> > > +    maximum: 4
> > > +
> > > +  num-lanes:
> > > +    maximum: 4
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - interrupts
> > > +  - clocks
> > > +  - power-domains
> > > +
> > > +unevaluatedProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +    #include <dt-bindings/power/r8a779f0-sysc.h>
> > > +
> > > +    soc {
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +
> > > +        pcie: pcie@e65d0000 {
> > > +            compatible = "renesas,r8a779f0-pcie", "renesas,rcar-gen4-pcie";
> > > +            reg = <0 0xe65d0000 0 0x3000>, <0 0xe65d3000 0 0x2000>,
> > > +                  <0 0xe65d5000 0 0x1200>, <0 0xe65d6200 0 0x0e00>,
> > > +                  <0 0xfe000000 0 0x400000>;
> > > +            reg-names = "dbi", "atu", "dma", "app", "config";
> > > +            #address-cells = <3>;
> > > +            #size-cells = <2>;
> > > +            bus-range = <0x00 0xff>;
> > > +            device_type = "pci";
> > > +            ranges = <0x81000000 0 0x00000000 0 0xfe000000 0 0x00400000
> > > +                      0x82000000 0 0x30000000 0 0x30000000 0 0x10000000>;
> > > +            dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x80000000>;
> > > +            interrupts = <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>,
> > > +                         <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
> > > +                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> > > +                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> > > +            interrupt-names = "msi", "dma", "sft_ce", "app";
> > > +            #interrupt-cells = <1>;
> > > +            interrupt-map-mask = <0 0 0 7>;
> > > +            interrupt-map = <0 0 0 1 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> > > +                             0 0 0 2 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> > > +                             0 0 0 3 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH
> > > +                             0 0 0 4 &gic GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH>;
> > > +            clocks = <&cpg CPG_MOD 624>;
> > > +            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> > > +            resets = <&cpg 624>;
> > > +            num-lanes = <2>;
> > > +            snps,enable-cdm-check;
> > > +            max-link-speed = <2>;
> > > +        };
> > > +    };
> > > --
> > > 2.25.1
> > >
> > >

[PATCH v11 04/13] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint

[Yoshihiro Shimoda]

Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
PCIe endpoint module.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/pci/rcar-gen4-pci-ep.yaml        | 97 +++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml

diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
new file mode 100644
index 000000000000..12d779a4ce8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car Gen4 PCIe Endpoint
+
+maintainers:
+  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
+
+allOf:
+  - $ref: snps,dw-pcie-ep.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: renesas,r8a779f0-pcie-ep   # R-Car S4-8
+      - const: renesas,rcar-gen4-pcie-ep  # R-Car Gen4
+
+  reg:
+    maxItems: 5
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: atu
+      - const: dma
+      - const: app
+      - const: addr_space
+
+  interrupts:
+    maxItems: 3
+
+  interrupt-names:
+    items:
+      - const: dma
+      - const: sft_ce
+      - const: app
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  max-functions:
+    maximum: 2
+
+  max-link-speed:
+    maximum: 4
+
+  num-lanes:
+    maximum: 4
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - resets
+  - power-domains
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/r8a779f0-sysc.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pcie0_ep: pcie-ep@e65d0000 {
+            compatible = "renesas,r8a779f0-pcie-ep", "renesas,rcar-gen4-pcie-ep";
+            reg = <0 0xe65d0000 0 0x3000>, <0 0xe65d3000 0 0x2000>,
+                  <0 0xe65d5000 0 0x1200>, <0 0xe65d6200 0 0x0e00>,
+                  <0 0xfe000000 0 0x400000>;
+            reg-names = "dbi", "atu", "dma", "app", "addr_space";
+            interrupts = <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "dma", "sft_ce", "app";
+            clocks = <&cpg CPG_MOD 624>;
+            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
+            resets = <&cpg 624>;
+            num-lanes = <2>;
+            max-link-speed = <2>;
+        };
+    };
-- 
2.25.1

Re: [PATCH v11 04/13] dt-bindings: PCI: renesas: Add R-Car Gen4 PCIe Endpoint

[Serge Semin]

On Fri, Mar 10, 2023 at 09:35:01PM +0900, Yoshihiro Shimoda wrote:
> Document bindings for Renesas R-Car Gen4 and R-Car S4-8 (R8A779F0)
> PCIe endpoint module.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Looking good now. Thanks.
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>

-Serge(y)

> ---
>  .../bindings/pci/rcar-gen4-pci-ep.yaml        | 97 +++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> new file mode 100644
> index 000000000000..12d779a4ce8d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/rcar-gen4-pci-ep.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 Renesas Electronics Corp.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/rcar-gen4-pci-ep.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas R-Car Gen4 PCIe Endpoint
> +
> +maintainers:
> +  - Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> +
> +allOf:
> +  - $ref: snps,dw-pcie-ep.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: renesas,r8a779f0-pcie-ep   # R-Car S4-8
> +      - const: renesas,rcar-gen4-pcie-ep  # R-Car Gen4
> +
> +  reg:
> +    maxItems: 5
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: atu
> +      - const: dma
> +      - const: app
> +      - const: addr_space
> +
> +  interrupts:
> +    maxItems: 3
> +
> +  interrupt-names:
> +    items:
> +      - const: dma
> +      - const: sft_ce
> +      - const: app
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  max-functions:
> +    maximum: 2
> +
> +  max-link-speed:
> +    maximum: 4
> +
> +  num-lanes:
> +    maximum: 4
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - resets
> +  - power-domains
> +  - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r8a779f0-cpg-mssr.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/r8a779f0-sysc.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pcie0_ep: pcie-ep@e65d0000 {
> +            compatible = "renesas,r8a779f0-pcie-ep", "renesas,rcar-gen4-pcie-ep";
> +            reg = <0 0xe65d0000 0 0x3000>, <0 0xe65d3000 0 0x2000>,
> +                  <0 0xe65d5000 0 0x1200>, <0 0xe65d6200 0 0x0e00>,
> +                  <0 0xfe000000 0 0x400000>;
> +            reg-names = "dbi", "atu", "dma", "app", "addr_space";
> +            interrupts = <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "dma", "sft_ce", "app";
> +            clocks = <&cpg CPG_MOD 624>;
> +            power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> +            resets = <&cpg 624>;
> +            num-lanes = <2>;
> +            max-link-speed = <2>;
> +        };
> +    };
> -- 
> 2.25.1
> 
> 

[PATCH v11 05/13] PCI: dwc: Refactor PCIE_PORT_LINK_CONTROL handling

[Yoshihiro Shimoda]

The previous code wrote this PCIE_PORT_LINK_CONTROL register twice
with redudant mask (PORT_LINK_FAST_LINK_MODE). So, refactor this.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 8e33e6e59e68..89b8ec29da7f 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -1008,19 +1008,13 @@ void dw_pcie_setup(struct dw_pcie *pci)
 		dw_pcie_writel_dbi(pci, PCIE_PL_CHK_REG_CONTROL_STATUS, val);
 	}
 
+	/* Set the number of lanes */
 	val = dw_pcie_readl_dbi(pci, PCIE_PORT_LINK_CONTROL);
 	val &= ~PORT_LINK_FAST_LINK_MODE;
 	val |= PORT_LINK_DLL_LINK_EN;
-	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
-
-	if (!pci->num_lanes) {
-		dev_dbg(pci->dev, "Using h/w default number of lanes\n");
-		return;
-	}
-
-	/* Set the number of lanes */
-	val &= ~PORT_LINK_FAST_LINK_MODE;
-	val &= ~PORT_LINK_MODE_MASK;
+	/* Mask LINK_MODE if num_lanes is not zero */
+	if (pci->num_lanes)
+		val &= ~PORT_LINK_MODE_MASK;
 	switch (pci->num_lanes) {
 	case 1:
 		val |= PORT_LINK_MODE_1_LANES;
@@ -1035,10 +1029,12 @@ void dw_pcie_setup(struct dw_pcie *pci)
 		val |= PORT_LINK_MODE_8_LANES;
 		break;
 	default:
-		dev_err(pci->dev, "num-lanes %u: invalid value\n", pci->num_lanes);
-		return;
+		dev_dbg(pci->dev, "Using h/w default number of lanes\n");
+		break;
 	}
 	dw_pcie_writel_dbi(pci, PCIE_PORT_LINK_CONTROL, val);
+	if (!pci->num_lanes)
+		return;
 
 	/* Set link width speed control register */
 	val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
-- 
2.25.1

[PATCH v11 06/13] PCI: Add PCI_EXP_LNKCAP_MLW macros

[Yoshihiro Shimoda]

Add macros defining Maximum Link Width bits in Link Capabilities
Register.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/uapi/linux/pci_regs.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index dc2000e0fe3a..5d48413ac28f 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -538,6 +538,12 @@
 #define  PCI_EXP_LNKCAP_SLS_16_0GB 0x00000004 /* LNKCAP2 SLS Vector bit 3 */
 #define  PCI_EXP_LNKCAP_SLS_32_0GB 0x00000005 /* LNKCAP2 SLS Vector bit 4 */
 #define  PCI_EXP_LNKCAP_SLS_64_0GB 0x00000006 /* LNKCAP2 SLS Vector bit 5 */
+#define  PCI_EXP_LNKCAP_MLW_X1	0x00000010 /* Maximum Link Width x1 */
+#define  PCI_EXP_LNKCAP_MLW_X2	0x00000020 /* Maximum Link Width x2 */
+#define  PCI_EXP_LNKCAP_MLW_X4	0x00000040 /* Maximum Link Width x4 */
+#define  PCI_EXP_LNKCAP_MLW_X8	0x00000080 /* Maximum Link Width x8 */
+#define  PCI_EXP_LNKCAP_MLW_X12	0x000000c0 /* Maximum Link Width x12 */
+#define  PCI_EXP_LNKCAP_MLW_X16	0x00000100 /* Maximum Link Width x16 */
 #define  PCI_EXP_LNKCAP_MLW	0x000003f0 /* Maximum Link Width */
 #define  PCI_EXP_LNKCAP_ASPMS	0x00000c00 /* ASPM Support */
 #define  PCI_EXP_LNKCAP_ASPM_L0S 0x00000400 /* ASPM L0s Support */
-- 
2.25.1

[PATCH v11 07/13] PCI: designware-ep: Expose dw_pcie_ep_exit() to module

[Yoshihiro Shimoda]

Expose dw_pcie_ep_exit() to module.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f9182f8d552f..95efe14f1036 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -622,6 +622,7 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 
 	pci_epc_mem_exit(epc);
 }
+EXPORT_SYMBOL_GPL(dw_pcie_ep_exit);
 
 static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
 {
-- 
2.25.1

[PATCH v11 08/13] PCI: dwc: Add dw_pcie_num_lanes_setup()

[Yoshihiro Shimoda]

Add dw_pcie_num_lanes_setup() to setup PCI_EXP_LNKCAP_MLW.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++++
 drivers/pci/controller/dwc/pcie-designware.h |  1 +
 drivers/pci/controller/dwc/pcie-tegra194.c   |  5 +----
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 89b8ec29da7f..47860da5738e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -696,6 +696,18 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci)
 }
 EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
 
+void dw_pcie_num_lanes_setup(struct dw_pcie *pci, int num_lanes)
+{
+	u8 cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
+	val &= ~PCI_EXP_LNKCAP_MLW;
+	val |= num_lanes << PCI_EXP_LNKSTA_NLW_SHIFT;
+	dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, val);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_num_lanes_setup);
+
 static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
 {
 	u32 cap, ctrl2, link_speed;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 79713ce075cc..36f3e2c818fe 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -415,6 +415,7 @@ void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
 void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
 int dw_pcie_link_up(struct dw_pcie *pci);
 void dw_pcie_upconfig_setup(struct dw_pcie *pci);
+void dw_pcie_num_lanes_setup(struct dw_pcie *pci, int num_lanes);
 int dw_pcie_wait_for_link(struct dw_pcie *pci);
 int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
 			      u64 cpu_addr, u64 pci_addr, u64 size);
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 09825b4a075e..befe17d57e2a 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -902,10 +902,7 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp)
 	dw_pcie_writel_dbi(pci, PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT, val);
 
 	/* Configure Max lane width from DT */
-	val = dw_pcie_readl_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP);
-	val &= ~PCI_EXP_LNKCAP_MLW;
-	val |= (pcie->num_lanes << PCI_EXP_LNKSTA_NLW_SHIFT);
-	dw_pcie_writel_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP, val);
+	dw_pcie_num_lanes_setup(pci, pcie->num_lanes);
 
 	/* Clear Slot Clock Configuration bit if SRNS configuration */
 	if (pcie->enable_srns) {
-- 
2.25.1

Re: [PATCH v11 08/13] PCI: dwc: Add dw_pcie_num_lanes_setup()

[Serge Semin]

On Fri, Mar 10, 2023 at 09:35:05PM +0900, Yoshihiro Shimoda wrote:
> Add dw_pcie_num_lanes_setup() to setup PCI_EXP_LNKCAP_MLW.

More details of why it's needed would be nice. For instance, in
accordance with the DW PCIe RC/EP HW manuals [1,2,3,...] aside with the
PORT_LINK_CTRL_OFF.LINK_CAPABLE and GEN2_CTRL_OFF.NUM_OF_LANES[8:0]
field there is another one which needs to be update. It's
LINK_CAPABILITIES_REG.PCIE_CAP_MAX_LINK_WIDTH. If it isn't done at the
very least the maximum link-width capability CSR won't expose the
actual maximum capability.

[1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
    Version 4.60a, March 2015, p.1032
[2] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
    Version 4.70a, March 2016, p.1065
[3] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
    Version 4.90a, March 2016, p.1057
...
[X] DesignWare Cores PCI Express Controller Databook - DWC PCIe Endpoint,
    Version 5.40a, March 2019, p.1396
[X+1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
    Version 5.40a, March 2019, p.1266

> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h |  1 +
>  drivers/pci/controller/dwc/pcie-tegra194.c   |  5 +----
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 89b8ec29da7f..47860da5738e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -696,6 +696,18 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci)
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
>  

> +void dw_pcie_num_lanes_setup(struct dw_pcie *pci, int num_lanes)
> +{
> +	u8 cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	u32 val;
> +
> +	val = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
> +	val &= ~PCI_EXP_LNKCAP_MLW;
> +	val |= num_lanes << PCI_EXP_LNKSTA_NLW_SHIFT;
> +	dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, val);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_num_lanes_setup);

That's not what I meant. I meant to implement that functionality in
the framework of dw_pcie_setup() function by moving all the
link-width-related initializations into a dedicated static function
dw_pcie_link_set_max_{link}_width() (thus the prototype would look
like the dw_pcie_link_set_max_speed()) and _calling_ it from
dw_pcie_setup() in place where the num-lanes initializations is
performed if pci->num_lanes isn't zero. As I already mentioned in my
comment above in accordance with the DW PCIe HW-manuals this should
have been done for all DW PCIe IP-cores in the first place. I also
checked the PCIE_CAP_MAX_LINK_WIDTH field access attribute. It turns
out to be the same
■ Wire: No access.
■ Dbi: if (DBI_RO_WR_EN == 1) then R/W else R
for all IP-cores which HW ref manuals I have at hands (v4.60a, v4.70a,
v4.90a, v5.20a, v5.30a, v5.40a).

* Note even though the dw_pcie_setup() method currently permits the
* 1, 2, 4 and 8 lanes only, in fact the x16 setup is also possible
* judging by the CX_NL DW PCIe IP-core synthesize parameter.

It would also be more readable to place the dw_pcie_link_set_max_{link}_width()
function below dw_pcie_link_set_max_speed() as per the calling order
in the framework of dw_pcie_setup().

By doing as I suggested above you not only would be able to implement
a correct link-width setup procedure for all IP-cores but also could
get rid of the PATCH #5 since you'll be moving the respective code
anyway and get rid of the dw_pcie_num_lanes_setup() method invocation
from your and Tegra glue-drivers.

-Serge(y)

> +
>  static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
>  {
>  	u32 cap, ctrl2, link_speed;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 79713ce075cc..36f3e2c818fe 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -415,6 +415,7 @@ void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
>  void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
>  int dw_pcie_link_up(struct dw_pcie *pci);
>  void dw_pcie_upconfig_setup(struct dw_pcie *pci);
> +void dw_pcie_num_lanes_setup(struct dw_pcie *pci, int num_lanes);
>  int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>  			      u64 cpu_addr, u64 pci_addr, u64 size);
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 09825b4a075e..befe17d57e2a 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -902,10 +902,7 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp)
>  	dw_pcie_writel_dbi(pci, PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT, val);
>  
>  	/* Configure Max lane width from DT */
> -	val = dw_pcie_readl_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP);
> -	val &= ~PCI_EXP_LNKCAP_MLW;
> -	val |= (pcie->num_lanes << PCI_EXP_LNKSTA_NLW_SHIFT);
> -	dw_pcie_writel_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP, val);
> +	dw_pcie_num_lanes_setup(pci, pcie->num_lanes);
>  
>  	/* Clear Slot Clock Configuration bit if SRNS configuration */
>  	if (pcie->enable_srns) {
> -- 
> 2.25.1
> 
> 

RE: [PATCH v11 08/13] PCI: dwc: Add dw_pcie_num_lanes_setup()

[Yoshihiro Shimoda]

Hi Serge,

> From: Serge Semin, Sent: Wednesday, March 22, 2023 3:57 PM
> 
> On Fri, Mar 10, 2023 at 09:35:05PM +0900, Yoshihiro Shimoda wrote:
> > Add dw_pcie_num_lanes_setup() to setup PCI_EXP_LNKCAP_MLW.
> 
> More details of why it's needed would be nice. For instance, in
> accordance with the DW PCIe RC/EP HW manuals [1,2,3,...] aside with the
> PORT_LINK_CTRL_OFF.LINK_CAPABLE and GEN2_CTRL_OFF.NUM_OF_LANES[8:0]
> field there is another one which needs to be update. It's
> LINK_CAPABILITIES_REG.PCIE_CAP_MAX_LINK_WIDTH. If it isn't done at the
> very least the maximum link-width capability CSR won't expose the
> actual maximum capability.

Thank you for your comments! I'll add them into the commit message on v12.

> [1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
>     Version 4.60a, March 2015, p.1032
> [2] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
>     Version 4.70a, March 2016, p.1065
> [3] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
>     Version 4.90a, March 2016, p.1057
> ...
> [X] DesignWare Cores PCI Express Controller Databook - DWC PCIe Endpoint,
>     Version 5.40a, March 2019, p.1396
> [X+1] DesignWare Cores PCI Express Controller Databook - DWC PCIe Root Port,
>     Version 5.40a, March 2019, p.1266
> 
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware.c | 12 ++++++++++++
> >  drivers/pci/controller/dwc/pcie-designware.h |  1 +
> >  drivers/pci/controller/dwc/pcie-tegra194.c   |  5 +----
> >  3 files changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 89b8ec29da7f..47860da5738e 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -696,6 +696,18 @@ void dw_pcie_upconfig_setup(struct dw_pcie *pci)
> >  }
> >  EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
> >
> 
> > +void dw_pcie_num_lanes_setup(struct dw_pcie *pci, int num_lanes)
> > +{
> > +	u8 cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > +	u32 val;
> > +
> > +	val = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
> > +	val &= ~PCI_EXP_LNKCAP_MLW;
> > +	val |= num_lanes << PCI_EXP_LNKSTA_NLW_SHIFT;
> > +	dw_pcie_writel_dbi(pci, cap + PCI_EXP_LNKCAP, val);
> > +}
> > +EXPORT_SYMBOL_GPL(dw_pcie_num_lanes_setup);
> 
> That's not what I meant. I meant to implement that functionality in
> the framework of dw_pcie_setup() function by moving all the
> link-width-related initializations into a dedicated static function
> dw_pcie_link_set_max_{link}_width() (thus the prototype would look
> like the dw_pcie_link_set_max_speed()) and _calling_ it from
> dw_pcie_setup() in place where the num-lanes initializations is
> performed if pci->num_lanes isn't zero. As I already mentioned in my
> comment above in accordance with the DW PCIe HW-manuals this should
> have been done for all DW PCIe IP-cores in the first place. I also
> checked the PCIE_CAP_MAX_LINK_WIDTH field access attribute. It turns
> out to be the same
> ■ Wire: No access.
> ■ Dbi: if (DBI_RO_WR_EN == 1) then R/W else R
> for all IP-cores which HW ref manuals I have at hands (v4.60a, v4.70a,
> v4.90a, v5.20a, v5.30a, v5.40a).
> 
> * Note even though the dw_pcie_setup() method currently permits the
> * 1, 2, 4 and 8 lanes only, in fact the x16 setup is also possible
> * judging by the CX_NL DW PCIe IP-core synthesize parameter.
> 
> It would also be more readable to place the dw_pcie_link_set_max_{link}_width()
> function below dw_pcie_link_set_max_speed() as per the calling order
> in the framework of dw_pcie_setup().
> 
> By doing as I suggested above you not only would be able to implement
> a correct link-width setup procedure for all IP-cores but also could
> get rid of the PATCH #5 since you'll be moving the respective code
> anyway and get rid of the dw_pcie_num_lanes_setup() method invocation
> from your and Tegra glue-drivers.

Thank you very much for your detailed explanation. I understood it.
I'll fix this on v12.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> > +
> >  static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
> >  {
> >  	u32 cap, ctrl2, link_speed;
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 79713ce075cc..36f3e2c818fe 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -415,6 +415,7 @@ void dw_pcie_write_dbi(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
> >  void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
> >  int dw_pcie_link_up(struct dw_pcie *pci);
> >  void dw_pcie_upconfig_setup(struct dw_pcie *pci);
> > +void dw_pcie_num_lanes_setup(struct dw_pcie *pci, int num_lanes);
> >  int dw_pcie_wait_for_link(struct dw_pcie *pci);
> >  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> >  			      u64 cpu_addr, u64 pci_addr, u64 size);
> > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> > index 09825b4a075e..befe17d57e2a 100644
> > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > @@ -902,10 +902,7 @@ static int tegra_pcie_dw_host_init(struct dw_pcie_rp *pp)
> >  	dw_pcie_writel_dbi(pci, PORT_LOGIC_AMBA_ERROR_RESPONSE_DEFAULT, val);
> >
> >  	/* Configure Max lane width from DT */
> > -	val = dw_pcie_readl_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP);
> > -	val &= ~PCI_EXP_LNKCAP_MLW;
> > -	val |= (pcie->num_lanes << PCI_EXP_LNKSTA_NLW_SHIFT);
> > -	dw_pcie_writel_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKCAP, val);
> > +	dw_pcie_num_lanes_setup(pci, pcie->num_lanes);
> >
> >  	/* Clear Slot Clock Configuration bit if SRNS configuration */
> >  	if (pcie->enable_srns) {
> > --
> > 2.25.1
> >
> >

[PATCH v11 09/13] PCI: dwc: Add support for triggering legacy IRQs

[Yoshihiro Shimoda]

Add support for triggering legacy IRQs by using outbound iATU.
Outbound iATU is utilized to send assert and de-assert INTx TLPs.
The message is generated based on the payloadless Msg TLP with type
0x14, where 0x4 is the routing code implying the terminated at
Receiver message. The message code is specified as b1000xx for
the INTx assertion and b1001xx for the INTx de-assertion.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 .../pci/controller/dwc/pcie-designware-ep.c   | 66 ++++++++++++--
 drivers/pci/controller/dwc/pcie-designware.c  | 88 +++++++++++++------
 drivers/pci/controller/dwc/pcie-designware.h  | 11 ++-
 3 files changed, 126 insertions(+), 39 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 95efe14f1036..73b3844e8a09 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -6,6 +6,7 @@
  * Author: Kishon Vijay Abraham I <kishon@ti.com>
  */
 
+#include <linux/delay.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 
@@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
 	return 0;
 }
 
-static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
-				   phys_addr_t phys_addr,
+static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
+				   u8 code, u8 routing, phys_addr_t phys_addr,
 				   u64 pci_addr, size_t size)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
@@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
 		return -EINVAL;
 	}
 
-	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
-					   phys_addr, pci_addr, size);
+	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
+					   code, routing, phys_addr, pci_addr,
+					   size);
 	if (ret)
 		return ret;
 
@@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 
-	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
+	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
+				      addr, pci_addr, size);
 	if (ret) {
 		dev_err(pci->dev, "Failed to enable address\n");
 		return ret;
@@ -479,14 +482,46 @@ static const struct pci_epc_ops epc_ops = {
 	.get_features		= dw_pcie_ep_get_features,
 };
 
+static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
+			       u8 routing)
+{
+	struct pci_epc *epc = ep->epc;
+	int ret;
+
+	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
+				      routing, ep->intx_mem_phys, 0,
+				      epc->mem->window.page_size);
+	if (ret)
+		return ret;
+	writel(0, ep->intx_mem);
+	dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
+
+	return 0;
+}
+
+static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
+					 int intx)
+{
+	int ret;
+
+	ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);
+	if (ret)
+		return ret;
+	usleep_range(1000, 2000);
+	return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
+}
+
 int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 	struct device *dev = pci->dev;
 
-	dev_err(dev, "EP cannot trigger legacy IRQs\n");
+	if (!ep->intx_mem) {
+		dev_err(dev, "EP cannot trigger legacy IRQs\n");
+		return -EINVAL;
+	}
 
-	return -EINVAL;
+	return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
 }
 EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_legacy_irq);
 
@@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 
 	dw_pcie_edma_remove(pci);
 
+	if (ep->intx_mem)
+		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
+				      epc->mem->window.page_size);
+
 	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
 			      epc->mem->window.page_size);
 
@@ -789,9 +828,14 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 		goto err_exit_epc_mem;
 	}
 
+	ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
+					      epc->mem->window.page_size);
+	if (!ep->intx_mem)
+		dev_warn(dev, "Failed to reserve memory for INTx\n");
+
 	ret = dw_pcie_edma_detect(pci);
 	if (ret)
-		goto err_free_epc_mem;
+		goto err_free_epc_mem_intx;
 
 	if (ep->ops->get_features) {
 		epc_features = ep->ops->get_features(ep);
@@ -808,7 +852,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 err_remove_edma:
 	dw_pcie_edma_remove(pci);
 
-err_free_epc_mem:
+err_free_epc_mem_intx:
+	if (ep->intx_mem)
+		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
+				      epc->mem->window.page_size);
+
 	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
 			      epc->mem->window.page_size);
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 47860da5738e..364926832126 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -23,6 +23,17 @@
 #include "../../pci.h"
 #include "pcie-designware.h"
 
+struct dw_pcie_outbound_atu {
+	u64 cpu_addr;
+	u64 pci_addr;
+	u64 size;
+	int index;
+	int type;
+	u8 func_no;
+	u8 code;
+	u8 routing;
+};
+
 static const char * const dw_pcie_app_clks[DW_PCIE_NUM_APP_CLKS] = {
 	[DW_PCIE_DBI_CLK] = "dbi",
 	[DW_PCIE_MSTR_CLK] = "mstr",
@@ -464,56 +475,58 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
 	return val | PCIE_ATU_TD;
 }
 
-static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
-				       int index, int type, u64 cpu_addr,
-				       u64 pci_addr, u64 size)
+static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
+				       struct dw_pcie_outbound_atu *atu)
 {
 	u32 retries, val;
 	u64 limit_addr;
 
 	if (pci->ops && pci->ops->cpu_addr_fixup)
-		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
+		atu->cpu_addr = pci->ops->cpu_addr_fixup(pci, atu->cpu_addr);
 
-	limit_addr = cpu_addr + size - 1;
+	limit_addr = atu->cpu_addr + atu->size - 1;
 
-	if ((limit_addr & ~pci->region_limit) != (cpu_addr & ~pci->region_limit) ||
-	    !IS_ALIGNED(cpu_addr, pci->region_align) ||
-	    !IS_ALIGNED(pci_addr, pci->region_align) || !size) {
+	if ((limit_addr & ~pci->region_limit) != (atu->cpu_addr & ~pci->region_limit) ||
+	    !IS_ALIGNED(atu->cpu_addr, pci->region_align) ||
+	    !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size) {
 		return -EINVAL;
 	}
 
-	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LOWER_BASE,
-			      lower_32_bits(cpu_addr));
-	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_BASE,
-			      upper_32_bits(cpu_addr));
+	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_BASE,
+			      lower_32_bits(atu->cpu_addr));
+	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_BASE,
+			      upper_32_bits(atu->cpu_addr));
 
-	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LIMIT,
+	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LIMIT,
 			      lower_32_bits(limit_addr));
 	if (dw_pcie_ver_is_ge(pci, 460A))
-		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_LIMIT,
+		dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_LIMIT,
 				      upper_32_bits(limit_addr));
 
-	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LOWER_TARGET,
-			      lower_32_bits(pci_addr));
-	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
-			      upper_32_bits(pci_addr));
+	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_TARGET,
+			      lower_32_bits(atu->pci_addr));
+	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_TARGET,
+			      upper_32_bits(atu->pci_addr));
 
-	val = type | PCIE_ATU_FUNC_NUM(func_no);
-	if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
+	val = atu->type | atu->routing | PCIE_ATU_FUNC_NUM(atu->func_no);
+	if (upper_32_bits(limit_addr) > upper_32_bits(atu->cpu_addr) &&
 	    dw_pcie_ver_is_ge(pci, 460A))
 		val |= PCIE_ATU_INCREASE_REGION_SIZE;
 	if (dw_pcie_ver_is(pci, 490A))
 		val = dw_pcie_enable_ecrc(val);
-	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
+	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val);
 
-	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);
+	val = PCIE_ATU_ENABLE;
+	if (atu->type == PCIE_ATU_TYPE_MSG)
+		val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | atu->code;
+	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, val);
 
 	/*
 	 * Make sure ATU enable takes effect before any subsequent config
 	 * and I/O accesses.
 	 */
 	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
-		val = dw_pcie_readl_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2);
+		val = dw_pcie_readl_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2);
 		if (val & PCIE_ATU_ENABLE)
 			return 0;
 
@@ -528,16 +541,33 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
 int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
 			      u64 cpu_addr, u64 pci_addr, u64 size)
 {
-	return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
-					   cpu_addr, pci_addr, size);
+	struct dw_pcie_outbound_atu atu;
+
+	memset(&atu, 0, sizeof(atu));
+	atu.index = index;
+	atu.type = type;
+	atu.cpu_addr = cpu_addr;
+	atu.pci_addr = pci_addr;
+	atu.size = size;
+	return __dw_pcie_prog_outbound_atu(pci, &atu);
 }
 
 int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
-				 int type, u64 cpu_addr, u64 pci_addr,
-				 u64 size)
+				 int type, u8 code, u8 routing, u64 cpu_addr,
+				 u64 pci_addr, u64 size)
 {
-	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
-					   cpu_addr, pci_addr, size);
+	struct dw_pcie_outbound_atu atu;
+
+	memset(&atu, 0, sizeof(atu));
+	atu.func_no = func_no;
+	atu.index = index;
+	atu.type = type;
+	atu.code = code;
+	atu.routing = routing;
+	atu.cpu_addr = cpu_addr;
+	atu.pci_addr = pci_addr;
+	atu.size = size;
+	return __dw_pcie_prog_outbound_atu(pci, &atu);
 }
 
 static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 36f3e2c818fe..3dbadb8043ab 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -147,11 +147,14 @@
 #define PCIE_ATU_TYPE_IO		0x2
 #define PCIE_ATU_TYPE_CFG0		0x4
 #define PCIE_ATU_TYPE_CFG1		0x5
+#define PCIE_ATU_TYPE_MSG		0x10
 #define PCIE_ATU_TD			BIT(8)
 #define PCIE_ATU_FUNC_NUM(pf)           ((pf) << 20)
 #define PCIE_ATU_REGION_CTRL2		0x004
 #define PCIE_ATU_ENABLE			BIT(31)
 #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
+#define PCIE_ATU_INHIBIT_PAYLOAD	BIT(22)
+#define PCIE_ATU_HEADER_SUB_ENABLE	BIT(21)
 #define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
 #define PCIE_ATU_LOWER_BASE		0x008
 #define PCIE_ATU_UPPER_BASE		0x00C
@@ -244,6 +247,9 @@
 /* Default eDMA LLP memory size */
 #define DMA_LLP_MEM_SIZE		PAGE_SIZE
 
+#define PCIE_MSG_ASSERT_INTA		0x20
+#define PCIE_MSG_DEASSERT_INTA		0x24
+
 struct dw_pcie;
 struct dw_pcie_rp;
 struct dw_pcie_ep;
@@ -352,6 +358,8 @@ struct dw_pcie_ep {
 	unsigned long		*ob_window_map;
 	void __iomem		*msi_mem;
 	phys_addr_t		msi_mem_phys;
+	void __iomem		*intx_mem;
+	phys_addr_t		intx_mem_phys;
 	struct pci_epf_bar	*epf_bar[PCI_STD_NUM_BARS];
 };
 
@@ -420,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
 int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
 			      u64 cpu_addr, u64 pci_addr, u64 size);
 int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
-				 int type, u64 cpu_addr, u64 pci_addr, u64 size);
+				 int type, u8 code, u8 routing, u64 cpu_addr,
+				 u64 pci_addr, u64 size);
 int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
 			     u64 cpu_addr, u64 pci_addr, u64 size);
 int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
-- 
2.25.1

Re: [PATCH v11 09/13] PCI: dwc: Add support for triggering legacy IRQs

[Serge Semin]

On Fri, Mar 10, 2023 at 09:35:06PM +0900, Yoshihiro Shimoda wrote:
> Add support for triggering legacy IRQs by using outbound iATU.
> Outbound iATU is utilized to send assert and de-assert INTx TLPs.
> The message is generated based on the payloadless Msg TLP with type
> 0x14, where 0x4 is the routing code implying the terminated at
> Receiver message. The message code is specified as b1000xx for
> the INTx assertion and b1001xx for the INTx de-assertion.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  .../pci/controller/dwc/pcie-designware-ep.c   | 66 ++++++++++++--
>  drivers/pci/controller/dwc/pcie-designware.c  | 88 +++++++++++++------
>  drivers/pci/controller/dwc/pcie-designware.h  | 11 ++-
>  3 files changed, 126 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 95efe14f1036..73b3844e8a09 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -6,6 +6,7 @@
>   * Author: Kishon Vijay Abraham I <kishon@ti.com>
>   */
>  
> +#include <linux/delay.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  
> @@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
>  	return 0;
>  }
>  
> -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> -				   phys_addr_t phys_addr,
> +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> +				   u8 code, u8 routing, phys_addr_t phys_addr,
>  				   u64 pci_addr, size_t size)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
>  		return -EINVAL;
>  	}
>  
> -	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
> -					   phys_addr, pci_addr, size);
> +	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
> +					   code, routing, phys_addr, pci_addr,
> +					   size);
>  	if (ret)
>  		return ret;
>  
> @@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  
> -	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> +	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
> +				      addr, pci_addr, size);
>  	if (ret) {
>  		dev_err(pci->dev, "Failed to enable address\n");
>  		return ret;
> @@ -479,14 +482,46 @@ static const struct pci_epc_ops epc_ops = {
>  	.get_features		= dw_pcie_ep_get_features,
>  };
>  
> +static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
> +			       u8 routing)
> +{
> +	struct pci_epc *epc = ep->epc;
> +	int ret;
> +
> +	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
> +				      routing, ep->intx_mem_phys, 0,
> +				      epc->mem->window.page_size);
> +	if (ret)
> +		return ret;

< newline pls

> +	writel(0, ep->intx_mem);

< nelinew pls

> +	dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
> +
> +	return 0;
> +}
> +
> +static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
> +					 int intx)
> +{
> +	int ret;
> +

> +	ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);

0x04 - magic number. Please create a macro for it.
* PCIe specification clearly states what 0x04 means.

> +	if (ret)
> +		return ret;

< newline pls

> +	usleep_range(1000, 2000);

< newline pls

> +	return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
> +}
> +
>  int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	struct device *dev = pci->dev;
>  
> -	dev_err(dev, "EP cannot trigger legacy IRQs\n");
> +	if (!ep->intx_mem) {
> +		dev_err(dev, "EP cannot trigger legacy IRQs\n");
> +		return -EINVAL;
> +	}
>  
> -	return -EINVAL;
> +	return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_legacy_irq);
>  
> @@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  
>  	dw_pcie_edma_remove(pci);
>  
> +	if (ep->intx_mem)
> +		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> +				      epc->mem->window.page_size);
> +
>  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
>  			      epc->mem->window.page_size);
>  
> @@ -789,9 +828,14 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  		goto err_exit_epc_mem;
>  	}
>  
> +	ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
> +					      epc->mem->window.page_size);
> +	if (!ep->intx_mem)
> +		dev_warn(dev, "Failed to reserve memory for INTx\n");
> +
>  	ret = dw_pcie_edma_detect(pci);
>  	if (ret)
> -		goto err_free_epc_mem;
> +		goto err_free_epc_mem_intx;
>  
>  	if (ep->ops->get_features) {
>  		epc_features = ep->ops->get_features(ep);
> @@ -808,7 +852,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  err_remove_edma:
>  	dw_pcie_edma_remove(pci);
>  
> -err_free_epc_mem:
> +err_free_epc_mem_intx:
> +	if (ep->intx_mem)
> +		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> +				      epc->mem->window.page_size);
> +
>  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
>  			      epc->mem->window.page_size);
>  
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 47860da5738e..364926832126 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -23,6 +23,17 @@
>  #include "../../pci.h"
>  #include "pcie-designware.h"
>  
> +struct dw_pcie_outbound_atu {
> +	u64 cpu_addr;
> +	u64 pci_addr;
> +	u64 size;
> +	int index;
> +	int type;
> +	u8 func_no;
> +	u8 code;
> +	u8 routing;
> +};

Please detach the __dw_pcie_prog_outbound_atu() method conversion to a
pre-requisite patch to make this change smaller and easier to review.

> +
>  static const char * const dw_pcie_app_clks[DW_PCIE_NUM_APP_CLKS] = {
>  	[DW_PCIE_DBI_CLK] = "dbi",
>  	[DW_PCIE_MSTR_CLK] = "mstr",
> @@ -464,56 +475,58 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
>  	return val | PCIE_ATU_TD;
>  }
>  
> -static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> -				       int index, int type, u64 cpu_addr,
> -				       u64 pci_addr, u64 size)
> +static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
> +				       struct dw_pcie_outbound_atu *atu)
>  {
>  	u32 retries, val;
>  	u64 limit_addr;
>  
>  	if (pci->ops && pci->ops->cpu_addr_fixup)
> -		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
> +		atu->cpu_addr = pci->ops->cpu_addr_fixup(pci, atu->cpu_addr);
>  
> -	limit_addr = cpu_addr + size - 1;
> +	limit_addr = atu->cpu_addr + atu->size - 1;
>  
> -	if ((limit_addr & ~pci->region_limit) != (cpu_addr & ~pci->region_limit) ||
> -	    !IS_ALIGNED(cpu_addr, pci->region_align) ||
> -	    !IS_ALIGNED(pci_addr, pci->region_align) || !size) {
> +	if ((limit_addr & ~pci->region_limit) != (atu->cpu_addr & ~pci->region_limit) ||
> +	    !IS_ALIGNED(atu->cpu_addr, pci->region_align) ||
> +	    !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size) {
>  		return -EINVAL;
>  	}
>  
> -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LOWER_BASE,
> -			      lower_32_bits(cpu_addr));
> -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_BASE,
> -			      upper_32_bits(cpu_addr));
> +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_BASE,
> +			      lower_32_bits(atu->cpu_addr));
> +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_BASE,
> +			      upper_32_bits(atu->cpu_addr));
>  
> -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LIMIT,
> +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LIMIT,
>  			      lower_32_bits(limit_addr));
>  	if (dw_pcie_ver_is_ge(pci, 460A))
> -		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_LIMIT,
> +		dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_LIMIT,
>  				      upper_32_bits(limit_addr));
>  
> -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LOWER_TARGET,
> -			      lower_32_bits(pci_addr));
> -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
> -			      upper_32_bits(pci_addr));
> +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_TARGET,
> +			      lower_32_bits(atu->pci_addr));
> +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_TARGET,
> +			      upper_32_bits(atu->pci_addr));
>  
> -	val = type | PCIE_ATU_FUNC_NUM(func_no);
> -	if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
> +	val = atu->type | atu->routing | PCIE_ATU_FUNC_NUM(atu->func_no);
> +	if (upper_32_bits(limit_addr) > upper_32_bits(atu->cpu_addr) &&
>  	    dw_pcie_ver_is_ge(pci, 460A))
>  		val |= PCIE_ATU_INCREASE_REGION_SIZE;
>  	if (dw_pcie_ver_is(pci, 490A))
>  		val = dw_pcie_enable_ecrc(val);
> -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
> +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val);
>  
> -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);
> +	val = PCIE_ATU_ENABLE;
> +	if (atu->type == PCIE_ATU_TYPE_MSG)
> +		val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | atu->code;
> +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, val);
>  
>  	/*
>  	 * Make sure ATU enable takes effect before any subsequent config
>  	 * and I/O accesses.
>  	 */
>  	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
> -		val = dw_pcie_readl_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2);
> +		val = dw_pcie_readl_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2);
>  		if (val & PCIE_ATU_ENABLE)
>  			return 0;
>  
> @@ -528,16 +541,33 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
>  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>  			      u64 cpu_addr, u64 pci_addr, u64 size)
>  {
> -	return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
> -					   cpu_addr, pci_addr, size);
> +	struct dw_pcie_outbound_atu atu;
> +

> +	memset(&atu, 0, sizeof(atu));

can be replaced with struct dw_pcie_outbound_atu atu = {0};

> +	atu.index = index;
> +	atu.type = type;
> +	atu.cpu_addr = cpu_addr;
> +	atu.pci_addr = pci_addr;
> +	atu.size = size;
> +	return __dw_pcie_prog_outbound_atu(pci, &atu);
>  }
>  
>  int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> -				 int type, u64 cpu_addr, u64 pci_addr,
> -				 u64 size)
> +				 int type, u8 code, u8 routing, u64 cpu_addr,
> +				 u64 pci_addr, u64 size)

The __dw_pcie_prog_outbound_atu() conversion has a near zero value
if dw_pcie_prog_ep_outbound_atu() will be left with the bunch of arguments.
The same concerns the dw_pcie_ep_outbound_atu() method which now accepts
eight arguments.

>  {
> -	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
> -					   cpu_addr, pci_addr, size);
> +	struct dw_pcie_outbound_atu atu;
> +

> +	memset(&atu, 0, sizeof(atu));

can be replaced with struct dw_pcie_outbound_atu atu = {0};

-Serge(y)

> +	atu.func_no = func_no;
> +	atu.index = index;
> +	atu.type = type;
> +	atu.code = code;
> +	atu.routing = routing;
> +	atu.cpu_addr = cpu_addr;
> +	atu.pci_addr = pci_addr;
> +	atu.size = size;
> +	return __dw_pcie_prog_outbound_atu(pci, &atu);
>  }
>  
>  static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 36f3e2c818fe..3dbadb8043ab 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -147,11 +147,14 @@
>  #define PCIE_ATU_TYPE_IO		0x2
>  #define PCIE_ATU_TYPE_CFG0		0x4
>  #define PCIE_ATU_TYPE_CFG1		0x5
> +#define PCIE_ATU_TYPE_MSG		0x10
>  #define PCIE_ATU_TD			BIT(8)
>  #define PCIE_ATU_FUNC_NUM(pf)           ((pf) << 20)
>  #define PCIE_ATU_REGION_CTRL2		0x004
>  #define PCIE_ATU_ENABLE			BIT(31)
>  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> +#define PCIE_ATU_INHIBIT_PAYLOAD	BIT(22)
> +#define PCIE_ATU_HEADER_SUB_ENABLE	BIT(21)
>  #define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
>  #define PCIE_ATU_LOWER_BASE		0x008
>  #define PCIE_ATU_UPPER_BASE		0x00C
> @@ -244,6 +247,9 @@
>  /* Default eDMA LLP memory size */
>  #define DMA_LLP_MEM_SIZE		PAGE_SIZE
>  
> +#define PCIE_MSG_ASSERT_INTA		0x20
> +#define PCIE_MSG_DEASSERT_INTA		0x24
> +
>  struct dw_pcie;
>  struct dw_pcie_rp;
>  struct dw_pcie_ep;
> @@ -352,6 +358,8 @@ struct dw_pcie_ep {
>  	unsigned long		*ob_window_map;
>  	void __iomem		*msi_mem;
>  	phys_addr_t		msi_mem_phys;
> +	void __iomem		*intx_mem;
> +	phys_addr_t		intx_mem_phys;
>  	struct pci_epf_bar	*epf_bar[PCI_STD_NUM_BARS];
>  };
>  
> @@ -420,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>  			      u64 cpu_addr, u64 pci_addr, u64 size);
>  int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> -				 int type, u64 cpu_addr, u64 pci_addr, u64 size);
> +				 int type, u8 code, u8 routing, u64 cpu_addr,
> +				 u64 pci_addr, u64 size);
>  int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
>  			     u64 cpu_addr, u64 pci_addr, u64 size);
>  int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> -- 
> 2.25.1
> 
> 

RE: [PATCH v11 09/13] PCI: dwc: Add support for triggering legacy IRQs

[Yoshihiro Shimoda]

Hi Serge,

> From: Serge Semin, Sent: Thursday, March 23, 2023 1:18 AM
> 
> On Fri, Mar 10, 2023 at 09:35:06PM +0900, Yoshihiro Shimoda wrote:
> > Add support for triggering legacy IRQs by using outbound iATU.
> > Outbound iATU is utilized to send assert and de-assert INTx TLPs.
> > The message is generated based on the payloadless Msg TLP with type
> > 0x14, where 0x4 is the routing code implying the terminated at
> > Receiver message. The message code is specified as b1000xx for
> > the INTx assertion and b1001xx for the INTx de-assertion.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  .../pci/controller/dwc/pcie-designware-ep.c   | 66 ++++++++++++--
> >  drivers/pci/controller/dwc/pcie-designware.c  | 88 +++++++++++++------
> >  drivers/pci/controller/dwc/pcie-designware.h  | 11 ++-
> >  3 files changed, 126 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 95efe14f1036..73b3844e8a09 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -6,6 +6,7 @@
> >   * Author: Kishon Vijay Abraham I <kishon@ti.com>
> >   */
> >
> > +#include <linux/delay.h>
> >  #include <linux/of.h>
> >  #include <linux/platform_device.h>
> >
> > @@ -182,8 +183,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> >  	return 0;
> >  }
> >
> > -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > -				   phys_addr_t phys_addr,
> > +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> > +				   u8 code, u8 routing, phys_addr_t phys_addr,
> >  				   u64 pci_addr, size_t size)
> >  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > @@ -196,8 +197,9 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> >  		return -EINVAL;
> >  	}
> >
> > -	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
> > -					   phys_addr, pci_addr, size);
> > +	ret = dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, type,
> > +					   code, routing, phys_addr, pci_addr,
> > +					   size);
> >  	if (ret)
> >  		return ret;
> >
> > @@ -306,7 +308,8 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> >  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >
> > -	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
> > +	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MEM, 0, 0,
> > +				      addr, pci_addr, size);
> >  	if (ret) {
> >  		dev_err(pci->dev, "Failed to enable address\n");
> >  		return ret;
> > @@ -479,14 +482,46 @@ static const struct pci_epc_ops epc_ops = {
> >  	.get_features		= dw_pcie_ep_get_features,
> >  };
> >
> > +static int dw_pcie_ep_send_msg(struct dw_pcie_ep *ep, u8 func_no, u8 code,
> > +			       u8 routing)
> > +{
> > +	struct pci_epc *epc = ep->epc;
> > +	int ret;
> > +
> > +	ret = dw_pcie_ep_outbound_atu(ep, func_no, PCIE_ATU_TYPE_MSG, code,
> > +				      routing, ep->intx_mem_phys, 0,
> > +				      epc->mem->window.page_size);
> > +	if (ret)
> > +		return ret;
> 
> < newline pls

I got it.

> > +	writel(0, ep->intx_mem);
> 
> < nelinew pls

I got it.

> > +	dw_pcie_ep_unmap_addr(epc, func_no, 0, ep->intx_mem_phys);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no,
> > +					 int intx)
> > +{
> > +	int ret;
> > +
> 
> > +	ret = dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_ASSERT_INTA + intx, 0x04);
> 
> 0x04 - magic number. Please create a macro for it.
> * PCIe specification clearly states what 0x04 means.

I understood it. The PCIE_MSG_ASSERT_INTA is also related to PCIe specification.
So, I'll create macros for it into include/linux/pci.h.

> > +	if (ret)
> > +		return ret;
> 
> < newline pls

I got it.

> > +	usleep_range(1000, 2000);
> 
> < newline pls

I got it.

> > +	return dw_pcie_ep_send_msg(ep, func_no, PCIE_MSG_DEASSERT_INTA + intx, 0x04);
> > +}
> > +
> >  int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
> >  {
> >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >  	struct device *dev = pci->dev;
> >
> > -	dev_err(dev, "EP cannot trigger legacy IRQs\n");
> > +	if (!ep->intx_mem) {
> > +		dev_err(dev, "EP cannot trigger legacy IRQs\n");
> > +		return -EINVAL;
> > +	}
> >
> > -	return -EINVAL;
> > +	return __dw_pcie_ep_raise_legacy_irq(ep, func_no, 0);
> >  }
> >  EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_legacy_irq);
> >
> > @@ -617,6 +652,10 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> >
> >  	dw_pcie_edma_remove(pci);
> >
> > +	if (ep->intx_mem)
> > +		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> > +				      epc->mem->window.page_size);
> > +
> >  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> >  			      epc->mem->window.page_size);
> >
> > @@ -789,9 +828,14 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> >  		goto err_exit_epc_mem;
> >  	}
> >
> > +	ep->intx_mem = pci_epc_mem_alloc_addr(epc, &ep->intx_mem_phys,
> > +					      epc->mem->window.page_size);
> > +	if (!ep->intx_mem)
> > +		dev_warn(dev, "Failed to reserve memory for INTx\n");
> > +
> >  	ret = dw_pcie_edma_detect(pci);
> >  	if (ret)
> > -		goto err_free_epc_mem;
> > +		goto err_free_epc_mem_intx;
> >
> >  	if (ep->ops->get_features) {
> >  		epc_features = ep->ops->get_features(ep);
> > @@ -808,7 +852,11 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> >  err_remove_edma:
> >  	dw_pcie_edma_remove(pci);
> >
> > -err_free_epc_mem:
> > +err_free_epc_mem_intx:
> > +	if (ep->intx_mem)
> > +		pci_epc_mem_free_addr(epc, ep->intx_mem_phys, ep->intx_mem,
> > +				      epc->mem->window.page_size);
> > +
> >  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> >  			      epc->mem->window.page_size);
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 47860da5738e..364926832126 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -23,6 +23,17 @@
> >  #include "../../pci.h"
> >  #include "pcie-designware.h"
> >
> > +struct dw_pcie_outbound_atu {
> > +	u64 cpu_addr;
> > +	u64 pci_addr;
> > +	u64 size;
> > +	int index;
> > +	int type;
> > +	u8 func_no;
> > +	u8 code;
> > +	u8 routing;
> > +};
> 
> Please detach the __dw_pcie_prog_outbound_atu() method conversion to a
> pre-requisite patch to make this change smaller and easier to review.

I got it. I'll make such a patch.

> > +
> >  static const char * const dw_pcie_app_clks[DW_PCIE_NUM_APP_CLKS] = {
> >  	[DW_PCIE_DBI_CLK] = "dbi",
> >  	[DW_PCIE_MSTR_CLK] = "mstr",
> > @@ -464,56 +475,58 @@ static inline u32 dw_pcie_enable_ecrc(u32 val)
> >  	return val | PCIE_ATU_TD;
> >  }
> >
> > -static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> > -				       int index, int type, u64 cpu_addr,
> > -				       u64 pci_addr, u64 size)
> > +static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
> > +				       struct dw_pcie_outbound_atu *atu)
> >  {
> >  	u32 retries, val;
> >  	u64 limit_addr;
> >
> >  	if (pci->ops && pci->ops->cpu_addr_fixup)
> > -		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
> > +		atu->cpu_addr = pci->ops->cpu_addr_fixup(pci, atu->cpu_addr);
> >
> > -	limit_addr = cpu_addr + size - 1;
> > +	limit_addr = atu->cpu_addr + atu->size - 1;
> >
> > -	if ((limit_addr & ~pci->region_limit) != (cpu_addr & ~pci->region_limit) ||
> > -	    !IS_ALIGNED(cpu_addr, pci->region_align) ||
> > -	    !IS_ALIGNED(pci_addr, pci->region_align) || !size) {
> > +	if ((limit_addr & ~pci->region_limit) != (atu->cpu_addr & ~pci->region_limit) ||
> > +	    !IS_ALIGNED(atu->cpu_addr, pci->region_align) ||
> > +	    !IS_ALIGNED(atu->pci_addr, pci->region_align) || !atu->size) {
> >  		return -EINVAL;
> >  	}
> >
> > -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LOWER_BASE,
> > -			      lower_32_bits(cpu_addr));
> > -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_BASE,
> > -			      upper_32_bits(cpu_addr));
> > +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_BASE,
> > +			      lower_32_bits(atu->cpu_addr));
> > +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_BASE,
> > +			      upper_32_bits(atu->cpu_addr));
> >
> > -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LIMIT,
> > +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LIMIT,
> >  			      lower_32_bits(limit_addr));
> >  	if (dw_pcie_ver_is_ge(pci, 460A))
> > -		dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_LIMIT,
> > +		dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_LIMIT,
> >  				      upper_32_bits(limit_addr));
> >
> > -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LOWER_TARGET,
> > -			      lower_32_bits(pci_addr));
> > -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_TARGET,
> > -			      upper_32_bits(pci_addr));
> > +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_LOWER_TARGET,
> > +			      lower_32_bits(atu->pci_addr));
> > +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_UPPER_TARGET,
> > +			      upper_32_bits(atu->pci_addr));
> >
> > -	val = type | PCIE_ATU_FUNC_NUM(func_no);
> > -	if (upper_32_bits(limit_addr) > upper_32_bits(cpu_addr) &&
> > +	val = atu->type | atu->routing | PCIE_ATU_FUNC_NUM(atu->func_no);
> > +	if (upper_32_bits(limit_addr) > upper_32_bits(atu->cpu_addr) &&
> >  	    dw_pcie_ver_is_ge(pci, 460A))
> >  		val |= PCIE_ATU_INCREASE_REGION_SIZE;
> >  	if (dw_pcie_ver_is(pci, 490A))
> >  		val = dw_pcie_enable_ecrc(val);
> > -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL1, val);
> > +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val);
> >
> > -	dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2, PCIE_ATU_ENABLE);
> > +	val = PCIE_ATU_ENABLE;
> > +	if (atu->type == PCIE_ATU_TYPE_MSG)
> > +		val |= PCIE_ATU_INHIBIT_PAYLOAD | PCIE_ATU_HEADER_SUB_ENABLE | atu->code;
> > +	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2, val);
> >
> >  	/*
> >  	 * Make sure ATU enable takes effect before any subsequent config
> >  	 * and I/O accesses.
> >  	 */
> >  	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
> > -		val = dw_pcie_readl_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2);
> > +		val = dw_pcie_readl_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL2);
> >  		if (val & PCIE_ATU_ENABLE)
> >  			return 0;
> >
> > @@ -528,16 +541,33 @@ static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no,
> >  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> >  			      u64 cpu_addr, u64 pci_addr, u64 size)
> >  {
> > -	return __dw_pcie_prog_outbound_atu(pci, 0, index, type,
> > -					   cpu_addr, pci_addr, size);
> > +	struct dw_pcie_outbound_atu atu;
> > +
> 
> > +	memset(&atu, 0, sizeof(atu));
> 
> can be replaced with struct dw_pcie_outbound_atu atu = {0};

I'll fix it.

> > +	atu.index = index;
> > +	atu.type = type;
> > +	atu.cpu_addr = cpu_addr;
> > +	atu.pci_addr = pci_addr;
> > +	atu.size = size;
> > +	return __dw_pcie_prog_outbound_atu(pci, &atu);
> >  }
> >
> >  int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > -				 int type, u64 cpu_addr, u64 pci_addr,
> > -				 u64 size)
> > +				 int type, u8 code, u8 routing, u64 cpu_addr,
> > +				 u64 pci_addr, u64 size)
> 
> The __dw_pcie_prog_outbound_atu() conversion has a near zero value
> if dw_pcie_prog_ep_outbound_atu() will be left with the bunch of arguments.
> The same concerns the dw_pcie_ep_outbound_atu() method which now accepts
> eight arguments.

I agree. So, I'll fix these functions somehow.

> >  {
> > -	return __dw_pcie_prog_outbound_atu(pci, func_no, index, type,
> > -					   cpu_addr, pci_addr, size);
> > +	struct dw_pcie_outbound_atu atu;
> > +
> 
> > +	memset(&atu, 0, sizeof(atu));
> 
> can be replaced with struct dw_pcie_outbound_atu atu = {0};

I got it.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> > +	atu.func_no = func_no;
> > +	atu.index = index;
> > +	atu.type = type;
> > +	atu.code = code;
> > +	atu.routing = routing;
> > +	atu.cpu_addr = cpu_addr;
> > +	atu.pci_addr = pci_addr;
> > +	atu.size = size;
> > +	return __dw_pcie_prog_outbound_atu(pci, &atu);
> >  }
> >
> >  static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg)
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 36f3e2c818fe..3dbadb8043ab 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -147,11 +147,14 @@
> >  #define PCIE_ATU_TYPE_IO		0x2
> >  #define PCIE_ATU_TYPE_CFG0		0x4
> >  #define PCIE_ATU_TYPE_CFG1		0x5
> > +#define PCIE_ATU_TYPE_MSG		0x10
> >  #define PCIE_ATU_TD			BIT(8)
> >  #define PCIE_ATU_FUNC_NUM(pf)           ((pf) << 20)
> >  #define PCIE_ATU_REGION_CTRL2		0x004
> >  #define PCIE_ATU_ENABLE			BIT(31)
> >  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> > +#define PCIE_ATU_INHIBIT_PAYLOAD	BIT(22)
> > +#define PCIE_ATU_HEADER_SUB_ENABLE	BIT(21)
> >  #define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
> >  #define PCIE_ATU_LOWER_BASE		0x008
> >  #define PCIE_ATU_UPPER_BASE		0x00C
> > @@ -244,6 +247,9 @@
> >  /* Default eDMA LLP memory size */
> >  #define DMA_LLP_MEM_SIZE		PAGE_SIZE
> >
> > +#define PCIE_MSG_ASSERT_INTA		0x20
> > +#define PCIE_MSG_DEASSERT_INTA		0x24
> > +
> >  struct dw_pcie;
> >  struct dw_pcie_rp;
> >  struct dw_pcie_ep;
> > @@ -352,6 +358,8 @@ struct dw_pcie_ep {
> >  	unsigned long		*ob_window_map;
> >  	void __iomem		*msi_mem;
> >  	phys_addr_t		msi_mem_phys;
> > +	void __iomem		*intx_mem;
> > +	phys_addr_t		intx_mem_phys;
> >  	struct pci_epf_bar	*epf_bar[PCI_STD_NUM_BARS];
> >  };
> >
> > @@ -420,7 +428,8 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
> >  int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> >  			      u64 cpu_addr, u64 pci_addr, u64 size);
> >  int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > -				 int type, u64 cpu_addr, u64 pci_addr, u64 size);
> > +				 int type, u8 code, u8 routing, u64 cpu_addr,
> > +				 u64 pci_addr, u64 size);
> >  int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
> >  			     u64 cpu_addr, u64 pci_addr, u64 size);
> >  int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> > --
> > 2.25.1
> >
> >

[PATCH v11 10/13] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Yoshihiro Shimoda]

Add R-Car Gen4 PCIe Host support. This controller is based on
Synopsys DesignWare PCIe.

This controller doesn't support MSI-X interrupt. So, introduce
no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
from dw_pcie_msi_domain_info.

Note that this controller on R-Car S4-8 has an unexpected register
value on the dbi+0x97b register. So, add a new capability flag
which would force the unrolled eDMA mapping for the problematic
device, as suggested by Serge Semin.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/Kconfig            |   9 +
 drivers/pci/controller/dwc/Makefile           |   2 +
 drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
 drivers/pci/controller/dwc/pcie-designware.h  |   5 +-
 .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 134 +++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 156 ++++++++++++++++++
 drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  56 +++++++
 7 files changed, 367 insertions(+), 3 deletions(-)
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 434f6a4f4041..94805ec31a8f 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -414,4 +414,13 @@ config PCIE_FU740
 	  Say Y here if you want PCIe controller support for the SiFive
 	  FU740.
 
+config PCIE_RCAR_GEN4
+	tristate "Renesas R-Car Gen4 PCIe Host controller"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on PCI_MSI
+	select PCIE_DW_HOST
+	help
+	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
+	  This uses the DesignWare core.
+
 endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index bf5c311875a1..486cf706b53d 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
 obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
 obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
 obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
+pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
+obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
 
 # The following drivers are for devices that use the generic ACPI
 # pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 364926832126..6827d42fcb2c 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -882,8 +882,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
 	 * Indirect eDMA CSRs access has been completely removed since v5.40a
 	 * thus no space is now reserved for the eDMA channels viewport and
 	 * former DMA CTRL register is no longer fixed to FFs.
+	 *
+	 * Note some devices for unknown reason may have zeros in the eDMA CTRL
+	 * register even though the HW-manual explicitly states there must FFs
+	 * if the unrolled mapping is enabled. For such cases the low-level
+	 * drivers are supposed to manually activate the unrolled mapping to
+	 * bypass the auto-detection procedure.
 	 */
-	if (dw_pcie_ver_is_ge(pci, 540A))
+	if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
 		val = 0xFFFFFFFF;
 	else
 		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 3dbadb8043ab..1be74d2c3729 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -51,8 +51,9 @@
 
 /* DWC PCIe controller capabilities */
 #define DW_PCIE_CAP_REQ_RES		0
-#define DW_PCIE_CAP_IATU_UNROLL		1
-#define DW_PCIE_CAP_CDM_CHECK		2
+#define DW_PCIE_CAP_EDMA_UNROLL		1
+#define DW_PCIE_CAP_IATU_UNROLL		2
+#define DW_PCIE_CAP_CDM_CHECK		3
 
 #define dw_pcie_cap_is(_pci, _cap) \
 	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
new file mode 100644
index 000000000000..4aaecc813d85
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+	int ret;
+	u32 val;
+
+	ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
+	if (ret < 0)
+		return ret;
+
+	dw_pcie_dbi_ro_wr_en(dw);
+
+	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
+	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
+
+	dw_pcie_dbi_ro_wr_dis(dw);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		/* Enable MSI interrupt signal */
+		val = readl(rcar->base + PCIEINTSTS0EN);
+		val |= MSI_CTRL_INT;
+		writel(val, rcar->base + PCIEINTSTS0EN);
+	}
+
+	gpiod_set_value_cansleep(dw->pe_rst, 0);
+
+	dw_pcie_setup_rc(pp);
+
+	dw_pcie_dbi_ro_wr_en(dw);
+	dw_pcie_num_lanes_setup(dw, dw->num_lanes);
+	dw_pcie_dbi_ro_wr_dis(dw);
+
+	return 0;
+}
+
+static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
+	.host_init = rcar_gen4_pcie_host_init,
+};
+
+static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
+				   struct platform_device *pdev)
+{
+	struct dw_pcie *dw = &rcar->dw;
+	struct dw_pcie_rp *pp = &dw->pp;
+
+	pp->ops = &rcar_gen4_pcie_host_ops;
+	dw_pcie_cap_set(dw, REQ_RES);
+
+	return dw_pcie_host_init(pp);
+}
+
+static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
+{
+	dw_pcie_host_deinit(&rcar->dw.pp);
+	gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
+}
+
+static int rcar_gen4_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen4_pcie *rcar;
+	int err;
+
+	rcar = rcar_gen4_pcie_devm_alloc(dev);
+	if (!rcar)
+		return -ENOMEM;
+
+	err = rcar_gen4_pcie_get_resources(rcar, pdev);
+	if (err < 0) {
+		dev_err(dev, "Failed to request resource: %d\n", err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, rcar);
+
+	err = rcar_gen4_pcie_prepare(rcar);
+	if (err < 0)
+		return err;
+
+	err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
+	if (err < 0)
+		goto err_add;
+
+	return 0;
+
+err_add:
+	rcar_gen4_pcie_unprepare(rcar);
+
+	return err;
+}
+
+static int rcar_gen4_pcie_remove(struct platform_device *pdev)
+{
+	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
+
+	rcar_gen4_remove_dw_pcie_rp(rcar);
+	rcar_gen4_pcie_unprepare(rcar);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_gen4_pcie_of_match[] = {
+	{ .compatible = "renesas,rcar-gen4-pcie", },
+	{},
+};
+
+static struct platform_driver rcar_gen4_pcie_driver = {
+	.driver = {
+		.name = "pcie-rcar-gen4",
+		.of_match_table = rcar_gen4_pcie_of_match,
+	},
+	.probe = rcar_gen4_pcie_probe,
+	.remove = rcar_gen4_pcie_remove,
+};
+module_platform_driver(rcar_gen4_pcie_driver);
+
+MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
new file mode 100644
index 000000000000..4908892e413b
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/io.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+/* Renesas-specific */
+#define PCIERSTCTRL1		0x0014
+#define  APP_HOLD_PHY_RST	BIT(16)
+#define  APP_LTSSM_ENABLE	BIT(0)
+
+static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
+					bool enable)
+{
+	u32 val;
+
+	val = readl(rcar->base + PCIERSTCTRL1);
+	if (enable) {
+		val |= APP_LTSSM_ENABLE;
+		val &= ~APP_HOLD_PHY_RST;
+	} else {
+		val &= ~APP_LTSSM_ENABLE;
+		val |= APP_HOLD_PHY_RST;
+	}
+	writel(val, rcar->base + PCIERSTCTRL1);
+}
+
+static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
+{
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+	u32 val, mask;
+
+	val = readl(rcar->base + PCIEINTSTS0);
+	mask = RDLH_LINK_UP | SMLH_LINK_UP;
+
+	return (val & mask) == mask;
+}
+
+static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
+{
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+
+	rcar_gen4_pcie_ltssm_enable(rcar, true);
+
+	return 0;
+}
+
+static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
+{
+	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+
+	rcar_gen4_pcie_ltssm_enable(rcar, false);
+}
+
+int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
+				   int num_lanes)
+{
+	u32 val;
+
+	/* Note: Assume the reset is asserted here */
+	val = readl(rcar->base + PCIEMSR0);
+	if (rc)
+		val |= DEVICE_TYPE_RC;
+	else
+		val |= DEVICE_TYPE_EP;
+	if (num_lanes < 4)
+		val |= BIFUR_MOD_SET_ON;
+	writel(val, rcar->base + PCIEMSR0);
+
+	return reset_control_deassert(rcar->rst);
+}
+
+void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
+{
+	dw_pcie_writel_dbi2(dw, bar_mask_reg, 0x0);
+}
+
+int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
+{
+	struct device *dev = rcar->dw.dev;
+	int err;
+
+	pm_runtime_enable(dev);
+	err = pm_runtime_resume_and_get(dev);
+	if (err < 0) {
+		dev_err(dev, "Failed to resume/get Runtime PM\n");
+		pm_runtime_disable(dev);
+	}
+
+	return err;
+}
+
+void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
+{
+	struct device *dev = rcar->dw.dev;
+
+	if (!reset_control_status(rcar->rst))
+		reset_control_assert(rcar->rst);
+	pm_runtime_put(dev);
+	pm_runtime_disable(dev);
+}
+
+static int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
+				  struct device *dev)
+{
+	rcar->rst = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(rcar->rst)) {
+		dev_err(dev, "Failed to get Cold-reset\n");
+		return PTR_ERR(rcar->rst);
+	}
+
+	return 0;
+}
+
+int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
+				 struct platform_device *pdev)
+{
+	struct dw_pcie *dw = &rcar->dw;
+
+	/* Renesas-specific registers */
+	rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
+	if (IS_ERR(rcar->base))
+		return PTR_ERR(rcar->base);
+
+	return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
+}
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.start_link = rcar_gen4_pcie_start_link,
+	.stop_link = rcar_gen4_pcie_stop_link,
+	.link_up = rcar_gen4_pcie_link_up,
+};
+
+struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
+{
+	struct rcar_gen4_pcie *rcar;
+
+	rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
+	if (!rcar)
+		return NULL;
+
+	rcar->dw.dev = dev;
+	rcar->dw.ops = &dw_pcie_ops;
+	dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
+
+	return rcar;
+}
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
new file mode 100644
index 000000000000..786f80418aab
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#ifndef _PCIE_RCAR_GEN4_H_
+#define _PCIE_RCAR_GEN4_H_
+
+#include <linux/io.h>
+#include <linux/pci.h>
+#include <linux/reset.h>
+
+#include "pcie-designware.h"
+
+/* BAR Mask registers */
+#define BAR0MASKF		0x1010
+#define BAR1MASKF		0x1014
+#define BAR2MASKF		0x1018
+#define BAR3MASKF		0x101c
+#define BAR4MASKF		0x1020
+#define BAR5MASKF		0x1024
+
+/* Renesas-specific */
+#define PCIEMSR0		0x0000
+#define  BIFUR_MOD_SET_ON	BIT(0)
+#define  DEVICE_TYPE_EP		0
+#define  DEVICE_TYPE_RC		BIT(4)
+
+#define PCIEINTSTS0		0x0084
+#define PCIEINTSTS0EN		0x0310
+#define  MSI_CTRL_INT		BIT(26)
+#define  SMLH_LINK_UP		BIT(7)
+#define  RDLH_LINK_UP		BIT(6)
+#define PCIEDMAINTSTSEN		0x0314
+#define  PCIEDMAINTSTSEN_INIT	GENMASK(15, 0)
+
+struct rcar_gen4_pcie {
+	struct dw_pcie		dw;
+	void __iomem		*base;
+	struct reset_control	*rst;
+};
+#define to_rcar_gen4_pcie(x)	dev_get_drvdata((x)->dev)
+
+u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
+void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
+int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
+				   int num_lanes);
+void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
+int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
+void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
+int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
+				 struct platform_device *pdev);
+struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
+
+#endif /* _PCIE_RCAR_GEN4_H_ */
-- 
2.25.1

Re: [PATCH v11 10/13] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Serge Semin]

On Fri, Mar 10, 2023 at 09:35:07PM +0900, Yoshihiro Shimoda wrote:
> Add R-Car Gen4 PCIe Host support. This controller is based on
> Synopsys DesignWare PCIe.
> 
> This controller doesn't support MSI-X interrupt. So, introduce
> no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> from dw_pcie_msi_domain_info.
> 
> Note that this controller on R-Car S4-8 has an unexpected register
> value on the dbi+0x97b register. So, add a new capability flag
> which would force the unrolled eDMA mapping for the problematic
> device, as suggested by Serge Semin.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/pci/controller/dwc/Kconfig            |   9 +
>  drivers/pci/controller/dwc/Makefile           |   2 +
>  drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
>  drivers/pci/controller/dwc/pcie-designware.h  |   5 +-
>  .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 134 +++++++++++++++
>  drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 156 ++++++++++++++++++
>  drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  56 +++++++
>  7 files changed, 367 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 434f6a4f4041..94805ec31a8f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -414,4 +414,13 @@ config PCIE_FU740
>  	  Say Y here if you want PCIe controller support for the SiFive
>  	  FU740.
>  
> +config PCIE_RCAR_GEN4
> +	tristate "Renesas R-Car Gen4 PCIe Host controller"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on PCI_MSI
> +	select PCIE_DW_HOST
> +	help
> +	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> +	  This uses the DesignWare core.
> +
>  endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index bf5c311875a1..486cf706b53d 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
>  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
>  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
>  
>  # The following drivers are for devices that use the generic ACPI
>  # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 364926832126..6827d42fcb2c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -882,8 +882,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
>  	 * Indirect eDMA CSRs access has been completely removed since v5.40a
>  	 * thus no space is now reserved for the eDMA channels viewport and
>  	 * former DMA CTRL register is no longer fixed to FFs.

> +	 *
> +	 * Note some devices for unknown reason may have zeros in the eDMA CTRL
> +	 * register even though the HW-manual explicitly states there must FFs
> +	 * if the unrolled mapping is enabled. For such cases the low-level
> +	 * drivers are supposed to manually activate the unrolled mapping to
> +	 * bypass the auto-detection procedure.
>  	 */
> -	if (dw_pcie_ver_is_ge(pci, 540A))
> +	if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))

Again. Please detach to a preparation patch. Even though it's related
to your driver it's still a generic separate feature and having it
applied together with your driver doesn't really make it more coherent
but more complex.

>  		val = 0xFFFFFFFF;
>  	else
>  		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 3dbadb8043ab..1be74d2c3729 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -51,8 +51,9 @@
>  
>  /* DWC PCIe controller capabilities */
>  #define DW_PCIE_CAP_REQ_RES		0
> -#define DW_PCIE_CAP_IATU_UNROLL		1
> -#define DW_PCIE_CAP_CDM_CHECK		2
> +#define DW_PCIE_CAP_EDMA_UNROLL		1
> +#define DW_PCIE_CAP_IATU_UNROLL		2
> +#define DW_PCIE_CAP_CDM_CHECK		3
>  
>  #define dw_pcie_cap_is(_pci, _cap) \
>  	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> new file mode 100644
> index 000000000000..4aaecc813d85
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +	int ret;
> +	u32 val;
> +
> +	ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> +	if (ret < 0)
> +		return ret;
> +
> +	dw_pcie_dbi_ro_wr_en(dw);
> +

> +	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> +	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);

I'll ask it one more time. Why do you need these calls? None of the
DW PCIe RC glue-drivers perform the BARs disabling. Moreover you won't
disable them by using the methods above because there is no shadow
DBI2 CSRs behind the TYPe1.BAR{0,1} CSRs. So AFAICS you can just drop
the calls.

> +
> +	dw_pcie_dbi_ro_wr_dis(dw);
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +		/* Enable MSI interrupt signal */
> +		val = readl(rcar->base + PCIEINTSTS0EN);
> +		val |= MSI_CTRL_INT;
> +		writel(val, rcar->base + PCIEINTSTS0EN);
> +	}
> +
> +	gpiod_set_value_cansleep(dw->pe_rst, 0);
> +

> +	dw_pcie_setup_rc(pp);

This will be done shortly after this function returns. Why do you need
it here? AFAICS you can drop it.

> +
> +	dw_pcie_dbi_ro_wr_en(dw);

> +	dw_pcie_num_lanes_setup(dw, dw->num_lanes);

* Please see my note to the respective patch.

So if all my suggestions are implemented the rcar_gen4_pcie_host_init()
will contain only the true platform-specific initializations.

> +	dw_pcie_dbi_ro_wr_dis(dw);
> +
> +	return 0;
> +}
> +
> +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> +	.host_init = rcar_gen4_pcie_host_init,
> +};
> +
> +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> +				   struct platform_device *pdev)
> +{
> +	struct dw_pcie *dw = &rcar->dw;
> +	struct dw_pcie_rp *pp = &dw->pp;
> +
> +	pp->ops = &rcar_gen4_pcie_host_ops;
> +	dw_pcie_cap_set(dw, REQ_RES);
> +
> +	return dw_pcie_host_init(pp);
> +}
> +
> +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> +{
> +	dw_pcie_host_deinit(&rcar->dw.pp);
> +	gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> +}
> +
> +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rcar_gen4_pcie *rcar;
> +	int err;
> +
> +	rcar = rcar_gen4_pcie_devm_alloc(dev);
> +	if (!rcar)
> +		return -ENOMEM;
> +
> +	err = rcar_gen4_pcie_get_resources(rcar, pdev);
> +	if (err < 0) {
> +		dev_err(dev, "Failed to request resource: %d\n", err);
> +		return err;
> +	}
> +
> +	platform_set_drvdata(pdev, rcar);
> +
> +	err = rcar_gen4_pcie_prepare(rcar);
> +	if (err < 0)
> +		return err;
> +
> +	err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> +	if (err < 0)
> +		goto err_add;
> +
> +	return 0;
> +
> +err_add:
> +	rcar_gen4_pcie_unprepare(rcar);
> +
> +	return err;
> +}
> +
> +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> +{
> +	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> +
> +	rcar_gen4_remove_dw_pcie_rp(rcar);
> +	rcar_gen4_pcie_unprepare(rcar);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> +	{ .compatible = "renesas,rcar-gen4-pcie", },
> +	{},
> +};
> +
> +static struct platform_driver rcar_gen4_pcie_driver = {
> +	.driver = {
> +		.name = "pcie-rcar-gen4",
> +		.of_match_table = rcar_gen4_pcie_of_match,
> +	},
> +	.probe = rcar_gen4_pcie_probe,
> +	.remove = rcar_gen4_pcie_remove,
> +};
> +module_platform_driver(rcar_gen4_pcie_driver);
> +
> +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> new file mode 100644
> index 000000000000..4908892e413b
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/io.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +/* Renesas-specific */
> +#define PCIERSTCTRL1		0x0014
> +#define  APP_HOLD_PHY_RST	BIT(16)
> +#define  APP_LTSSM_ENABLE	BIT(0)
> +
> +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> +					bool enable)
> +{
> +	u32 val;
> +
> +	val = readl(rcar->base + PCIERSTCTRL1);
> +	if (enable) {
> +		val |= APP_LTSSM_ENABLE;
> +		val &= ~APP_HOLD_PHY_RST;
> +	} else {
> +		val &= ~APP_LTSSM_ENABLE;
> +		val |= APP_HOLD_PHY_RST;
> +	}
> +	writel(val, rcar->base + PCIERSTCTRL1);
> +}
> +
> +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> +{
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +	u32 val, mask;
> +
> +	val = readl(rcar->base + PCIEINTSTS0);
> +	mask = RDLH_LINK_UP | SMLH_LINK_UP;
> +
> +	return (val & mask) == mask;
> +}
> +
> +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> +{
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> +	rcar_gen4_pcie_ltssm_enable(rcar, true);
> +
> +	return 0;
> +}
> +
> +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> +{
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +
> +	rcar_gen4_pcie_ltssm_enable(rcar, false);
> +}
> +
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> +				   int num_lanes)
> +{
> +	u32 val;
> +
> +	/* Note: Assume the reset is asserted here */
> +	val = readl(rcar->base + PCIEMSR0);
> +	if (rc)
> +		val |= DEVICE_TYPE_RC;
> +	else
> +		val |= DEVICE_TYPE_EP;
> +	if (num_lanes < 4)
> +		val |= BIFUR_MOD_SET_ON;
> +	writel(val, rcar->base + PCIEMSR0);
> +
> +	return reset_control_deassert(rcar->rst);
> +}
> +
> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> +{
> +	dw_pcie_writel_dbi2(dw, bar_mask_reg, 0x0);
> +}
> +
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> +{
> +	struct device *dev = rcar->dw.dev;
> +	int err;
> +
> +	pm_runtime_enable(dev);
> +	err = pm_runtime_resume_and_get(dev);
> +	if (err < 0) {
> +		dev_err(dev, "Failed to resume/get Runtime PM\n");
> +		pm_runtime_disable(dev);
> +	}
> +
> +	return err;
> +}
> +
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> +{
> +	struct device *dev = rcar->dw.dev;
> +
> +	if (!reset_control_status(rcar->rst))
> +		reset_control_assert(rcar->rst);
> +	pm_runtime_put(dev);
> +	pm_runtime_disable(dev);
> +}
> +
> +static int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> +				  struct device *dev)
> +{
> +	rcar->rst = devm_reset_control_get(dev, NULL);
> +	if (IS_ERR(rcar->rst)) {
> +		dev_err(dev, "Failed to get Cold-reset\n");
> +		return PTR_ERR(rcar->rst);
> +	}
> +
> +	return 0;
> +}
> +
> +int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> +				 struct platform_device *pdev)
> +{
> +	struct dw_pcie *dw = &rcar->dw;
> +
> +	/* Renesas-specific registers */
> +	rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> +	if (IS_ERR(rcar->base))
> +		return PTR_ERR(rcar->base);
> +
> +	return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> +}
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = rcar_gen4_pcie_start_link,
> +	.stop_link = rcar_gen4_pcie_stop_link,
> +	.link_up = rcar_gen4_pcie_link_up,
> +};
> +
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> +{
> +	struct rcar_gen4_pcie *rcar;
> +
> +	rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> +	if (!rcar)
> +		return NULL;
> +
> +	rcar->dw.dev = dev;
> +	rcar->dw.ops = &dw_pcie_ops;
> +	dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> +
> +	return rcar;
> +}
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> new file mode 100644
> index 000000000000..786f80418aab
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#ifndef _PCIE_RCAR_GEN4_H_
> +#define _PCIE_RCAR_GEN4_H_
> +
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +/* BAR Mask registers */

> +#define BAR0MASKF		0x1010
> +#define BAR1MASKF		0x1014
> +#define BAR2MASKF		0x1018
> +#define BAR3MASKF		0x101c
> +#define BAR4MASKF		0x1020
> +#define BAR5MASKF		0x1024

I don't get it. You have DBI2 at the 0x1000 offset with respect to
the DBI base address. But the BARx CSRs are defined in additional
0x1000 offset. So it's 0x2000 all together. How come? This doesn't
seem right. In accordance with the DW PCIe EP manuals the shadow BARx
CSRs are defined within the DBI2 space at the normal offsets (0x10,
0x14, 0x18, 0x1c, 0x20, 0x24). So in case of the DW PCIe _EP_ you'll
only need to call dw_pcie_writel_dbi2(dw, {0x10, 0x14, 0x18, 0x1c, 0x20, 0x24}, 0x0);
in order to disable the BARs. Am I missing something? Could you double
check this for the _end-point_ part of the driver?

-Serge(y)

> +
> +/* Renesas-specific */
> +#define PCIEMSR0		0x0000
> +#define  BIFUR_MOD_SET_ON	BIT(0)
> +#define  DEVICE_TYPE_EP		0
> +#define  DEVICE_TYPE_RC		BIT(4)
> +
> +#define PCIEINTSTS0		0x0084
> +#define PCIEINTSTS0EN		0x0310
> +#define  MSI_CTRL_INT		BIT(26)
> +#define  SMLH_LINK_UP		BIT(7)
> +#define  RDLH_LINK_UP		BIT(6)
> +#define PCIEDMAINTSTSEN		0x0314
> +#define  PCIEDMAINTSTSEN_INIT	GENMASK(15, 0)
> +
> +struct rcar_gen4_pcie {
> +	struct dw_pcie		dw;
> +	void __iomem		*base;
> +	struct reset_control	*rst;
> +};
> +#define to_rcar_gen4_pcie(x)	dev_get_drvdata((x)->dev)
> +
> +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> +				   int num_lanes);
> +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> +int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> +				 struct platform_device *pdev);
> +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> +
> +#endif /* _PCIE_RCAR_GEN4_H_ */
> -- 
> 2.25.1
> 
> 

RE: [PATCH v11 10/13] PCI: rcar-gen4: Add R-Car Gen4 PCIe Host support

[Yoshihiro Shimoda]

Hi Serge,

> From: Serge Semin, Sent: Thursday, March 23, 2023 2:16 AM
> 
> On Fri, Mar 10, 2023 at 09:35:07PM +0900, Yoshihiro Shimoda wrote:
> > Add R-Car Gen4 PCIe Host support. This controller is based on
> > Synopsys DesignWare PCIe.
> >
> > This controller doesn't support MSI-X interrupt. So, introduce
> > no_msix flag in struct dw_pcie_host_ops to clear MSI_FLAG_PCI_MSIX
> > from dw_pcie_msi_domain_info.
> >
> > Note that this controller on R-Car S4-8 has an unexpected register
> > value on the dbi+0x97b register. So, add a new capability flag
> > which would force the unrolled eDMA mapping for the problematic
> > device, as suggested by Serge Semin.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  drivers/pci/controller/dwc/Kconfig            |   9 +
> >  drivers/pci/controller/dwc/Makefile           |   2 +
> >  drivers/pci/controller/dwc/pcie-designware.c  |   8 +-
> >  drivers/pci/controller/dwc/pcie-designware.h  |   5 +-
> >  .../pci/controller/dwc/pcie-rcar-gen4-host.c  | 134 +++++++++++++++
> >  drivers/pci/controller/dwc/pcie-rcar-gen4.c   | 156 ++++++++++++++++++
> >  drivers/pci/controller/dwc/pcie-rcar-gen4.h   |  56 +++++++
> >  7 files changed, 367 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.c
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4.h
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 434f6a4f4041..94805ec31a8f 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -414,4 +414,13 @@ config PCIE_FU740
> >  	  Say Y here if you want PCIe controller support for the SiFive
> >  	  FU740.
> >
> > +config PCIE_RCAR_GEN4
> > +	tristate "Renesas R-Car Gen4 PCIe Host controller"
> > +	depends on ARCH_RENESAS || COMPILE_TEST
> > +	depends on PCI_MSI
> > +	select PCIE_DW_HOST
> > +	help
> > +	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> > +	  This uses the DesignWare core.
> > +
> >  endmenu
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index bf5c311875a1..486cf706b53d 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -26,6 +26,8 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
> >  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
> >  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> >  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> > +pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> > +obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> >
> >  # The following drivers are for devices that use the generic ACPI
> >  # pci_root.c driver but don't support standard ECAM config access.
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 364926832126..6827d42fcb2c 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -882,8 +882,14 @@ static int dw_pcie_edma_find_chip(struct dw_pcie *pci)
> >  	 * Indirect eDMA CSRs access has been completely removed since v5.40a
> >  	 * thus no space is now reserved for the eDMA channels viewport and
> >  	 * former DMA CTRL register is no longer fixed to FFs.
> 
> > +	 *
> > +	 * Note some devices for unknown reason may have zeros in the eDMA CTRL
> > +	 * register even though the HW-manual explicitly states there must FFs
> > +	 * if the unrolled mapping is enabled. For such cases the low-level
> > +	 * drivers are supposed to manually activate the unrolled mapping to
> > +	 * bypass the auto-detection procedure.
> >  	 */
> > -	if (dw_pcie_ver_is_ge(pci, 540A))
> > +	if (dw_pcie_ver_is_ge(pci, 540A) || dw_pcie_cap_is(pci, EDMA_UNROLL))
> 
> Again. Please detach to a preparation patch. Even though it's related
> to your driver it's still a generic separate feature and having it
> applied together with your driver doesn't really make it more coherent
> but more complex.

I got it. I'll make such a patch on v12.

> >  		val = 0xFFFFFFFF;
> >  	else
> >  		val = dw_pcie_readl_dbi(pci, PCIE_DMA_VIEWPORT_BASE + PCIE_DMA_CTRL);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 3dbadb8043ab..1be74d2c3729 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -51,8 +51,9 @@
> >
> >  /* DWC PCIe controller capabilities */
> >  #define DW_PCIE_CAP_REQ_RES		0
> > -#define DW_PCIE_CAP_IATU_UNROLL		1
> > -#define DW_PCIE_CAP_CDM_CHECK		2
> > +#define DW_PCIE_CAP_EDMA_UNROLL		1
> > +#define DW_PCIE_CAP_IATU_UNROLL		2
> > +#define DW_PCIE_CAP_CDM_CHECK		3
> >
> >  #define dw_pcie_cap_is(_pci, _cap) \
> >  	test_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > new file mode 100644
> > index 000000000000..4aaecc813d85
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-host.c
> > @@ -0,0 +1,134 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> > +{
> > +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +	int ret;
> > +	u32 val;
> > +
> > +	ret = rcar_gen4_pcie_set_device_type(rcar, true, dw->num_lanes);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	dw_pcie_dbi_ro_wr_en(dw);
> > +
> 
> > +	rcar_gen4_pcie_disable_bar(dw, BAR0MASKF);
> > +	rcar_gen4_pcie_disable_bar(dw, BAR1MASKF);
> 
> I'll ask it one more time. Why do you need these calls? None of the
> DW PCIe RC glue-drivers perform the BARs disabling. Moreover you won't
> disable them by using the methods above because there is no shadow
> DBI2 CSRs behind the TYPe1.BAR{0,1} CSRs. So AFAICS you can just drop
> the calls.

If I remove these setting, the controller cannot work correctly. However,
To be honest, I don't know why these settings are needed. So, I'll investigate why
and add comments.

> > +
> > +	dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > +		/* Enable MSI interrupt signal */
> > +		val = readl(rcar->base + PCIEINTSTS0EN);
> > +		val |= MSI_CTRL_INT;
> > +		writel(val, rcar->base + PCIEINTSTS0EN);
> > +	}
> > +
> > +	gpiod_set_value_cansleep(dw->pe_rst, 0);
> > +
> 
> > +	dw_pcie_setup_rc(pp);
> 
> This will be done shortly after this function returns. Why do you need
> it here? AFAICS you can drop it.

Thank you for pointed it out. You're correct. I can drop it.

> > +
> > +	dw_pcie_dbi_ro_wr_en(dw);
> 
> > +	dw_pcie_num_lanes_setup(dw, dw->num_lanes);
> 
> * Please see my note to the respective patch.
> 
> So if all my suggestions are implemented the rcar_gen4_pcie_host_init()
> will contain only the true platform-specific initializations.

I got it. I'll drop this.

> > +	dw_pcie_dbi_ro_wr_dis(dw);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
> > +	.host_init = rcar_gen4_pcie_host_init,
> > +};
> > +
> > +static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar,
> > +				   struct platform_device *pdev)
> > +{
> > +	struct dw_pcie *dw = &rcar->dw;
> > +	struct dw_pcie_rp *pp = &dw->pp;
> > +
> > +	pp->ops = &rcar_gen4_pcie_host_ops;
> > +	dw_pcie_cap_set(dw, REQ_RES);
> > +
> > +	return dw_pcie_host_init(pp);
> > +}
> > +
> > +static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> > +{
> > +	dw_pcie_host_deinit(&rcar->dw.pp);
> > +	gpiod_set_value_cansleep(rcar->dw.pe_rst, 1);
> > +}
> > +
> > +static int rcar_gen4_pcie_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct rcar_gen4_pcie *rcar;
> > +	int err;
> > +
> > +	rcar = rcar_gen4_pcie_devm_alloc(dev);
> > +	if (!rcar)
> > +		return -ENOMEM;
> > +
> > +	err = rcar_gen4_pcie_get_resources(rcar, pdev);
> > +	if (err < 0) {
> > +		dev_err(dev, "Failed to request resource: %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, rcar);
> > +
> > +	err = rcar_gen4_pcie_prepare(rcar);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	err = rcar_gen4_add_dw_pcie_rp(rcar, pdev);
> > +	if (err < 0)
> > +		goto err_add;
> > +
> > +	return 0;
> > +
> > +err_add:
> > +	rcar_gen4_pcie_unprepare(rcar);
> > +
> > +	return err;
> > +}
> > +
> > +static int rcar_gen4_pcie_remove(struct platform_device *pdev)
> > +{
> > +	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> > +
> > +	rcar_gen4_remove_dw_pcie_rp(rcar);
> > +	rcar_gen4_pcie_unprepare(rcar);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > +	{ .compatible = "renesas,rcar-gen4-pcie", },
> > +	{},
> > +};
> > +
> > +static struct platform_driver rcar_gen4_pcie_driver = {
> > +	.driver = {
> > +		.name = "pcie-rcar-gen4",
> > +		.of_match_table = rcar_gen4_pcie_of_match,
> > +	},
> > +	.probe = rcar_gen4_pcie_probe,
> > +	.remove = rcar_gen4_pcie_remove,
> > +};
> > +module_platform_driver(rcar_gen4_pcie_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe host controller driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > new file mode 100644
> > index 000000000000..4908892e413b
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > @@ -0,0 +1,156 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/io.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +/* Renesas-specific */
> > +#define PCIERSTCTRL1		0x0014
> > +#define  APP_HOLD_PHY_RST	BIT(16)
> > +#define  APP_LTSSM_ENABLE	BIT(0)
> > +
> > +static void rcar_gen4_pcie_ltssm_enable(struct rcar_gen4_pcie *rcar,
> > +					bool enable)
> > +{
> > +	u32 val;
> > +
> > +	val = readl(rcar->base + PCIERSTCTRL1);
> > +	if (enable) {
> > +		val |= APP_LTSSM_ENABLE;
> > +		val &= ~APP_HOLD_PHY_RST;
> > +	} else {
> > +		val &= ~APP_LTSSM_ENABLE;
> > +		val |= APP_HOLD_PHY_RST;
> > +	}
> > +	writel(val, rcar->base + PCIERSTCTRL1);
> > +}
> > +
> > +static int rcar_gen4_pcie_link_up(struct dw_pcie *dw)
> > +{
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +	u32 val, mask;
> > +
> > +	val = readl(rcar->base + PCIEINTSTS0);
> > +	mask = RDLH_LINK_UP | SMLH_LINK_UP;
> > +
> > +	return (val & mask) == mask;
> > +}
> > +
> > +static int rcar_gen4_pcie_start_link(struct dw_pcie *dw)
> > +{
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > +	rcar_gen4_pcie_ltssm_enable(rcar, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static void rcar_gen4_pcie_stop_link(struct dw_pcie *dw)
> > +{
> > +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> > +
> > +	rcar_gen4_pcie_ltssm_enable(rcar, false);
> > +}
> > +
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > +				   int num_lanes)
> > +{
> > +	u32 val;
> > +
> > +	/* Note: Assume the reset is asserted here */
> > +	val = readl(rcar->base + PCIEMSR0);
> > +	if (rc)
> > +		val |= DEVICE_TYPE_RC;
> > +	else
> > +		val |= DEVICE_TYPE_EP;
> > +	if (num_lanes < 4)
> > +		val |= BIFUR_MOD_SET_ON;
> > +	writel(val, rcar->base + PCIEMSR0);
> > +
> > +	return reset_control_deassert(rcar->rst);
> > +}
> > +
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg)
> > +{
> > +	dw_pcie_writel_dbi2(dw, bar_mask_reg, 0x0);
> > +}
> > +
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *rcar)
> > +{
> > +	struct device *dev = rcar->dw.dev;
> > +	int err;
> > +
> > +	pm_runtime_enable(dev);
> > +	err = pm_runtime_resume_and_get(dev);
> > +	if (err < 0) {
> > +		dev_err(dev, "Failed to resume/get Runtime PM\n");
> > +		pm_runtime_disable(dev);
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
> > +{
> > +	struct device *dev = rcar->dw.dev;
> > +
> > +	if (!reset_control_status(rcar->rst))
> > +		reset_control_assert(rcar->rst);
> > +	pm_runtime_put(dev);
> > +	pm_runtime_disable(dev);
> > +}
> > +
> > +static int rcar_gen4_pcie_devm_reset_get(struct rcar_gen4_pcie *rcar,
> > +				  struct device *dev)
> > +{
> > +	rcar->rst = devm_reset_control_get(dev, NULL);
> > +	if (IS_ERR(rcar->rst)) {
> > +		dev_err(dev, "Failed to get Cold-reset\n");
> > +		return PTR_ERR(rcar->rst);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> > +				 struct platform_device *pdev)
> > +{
> > +	struct dw_pcie *dw = &rcar->dw;
> > +
> > +	/* Renesas-specific registers */
> > +	rcar->base = devm_platform_ioremap_resource_byname(pdev, "app");
> > +	if (IS_ERR(rcar->base))
> > +		return PTR_ERR(rcar->base);
> > +
> > +	return rcar_gen4_pcie_devm_reset_get(rcar, dw->dev);
> > +}
> > +
> > +static const struct dw_pcie_ops dw_pcie_ops = {
> > +	.start_link = rcar_gen4_pcie_start_link,
> > +	.stop_link = rcar_gen4_pcie_stop_link,
> > +	.link_up = rcar_gen4_pcie_link_up,
> > +};
> > +
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev)
> > +{
> > +	struct rcar_gen4_pcie *rcar;
> > +
> > +	rcar = devm_kzalloc(dev, sizeof(*rcar), GFP_KERNEL);
> > +	if (!rcar)
> > +		return NULL;
> > +
> > +	rcar->dw.dev = dev;
> > +	rcar->dw.ops = &dw_pcie_ops;
> > +	dw_pcie_cap_set(&rcar->dw, EDMA_UNROLL);
> > +
> > +	return rcar;
> > +}
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.h b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > new file mode 100644
> > index 000000000000..786f80418aab
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.h
> > @@ -0,0 +1,56 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * PCIe host/endpoint controller driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#ifndef _PCIE_RCAR_GEN4_H_
> > +#define _PCIE_RCAR_GEN4_H_
> > +
> > +#include <linux/io.h>
> > +#include <linux/pci.h>
> > +#include <linux/reset.h>
> > +
> > +#include "pcie-designware.h"
> > +
> > +/* BAR Mask registers */
> 
> > +#define BAR0MASKF		0x1010
> > +#define BAR1MASKF		0x1014
> > +#define BAR2MASKF		0x1018
> > +#define BAR3MASKF		0x101c
> > +#define BAR4MASKF		0x1020
> > +#define BAR5MASKF		0x1024
> 
> I don't get it. You have DBI2 at the 0x1000 offset with respect to
> the DBI base address. But the BARx CSRs are defined in additional
> 0x1000 offset. So it's 0x2000 all together. How come? This doesn't
> seem right. In accordance with the DW PCIe EP manuals the shadow BARx
> CSRs are defined within the DBI2 space at the normal offsets (0x10,
> 0x14, 0x18, 0x1c, 0x20, 0x24). So in case of the DW PCIe _EP_ you'll
> only need to call dw_pcie_writel_dbi2(dw, {0x10, 0x14, 0x18, 0x1c, 0x20, 0x24}, 0x0);
> in order to disable the BARs. Am I missing something? Could you double
> check this for the _end-point_ part of the driver?

Thank you for your comments. I checked the datasheet again and then
I completely misunderstood the shadow register and address map of
this controller. On the R-Car S4-8 SoC, the address map of this
controller is:

 +0x0000 : Function 0 (common address in Root complex and Endpoint mode)
 +0x1000 : Function 1 (Endpoint mode only)
 +0x2000 : Shadow register for Function 0
 +0x2800 : Shadow register for Function 1

So, I'll revise the dt-binding docs for both host and endpoint to add "dbi2".
And, I'll drop these BARnMASKF macros.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> > +
> > +/* Renesas-specific */
> > +#define PCIEMSR0		0x0000
> > +#define  BIFUR_MOD_SET_ON	BIT(0)
> > +#define  DEVICE_TYPE_EP		0
> > +#define  DEVICE_TYPE_RC		BIT(4)
> > +
> > +#define PCIEINTSTS0		0x0084
> > +#define PCIEINTSTS0EN		0x0310
> > +#define  MSI_CTRL_INT		BIT(26)
> > +#define  SMLH_LINK_UP		BIT(7)
> > +#define  RDLH_LINK_UP		BIT(6)
> > +#define PCIEDMAINTSTSEN		0x0314
> > +#define  PCIEDMAINTSTSEN_INIT	GENMASK(15, 0)
> > +
> > +struct rcar_gen4_pcie {
> > +	struct dw_pcie		dw;
> > +	void __iomem		*base;
> > +	struct reset_control	*rst;
> > +};
> > +#define to_rcar_gen4_pcie(x)	dev_get_drvdata((x)->dev)
> > +
> > +u32 rcar_gen4_pcie_readl(struct rcar_gen4_pcie *pcie, u32 reg);
> > +void rcar_gen4_pcie_writel(struct rcar_gen4_pcie *pcie, u32 reg, u32 val);
> > +int rcar_gen4_pcie_set_device_type(struct rcar_gen4_pcie *rcar, bool rc,
> > +				   int num_lanes);
> > +void rcar_gen4_pcie_disable_bar(struct dw_pcie *dw, u32 bar_mask_reg);
> > +int rcar_gen4_pcie_prepare(struct rcar_gen4_pcie *pcie);
> > +void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *pcie);
> > +int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar,
> > +				 struct platform_device *pdev);
> > +struct rcar_gen4_pcie *rcar_gen4_pcie_devm_alloc(struct device *dev);
> > +
> > +#endif /* _PCIE_RCAR_GEN4_H_ */
> > --
> > 2.25.1
> >
> >

[PATCH v11 11/13] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support

[Yoshihiro Shimoda]

Add R-Car Gen4 PCIe Endpoint support. This controller is based on
Synopsys DesignWare PCIe.

This controller requires vender-specific initialization before
.ep_init(). To use dw->dbi and dw->num-lanes in the initialization
code, introduce .ep_pre_init() into struct dw_pcie_ep_ops.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pci/controller/dwc/Kconfig            |   9 +
 drivers/pci/controller/dwc/Makefile           |   2 +
 .../pci/controller/dwc/pcie-designware-ep.c   |   3 +
 drivers/pci/controller/dwc/pcie-designware.h  |   1 +
 .../pci/controller/dwc/pcie-rcar-gen4-ep.c    | 170 ++++++++++++++++++
 5 files changed, 185 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 94805ec31a8f..f33e403c7b61 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -423,4 +423,13 @@ config PCIE_RCAR_GEN4
 	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
 	  This uses the DesignWare core.
 
+config PCIE_RCAR_GEN4_EP
+	tristate "Renesas R-Car Gen4 PCIe Endpoint controller"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on PCI_ENDPOINT
+	select PCIE_DW_EP
+	help
+	  Say Y here if you want PCIe endpoint controller support on R-Car Gen4
+	  SoCs. This uses the DesignWare core.
+
 endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 486cf706b53d..0fb0bde26ac4 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -28,6 +28,8 @@ obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
 obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
 pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
 obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
+pcie-rcar-gen4-ep-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-ep.o
+obj-$(CONFIG_PCIE_RCAR_GEN4_EP) += pcie-rcar-gen4-ep-drv.o
 
 # The following drivers are for devices that use the generic ACPI
 # pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 73b3844e8a09..8302053fa2da 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -763,6 +763,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->ep_pre_init)
+		ep->ops->ep_pre_init(ep);
+
 	dw_pcie_version_detect(pci);
 
 	dw_pcie_iatu_detect(pci);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 1be74d2c3729..f2026ac8b02f 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -325,6 +325,7 @@ struct dw_pcie_rp {
 };
 
 struct dw_pcie_ep_ops {
+	void	(*ep_pre_init)(struct dw_pcie_ep *ep);
 	void	(*ep_init)(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);
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
new file mode 100644
index 000000000000..4c763e5a6793
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe Endpoint driver for Renesas R-Car Gen4 Series SoCs
+ * Copyright (C) 2022-2023 Renesas Electronics Corporation
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-rcar-gen4.h"
+#include "pcie-designware.h"
+
+/* Configuration */
+#define PCICONF3		0x000c
+#define  MULTI_FUNC		BIT(23)
+
+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 val;
+
+	rcar_gen4_pcie_set_device_type(rcar, false, dw->num_lanes);
+
+	dw_pcie_dbi_ro_wr_en(dw);
+
+	/* Single function */
+	val = dw_pcie_readl_dbi(dw, PCICONF3);
+	val &= ~MULTI_FUNC;
+	dw_pcie_writel_dbi(dw, PCICONF3, val);
+
+	rcar_gen4_pcie_disable_bar(dw, BAR5MASKF);
+
+	dw_pcie_num_lanes_setup(dw, dw->num_lanes);
+
+	dw_pcie_dbi_ro_wr_dis(dw);
+}
+
+static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
+				       enum pci_epc_irq_type type,
+				       u16 interrupt_num)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
+
+	switch (type) {
+	case PCI_EPC_IRQ_LEGACY:
+		return dw_pcie_ep_raise_legacy_irq(ep, func_no);
+	case PCI_EPC_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_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 const struct dw_pcie_ep_ops pcie_ep_ops = {
+	.ep_pre_init = rcar_gen4_pcie_ep_pre_init,
+	.raise_irq = rcar_gen4_pcie_ep_raise_irq,
+	.get_features = rcar_gen4_pcie_ep_get_features,
+};
+
+static int rcar_gen4_add_pcie_ep(struct rcar_gen4_pcie *rcar,
+				 struct platform_device *pdev)
+{
+	struct dw_pcie *dw = &rcar->dw;
+	struct dw_pcie_ep *ep;
+	int ret;
+
+	ep = &dw->ep;
+	ep->ops = &pcie_ep_ops;
+
+	ret = dw_pcie_ep_init(ep);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize endpoint\n");
+		return ret;
+	}
+
+	writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
+
+	dw->ops->start_link(dw);
+
+	return 0;
+}
+
+static void rcar_gen4_remove_pcie_ep(struct rcar_gen4_pcie *rcar)
+{
+	writel(0, rcar->base + PCIEDMAINTSTSEN);
+	dw_pcie_ep_exit(&rcar->dw.ep);
+}
+
+static int rcar_gen4_pcie_ep_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rcar_gen4_pcie *rcar;
+	int err;
+
+	rcar = rcar_gen4_pcie_devm_alloc(dev);
+	if (!rcar)
+		return -ENOMEM;
+
+	err = rcar_gen4_pcie_get_resources(rcar, pdev);
+	if (err < 0) {
+		dev_err(dev, "Failed to request resource: %d\n", err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, rcar);
+
+	err = rcar_gen4_pcie_prepare(rcar);
+	if (err < 0)
+		return err;
+
+	err = rcar_gen4_add_pcie_ep(rcar, pdev);
+	if (err < 0)
+		goto err_add;
+
+	return 0;
+
+err_add:
+	rcar_gen4_pcie_unprepare(rcar);
+
+	return err;
+}
+
+static int rcar_gen4_pcie_ep_remove(struct platform_device *pdev)
+{
+	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
+
+	rcar_gen4_remove_pcie_ep(rcar);
+	rcar_gen4_pcie_unprepare(rcar);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_gen4_pcie_of_match[] = {
+	{ .compatible = "renesas,rcar-gen4-pcie-ep", },
+	{},
+};
+
+static struct platform_driver rcar_gen4_pcie_ep_driver = {
+	.driver = {
+		.name = "pcie-rcar-gen4-ep",
+		.of_match_table = rcar_gen4_pcie_of_match,
+	},
+	.probe = rcar_gen4_pcie_ep_probe,
+	.remove = rcar_gen4_pcie_ep_remove,
+};
+module_platform_driver(rcar_gen4_pcie_ep_driver);
+
+MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe endpoint controller driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v11 11/13] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support

[Serge Semin]

On Fri, Mar 10, 2023 at 09:35:08PM +0900, Yoshihiro Shimoda wrote:
> Add R-Car Gen4 PCIe Endpoint support. This controller is based on
> Synopsys DesignWare PCIe.
> 
> This controller requires vender-specific initialization before
> .ep_init(). To use dw->dbi and dw->num-lanes in the initialization
> code, introduce .ep_pre_init() into struct dw_pcie_ep_ops.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  drivers/pci/controller/dwc/Kconfig            |   9 +
>  drivers/pci/controller/dwc/Makefile           |   2 +
>  .../pci/controller/dwc/pcie-designware-ep.c   |   3 +
>  drivers/pci/controller/dwc/pcie-designware.h  |   1 +
>  .../pci/controller/dwc/pcie-rcar-gen4-ep.c    | 170 ++++++++++++++++++
>  5 files changed, 185 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 94805ec31a8f..f33e403c7b61 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -423,4 +423,13 @@ config PCIE_RCAR_GEN4
>  	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
>  	  This uses the DesignWare core.
>  
> +config PCIE_RCAR_GEN4_EP
> +	tristate "Renesas R-Car Gen4 PCIe Endpoint controller"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on PCI_ENDPOINT
> +	select PCIE_DW_EP
> +	help
> +	  Say Y here if you want PCIe endpoint controller support on R-Car Gen4
> +	  SoCs. This uses the DesignWare core.
> +
>  endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 486cf706b53d..0fb0bde26ac4 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -28,6 +28,8 @@ obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
>  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
>  pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
>  obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> +pcie-rcar-gen4-ep-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-ep.o
> +obj-$(CONFIG_PCIE_RCAR_GEN4_EP) += pcie-rcar-gen4-ep-drv.o
>  
>  # The following drivers are for devices that use the generic ACPI
>  # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 73b3844e8a09..8302053fa2da 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -763,6 +763,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->ep_pre_init)
> +		ep->ops->ep_pre_init(ep);
> +
>  	dw_pcie_version_detect(pci);
>  
>  	dw_pcie_iatu_detect(pci);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 1be74d2c3729..f2026ac8b02f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -325,6 +325,7 @@ struct dw_pcie_rp {
>  };
>  
>  struct dw_pcie_ep_ops {
> +	void	(*ep_pre_init)(struct dw_pcie_ep *ep);
>  	void	(*ep_init)(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);
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
> new file mode 100644
> index 000000000000..4c763e5a6793
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PCIe Endpoint driver for Renesas R-Car Gen4 Series SoCs
> + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-rcar-gen4.h"
> +#include "pcie-designware.h"
> +
> +/* Configuration */
> +#define PCICONF3		0x000c
> +#define  MULTI_FUNC		BIT(23)
> +
> +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 val;
> +
> +	rcar_gen4_pcie_set_device_type(rcar, false, dw->num_lanes);
> +
> +	dw_pcie_dbi_ro_wr_en(dw);
> +
> +	/* Single function */

> +	val = dw_pcie_readl_dbi(dw, PCICONF3);

There is a special macro PCI_HEADER_TYPE for the respective 8bit
field. You can use it together with the dw_pcie_readb_dbi() method.

> +	val &= ~MULTI_FUNC;

MULTI_FUNC is defined in the PCIe specification. What about updating
the include/uapi/linux/pci_regs.h file instead of defining a local
macro?

> +	dw_pcie_writel_dbi(dw, PCICONF3, val);
> +

> +	rcar_gen4_pcie_disable_bar(dw, BAR5MASKF);

Generically this can be done by calling dw_pcie_ep_reset_bar(). It also
writes zero to the shadow BARx CSRs. Otherwise please explain what is
mapped at the 0x2000 offset with respect to the DBI base address.

> +
> +	dw_pcie_num_lanes_setup(dw, dw->num_lanes);

* Please see my note to the respective patch.

> +
> +	dw_pcie_dbi_ro_wr_dis(dw);
> +}
> +
> +static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> +				       enum pci_epc_irq_type type,
> +				       u16 interrupt_num)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> +
> +	switch (type) {
> +	case PCI_EPC_IRQ_LEGACY:
> +		return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> +	case PCI_EPC_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_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 const struct dw_pcie_ep_ops pcie_ep_ops = {
> +	.ep_pre_init = rcar_gen4_pcie_ep_pre_init,
> +	.raise_irq = rcar_gen4_pcie_ep_raise_irq,
> +	.get_features = rcar_gen4_pcie_ep_get_features,
> +};
> +
> +static int rcar_gen4_add_pcie_ep(struct rcar_gen4_pcie *rcar,
> +				 struct platform_device *pdev)
> +{
> +	struct dw_pcie *dw = &rcar->dw;
> +	struct dw_pcie_ep *ep;
> +	int ret;
> +
> +	ep = &dw->ep;
> +	ep->ops = &pcie_ep_ops;
> +
> +	ret = dw_pcie_ep_init(ep);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize endpoint\n");
> +		return ret;
> +	}
> +

> +	writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);

Most likely this needs to be done either in ep_pre_init or in the
ep_init callback.

> +
> +	dw->ops->start_link(dw);

Why do you need to start the link right away thus interfering with the
PCI EP subsystem? It is supposed to be done by the PCIe EP core on
demand from the user-space (see pci_epc_start() usage).

> +
> +	return 0;
> +}
> +
> +static void rcar_gen4_remove_pcie_ep(struct rcar_gen4_pcie *rcar)
> +{

> +	writel(0, rcar->base + PCIEDMAINTSTSEN);

If we had dw_pcie_ep_ops.ep_deinit() this should have been done
there...

-Serge(y)

> +	dw_pcie_ep_exit(&rcar->dw.ep);
> +}
> +
> +static int rcar_gen4_pcie_ep_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rcar_gen4_pcie *rcar;
> +	int err;
> +
> +	rcar = rcar_gen4_pcie_devm_alloc(dev);
> +	if (!rcar)
> +		return -ENOMEM;
> +
> +	err = rcar_gen4_pcie_get_resources(rcar, pdev);
> +	if (err < 0) {
> +		dev_err(dev, "Failed to request resource: %d\n", err);
> +		return err;
> +	}
> +
> +	platform_set_drvdata(pdev, rcar);
> +
> +	err = rcar_gen4_pcie_prepare(rcar);
> +	if (err < 0)
> +		return err;
> +
> +	err = rcar_gen4_add_pcie_ep(rcar, pdev);
> +	if (err < 0)
> +		goto err_add;
> +
> +	return 0;
> +
> +err_add:
> +	rcar_gen4_pcie_unprepare(rcar);
> +
> +	return err;
> +}
> +
> +static int rcar_gen4_pcie_ep_remove(struct platform_device *pdev)
> +{
> +	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> +
> +	rcar_gen4_remove_pcie_ep(rcar);
> +	rcar_gen4_pcie_unprepare(rcar);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> +	{ .compatible = "renesas,rcar-gen4-pcie-ep", },
> +	{},
> +};
> +
> +static struct platform_driver rcar_gen4_pcie_ep_driver = {
> +	.driver = {
> +		.name = "pcie-rcar-gen4-ep",
> +		.of_match_table = rcar_gen4_pcie_of_match,
> +	},
> +	.probe = rcar_gen4_pcie_ep_probe,
> +	.remove = rcar_gen4_pcie_ep_remove,
> +};
> +module_platform_driver(rcar_gen4_pcie_ep_driver);
> +
> +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe endpoint controller driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 
> 

RE: [PATCH v11 11/13] PCI: rcar-gen4-ep: Add R-Car Gen4 PCIe Endpoint support

[Yoshihiro Shimoda]

Hi Serge,

> From: Serge Semin, Sent: Thursday, March 23, 2023 2:57 AM
> 
> On Fri, Mar 10, 2023 at 09:35:08PM +0900, Yoshihiro Shimoda wrote:
> > Add R-Car Gen4 PCIe Endpoint support. This controller is based on
> > Synopsys DesignWare PCIe.
> >
> > This controller requires vender-specific initialization before
> > .ep_init(). To use dw->dbi and dw->num-lanes in the initialization
> > code, introduce .ep_pre_init() into struct dw_pcie_ep_ops.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  drivers/pci/controller/dwc/Kconfig            |   9 +
> >  drivers/pci/controller/dwc/Makefile           |   2 +
> >  .../pci/controller/dwc/pcie-designware-ep.c   |   3 +
> >  drivers/pci/controller/dwc/pcie-designware.h  |   1 +
> >  .../pci/controller/dwc/pcie-rcar-gen4-ep.c    | 170 ++++++++++++++++++
> >  5 files changed, 185 insertions(+)
> >  create mode 100644 drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 94805ec31a8f..f33e403c7b61 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -423,4 +423,13 @@ config PCIE_RCAR_GEN4
> >  	  Say Y here if you want PCIe host controller support on R-Car Gen4 SoCs.
> >  	  This uses the DesignWare core.
> >
> > +config PCIE_RCAR_GEN4_EP
> > +	tristate "Renesas R-Car Gen4 PCIe Endpoint controller"
> > +	depends on ARCH_RENESAS || COMPILE_TEST
> > +	depends on PCI_ENDPOINT
> > +	select PCIE_DW_EP
> > +	help
> > +	  Say Y here if you want PCIe endpoint controller support on R-Car Gen4
> > +	  SoCs. This uses the DesignWare core.
> > +
> >  endmenu
> > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> > index 486cf706b53d..0fb0bde26ac4 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -28,6 +28,8 @@ obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> >  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> >  pcie-rcar-gen4-host-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-host.o
> >  obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4-host-drv.o
> > +pcie-rcar-gen4-ep-drv-objs := pcie-rcar-gen4.o pcie-rcar-gen4-ep.o
> > +obj-$(CONFIG_PCIE_RCAR_GEN4_EP) += pcie-rcar-gen4-ep-drv.o
> >
> >  # The following drivers are for devices that use the generic ACPI
> >  # pci_root.c driver but don't support standard ECAM config access.
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 73b3844e8a09..8302053fa2da 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -763,6 +763,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->ep_pre_init)
> > +		ep->ops->ep_pre_init(ep);
> > +
> >  	dw_pcie_version_detect(pci);
> >
> >  	dw_pcie_iatu_detect(pci);
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 1be74d2c3729..f2026ac8b02f 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -325,6 +325,7 @@ struct dw_pcie_rp {
> >  };
> >
> >  struct dw_pcie_ep_ops {
> > +	void	(*ep_pre_init)(struct dw_pcie_ep *ep);
> >  	void	(*ep_init)(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);
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
> > new file mode 100644
> > index 000000000000..4c763e5a6793
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4-ep.c
> > @@ -0,0 +1,170 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe Endpoint driver for Renesas R-Car Gen4 Series SoCs
> > + * Copyright (C) 2022-2023 Renesas Electronics Corporation
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include "pcie-rcar-gen4.h"
> > +#include "pcie-designware.h"
> > +
> > +/* Configuration */
> > +#define PCICONF3		0x000c
> > +#define  MULTI_FUNC		BIT(23)
> > +
> > +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 val;
> > +
> > +	rcar_gen4_pcie_set_device_type(rcar, false, dw->num_lanes);
> > +
> > +	dw_pcie_dbi_ro_wr_en(dw);
> > +
> > +	/* Single function */
> 
> > +	val = dw_pcie_readl_dbi(dw, PCICONF3);
> 
> There is a special macro PCI_HEADER_TYPE for the respective 8bit
> field. You can use it together with the dw_pcie_readb_dbi() method.

Thank you for your comment. I got it.

> > +	val &= ~MULTI_FUNC;
> 
> MULTI_FUNC is defined in the PCIe specification. What about updating
> the include/uapi/linux/pci_regs.h file instead of defining a local
> macro?

I got it. I'll fix this on v12.

> > +	dw_pcie_writel_dbi(dw, PCICONF3, val);
> > +
> 
> > +	rcar_gen4_pcie_disable_bar(dw, BAR5MASKF);
> 
> Generically this can be done by calling dw_pcie_ep_reset_bar(). It also
> writes zero to the shadow BARx CSRs. Otherwise please explain what is
> mapped at the 0x2000 offset with respect to the DBI base address.

You're correct. I'll drop this on v12.

> > +
> > +	dw_pcie_num_lanes_setup(dw, dw->num_lanes);
> 
> * Please see my note to the respective patch.

I got it.

> > +
> > +	dw_pcie_dbi_ro_wr_dis(dw);
> > +}
> > +
> > +static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > +				       enum pci_epc_irq_type type,
> > +				       u16 interrupt_num)
> > +{
> > +	struct dw_pcie *dw = to_dw_pcie_from_ep(ep);
> > +
> > +	switch (type) {
> > +	case PCI_EPC_IRQ_LEGACY:
> > +		return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> > +	case PCI_EPC_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_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 const struct dw_pcie_ep_ops pcie_ep_ops = {
> > +	.ep_pre_init = rcar_gen4_pcie_ep_pre_init,
> > +	.raise_irq = rcar_gen4_pcie_ep_raise_irq,
> > +	.get_features = rcar_gen4_pcie_ep_get_features,
> > +};
> > +
> > +static int rcar_gen4_add_pcie_ep(struct rcar_gen4_pcie *rcar,
> > +				 struct platform_device *pdev)
> > +{
> > +	struct dw_pcie *dw = &rcar->dw;
> > +	struct dw_pcie_ep *ep;
> > +	int ret;
> > +
> > +	ep = &dw->ep;
> > +	ep->ops = &pcie_ep_ops;
> > +
> > +	ret = dw_pcie_ep_init(ep);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to initialize endpoint\n");
> > +		return ret;
> > +	}
> > +
> 
> > +	writel(PCIEDMAINTSTSEN_INIT, rcar->base + PCIEDMAINTSTSEN);
> 
> Most likely this needs to be done either in ep_pre_init or in the
> ep_init callback.

I think so. I'll fix it.

> > +
> > +	dw->ops->start_link(dw);
> 
> Why do you need to start the link right away thus interfering with the
> PCI EP subsystem? It is supposed to be done by the PCIe EP core on
> demand from the user-space (see pci_epc_start() usage).

Thank you for pointed it out. I also realized this is strange.
I think I can drop this on v12.

> > +
> > +	return 0;
> > +}
> > +
> > +static void rcar_gen4_remove_pcie_ep(struct rcar_gen4_pcie *rcar)
> > +{
> 
> > +	writel(0, rcar->base + PCIEDMAINTSTSEN);
> 
> If we had dw_pcie_ep_ops.ep_deinit() this should have been done
> there...

I see. I'll create such a patch which add ep_deinit(), and disable
the irq from the function.

Best regards,
Yoshihiro Shimoda

> -Serge(y)
> 
> > +	dw_pcie_ep_exit(&rcar->dw.ep);
> > +}
> > +
> > +static int rcar_gen4_pcie_ep_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct rcar_gen4_pcie *rcar;
> > +	int err;
> > +
> > +	rcar = rcar_gen4_pcie_devm_alloc(dev);
> > +	if (!rcar)
> > +		return -ENOMEM;
> > +
> > +	err = rcar_gen4_pcie_get_resources(rcar, pdev);
> > +	if (err < 0) {
> > +		dev_err(dev, "Failed to request resource: %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, rcar);
> > +
> > +	err = rcar_gen4_pcie_prepare(rcar);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	err = rcar_gen4_add_pcie_ep(rcar, pdev);
> > +	if (err < 0)
> > +		goto err_add;
> > +
> > +	return 0;
> > +
> > +err_add:
> > +	rcar_gen4_pcie_unprepare(rcar);
> > +
> > +	return err;
> > +}
> > +
> > +static int rcar_gen4_pcie_ep_remove(struct platform_device *pdev)
> > +{
> > +	struct rcar_gen4_pcie *rcar = platform_get_drvdata(pdev);
> > +
> > +	rcar_gen4_remove_pcie_ep(rcar);
> > +	rcar_gen4_pcie_unprepare(rcar);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id rcar_gen4_pcie_of_match[] = {
> > +	{ .compatible = "renesas,rcar-gen4-pcie-ep", },
> > +	{},
> > +};
> > +
> > +static struct platform_driver rcar_gen4_pcie_ep_driver = {
> > +	.driver = {
> > +		.name = "pcie-rcar-gen4-ep",
> > +		.of_match_table = rcar_gen4_pcie_of_match,
> > +	},
> > +	.probe = rcar_gen4_pcie_ep_probe,
> > +	.remove = rcar_gen4_pcie_ep_remove,
> > +};
> > +module_platform_driver(rcar_gen4_pcie_ep_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas R-Car Gen4 PCIe endpoint controller driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
> >
> >

[PATCH v11 12/13] MAINTAINERS: Update PCI DRIVER FOR RENESAS R-CAR for R-Car Gen4

[Yoshihiro Shimoda]

Update this entry for R-Car Gen4's source code.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7002a5d3eb62..5e0268f5121b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16050,6 +16050,7 @@ L:	linux-renesas-soc@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/pci/*rcar*
 F:	drivers/pci/controller/*rcar*
+F:	drivers/pci/controller/dwc/*rcar*
 
 PCI DRIVER FOR SAMSUNG EXYNOS
 M:	Jingoo Han <jingoohan1@gmail.com>
-- 
2.25.1

[PATCH v11 13/13] misc: pci_endpoint_test: Add Device ID for R-Car S4-8 PCIe controller

[Yoshihiro Shimoda]

Add Renesas R8A779F0 in pci_device_id table so that pci-epf-test
can be used for testing PCIe EP on R-Car S4-8.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/misc/pci_endpoint_test.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index a7244de081ec..1d8f72b42c0a 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -81,6 +81,7 @@
 #define PCI_DEVICE_ID_RENESAS_R8A774B1		0x002b
 #define PCI_DEVICE_ID_RENESAS_R8A774C0		0x002d
 #define PCI_DEVICE_ID_RENESAS_R8A774E1		0x0025
+#define PCI_DEVICE_ID_RENESAS_R8A779F0		0x0031
 
 static DEFINE_IDA(pci_endpoint_test_ida);
 
@@ -993,6 +994,9 @@ static const struct pci_device_id pci_endpoint_test_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, PCI_DEVICE_ID_RENESAS_R8A774B1),},
 	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, PCI_DEVICE_ID_RENESAS_R8A774C0),},
 	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, PCI_DEVICE_ID_RENESAS_R8A774E1),},
+	{ PCI_DEVICE(PCI_VENDOR_ID_RENESAS, PCI_DEVICE_ID_RENESAS_R8A779F0),
+	  .driver_data = (kernel_ulong_t)&default_data,
+	},
 	{ PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_J721E),
 	  .driver_data = (kernel_ulong_t)&j721e_data,
 	},
-- 
2.25.1