[PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Stefan Roese]

This patch adds basic endpoint support to the 4xx PCIe driver.

This is done by checking whether the PCIe port is already configured as
root-complex or as endpoint. This has been done previously in U-Boot and
can be configured there dynamically by setting the "pcie_mode"
environment variable (reboot necessary of course). Here an example:

  pcie_mode = 'RP:RP:EP'

Port 0 & 1 are configured as root-complex and port 2 as endpoint.
This mode will now be used in the Linux driver too.

Note: Currently we map a fixed 64MByte window to PLB address 0 (SDRAM).
This should probably be configurable via a dts property.

Signed-off-by: Stefan Roese <sr@denx.de>
---
 arch/powerpc/sysdev/ppc4xx_pci.c |  133 +++++++++++++++++++++++++++-----------
 1 files changed, 96 insertions(+), 37 deletions(-)

diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
index d183b83..013fd05 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.c
+++ b/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -563,6 +563,18 @@ struct ppc4xx_pciex_hwops
 
 static struct ppc4xx_pciex_hwops *ppc4xx_pciex_hwops;
 
+static int is_endpoint(struct ppc4xx_pciex_port *port)
+{
+	u32 val;
+
+	val = mfdcri(SDR0, port->sdr_base + PESDRn_DLPSET);
+
+	if (((val >> 20) & 0xf) == PTYPE_LEGACY_ENDPOINT)
+		return 1;
+	else
+		return 0;
+}
+
 #ifdef CONFIG_44x
 
 /* Check various reset bits of the 440SPe PCIe core */
@@ -1400,28 +1412,59 @@ static void __init ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
 	resource_size_t size = res->end - res->start + 1;
 	u64 sa;
 
-	/* Calculate window size */
-	sa = (0xffffffffffffffffull << ilog2(size));;
-	if (res->flags & IORESOURCE_PREFETCH)
-		sa |= 0x8;
+	if (port->endpoint) {
+		resource_size_t ep_addr = 0;
+		resource_size_t ep_size = 32 << 20;
 
-	out_le32(mbase + PECFG_BAR0HMPA, RES_TO_U32_HIGH(sa));
-	out_le32(mbase + PECFG_BAR0LMPA, RES_TO_U32_LOW(sa));
+		/* Currently we map a fixed 64MByte window to PLB address
+		 * 0 (SDRAM). This should probably be configurable via a dts
+		 * property.
+		 */
+
+		/* Calculate window size */
+		sa = (0xffffffffffffffffull << ilog2(ep_size));;
+
+		/* Setup BAR0 */
+		out_le32(mbase + PECFG_BAR0HMPA, RES_TO_U32_HIGH(sa));
+		out_le32(mbase + PECFG_BAR0LMPA, RES_TO_U32_LOW(sa) |
+			 PCI_BASE_ADDRESS_MEM_TYPE_64);
 
-	/* The setup of the split looks weird to me ... let's see if it works */
-	out_le32(mbase + PECFG_PIM0LAL, 0x00000000);
-	out_le32(mbase + PECFG_PIM0LAH, 0x00000000);
-	out_le32(mbase + PECFG_PIM1LAL, 0x00000000);
-	out_le32(mbase + PECFG_PIM1LAH, 0x00000000);
-	out_le32(mbase + PECFG_PIM01SAH, 0xffff0000);
-	out_le32(mbase + PECFG_PIM01SAL, 0x00000000);
+		/* Disable BAR1 & BAR2 */
+		out_le32(mbase + PECFG_BAR1MPA, 0);
+		out_le32(mbase + PECFG_BAR2HMPA, 0);
+		out_le32(mbase + PECFG_BAR2LMPA, 0);
+
+		out_le32(mbase + PECFG_PIM01SAH, RES_TO_U32_HIGH(sa));
+		out_le32(mbase + PECFG_PIM01SAL, RES_TO_U32_LOW(sa));
+
+		out_le32(mbase + PCI_BASE_ADDRESS_0, RES_TO_U32_LOW(ep_addr));
+		out_le32(mbase + PCI_BASE_ADDRESS_1, RES_TO_U32_HIGH(ep_addr));
+	} else {
+		/* Calculate window size */
+		sa = (0xffffffffffffffffull << ilog2(size));;
+		if (res->flags & IORESOURCE_PREFETCH)
+			sa |= 0x8;
+
+		out_le32(mbase + PECFG_BAR0HMPA, RES_TO_U32_HIGH(sa));
+		out_le32(mbase + PECFG_BAR0LMPA, RES_TO_U32_LOW(sa));
+
+		/* The setup of the split looks weird to me ... let's see
+		 * if it works
+		 */
+		out_le32(mbase + PECFG_PIM0LAL, 0x00000000);
+		out_le32(mbase + PECFG_PIM0LAH, 0x00000000);
+		out_le32(mbase + PECFG_PIM1LAL, 0x00000000);
+		out_le32(mbase + PECFG_PIM1LAH, 0x00000000);
+		out_le32(mbase + PECFG_PIM01SAH, 0xffff0000);
+		out_le32(mbase + PECFG_PIM01SAL, 0x00000000);
+
+		out_le32(mbase + PCI_BASE_ADDRESS_0, RES_TO_U32_LOW(res->start));
+		out_le32(mbase + PCI_BASE_ADDRESS_1, RES_TO_U32_HIGH(res->start));
+	}
 
 	/* Enable inbound mapping */
 	out_le32(mbase + PECFG_PIMEN, 0x1);
 
