[PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[Jason Gunthorpe]

Otherwise hotplugging the PEX doesn't work at all since the driver
detects the link state at probe time. Simply replacing the two tests
of haslink with a register read is enough to fix it.

Tested on kirkwood with repeated plug/unplug of the link partner.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/pci/host/pci-mvebu.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 729d5a1..f2d61f5 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -115,7 +115,6 @@ struct mvebu_pcie_port {
 	char *name;
 	void __iomem *base;
 	spinlock_t conf_lock;
-	int haslink;
 	u32 port;
 	u32 lane;
 	int devfn;
@@ -552,7 +551,7 @@ static int mvebu_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 	if (bus->number == 0)
 		return mvebu_sw_pci_bridge_write(port, where, size, val);
 
-	if (!port->haslink)
+	if (!mvebu_pcie_link_up(port))
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
 	/*
@@ -594,7 +593,7 @@ static int mvebu_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 	if (bus->number == 0)
 		return mvebu_sw_pci_bridge_read(port, where, size, val);
 
-	if (!port->haslink) {
+	if (!mvebu_pcie_link_up(port)) {
 		*val = 0xffffffff;
 		return PCIBIOS_DEVICE_NOT_FOUND;
 	}
@@ -883,22 +882,11 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
 
 		mvebu_pcie_set_local_dev_nr(port, 1);
 
-		if (mvebu_pcie_link_up(port)) {
-			port->haslink = 1;
-			dev_info(&pdev->dev, "PCIe%d.%d: link up\n",
-				 port->port, port->lane);
-		} else {
-			port->haslink = 0;
-			dev_info(&pdev->dev, "PCIe%d.%d: link down\n",
-				 port->port, port->lane);
-		}
-
 		port->clk = of_clk_get_by_name(child, NULL);
 		if (IS_ERR(port->clk)) {
 			dev_err(&pdev->dev, "PCIe%d.%d: cannot get clock\n",
 			       port->port, port->lane);
 			iounmap(port->base);
-			port->haslink = 0;
 			continue;
 		}
 
-- 
1.8.1.2

[PATCH RESEND] PCI: mvebu - Support a bridge with no IO port window

[Jason Gunthorpe]

Make pcie-io-aperture and the IO port MBUS ID in ranges optional.
If not provided the bridge reports to Linux that IO space mapping is
not supported and refuses to configure an IO mbus window.

This allows both complete disable (do not specify pcie-io-aperture) and
per-port disable (do not specify a IO target ranges entry for the port)

Most PCIE devices these days do not require IO support to function,
so having an option to disable it in the driver is useful.

This depends on 'bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties'
to work properly.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 drivers/pci/host/pci-mvebu.c | 62 ++++++++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index f2d61f5..5ddba68 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -132,6 +132,11 @@ struct mvebu_pcie_port {
 	size_t iowin_size;
 };
 
+static inline bool mvebu_has_ioport(struct mvebu_pcie_port *port)
+{
+	return port->io_target == -1 && port->io_attr == -1;
+}
+
 static bool mvebu_pcie_link_up(struct mvebu_pcie_port *port)
 {
 	return !(readl(port->base + PCIE_STAT_OFF) & PCIE_STAT_LINK_DOWN);
@@ -292,6 +297,12 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 		return;
 	}
 
+	if (!mvebu_has_ioport(port)) {
+		dev_WARN(&port->pcie->pdev->dev,
+			 "Attempt to set IO when IO is disabled\n");
+		return;
+	}
+
 	/*
 	 * We read the PCI-to-PCI bridge emulated registers, and
 	 * calculate the base address and size of the address decoding
@@ -405,9 +416,12 @@ static int mvebu_sw_pci_bridge_read(struct mvebu_pcie_port *port,
 		break;
 
 	case PCI_IO_BASE:
-		*value = (bridge->secondary_status << 16 |
-			  bridge->iolimit          <<  8 |
-			  bridge->iobase);
+		if (!mvebu_has_ioport(port))
+			*value = 0;
+		else
+			*value = (bridge->secondary_status << 16 |
+				  bridge->iolimit          <<  8 |
+				  bridge->iobase);
 		break;
 
 	case PCI_MEMORY_BASE:
@@ -465,6 +479,8 @@ static int mvebu_sw_pci_bridge_write(struct mvebu_pcie_port *port,
 	switch (where & ~3) {
 	case PCI_COMMAND:
 		bridge->command = value & 0xffff;
+		if (!mvebu_has_ioport(port))
+			bridge->command &= ~PCI_COMMAND_IO;
 		break;
 
 	case PCI_BASE_ADDRESS_0 ... PCI_BASE_ADDRESS_1:
@@ -630,7 +646,9 @@ static int __init mvebu_pcie_setup(int nr, struct pci_sys_data *sys)
 	struct mvebu_pcie *pcie = sys_to_pcie(sys);
 	int i;
 
-	pci_add_resource_offset(&sys->resources, &pcie->realio, sys->io_offset);
+	if (resource_size(&pcie->realio) != 0)
+		pci_add_resource_offset(&sys->resources, &pcie->realio,
+					sys->io_offset);
 	pci_add_resource_offset(&sys->resources, &pcie->mem, sys->mem_offset);
 	pci_add_resource(&sys->resources, &pcie->busn);
 
@@ -739,12 +757,17 @@ mvebu_pcie_map_registers(struct platform_device *pdev,
 #define DT_CPUADDR_TO_ATTR(cpuaddr)   (((cpuaddr) >> 48) & 0xFF)
 
 static int mvebu_get_tgt_attr(struct device_node *np, int devfn,
-			      unsigned long type, int *tgt, int *attr)
+			      unsigned long type,
+			      unsigned int *tgt,
+			      unsigned int *attr)
 {
 	const int na = 3, ns = 2;
 	const __be32 *range;
 	int rlen, nranges, rangesz, pna, i;
 
+	*tgt = -1;
+	*attr = -1;
+
 	range = of_get_property(np, "ranges", &rlen);
 	if (!range)
 		return -EINVAL;
@@ -798,16 +821,15 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
 	}
 
 	mvebu_mbus_get_pcie_io_aperture(&pcie->io);
-	if (resource_size(&pcie->io) == 0) {
-		dev_err(&pdev->dev, "invalid I/O aperture size\n");
-		return -EINVAL;
-	}
 
-	pcie->realio.flags = pcie->io.flags;
-	pcie->realio.start = PCIBIOS_MIN_IO;
-	pcie->realio.end = min_t(resource_size_t,
-				  IO_SPACE_LIMIT,
-				  resource_size(&pcie->io));
+	if (resource_size(&pcie->io) != 0) {
+		pcie->realio.flags = pcie->io.flags;
+		pcie->realio.start = PCIBIOS_MIN_IO;
+		pcie->realio.end = min_t(resource_size_t,
+					 IO_SPACE_LIMIT,
+					 resource_size(&pcie->io));
+	} else
+		pcie->realio = pcie->io;
 
 	/* Get the bus range */
 	ret = of_pci_parse_bus_range(np, &pcie->busn);
@@ -864,12 +886,12 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
 			continue;
 		}
 
-		ret = mvebu_get_tgt_attr(np, port->devfn, IORESOURCE_IO,
-					 &port->io_target, &port->io_attr);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "PCIe%d.%d: cannot get tgt/attr for io window\n",
-				port->port, port->lane);
-			continue;
+		if (resource_size(&pcie->io) != 0)
+			mvebu_get_tgt_attr(np, port->devfn, IORESOURCE_IO,
+					   &port->io_target, &port->io_attr);
+		else {
+			port->io_target = -1;
+			port->io_attr = -1;
 		}
 
 		port->base = mvebu_pcie_map_registers(pdev, child, port);
