All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
	linux-pci@vger.kernel.org, "Krzysztof Hałasa" <khalasa@piap.pl>,
	linux-kernel@vger.kernel.org,
	"Russell King" <linux@armlinux.org.uk>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] Revert "ARM: cns3xxx: pci: avoid potential stack overflow"
Date: Tue, 31 May 2016 17:02:24 -0500	[thread overview]
Message-ID: <20160531220224.GA30841@localhost> (raw)
In-Reply-To: <20160531215802.30590.97398.stgit@bhelgaas-glaptop2.roam.corp.google.com>

[+cc Russell, linux-arm-kernel]

On Tue, May 31, 2016 at 04:58:02PM -0500, Bjorn Helgaas wrote:
> This reverts commit 498a92d42596a7a32c042319eb62a4c3d8081cf1.
> 
> Krzysztof reported that this change broke Cavium CNS3xxx, ARMv6 (Laguna
> GW-2388) because the MRRS setting is never written to the hardware.

Krzysztof, can you test this and see whether it fixes the problem for
you?

> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Krzysztof Hałasa <khalasa@piap.pl>
> ---
>  arch/arm/mach-cns3xxx/pcie.c |   71 ++++++++++++++++++++++++------------------
>  1 file changed, 41 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
> index 318394e..c622c30 100644
> --- a/arch/arm/mach-cns3xxx/pcie.c
> +++ b/arch/arm/mach-cns3xxx/pcie.c
> @@ -65,9 +65,8 @@ static void __iomem *cns3xxx_pci_map_bus(struct pci_bus *bus,
>  
>  	/*
>  	 * The CNS PCI bridge doesn't fit into the PCI hierarchy, though
> -	 * we still want to access it.
> -	 * We place the host bridge on bus 0, and the directly connected
> -	 * device on bus 1, slot 0.
> +	 * we still want to access it. For this to work, we must place
> +	 * the first device on the same bus as the CNS PCI bridge.
>  	 */
>  	if (busno == 0) { /* internal PCIe bus, host bridge device */
>  		if (devfn == 0) /* device# and function# are ignored by hw */
> @@ -212,46 +211,58 @@ static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
>  	}
>  }
>  
> -static void cns3xxx_write_config(struct cns3xxx_pcie *cnspci,
> -					 int where, int size, u32 val)
> -{
> -	void __iomem *base = cnspci->host_regs + (where & 0xffc);
> -	u32 v;
> -	u32 mask = (0x1ull << (size * 8)) - 1;
> -	int shift = (where % 4) * 8;
> -
> -	v = readl_relaxed(base);
> -
> -	v &= ~(mask << shift);
> -	v |= (val & mask) << shift;
> -
> -	writel_relaxed(v, base);
> -	readl_relaxed(base);
> -}
> -
>  static void __init cns3xxx_pcie_hw_init(struct cns3xxx_pcie *cnspci)
>  {
> +	int port = cnspci->port;
> +	struct pci_sys_data sd = {
> +		.private_data = cnspci,
> +	};
> +	struct pci_bus bus = {
> +		.number = 0,
> +		.ops = &cns3xxx_pcie_ops,
> +		.sysdata = &sd,
> +	};
>  	u16 mem_base  = cnspci->res_mem.start >> 16;
>  	u16 mem_limit = cnspci->res_mem.end   >> 16;
>  	u16 io_base   = cnspci->res_io.start  >> 16;
>  	u16 io_limit  = cnspci->res_io.end    >> 16;
> +	u32 devfn = 0;
> +	u8 tmp8;
> +	u16 pos;
> +	u16 dc;
> +
> +	pci_bus_write_config_byte(&bus, devfn, PCI_PRIMARY_BUS, 0);
> +	pci_bus_write_config_byte(&bus, devfn, PCI_SECONDARY_BUS, 1);
> +	pci_bus_write_config_byte(&bus, devfn, PCI_SUBORDINATE_BUS, 1);
>  
> -	cns3xxx_write_config(cnspci, PCI_PRIMARY_BUS, 1, 0);
> -	cns3xxx_write_config(cnspci, PCI_SECONDARY_BUS, 1, 1);
> -	cns3xxx_write_config(cnspci, PCI_SUBORDINATE_BUS, 1, 1);
> -	cns3xxx_write_config(cnspci, PCI_MEMORY_BASE, 2, mem_base);
> -	cns3xxx_write_config(cnspci, PCI_MEMORY_LIMIT, 2, mem_limit);
> -	cns3xxx_write_config(cnspci, PCI_IO_BASE_UPPER16, 2, io_base);
> -	cns3xxx_write_config(cnspci, PCI_IO_LIMIT_UPPER16, 2, io_limit);
> +	pci_bus_read_config_byte(&bus, devfn, PCI_PRIMARY_BUS, &tmp8);
> +	pci_bus_read_config_byte(&bus, devfn, PCI_SECONDARY_BUS, &tmp8);
> +	pci_bus_read_config_byte(&bus, devfn, PCI_SUBORDINATE_BUS, &tmp8);
> +
> +	pci_bus_write_config_word(&bus, devfn, PCI_MEMORY_BASE, mem_base);
> +	pci_bus_write_config_word(&bus, devfn, PCI_MEMORY_LIMIT, mem_limit);
> +	pci_bus_write_config_word(&bus, devfn, PCI_IO_BASE_UPPER16, io_base);
> +	pci_bus_write_config_word(&bus, devfn, PCI_IO_LIMIT_UPPER16, io_limit);
>  
>  	if (!cnspci->linked)
>  		return;
>  
>  	/* Set Device Max_Read_Request_Size to 128 byte */
> -	pcie_bus_config = PCIE_BUS_PEER2PEER;
> -
> +	bus.number = 1; /* directly connected PCIe device */
> +	devfn = PCI_DEVFN(0, 0);
> +	pos = pci_bus_find_capability(&bus, devfn, PCI_CAP_ID_EXP);
> +	pci_bus_read_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, &dc);
> +	if (dc & PCI_EXP_DEVCTL_READRQ) {
> +		dc &= ~PCI_EXP_DEVCTL_READRQ;
> +		pci_bus_write_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, dc);
> +		pci_bus_read_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, &dc);
> +		if (dc & PCI_EXP_DEVCTL_READRQ)
> +			pr_warn("PCIe: Unable to set device Max_Read_Request_Size\n");
> +		else
> +			pr_info("PCIe: Max_Read_Request_Size set to 128 bytes\n");
> +	}
>  	/* Disable PCIe0 Interrupt Mask INTA to INTD */
> -	__raw_writel(~0x3FFF, MISC_PCIE_INT_MASK(cnspci->port));
> +	__raw_writel(~0x3FFF, MISC_PCIE_INT_MASK(port));
>  }
>  
>  static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
> 
> --
> 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