-	out_le32(mbase + PCI_BASE_ADDRESS_0, RES_TO_U32_LOW(res->start));
-	out_le32(mbase + PCI_BASE_ADDRESS_1, RES_TO_U32_HIGH(res->start));
-
 	/* Enable I/O, Mem, and Busmaster cycles */
 	out_le16(mbase + PCI_COMMAND,
 		 in_le16(mbase + PCI_COMMAND) |
@@ -1436,13 +1479,6 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	int primary = 0, busses;
 	void __iomem *mbase = NULL, *cfg_data = NULL;
 
-	/* XXX FIXME: Handle endpoint mode properly */
-	if (port->endpoint) {
-		printk(KERN_WARNING "PCIE%d: Port in endpoint mode !\n",
-		       port->index);
-		return;
-	}
-
 	/* Check if primary bridge */
 	if (of_get_property(port->node, "primary", NULL))
 		primary = 1;
@@ -1502,12 +1538,14 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	port->hose = hose;
 	mbase = (void __iomem *)hose->cfg_addr;
 
-	/*
-	 * Set bus numbers on our root port
-	 */
-	out_8(mbase + PCI_PRIMARY_BUS, hose->first_busno);
-	out_8(mbase + PCI_SECONDARY_BUS, hose->first_busno + 1);
-	out_8(mbase + PCI_SUBORDINATE_BUS, hose->last_busno);
+	if (!port->endpoint) {
+		/*
+		 * Set bus numbers on our root port
+		 */
+		out_8(mbase + PCI_PRIMARY_BUS, hose->first_busno);
+		out_8(mbase + PCI_SECONDARY_BUS, hose->first_busno + 1);
+		out_8(mbase + PCI_SUBORDINATE_BUS, hose->last_busno);
+	}
 
 	/*
 	 * OMRs are already reset, also disable PIMs
@@ -1531,14 +1569,26 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	 * and device IDs into it. Those are the same bogus one that the
 	 * initial code in arch/ppc add. We might want to change that.
 	 */
-	out_le16(mbase + 0x200, 0xaaa0 + port->index);
-	out_le16(mbase + 0x202, 0xbed0 + port->index);
+	if (!port->endpoint) {
+		out_le16(mbase + 0x200, 0xaaa0 + port->index);
+		out_le16(mbase + 0x202, 0xbed0 + port->index);
 
-	/* Set Class Code to PCI-PCI bridge and Revision Id to 1 */
-	out_le32(mbase + 0x208, 0x06040001);
+		/* Set Class Code to PCI-PCI bridge and Revision Id to 1 */
+		out_le32(mbase + 0x208, 0x06040001);
+
+		printk(KERN_INFO "PCIE%d: successfully set as root-complex\n",
+		       port->index);
+	} else {
+		out_le16(mbase + 0x200, 0xeee0 + port->index);
+		out_le16(mbase + 0x202, 0xfed0 + port->index);
+
+		/* Set Class Code to Processor/PPC */
+		out_le32(mbase + 0x208, 0x0b200001);
+
+		printk(KERN_INFO "PCIE%d: successfully set as endpoint\n",
+		       port->index);
+	}
 
-	printk(KERN_INFO "PCIE%d: successfully set as root-complex\n",
-	       port->index);
 	return;
  fail:
 	if (hose)
@@ -1586,8 +1636,17 @@ static void __init ppc4xx_probe_pciex_bridge(struct device_node *np)
 	}
 	port->sdr_base = *pval;
 