-- 
1.8.1.2

Re: [PATCH RESEND] PCI: mvebu - Support a bridge with no IO port window

[Thomas Petazzoni]

Dear Jason Gunthorpe,

On Tue,  1 Oct 2013 11:58:01 -0600, Jason Gunthorpe wrote:
> Make pcie-io-aperture and the IO port MBUS ID in ranges optional.
> If not provided the bridge reports to Linux that IO space mapping is
> not supported and refuses to configure an IO mbus window.
> 
> This allows both complete disable (do not specify pcie-io-aperture) and
> per-port disable (do not specify a IO target ranges entry for the port)
> 
> Most PCIE devices these days do not require IO support to function,
> so having an option to disable it in the driver is useful.
> 
> This depends on 'bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties'
> to work properly.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

This doesn't work here, I get multiple warnings "Attempt to set IO when
IO is disabled" :

------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at /home/thomas/projets/linux-2.6/drivers/pci/host/pci-mvebu.c:302 mvebu_pcie_wr_conf+0x2c0/0x44c()
Device: mvebu-pcie
Attempt to set IO when IO is disabled
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.12.0-rc1-00003-ga85b362 #6
[<c0015798>] (unwind_backtrace+0x0/0xf8) from [<c0011770>] (show_stack+0x10/0x14)
[<c0011770>] (show_stack+0x10/0x14) from [<c0372c08>] (dump_stack+0x70/0x8c)
[<c0372c08>] (dump_stack+0x70/0x8c) from [<c001d5bc>] (warn_slowpath_common+0x64/0x88)
[<c001d5bc>] (warn_slowpath_common+0x64/0x88) from [<c001d674>] (warn_slowpath_fmt+0x30/0x40)
[<c001d674>] (warn_slowpath_fmt+0x30/0x40) from [<c0187080>] (mvebu_pcie_wr_conf+0x2c0/0x44c)
[<c0187080>] (mvebu_pcie_wr_conf+0x2c0/0x44c) from [<c01770fc>] (pci_bus_write_config_word+0x60/0x78)
[<c01770fc>] (pci_bus_write_config_word+0x60/0x78) from [<c03708b4>] (__pci_bus_size_bridges+0x708/0x7c0)
[<c03708b4>] (__pci_bus_size_bridges+0x708/0x7c0) from [<c03701e0>] (__pci_bus_size_bridges+0x34/0x7c0)
[<c03701e0>] (__pci_bus_size_bridges+0x34/0x7c0) from [<c00134c4>] (pci_common_init_dev+0x1b0/0x2dc)
[<c00134c4>] (pci_common_init_dev+0x1b0/0x2dc) from [<c04a3f68>] (mvebu_pcie_probe+0x444/0x48c)
[<c04a3f68>] (mvebu_pcie_probe+0x444/0x48c) from [<c01b69dc>] (platform_drv_probe+0x18/0x1c)
[<c01b69dc>] (platform_drv_probe+0x18/0x1c) from [<c01b5858>] (driver_probe_device+0xf4/0x210)
[<c01b5858>] (driver_probe_device+0xf4/0x210) from [<c01b5a00>] (__driver_attach+0x8c/0x90)
[<c01b5a00>] (__driver_attach+0x8c/0x90) from [<c01b3eb8>] (bus_for_each_dev+0x54/0x88)
[<c01b3eb8>] (bus_for_each_dev+0x54/0x88) from [<c01b4f60>] (bus_add_driver+0xd4/0x258)
[<c01b4f60>] (bus_add_driver+0xd4/0x258) from [<c01b6038>] (driver_register+0x78/0xf4)
[<c01b6038>] (driver_register+0x78/0xf4) from [<c01b6bc0>] (platform_driver_probe+0x1c/0xa0)
[<c01b6bc0>] (platform_driver_probe+0x1c/0xa0) from [<c0008878>] (do_one_initcall+0xe4/0x140)
[<c0008878>] (do_one_initcall+0xe4/0x140) from [<c0492b60>] (kernel_init_freeable+0xfc/0x1c4)
[<c0492b60>] (kernel_init_freeable+0xfc/0x1c4) from [<c036f0cc>] (kernel_init+0x8/0xe4)
[<c036f0cc>] (kernel_init+0x8/0xe4) from [<c000e3d8>] (ret_from_fork+0x14/0x3c)
---[ end trace 70bc10e370e10347 ]---