WARNING: multiple messages have this Message-ID (diff)
From: helgaas@kernel.org (Bjorn Helgaas)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] Revert "ARM: cns3xxx: pci: avoid potential stack overflow"
Date: Tue, 31 May 2016 17:02:24 -0500	[thread overview]
Message-ID: <20160531220224.GA30841@localhost> (raw)
In-Reply-To: <20160531215802.30590.97398.stgit@bhelgaas-glaptop2.roam.corp.google.com>

[+cc Russell, linux-arm-kernel]

On Tue, May 31, 2016 at 04:58:02PM -0500, Bjorn Helgaas wrote:
> This reverts commit 498a92d42596a7a32c042319eb62a4c3d8081cf1.
> 
> Krzysztof reported that this change broke Cavium CNS3xxx, ARMv6 (Laguna
> GW-2388) because the MRRS setting is never written to the hardware.

Krzysztof, can you test this and see whether it fixes the problem for
you?

> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Krzysztof Ha?asa <khalasa@piap.pl>
> ---
>  arch/arm/mach-cns3xxx/pcie.c |   71 ++++++++++++++++++++++++------------------
>  1 file changed, 41 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
> index 318394e..c622c30 100644
> --- a/arch/arm/mach-cns3xxx/pcie.c
> +++ b/arch/arm/mach-cns3xxx/pcie.c
> @@ -65,9 +65,8 @@ static void __iomem *cns3xxx_pci_map_bus(struct pci_bus *bus,
>  
>  	/*
>  	 * The CNS PCI bridge doesn't fit into the PCI hierarchy, though
> -	 * we still want to access it.
> -	 * We place the host bridge on bus 0, and the directly connected
> -	 * device on bus 1, slot 0.
> +	 * we still want to access it. For this to work, we must place
> +	 * the first device on the same bus as the CNS PCI bridge.
>  	 */
>  	if (busno == 0) { /* internal PCIe bus, host bridge device */
>  		if (devfn == 0) /* device# and function# are ignored by hw */
> @@ -212,46 +211,58 @@ static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
>  	}
>  }
>  
> -static void cns3xxx_write_config(struct cns3xxx_pcie *cnspci,
> -					 int where, int size, u32 val)
> -{
> -	void __iomem *base = cnspci->host_regs + (where & 0xffc);
> -	u32 v;
> -	u32 mask = (0x1ull << (size * 8)) - 1;
> -	int shift = (where % 4) * 8;
> -
> -	v = readl_relaxed(base);
> -
> -	v &= ~(mask << shift);
> -	v |= (val & mask) << shift;
> -
> -	writel_relaxed(v, base);
> -	readl_relaxed(base);
> -}
> -
>  static void __init cns3xxx_pcie_hw_init(struct cns3xxx_pcie *cnspci)
>  {
> +	int port = cnspci->port;
> +	struct pci_sys_data sd = {
> +		.private_data = cnspci,
> +	};
> +	struct pci_bus bus = {
> +		.number = 0,
> +		.ops = &cns3xxx_pcie_ops,
> +		.sysdata = &sd,
> +	};
>  	u16 mem_base  = cnspci->res_mem.start >> 16;
>  	u16 mem_limit = cnspci->res_mem.end   >> 16;
>  	u16 io_base   = cnspci->res_io.start  >> 16;
>  	u16 io_limit  = cnspci->res_io.end    >> 16;
> +	u32 devfn = 0;
> +	u8 tmp8;
> +	u16 pos;
> +	u16 dc;
> +
> +	pci_bus_write_config_byte(&bus, devfn, PCI_PRIMARY_BUS, 0);
> +	pci_bus_write_config_byte(&bus, devfn, PCI_SECONDARY_BUS, 1);
> +	pci_bus_write_config_byte(&bus, devfn, PCI_SUBORDINATE_BUS, 1);
>  
> -	cns3xxx_write_config(cnspci, PCI_PRIMARY_BUS, 1, 0);
> -	cns3xxx_write_config(cnspci, PCI_SECONDARY_BUS, 1, 1);
> -	cns3xxx_write_config(cnspci, PCI_SUBORDINATE_BUS, 1, 1);
> -	cns3xxx_write_config(cnspci, PCI_MEMORY_BASE, 2, mem_base);
> -	cns3xxx_write_config(cnspci, PCI_MEMORY_LIMIT, 2, mem_limit);
> -	cns3xxx_write_config(cnspci, PCI_IO_BASE_UPPER16, 2, io_base);
> -	cns3xxx_write_config(cnspci, PCI_IO_LIMIT_UPPER16, 2, io_limit);
> +	pci_bus_read_config_byte(&bus, devfn, PCI_PRIMARY_BUS, &tmp8);
> +	pci_bus_read_config_byte(&bus, devfn, PCI_SECONDARY_BUS, &tmp8);
> +	pci_bus_read_config_byte(&bus, devfn, PCI_SUBORDINATE_BUS, &tmp8);
> +
> +	pci_bus_write_config_word(&bus, devfn, PCI_MEMORY_BASE, mem_base);
> +	pci_bus_write_config_word(&bus, devfn, PCI_MEMORY_LIMIT, mem_limit);
> +	pci_bus_write_config_word(&bus, devfn, PCI_IO_BASE_UPPER16, io_base);
> +	pci_bus_write_config_word(&bus, devfn, PCI_IO_LIMIT_UPPER16, io_limit);
>  
>  	if (!cnspci->linked)
>  		return;
>  
>  	/* Set Device Max_Read_Request_Size to 128 byte */
> -	pcie_bus_config = PCIE_BUS_PEER2PEER;
> -
> +	bus.number = 1; /* directly connected PCIe device */
> +	devfn = PCI_DEVFN(0, 0);
> +	pos = pci_bus_find_capability(&bus, devfn, PCI_CAP_ID_EXP);
> +	pci_bus_read_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, &dc);
> +	if (dc & PCI_EXP_DEVCTL_READRQ) {
> +		dc &= ~PCI_EXP_DEVCTL_READRQ;
> +		pci_bus_write_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, dc);
> +		pci_bus_read_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, &dc);
> +		if (dc & PCI_EXP_DEVCTL_READRQ)
> +			pr_warn("PCIe: Unable to set device Max_Read_Request_Size\n");
> +		else
> +			pr_info("PCIe: Max_Read_Request_Size set to 128 bytes\n");
> +	}
>  	/* Disable PCIe0 Interrupt Mask INTA to INTD */
> -	__raw_writel(~0x3FFF, MISC_PCIE_INT_MASK(cnspci->port));
> +	__raw_writel(~0x3FFF, MISC_PCIE_INT_MASK(port));
>  }
>  
>  static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2016-05-31 22:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-31 21:58 [PATCH] Revert "ARM: cns3xxx: pci: avoid potential stack overflow" Bjorn Helgaas
2016-05-31 22:02 ` Bjorn Helgaas [this message]
2016-05-31 22:02   ` Bjorn Helgaas
2016-06-01 11:08 ` Krzysztof Hałasa
2016-06-01 21:09   ` Arnd Bergmann
2016-06-09  5:42     ` Krzysztof Hałasa
2016-06-09 14:42       ` Arnd Bergmann
2016-06-10 10:10         ` Krzysztof Hałasa
2016-06-10 21:57           ` Arnd Bergmann
2016-06-16  4:39             ` Krzysztof Hałasa
2016-06-09 17:01     ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160531220224.GA30841@localhost \
    --to=helgaas@kernel.org \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=khalasa@piap.pl \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.