-	/* XXX Currently, we only support root complex mode */
-	port->endpoint = 0;
+	/* Check whether the PCIe port is already configured as root-complex
+	 * or as endpoint. This has been done previously in U-Boot and can
+	 * be configured there dynamically by setting the "pcie_mode"
+	 * environment variable (reboot necessary of course). Here an example:
+	 *
+	 * pcie_mode = 'RP:RP:EP'
+	 *
+	 * Port 0 & 1 are configured as root-complex and port 2 as endpoint.
+	 * This mode will now be used in the Linux driver too.
+	 */
+	port->endpoint = is_endpoint(port);
 
 	/* Fetch config space registers address */
 	if (of_address_to_resource(np, 0, &port->cfg_space)) {
-- 
1.5.4.5

Re: [PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Stefan Roese]

On Wednesday 02 April 2008, Stefan Roese wrote:
> This patch adds basic endpoint support to the 4xx PCIe driver.

This should have been an RFC. Sorry I missed adding it to the subject.

Best regards,
Stefan

Re: [PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Benjamin Herrenschmidt]

On Wed, 2008-04-02 at 17:12 +0200, Stefan Roese wrote:

   ../..

Overall looks good, just a few things:

In general, while I have nothing against the idea of reading the HW
setup left by uboot, I wonder if it wouldn't be best to have this file
capable of fully configuring it in either mode based on DT properties
instead. The PCI node in endpoint mode would be called something
different, such as "pci-endpoint" and would contain some kind of
"endpoint-mode" property, maybe ? That way people using other
bootloaders or even booting of straight flash kernels can still use
this.

> +
> +               out_le32(mbase + PCI_BASE_ADDRESS_0, RES_TO_U32_LOW(res->start));
> +               out_le32(mbase + PCI_BASE_ADDRESS_1, RES_TO_U32_HIGH(res->start));
> +       }
>  
>         /* Enable inbound mapping */
>         out_le32(mbase + PECFG_PIMEN, 0x1);
>  
> -       out_le32(mbase + PCI_BASE_ADDRESS_0, RES_TO_U32_LOW(res->start));
> -       out_le32(mbase + PCI_BASE_ADDRESS_1, RES_TO_U32_HIGH(res->start));
> -

does it work properly to setup the BARs before enabling the inbound
mapping ?