> +static inline bool mvebu_has_ioport(struct mvebu_pcie_port *port)
> +{
> +	return port->io_target == -1 && port->io_attr == -1;

Are you sure the logic is not reverted here? I.e, this shouldn't be !=
-1 in both cases?

> +}
> +
>  static bool mvebu_pcie_link_up(struct mvebu_pcie_port *port)
>  {
>  	return !(readl(port->base + PCIE_STAT_OFF) & PCIE_STAT_LINK_DOWN);
> @@ -292,6 +297,12 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
>  		return;
>  	}
>  
> +	if (!mvebu_has_ioport(port)) {
> +		dev_WARN(&port->pcie->pdev->dev,
> +			 "Attempt to set IO when IO is disabled\n");
> +		return;
> +	}
> +
>  	/*
>  	 * We read the PCI-to-PCI bridge emulated registers, and
>  	 * calculate the base address and size of the address decoding
> @@ -405,9 +416,12 @@ static int mvebu_sw_pci_bridge_read(struct mvebu_pcie_port *port,
>  		break;
>  
>  	case PCI_IO_BASE:
> -		*value = (bridge->secondary_status << 16 |
> -			  bridge->iolimit          <<  8 |
> -			  bridge->iobase);
> +		if (!mvebu_has_ioport(port))
> +			*value = 0;
> +		else
> +			*value = (bridge->secondary_status << 16 |
> +				  bridge->iolimit          <<  8 |
> +				  bridge->iobase);

While I do understand that you're returning 0 for iolimit and iobase,
I'm not sure why the secondary status is affected by the value of
mvebu_has_ioport().

>  static int mvebu_get_tgt_attr(struct device_node *np, int devfn,
> -			      unsigned long type, int *tgt, int *attr)
> +			      unsigned long type,
> +			      unsigned int *tgt,
> +			      unsigned int *attr)
>  {
>  	const int na = 3, ns = 2;
>  	const __be32 *range;
>  	int rlen, nranges, rangesz, pna, i;
>  
> +	*tgt = -1;
> +	*attr = -1;
> +
>  	range = of_get_property(np, "ranges", &rlen);
>  	if (!range)
>  		return -EINVAL;
> @@ -798,16 +821,15 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
>  	}
>  
>  	mvebu_mbus_get_pcie_io_aperture(&pcie->io);
> -	if (resource_size(&pcie->io) == 0) {
> -		dev_err(&pdev->dev, "invalid I/O aperture size\n");
> -		return -EINVAL;
> -	}
>  
> -	pcie->realio.flags = pcie->io.flags;
> -	pcie->realio.start = PCIBIOS_MIN_IO;
> -	pcie->realio.end = min_t(resource_size_t,
> -				  IO_SPACE_LIMIT,
> -				  resource_size(&pcie->io));
> +	if (resource_size(&pcie->io) != 0) {
> +		pcie->realio.flags = pcie->io.flags;
> +		pcie->realio.start = PCIBIOS_MIN_IO;
> +		pcie->realio.end = min_t(resource_size_t,
> +					 IO_SPACE_LIMIT,
> +					 resource_size(&pcie->io));
> +	} else
> +		pcie->realio = pcie->io;
>  
>  	/* Get the bus range */
>  	ret = of_pci_parse_bus_range(np, &pcie->busn);
> @@ -864,12 +886,12 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
>  			continue;
>  		}
>  
> -		ret = mvebu_get_tgt_attr(np, port->devfn, IORESOURCE_IO,
> -					 &port->io_target, &port->io_attr);
> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "PCIe%d.%d: cannot get tgt/attr for io window\n",
> -				port->port, port->lane);
> -			continue;
> +		if (resource_size(&pcie->io) != 0)
> +			mvebu_get_tgt_attr(np, port->devfn, IORESOURCE_IO,
> +					   &port->io_target, &port->io_attr);
> +		else {
> +			port->io_target = -1;
> +			port->io_attr = -1;
>  		}
>  
>  		port->base = mvebu_pcie_map_registers(pdev, child, port);

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[Thomas Petazzoni]

Dear Jason Gunthorpe,

On Tue,  1 Oct 2013 11:58:00 -0600, Jason Gunthorpe wrote:
> Otherwise hotplugging the PEX doesn't work at all since the driver
> detects the link state at probe time. Simply replacing the two tests
> of haslink with a register read is enough to fix it.
> 
> Tested on kirkwood with repeated plug/unplug of the link partner.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[Bjorn Helgaas]

[+cc Seungwon]

On Tue, Oct 1, 2013 at 11:58 AM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> Otherwise hotplugging the PEX doesn't work at all since the driver
> detects the link state at probe time. Simply replacing the two tests
> of haslink with a register read is enough to fix it.
>
> Tested on kirkwood with repeated plug/unplug of the link partner.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

Last I heard (Sep 26), you had some mvebu changes in your tree
already, Jason, so I was assuming you'd handle pci-mvebu.c changes, at
least for the v3.13 merge window.  But you marked this "resend"; does
that mean you're waiting for me to do something with it?

I see the following mvebu patches in patchwork:

  PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug
  PCI: mvebu - Support a bridge with no IO port window
  PCI: mvebu: add I/O access wrappers

Let me know if I need to do anything with any of them.

Bjorn

> ---
>  drivers/pci/host/pci-mvebu.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index 729d5a1..f2d61f5 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -115,7 +115,6 @@ struct mvebu_pcie_port {
>         char *name;
>         void __iomem *base;
>         spinlock_t conf_lock;
> -       int haslink;
>         u32 port;
>         u32 lane;
>         int devfn;
> @@ -552,7 +551,7 @@ static int mvebu_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>         if (bus->number == 0)
>                 return mvebu_sw_pci_bridge_write(port, where, size, val);
>
> -       if (!port->haslink)
> +       if (!mvebu_pcie_link_up(port))
>                 return PCIBIOS_DEVICE_NOT_FOUND;
>
>         /*
> @@ -594,7 +593,7 @@ static int mvebu_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>         if (bus->number == 0)
>                 return mvebu_sw_pci_bridge_read(port, where, size, val);
>
> -       if (!port->haslink) {
> +       if (!mvebu_pcie_link_up(port)) {
>                 *val = 0xffffffff;
>                 return PCIBIOS_DEVICE_NOT_FOUND;
>         }
> @@ -883,22 +882,11 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
>
>                 mvebu_pcie_set_local_dev_nr(port, 1);
>
> -               if (mvebu_pcie_link_up(port)) {
> -                       port->haslink = 1;
> -                       dev_info(&pdev->dev, "PCIe%d.%d: link up\n",
> -                                port->port, port->lane);
> -               } else {
> -                       port->haslink = 0;
> -                       dev_info(&pdev->dev, "PCIe%d.%d: link down\n",
> -                                port->port, port->lane);
> -               }
> -
>                 port->clk = of_clk_get_by_name(child, NULL);
>                 if (IS_ERR(port->clk)) {
>                         dev_err(&pdev->dev, "PCIe%d.%d: cannot get clock\n",
>                                port->port, port->lane);
>                         iounmap(port->base);
> -                       port->haslink = 0;
>                         continue;
>                 }
>
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[Jason Gunthorpe]

On Mon, Oct 07, 2013 at 04:28:52PM -0600, Bjorn Helgaas wrote:
> [+cc Seungwon]
> 
> On Tue, Oct 1, 2013 at 11:58 AM, Jason Gunthorpe
> <jgunthorpe@obsidianresearch.com> wrote:
> > Otherwise hotplugging the PEX doesn't work at all since the driver
> > detects the link state at probe time. Simply replacing the two tests
> > of haslink with a register read is enough to fix it.
> >
> > Tested on kirkwood with repeated plug/unplug of the link partner.
> >
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> 
> Last I heard (Sep 26), you had some mvebu changes in your tree
> already, Jason, so I was assuming you'd handle pci-mvebu.c changes, at
> least for the v3.13 merge window.  But you marked this "resend"; does
> that mean you're waiting for me to do something with it?

Are you thinking of Jason C.? He is handling much of the mvebu stuff.

Hopefully Jason C. can clarify which route these patches should take
to hit the 3.13 merge window.

This was reposted at the request of Thomas, IIRC.

>   PCI: mvebu - Support a bridge with no IO port window

This one is broken, I have yet to fix it still.

>   PCI: mvebu: add I/O access wrappers

This isn't one of mine ..

Regards,
Jason

Re: [PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[Bjorn Helgaas]

On Mon, Oct 7, 2013 at 4:40 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Mon, Oct 07, 2013 at 04:28:52PM -0600, Bjorn Helgaas wrote:
>> [+cc Seungwon]
>>
>> On Tue, Oct 1, 2013 at 11:58 AM, Jason Gunthorpe
>> <jgunthorpe@obsidianresearch.com> wrote:
>> > Otherwise hotplugging the PEX doesn't work at all since the driver
>> > detects the link state at probe time. Simply replacing the two tests
>> > of haslink with a register read is enough to fix it.
>> >
>> > Tested on kirkwood with repeated plug/unplug of the link partner.
>> >
>> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>>
>> Last I heard (Sep 26), you had some mvebu changes in your tree
>> already, Jason, so I was assuming you'd handle pci-mvebu.c changes, at
>> least for the v3.13 merge window.  But you marked this "resend"; does
>> that mean you're waiting for me to do something with it?
>
> Are you thinking of Jason C.? He is handling much of the mvebu stuff.

Oops, sorry, yep, indeed I meant Jason C.  Sorry about confusing you guys :)

> Hopefully Jason C. can clarify which route these patches should take
> to hit the 3.13 merge window.
>
> This was reposted at the request of Thomas, IIRC.
>
>>   PCI: mvebu - Support a bridge with no IO port window
>
> This one is broken, I have yet to fix it still.
>
>>   PCI: mvebu: add I/O access wrappers
>
> This isn't one of mine ..

That's Seungwon's.  I just wanted to collect all the mvebu patches
from the PCI patchwork.

Bjorn

Re: [PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[Thomas Petazzoni]

Dear Bjorn Helgaas,

On Mon, 7 Oct 2013 16:28:52 -0600, Bjorn Helgaas wrote:

> I see the following mvebu patches in patchwork:
> 
>   PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

This one got my Acked-by, see <20131003144457.4131aea3@skate>.

>   PCI: mvebu - Support a bridge with no IO port window

This one has issues, as I reported to Jason Gunthorpe.

>   PCI: mvebu: add I/O access wrappers

I need to test this one.

Jason Cooper will confirm, but my understanding is that for the 3.13,
he wanted to merge all the pci-mvebu patches, because he already has
MSI related changes (that span PCI and irqchip drivers). Of course, if
you agree with this.

But indeed, this means that Jason C. should also take care of all other
pci-mvebu patches :)

Jason C, do not hesitate to ping me if you need testing/confirmation on
some pci-mvebu patches I might have overlooked.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[Jason Cooper]

On Mon, Oct 07, 2013 at 04:28:52PM -0600, Bjorn Helgaas wrote:
> [+cc Seungwon]
> 
> On Tue, Oct 1, 2013 at 11:58 AM, Jason Gunthorpe
> <jgunthorpe@obsidianresearch.com> wrote:
> > Otherwise hotplugging the PEX doesn't work at all since the driver
> > detects the link state at probe time. Simply replacing the two tests
> > of haslink with a register read is enough to fix it.
> >
> > Tested on kirkwood with repeated plug/unplug of the link partner.
> >
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> 
> Last I heard (Sep 26), you had some mvebu changes in your tree
> already, Jason, so I was assuming you'd handle pci-mvebu.c changes, at
> least for the v3.13 merge window.  But you marked this "resend"; 

That's correct.  I somehow missed these two and I asked JasonG to resend
them to me.

> does that mean you're waiting for me to do something with it?

Nope, It means I'm looking for a slice of time to get caught up :-P

> I see the following mvebu patches in patchwork:
> 
>   PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug
>   PCI: mvebu - Support a bridge with no IO port window
>   PCI: mvebu: add I/O access wrappers
> 
> Let me know if I need to do anything with any of them.

I always appreciate an extra set of eyes ;-)  Hopefully, after v3.13, we
can get back to normal wrt patches going through proper trees.

thx,

Jason.

Re: [PATCH RESEND] PCI: mvebu: Dynamically detect if the PEX link is up to enable hot plug

[Jason Cooper]

On Tue, Oct 01, 2013 at 11:58:00AM -0600, Jason Gunthorpe wrote:
> Otherwise hotplugging the PEX doesn't work at all since the driver
> detects the link state at probe time. Simply replacing the two tests
> of haslink with a register read is enough to fix it.
> 
> Tested on kirkwood with repeated plug/unplug of the link partner.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  drivers/pci/host/pci-mvebu.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)

Applied to mvebu/drivers with Thomas' Ack and Tested-by.

thx,

Jason.

Re: [PATCH RESEND] PCI: mvebu - Support a bridge with no IO port window

[Jason Gunthorpe]

On Thu, Oct 03, 2013 at 02:06:48PM +0200, Thomas Petazzoni wrote:
> Dear Jason Gunthorpe,
> 
> On Tue,  1 Oct 2013 11:58:01 -0600, Jason Gunthorpe wrote:
> > Make pcie-io-aperture and the IO port MBUS ID in ranges optional.
> > If not provided the bridge reports to Linux that IO space mapping is
> > not supported and refuses to configure an IO mbus window.
> > 
> > This allows both complete disable (do not specify pcie-io-aperture) and
> > per-port disable (do not specify a IO target ranges entry for the port)
> > 
> > Most PCIE devices these days do not require IO support to function,
> > so having an option to disable it in the driver is useful.
> > 
> > This depends on 'bus: mvebu-mbus: Fix optional pcie-mem/io-aperture properties'
> > to work properly.
> 
> This doesn't work here, I get multiple warnings "Attempt to set IO when
> IO is disabled" :

Okay, I looked into this.

The warnings are caused by the bogus mvebu_has_ioport you noted, 
and this is also required:

        /* Are the new iobase/iolimit values invalid? */
-       if (port->bridge.iolimit < port->bridge.iobase ||
+       if (port->bridge.iolimit <= port->bridge.iobase ||
            port->bridge.iolimitupper < port->bridge.iobaseupper) {

> >  	case PCI_IO_BASE:
> > -		*value = (bridge->secondary_status << 16 |
> > -			  bridge->iolimit          <<  8 |
> > -			  bridge->iobase);
> > +		if (!mvebu_has_ioport(port))
> > +			*value = 0;
> > +		else
> > +			*value = (bridge->secondary_status << 16 |
> > +				  bridge->iolimit          <<  8 |
> > +				  bridge->iobase);
> 
> While I do understand that you're returning 0 for iolimit and iobase,
> I'm not sure why the secondary status is affected by the value of
> mvebu_has_ioport().

This is an error, of sorts. 0 is actually the correct value for 
secondary status with the current capabilities of the driver.. I'll
fix it here as you noted and send a patch to fix that up separately.

New patches in a sec..

Thanks,
Jason