>          * OMRs are already reset, also disable PIMs
> @@ -1531,14 +1569,26 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
>          * and device IDs into it. Those are the same bogus one that the
>          * initial code in arch/ppc add. We might want to change that.
>          */
> -       out_le16(mbase + 0x200, 0xaaa0 + port->index);
> -       out_le16(mbase + 0x202, 0xbed0 + port->index);
> +       if (!port->endpoint) {
> +               out_le16(mbase + 0x200, 0xaaa0 + port->index);
> +               out_le16(mbase + 0x202, 0xbed0 + port->index);
>  

We should probably setup the config space IDs based on some device-tree
properties no ?

> -       /* Set Class Code to PCI-PCI bridge and Revision Id to 1 */
> -       out_le32(mbase + 0x208, 0x06040001);
> +               /* Set Class Code to PCI-PCI bridge and Revision Id to 1 */
> +               out_le32(mbase + 0x208, 0x06040001);
> +
> +               printk(KERN_INFO "PCIE%d: successfully set as root-complex\n",
> +                      port->index);
> +       } else {
> +               out_le16(mbase + 0x200, 0xeee0 + port->index);
> +               out_le16(mbase + 0x202, 0xfed0 + port->index);
> +
> +               /* Set Class Code to Processor/PPC */
> +               out_le32(mbase + 0x208, 0x0b200001);
> +
> +               printk(KERN_INFO "PCIE%d: successfully set as endpoint\n",
> +                      port->index);
> +       }
>  
> -       printk(KERN_INFO "PCIE%d: successfully set as root-complex\n",
> -              port->index);
>         return;
>   fail:
>         if (hose)
> @@ -1586,8 +1636,17 @@ static void __init ppc4xx_probe_pciex_bridge(struct device_node *np)
>         }
>         port->sdr_base = *pval;
>  
> -       /* XXX Currently, we only support root complex mode */
> -       port->endpoint = 0;
> +       /* Check whether the PCIe port is already configured as root-complex
> +        * or as endpoint. This has been done previously in U-Boot and can
> +        * be configured there dynamically by setting the "pcie_mode"
> +        * environment variable (reboot necessary of course). Here an example:
> +        *
> +        * pcie_mode = 'RP:RP:EP'
> +        *
> +        * Port 0 & 1 are configured as root-complex and port 2 as endpoint.
> +        * This mode will now be used in the Linux driver too.
> +        */
> +       port->endpoint = is_endpoint(port);
>  
>         /* Fetch config space registers address */
>         if (of_address_to_resource(np, 0, &port->cfg_space)) {
> -- 
> 1.5.4.5
> 

Re: [PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Stefan Roese]

On Thursday 10 April 2008, Benjamin Herrenschmidt wrote:
> On Wed, 2008-04-02 at 17:12 +0200, Stefan Roese wrote:
>
>    ../..
>
> Overall looks good, just a few things:

Ben, thanks for the review.

> In general, while I have nothing against the idea of reading the HW
> setup left by uboot, I wonder if it wouldn't be best to have this file
> capable of fully configuring it in either mode based on DT properties
> instead.

Sure, this would be optimal. But frankly, I currently have no need for this 
non U-Boot mode, and therefore I didn't implement it. This can be added later 
when really needed don't you think?

> The PCI node in endpoint mode would be called something 
> different, such as "pci-endpoint" and would contain some kind of
> "endpoint-mode" property, maybe ? That way people using other
> bootloaders or even booting of straight flash kernels can still use
> this.

Good. But again, I would really prefer to first include this U-Boot style 
endpoint support and later add this independent device tree endpoint 
configuration when really needed.

> > +
> > +               out_le32(mbase + PCI_BASE_ADDRESS_0,
> > RES_TO_U32_LOW(res->start)); +               out_le32(mbase +
> > PCI_BASE_ADDRESS_1, RES_TO_U32_HIGH(res->start)); +       }
> >
> >         /* Enable inbound mapping */
> >         out_le32(mbase + PECFG_PIMEN, 0x1);
> >
> > -       out_le32(mbase + PCI_BASE_ADDRESS_0, RES_TO_U32_LOW(res->start));
> > -       out_le32(mbase + PCI_BASE_ADDRESS_1,
> > RES_TO_U32_HIGH(res->start)); -
>
> does it work properly to setup the BARs before enabling the inbound
> mapping ?

Yes. At least I have seen no problems so far.

> >          * OMRs are already reset, also disable PIMs
> > @@ -1531,14 +1569,26 @@ static void __init
> > ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port) * and device
> > IDs into it. Those are the same bogus one that the * initial code in
> > arch/ppc add. We might want to change that. */
> > -       out_le16(mbase + 0x200, 0xaaa0 + port->index);
> > -       out_le16(mbase + 0x202, 0xbed0 + port->index);
> > +       if (!port->endpoint) {
> > +               out_le16(mbase + 0x200, 0xaaa0 + port->index);
> > +               out_le16(mbase + 0x202, 0xbed0 + port->index);
>
> We should probably setup the config space IDs based on some device-tree
> properties no ?

Good idea. How about "vendor-id" and "device-id"? And we probably need some 
for endpoint mode too now. "vendor-id-endpoint" and "device-id-endpoint"?

And how should we handle the backward compatibility? Should I set the old 
values as default when those properties are not available?

Thanks.

Best regards,
Stefan

Re: [PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Benjamin Herrenschmidt]

On Thu, 2008-04-10 at 12:21 +0200, Stefan Roese wrote:
> 
> Sure, this would be optimal. But frankly, I currently have no need for
> this 
> non U-Boot mode, and therefore I didn't implement it. This can be
> added later 
> when really needed don't you think?

It can... but on the other hand, it's not hard :-)

> Good. But again, I would really prefer to first include this U-Boot
> style 
> endpoint support and later add this independent device tree endpoint 
> configuration when really needed.

Well, you are the one with a use case... but I do think we should
differenciate a PCI in host mode vs. endpoint mode right away. We don't
want the PCI stack from kicking in at all in endpoint mode, we don't
want to bother with creating the PCI infrastructure and config space
accessors etc...

> Good idea. How about "vendor-id" and "device-id"? And we probably need
> some 
> for endpoint mode too now. "vendor-id-endpoint" and
> "device-id-endpoint"?

If the host bridge in EP mode has a different name/type than the host
bridge one, just 'vendor-id' and 'device-id' will do there too.

> And how should we handle the backward compatibility? Should I set the
> old 
> values as default when those properties are not available?

Yup.

Cheers,
Ben.

Re: [PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Stefan Roese]

On Thursday 10 April 2008, Benjamin Herrenschmidt wrote:
> On Thu, 2008-04-10 at 12:21 +0200, Stefan Roese wrote:
> > Sure, this would be optimal. But frankly, I currently have no need for
> > this
> > non U-Boot mode, and therefore I didn't implement it. This can be
> > added later
> > when really needed don't you think?
>
> It can... but on the other hand, it's not hard :-)

But it takes time. And time is as always short. :)

> > Good. But again, I would really prefer to first include this U-Boot
> > style
> > endpoint support and later add this independent device tree endpoint
> > configuration when really needed.
>
> Well, you are the one with a use case... but I do think we should
> differenciate a PCI in host mode vs. endpoint mode right away. We don't
> want the PCI stack from kicking in at all in endpoint mode, we don't
> want to bother with creating the PCI infrastructure and config space
> accessors etc...

Understood. But for this "dynamic" root-complex/endpoint configuration as 
implemented with the current version of the patch, I can't change the PCIe 
node in the dts from root-complex mode to endpoint mode, since I would loose 
root-complex functionality. Perhaps I'm missing something here.

Best regards,
Stefan

Re: [PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Benjamin Herrenschmidt]

On Thu, 2008-04-10 at 13:59 +0200, Stefan Roese wrote:
> Understood. But for this "dynamic" root-complex/endpoint configuration as 
> implemented with the current version of the patch, I can't change the PCIe 
> node in the dts from root-complex mode to endpoint mode, since I would loose 
> root-complex functionality. Perhaps I'm missing something here.

Well, I would expect the firmware (ie. uboot) to provide the right
device-node type...

Ben.

Re: [PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Stefan Roese]

On Thursday 10 April 2008, Benjamin Herrenschmidt wrote:
> > Understood. But for this "dynamic" root-complex/endpoint configuration as
> > implemented with the current version of the patch, I can't change the
> > PCIe node in the dts from root-complex mode to endpoint mode, since I
> > would loose root-complex functionality. Perhaps I'm missing something
> > here.
>
> Well, I would expect the firmware (ie. uboot) to provide the right
> device-node type...

Now I have to re-check to see what you really have in mind. Do you think about 
creating two different PCIe nodes, one for root-complex and one for endpoint 
functionality? Or is your idea to add a property to the existing PCIe 
device-tree node (like "mode = endpoint"), or perhaps change the device_type 
from "pci" to "pci-endpoint"? I would vote for the latter.

Best regards,
Stefan

Re: [PATCH] [POWERPC] 4xx: Add endpoint support to 4xx PCIe driver

[Benjamin Herrenschmidt]

On Thu, 2008-04-10 at 15:37 +0200, Stefan Roese wrote:
> 
> Now I have to re-check to see what you really have in mind. Do you think about 
> creating two different PCIe nodes, one for root-complex and one for endpoint 
> functionality? Or is your idea to add a property to the existing PCIe 
> device-tree node (like "mode = endpoint"), or perhaps change the device_type 
> from "pci" to "pci-endpoint"? I would vote for the latter.

Make the name and type of the node different. The rest can mostly stay
the same.

Cheers,
Ben.