[PATCH 1/6] ARM: asm: Add convenience fucntions to access VBAR

[PATCH 1/6] ARM: asm: Add convenience fucntions to access VBAR

[Andrey Smirnov]

Add two functions to get/set VBAR register.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/include/asm/system.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 26fb18c..2e37675 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -71,6 +71,31 @@ static inline void set_cr(unsigned int val)
 	isb();
 }

+#if __LINUX_ARM_ARCH__ >= 7
+static inline unsigned int get_vbar(void)
+{
+	unsigned int vbar;
+	asm volatile("mrc p15, 0, %0, c12, c0, 0 @ get VBAR"
+		     : "=r" (vbar) : : "cc");
+	return vbar;
+}
+
+static inline void set_vbar(unsigned int vbar)
+{
+	asm volatile("mcr p15, 0, %0, c12, c0, 0 @ set VBAR"
+		     : : "r" (vbar) : "cc");
+	isb();
+}
+#else
+/*
+   Pre ARMv7 CPUs do not implement Security Extensions so normal
+   exceptions base address cannot be re-mapped from 0x00000000
+ */
+static inline unsigned int get_vbar(void) { return 0; }
+static inline void set_vbar(u32 vbar) {}
+#endif
+
+
 #endif

 #endif /* __ASM_ARM_SYSTEM_H */
--
2.5.0

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/6] ARM: mmu: Add VBAR setup

[Andrey Smirnov]

Add code to make sure that normal vector exception table, when it is
used due to unavailability of the high vector table, was not re-mapped
from 0x0 via VBAR by someone else before us.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 784221c..e32523b 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -302,6 +302,12 @@ static void vectors_init(void)
 		 * live without being able to catch NULL pointer dereferences
 		 */
 		exc = arm_create_pte(0x0);
+
+		/*
+		  Make sure that vecotrs are not re-mapped from 0x0
+		  via VBAR (no-op on non-ARMv7)
+		 */
+		set_vbar(0x0);
 	}

 	arm_fixup_vectors();
--
2.5.0

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/6] ARM: Fix exception table setup in MMU-less mode

[Andrey Smirnov]

Add code necessary for correct initialization of exception vector
table when MMU is disabled.

Note: Please be aware that non ARMv7 implementation of this
functionality was not fully tested due to the lack of any such
hardware. It should theoretically work, but only testing that I did
was to test place_vector_table() failure path (the best I could do on
a i.MX6)

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/Makefile |   6 +++
 arch/arm/cpu/no-mmu.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 126 insertions(+)
 create mode 100644 arch/arm/cpu/no-mmu.c

diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
index 418bcab..f708e8f 100644
--- a/arch/arm/cpu/Makefile
+++ b/arch/arm/cpu/Makefile
@@ -11,6 +11,12 @@ obj-$(CONFIG_CMD_ARM_MMUINFO) += mmuinfo.o
 obj-$(CONFIG_OFDEVICE) += dtb.o
 obj-$(CONFIG_MMU) += mmu.o cache.o mmu-early.o
 pbl-$(CONFIG_MMU) += mmu-early.o
+
+ifeq ($(CONFIG_MMU),)
+obj-y += no-mmu.o
+pbl-y += no-mmu.o
+endif
+
 obj-$(CONFIG_CPU_32v4T) += cache-armv4.o
 pbl-$(CONFIG_CPU_32v4T) += cache-armv4.o
 obj-$(CONFIG_CPU_32v5) += cache-armv5.o
diff --git a/arch/arm/cpu/no-mmu.c b/arch/arm/cpu/no-mmu.c
new file mode 100644
index 0000000..591cc2c
--- /dev/null
+++ b/arch/arm/cpu/no-mmu.c
@@ -0,0 +1,120 @@
+/*
+ * Copyright (c) 2015 Zodiac Inflight Innovation
+ * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#define pr_fmt(fmt)	"nommu: " fmt
+
+#include <common.h>
+#include <dma-dir.h>
+#include <init.h>
+#include <mmu.h>
+#include <errno.h>
+#include <linux/sizes.h>
+#include <asm/memory.h>
+#include <asm/barebox-arm.h>
+#include <asm/system.h>
+#include <asm/cache.h>
+#include <memory.h>
+#include <asm/system_info.h>
+#include <debug_ll.h>
+
+
+#define __exceptions_size (__exceptions_stop - __exceptions_start)
+
+#if __LINUX_ARM_ARCH__ >= 7
+
+static int nommu_v7_vectors_init(void)
+{
+	void *vectors;
+	u32 cr;
+
+	/*
+	   High vectors cannot be re-mapped, so we have to use normal
+	   vectors
+	 */
+	cr = get_cr();
+	cr &= ~CR_V;
+	set_cr(cr);
+
+	arm_fixup_vectors();
+
+	vectors = xmemalign(PAGE_SIZE, PAGE_SIZE);
+	memset(vectors, 0, PAGE_SIZE);
+	memcpy(vectors, __exceptions_start, __exceptions_size);
+
+	set_vbar((unsigned int)vectors);
+
+	return 0;
+}
+mmu_initcall(nommu_v7_vectors_init);
+
+#else
+
+#warning "This code has seen only rudimentary testing (only failure path). Proceed with caution"
+
+static struct resource *place_vector_table(void)
+{
+	int i;
+	struct resource *vectors = NULL;
+	resource_size_t addr[2] = { 0x00000000, 0xFFFF0000 };
+
+	for (i = 0; i < ARRAY_SIZE(addr); i++) {
+		vectors = request_sdram_region("exceptions",
+					       addr[i],
+					       __exceptions_size);
+		if (vectors)
+			break;
+	}
+
+	return vectors;
+}
+
+static int nommu_v4_vectors_init(void)
+{
+	u32 cr;
+	struct resource *vectors;
+
+	vectors = place_vector_table();
+	if (!vectors) {
+		pr_crit("Critical Error: Can't place exception vector table\n");
+		return 0;
+	}
+
+	cr = get_cr();
+
+	if (vectors->start) {
+		cr |= CR_V;
+		set_cr(cr);
+
+		if (!(get_cr() & CR_V)) {
+			pr_crit("Critical Error: "
+				"High vector table is unavailble\n");
+			release_sdram_region(vectors);
+			return 0;
+		}
+	} else {
+		cr &= ~CR_V;
+		set_cr(cr);
+	}
+
+	arm_fixup_vectors();
+
+	memcpy((void *)vectors->start,
+	       __exceptions_start, resource_size(vectors));
+
+	return 0;
+}
+mmu_initcall(nommu_v4_vectors_init);
+
+#endif
--
2.5.0

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/6] i.MX6: pci: Replace magic number with a named constant

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/pci-imx6.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-imx6.c b/drivers/pci/pci-imx6.c
index eaa5f0e..84a937f 100644
--- a/drivers/pci/pci-imx6.c
+++ b/drivers/pci/pci-imx6.c
@@ -51,6 +51,8 @@ struct imx6_pcie {
 #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2	0x2
 #define PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK	0xf

+#define PCIE_RC_LCSR				0x80
+
 /* PCIe Port Logic registers (memory-mapped) */
 #define PL_OFFSET 0x700
 #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
@@ -418,7 +420,7 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
 	if (ret) {
 		dev_err(pp->dev, "Failed to bring link up!\n");
 	} else {
-		tmp = readl(pp->dbi_base + 0x80);
+		tmp = readl(pp->dbi_base + PCIE_RC_LCSR);
 		dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
 	}

--
2.5.0

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/6] i.MX6: pci: Reconcile imx6_pcie_start_link with the kernel code

[Andrey Smirnov]

Reconcile imx6_pcie_start_link with almost identical
imx6_pcie_establish_link from analogous Linux kernel driver. This
change is purely cosmetical, but refactoring the code this way
simplifies implementation comparison.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/pci-imx6.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/pci-imx6.c b/drivers/pci/pci-imx6.c
index 84a937f..713007b 100644
--- a/drivers/pci/pci-imx6.c
+++ b/drivers/pci/pci-imx6.c
@@ -362,13 +362,29 @@ static int imx6_pcie_wait_for_link(struct pcie_port *pp)
 	}
 }

+static int imx6_pcie_wait_for_speed_change(struct pcie_port *pp)
+{
+	uint32_t tmp;
+	uint64_t start = get_time_ns();
+
+	while (!is_timeout(start, SECOND)) {
+		tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+		/* Test if the speed change finished. */
+		if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
+			return 0;
+	}
+
+	dev_err(pp->dev, "Speed change timeout\n");
+	return -EINVAL;
+}
+
+
 static int imx6_pcie_start_link(struct pcie_port *pp)
 {
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
 	uint32_t tmp;
 	int ret;
 	u32 gpr12;
-	u64 start;

 	/*
 	 * Force Gen1 operation when starting the link.  In case the link is
@@ -403,28 +419,22 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
 	tmp |= PORT_LOGIC_SPEED_CHANGE;
 	writel(tmp, pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);

-	start = get_time_ns();
-	while (!is_timeout(start, SECOND)) {
-		tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
-		/* Test if the speed change finished. */
-		if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
-			break;
+	ret = imx6_pcie_wait_for_speed_change(pp);
+	if (ret) {
+		dev_err(pp->dev, "Failed to bring link up!\n");
+		return ret;
 	}

 	/* Make sure link training is finished as well! */
-	if (tmp & PORT_LOGIC_SPEED_CHANGE)
-		ret = -EINVAL;
-	else
-		ret = imx6_pcie_wait_for_link(pp);
-
+	ret = imx6_pcie_wait_for_link(pp);
 	if (ret) {
 		dev_err(pp->dev, "Failed to bring link up!\n");
-	} else {
-		tmp = readl(pp->dbi_base + PCIE_RC_LCSR);
-		dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
+		return ret;
 	}

-	return ret;
+	tmp = readl(pp->dbi_base + PCIE_RC_LCSR);
+	dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
+	return 0;
 }

 static void imx6_pcie_host_init(struct pcie_port *pp)
--
2.5.0

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/6] i.MX6: pci: Avoid aborts when asserting PCIe reset

[Andrey Smirnov]

When booting Barebox in the HW environment where PCIe core has been
used but not properly shut down, writing to PCIE_PL_PFLR in
imx6_pcie_assert_core_reset would cause data abort exception.

The problem can be easily reproduced on a i.MX6 based board with PCIe
slot populated with some device by doing:

> bootm -f -e 0x1050 <your board's>.img

Ignoring this exception seem to allow PCIe core to successfully
initialize and enumerate devices properly. This is also how Linux
Kernel version of the driver handles this situation -- it installs
dummy no-op abort handler at the beginning of device's probing.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/pci-imx6.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/pci-imx6.c b/drivers/pci/pci-imx6.c
index 713007b..3a3edd8 100644
--- a/drivers/pci/pci-imx6.c
+++ b/drivers/pci/pci-imx6.c
@@ -13,6 +13,7 @@

 #include <common.h>
 #include <clock.h>
+#include <abort.h>
 #include <malloc.h>
 #include <io.h>
 #include <init.h>
@@ -237,7 +238,10 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 		val = readl(pp->dbi_base + PCIE_PL_PFLR);
 		val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
 		val |= PCIE_PL_PFLR_FORCE_LINK;
+
+		data_abort_mask();
 		writel(val, pp->dbi_base + PCIE_PL_PFLR);
+		data_abort_unmask();

 		gpr12 &= ~IMX6Q_GPR12_PCIE_CTL_2;
 		writel(gpr12, imx6_pcie->iomuxc_gpr + IOMUXC_GPR12);
--
2.5.0

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/6] ARM: mmu: Add VBAR setup

[Sascha Hauer]

On Thu, Dec 31, 2015 at 09:58:34PM -0800, Andrey Smirnov wrote:
> Add code to make sure that normal vector exception table, when it is
> used due to unavailability of the high vector table, was not re-mapped
> from 0x0 via VBAR by someone else before us.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/mmu.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
> index 784221c..e32523b 100644
> --- a/arch/arm/cpu/mmu.c
> +++ b/arch/arm/cpu/mmu.c
> @@ -302,6 +302,12 @@ static void vectors_init(void)
>  		 * live without being able to catch NULL pointer dereferences
>  		 */
>  		exc = arm_create_pte(0x0);
> +
> +		/*
> +		  Make sure that vecotrs are not re-mapped from 0x0
> +		  via VBAR (no-op on non-ARMv7)
> +		 */

s/vecotrs/vectors/

/*
 * Please use this multimline
 * comment style.
 */

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/6] ARM: Fix exception table setup in MMU-less mode

[Sascha Hauer]

On Thu, Dec 31, 2015 at 09:58:35PM -0800, Andrey Smirnov wrote:
> Add code necessary for correct initialization of exception vector
> table when MMU is disabled.
> 
> Note: Please be aware that non ARMv7 implementation of this
> functionality was not fully tested due to the lack of any such
> hardware. It should theoretically work, but only testing that I did
> was to test place_vector_table() failure path (the best I could do on
> a i.MX6)
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/Makefile |   6 +++
>  arch/arm/cpu/no-mmu.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 126 insertions(+)
>  create mode 100644 arch/arm/cpu/no-mmu.c
> 
> diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
> index 418bcab..f708e8f 100644
> --- a/arch/arm/cpu/Makefile
> +++ b/arch/arm/cpu/Makefile
> @@ -11,6 +11,12 @@ obj-$(CONFIG_CMD_ARM_MMUINFO) += mmuinfo.o
>  obj-$(CONFIG_OFDEVICE) += dtb.o
>  obj-$(CONFIG_MMU) += mmu.o cache.o mmu-early.o
>  pbl-$(CONFIG_MMU) += mmu-early.o
> +
> +ifeq ($(CONFIG_MMU),)
> +obj-y += no-mmu.o
> +pbl-y += no-mmu.o
> +endif
> +
>  obj-$(CONFIG_CPU_32v4T) += cache-armv4.o
>  pbl-$(CONFIG_CPU_32v4T) += cache-armv4.o
>  obj-$(CONFIG_CPU_32v5) += cache-armv5.o
> diff --git a/arch/arm/cpu/no-mmu.c b/arch/arm/cpu/no-mmu.c
> new file mode 100644
> index 0000000..591cc2c
> --- /dev/null
> +++ b/arch/arm/cpu/no-mmu.c
> @@ -0,0 +1,120 @@
> +/*
> + * Copyright (c) 2015 Zodiac Inflight Innovation
> + * Author: Andrey Smirnov <andrew.smirnov@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#define pr_fmt(fmt)	"nommu: " fmt
> +
> +#include <common.h>
> +#include <dma-dir.h>
> +#include <init.h>
> +#include <mmu.h>
> +#include <errno.h>
> +#include <linux/sizes.h>
> +#include <asm/memory.h>
> +#include <asm/barebox-arm.h>
> +#include <asm/system.h>
> +#include <asm/cache.h>
> +#include <memory.h>
> +#include <asm/system_info.h>
> +#include <debug_ll.h>
> +
> +
> +#define __exceptions_size (__exceptions_stop - __exceptions_start)
> +
> +#if __LINUX_ARM_ARCH__ >= 7
> +

This does not work. In arch/arm/Makefile we have:

arch-$(CONFIG_CPU_32v7)         :=-D__LINUX_ARM_ARCH__=7 $(call cc-option,-march=armv7-a,-march=armv5t -Wa$(comma)-march=armv7-a)
arch-$(CONFIG_CPU_32v6)         :=-D__LINUX_ARM_ARCH__=6 $(call cc-option,-march=armv6,-march=armv5t -Wa$(comma)-march=armv6)
arch-$(CONFIG_CPU_32v5)         :=-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)
arch-$(CONFIG_CPU_32v4T)        :=-D__LINUX_ARM_ARCH__=4 -march=armv4t

We can build barebox with support for multiple ARM architectures, in this
case __LINUX_ARM_ARCH__ is set to the smallest supported ARM architecture.

You can encapsulate this code in a #ifdef CONFIG_CPU_32v7 to make sure
it's only compiled when ARMv7 support is enabled. Then we still can not
be sure that we actually run on ARMv7, we'll need an additional runtime
check for:

	if (cpu_architecture() >= CPU_ARCH_ARMv7)

> +static struct resource *place_vector_table(void)
> +{
> +	int i;
> +	struct resource *vectors = NULL;
> +	resource_size_t addr[2] = { 0x00000000, 0xFFFF0000 };
> +
> +	for (i = 0; i < ARRAY_SIZE(addr); i++) {
> +		vectors = request_sdram_region("exceptions",
> +					       addr[i],
> +					       __exceptions_size);
> +		if (vectors)
> +			break;
> +	}
> +
> +	return vectors;
> +}
> +
> +static int nommu_v4_vectors_init(void)
> +{
> +	u32 cr;
> +	struct resource *vectors;
> +
> +	vectors = place_vector_table();
> +	if (!vectors) {
> +		pr_crit("Critical Error: Can't place exception vector table\n");
> +		return 0;
> +	}

Several SoCs do not have SDRAM at 0x0 and 0xFFFF0000, so on these SoCs
we would always see this message and have no chance to fix it.

Given that the < ARMv7 path is untested anyway I suggest to just skip it
and require MMU support to get exception support (unless someone has a
hardware to test this on).

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 6/6] i.MX6: pci: Avoid aborts when asserting PCIe reset

[Sascha Hauer]

The PCI patches look good to me. I'll give Lucas the change to look over
them.

Sascha

On Thu, Dec 31, 2015 at 09:58:38PM -0800, Andrey Smirnov wrote:
> When booting Barebox in the HW environment where PCIe core has been
> used but not properly shut down, writing to PCIE_PL_PFLR in
> imx6_pcie_assert_core_reset would cause data abort exception.
> 
> The problem can be easily reproduced on a i.MX6 based board with PCIe
> slot populated with some device by doing:
> 
> > bootm -f -e 0x1050 <your board's>.img
> 
> Ignoring this exception seem to allow PCIe core to successfully
> initialize and enumerate devices properly. This is also how Linux
> Kernel version of the driver handles this situation -- it installs
> dummy no-op abort handler at the beginning of device's probing.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  drivers/pci/pci-imx6.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/pci-imx6.c b/drivers/pci/pci-imx6.c
> index 713007b..3a3edd8 100644
> --- a/drivers/pci/pci-imx6.c
> +++ b/drivers/pci/pci-imx6.c
> @@ -13,6 +13,7 @@
> 
>  #include <common.h>
>  #include <clock.h>
> +#include <abort.h>
>  #include <malloc.h>
>  #include <io.h>
>  #include <init.h>
> @@ -237,7 +238,10 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
>  		val = readl(pp->dbi_base + PCIE_PL_PFLR);
>  		val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
>  		val |= PCIE_PL_PFLR_FORCE_LINK;
> +
> +		data_abort_mask();
>  		writel(val, pp->dbi_base + PCIE_PL_PFLR);
> +		data_abort_unmask();
> 
>  		gpr12 &= ~IMX6Q_GPR12_PCIE_CTL_2;
>  		writel(gpr12, imx6_pcie->iomuxc_gpr + IOMUXC_GPR12);
> --
> 2.5.0
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/6] ARM: Fix exception table setup in MMU-less mode

[Andrey Smirnov]

>> +
>> +#define __exceptions_size (__exceptions_stop - __exceptions_start)
>> +
>> +#if __LINUX_ARM_ARCH__ >= 7
>> +
>
> This does not work. In arch/arm/Makefile we have:
>
> arch-$(CONFIG_CPU_32v7)         :=-D__LINUX_ARM_ARCH__=7 $(call cc-option,-march=armv7-a,-march=armv5t -Wa$(comma)-march=armv7-a)
> arch-$(CONFIG_CPU_32v6)         :=-D__LINUX_ARM_ARCH__=6 $(call cc-option,-march=armv6,-march=armv5t -Wa$(comma)-march=armv6)
> arch-$(CONFIG_CPU_32v5)         :=-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)
> arch-$(CONFIG_CPU_32v4T)        :=-D__LINUX_ARM_ARCH__=4 -march=armv4t
>
> We can build barebox with support for multiple ARM architectures, in this
> case __LINUX_ARM_ARCH__ is set to the smallest supported ARM architecture.
>
> You can encapsulate this code in a #ifdef CONFIG_CPU_32v7 to make sure
> it's only compiled when ARMv7 support is enabled. Then we still can not
> be sure that we actually run on ARMv7, we'll need an additional runtime
> check for:
>
>         if (cpu_architecture() >= CPU_ARCH_ARMv7)
>

Ah, good point. Will fix.

>> +static struct resource *place_vector_table(void)
>> +{
>> +     int i;
>> +     struct resource *vectors = NULL;
>> +     resource_size_t addr[2] = { 0x00000000, 0xFFFF0000 };
>> +
>> +     for (i = 0; i < ARRAY_SIZE(addr); i++) {
>> +             vectors = request_sdram_region("exceptions",
>> +                                            addr[i],
>> +                                            __exceptions_size);
>> +             if (vectors)
>> +                     break;
>> +     }
>> +
>> +     return vectors;
>> +}
>> +
>> +static int nommu_v4_vectors_init(void)
>> +{
>> +     u32 cr;
>> +     struct resource *vectors;
>> +
>> +     vectors = place_vector_table();
>> +     if (!vectors) {
>> +             pr_crit("Critical Error: Can't place exception vector table\n");
>> +             return 0;
>> +     }
>
> Several SoCs do not have SDRAM at 0x0 and 0xFFFF0000, so on these SoCs
> we would always see this message and have no chance to fix it.

I am not sure I see why this is a problem. Those SoC physically can't
support this feature, so if you disable MMU you basically choose for
boot ROM to handle the exceptions.

>
> Given that the < ARMv7 path is untested anyway I suggest to just skip it
> and require MMU support to get exception support (unless someone has a
> hardware to test this on).

The code seemed rather trivial, so I was hoping to save people some
legwork, but sure I'll drop that portion in the next version.

Andrey

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/6] ARM: Fix exception table setup in MMU-less mode

[Sascha Hauer]

On Mon, Jan 04, 2016 at 09:01:23AM -0800, Andrey Smirnov wrote:
> >> +
> >> +#define __exceptions_size (__exceptions_stop - __exceptions_start)
> >> +
> >> +#if __LINUX_ARM_ARCH__ >= 7
> >> +
> >
> > This does not work. In arch/arm/Makefile we have:
> >
> > arch-$(CONFIG_CPU_32v7)         :=-D__LINUX_ARM_ARCH__=7 $(call cc-option,-march=armv7-a,-march=armv5t -Wa$(comma)-march=armv7-a)
> > arch-$(CONFIG_CPU_32v6)         :=-D__LINUX_ARM_ARCH__=6 $(call cc-option,-march=armv6,-march=armv5t -Wa$(comma)-march=armv6)
> > arch-$(CONFIG_CPU_32v5)         :=-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)
> > arch-$(CONFIG_CPU_32v4T)        :=-D__LINUX_ARM_ARCH__=4 -march=armv4t
> >
> > We can build barebox with support for multiple ARM architectures, in this
> > case __LINUX_ARM_ARCH__ is set to the smallest supported ARM architecture.
> >
> > You can encapsulate this code in a #ifdef CONFIG_CPU_32v7 to make sure
> > it's only compiled when ARMv7 support is enabled. Then we still can not
> > be sure that we actually run on ARMv7, we'll need an additional runtime
> > check for:
> >
> >         if (cpu_architecture() >= CPU_ARCH_ARMv7)
> >
> 
> Ah, good point. Will fix.
> 
> >> +static struct resource *place_vector_table(void)
> >> +{
> >> +     int i;
> >> +     struct resource *vectors = NULL;
> >> +     resource_size_t addr[2] = { 0x00000000, 0xFFFF0000 };
> >> +
> >> +     for (i = 0; i < ARRAY_SIZE(addr); i++) {
> >> +             vectors = request_sdram_region("exceptions",
> >> +                                            addr[i],
> >> +                                            __exceptions_size);
> >> +             if (vectors)
> >> +                     break;
> >> +     }
> >> +
> >> +     return vectors;
> >> +}
> >> +
> >> +static int nommu_v4_vectors_init(void)
> >> +{
> >> +     u32 cr;
> >> +     struct resource *vectors;
> >> +
> >> +     vectors = place_vector_table();
> >> +     if (!vectors) {
> >> +             pr_crit("Critical Error: Can't place exception vector table\n");
> >> +             return 0;
> >> +     }
> >
> > Several SoCs do not have SDRAM at 0x0 and 0xFFFF0000, so on these SoCs
> > we would always see this message and have no chance to fix it.
> 
> I am not sure I see why this is a problem. Those SoC physically can't
> support this feature, so if you disable MMU you basically choose for
> boot ROM to handle the exceptions.

"Critical Error: Can't place exception vector table" implies that something
went really wrong. We shouldn't print such a message when everything is
fine. If we want to merge this code we should lower the message to
pr_debug.

> 
> >
> > Given that the < ARMv7 path is untested anyway I suggest to just skip it
> > and require MMU support to get exception support (unless someone has a
> > hardware to test this on).
> 
> The code seemed rather trivial, so I was hoping to save people some
> legwork, but sure I'll drop that portion in the next version.

From a quick grep through the code it seems that Nomadik is the only
architecture that could currently make use of the code.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/6] i.MX6: pci: Replace magic number with a named constant

[Lucas Stach]

Am Donnerstag, den 31.12.2015, 21:58 -0800 schrieb Andrey Smirnov:
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/pci-imx6.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-imx6.c b/drivers/pci/pci-imx6.c
> index eaa5f0e..84a937f 100644
> --- a/drivers/pci/pci-imx6.c
> +++ b/drivers/pci/pci-imx6.c
> @@ -51,6 +51,8 @@ struct imx6_pcie {
>  #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2	0x2
>  #define PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK	0xf
> 
> +#define PCIE_RC_LCSR				0x80
> +
>  /* PCIe Port Logic registers (memory-mapped) */
>  #define PL_OFFSET 0x700
>  #define PCIE_PL_PFLR (PL_OFFSET + 0x08)
> @@ -418,7 +420,7 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
>  	if (ret) {
>  		dev_err(pp->dev, "Failed to bring link up!\n");
>  	} else {
> -		tmp = readl(pp->dbi_base + 0x80);
> +		tmp = readl(pp->dbi_base + PCIE_RC_LCSR);
>  		dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
>  	}
> 
> --
> 2.5.0
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 5/6] i.MX6: pci: Reconcile imx6_pcie_start_link with the kernel code

[Lucas Stach]

Am Donnerstag, den 31.12.2015, 21:58 -0800 schrieb Andrey Smirnov:
> Reconcile imx6_pcie_start_link with almost identical
> imx6_pcie_establish_link from analogous Linux kernel driver. This
> change is purely cosmetical, but refactoring the code this way
> simplifies implementation comparison.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  drivers/pci/pci-imx6.c | 42 ++++++++++++++++++++++++++----------------
>  1 file changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/pci/pci-imx6.c b/drivers/pci/pci-imx6.c
> index 84a937f..713007b 100644
> --- a/drivers/pci/pci-imx6.c
> +++ b/drivers/pci/pci-imx6.c
> @@ -362,13 +362,29 @@ static int imx6_pcie_wait_for_link(struct pcie_port *pp)
>  	}
>  }
> 
> +static int imx6_pcie_wait_for_speed_change(struct pcie_port *pp)
> +{
> +	uint32_t tmp;
> +	uint64_t start = get_time_ns();
> +
> +	while (!is_timeout(start, SECOND)) {
> +		tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
> +		/* Test if the speed change finished. */
> +		if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
> +			return 0;
> +	}
> +
> +	dev_err(pp->dev, "Speed change timeout\n");
> +	return -EINVAL;
> +}
> +
> +
>  static int imx6_pcie_start_link(struct pcie_port *pp)
>  {
>  	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
>  	uint32_t tmp;
>  	int ret;
>  	u32 gpr12;
> -	u64 start;
> 
>  	/*
>  	 * Force Gen1 operation when starting the link.  In case the link is
> @@ -403,28 +419,22 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
>  	tmp |= PORT_LOGIC_SPEED_CHANGE;
>  	writel(tmp, pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
> 
> -	start = get_time_ns();
> -	while (!is_timeout(start, SECOND)) {
> -		tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
> -		/* Test if the speed change finished. */
> -		if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
> -			break;
> +	ret = imx6_pcie_wait_for_speed_change(pp);
> +	if (ret) {
> +		dev_err(pp->dev, "Failed to bring link up!\n");
> +		return ret;
>  	}
> 
>  	/* Make sure link training is finished as well! */
> -	if (tmp & PORT_LOGIC_SPEED_CHANGE)
> -		ret = -EINVAL;
> -	else
> -		ret = imx6_pcie_wait_for_link(pp);
> -
> +	ret = imx6_pcie_wait_for_link(pp);
>  	if (ret) {
>  		dev_err(pp->dev, "Failed to bring link up!\n");
> -	} else {
> -		tmp = readl(pp->dbi_base + PCIE_RC_LCSR);
> -		dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
> +		return ret;
>  	}
> 
> -	return ret;
> +	tmp = readl(pp->dbi_base + PCIE_RC_LCSR);
> +	dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
> +	return 0;
>  }
> 
>  static void imx6_pcie_host_init(struct pcie_port *pp)
> --
> 2.5.0
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 6/6] i.MX6: pci: Avoid aborts when asserting PCIe reset

[Lucas Stach]

Am Donnerstag, den 31.12.2015, 21:58 -0800 schrieb Andrey Smirnov:
> When booting Barebox in the HW environment where PCIe core has been
> used but not properly shut down, writing to PCIE_PL_PFLR in
> imx6_pcie_assert_core_reset would cause data abort exception.
> 
> The problem can be easily reproduced on a i.MX6 based board with PCIe
> slot populated with some device by doing:
> 
> > bootm -f -e 0x1050 <your board's>.img
> 
> Ignoring this exception seem to allow PCIe core to successfully
> initialize and enumerate devices properly. This is also how Linux
> Kernel version of the driver handles this situation -- it installs
> dummy no-op abort handler at the beginning of device's probing.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

This isn't a complete fix, as it will only work if the clock tree is
unchanged. The proper solution is to shut the pcie core down before
jumping into the next image. I'll post a patch for this shortly.

But as it is increasing the odds that reinitializing an improperly
shutdown core succeeds, it is useful by itself, so:

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  drivers/pci/pci-imx6.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/pci-imx6.c b/drivers/pci/pci-imx6.c
> index 713007b..3a3edd8 100644
> --- a/drivers/pci/pci-imx6.c
> +++ b/drivers/pci/pci-imx6.c
> @@ -13,6 +13,7 @@
> 
>  #include <common.h>
>  #include <clock.h>
> +#include <abort.h>
>  #include <malloc.h>
>  #include <io.h>
>  #include <init.h>
> @@ -237,7 +238,10 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
>  		val = readl(pp->dbi_base + PCIE_PL_PFLR);
>  		val &= ~PCIE_PL_PFLR_LINK_STATE_MASK;
>  		val |= PCIE_PL_PFLR_FORCE_LINK;
> +
> +		data_abort_mask();
>  		writel(val, pp->dbi_base + PCIE_PL_PFLR);
> +		data_abort_unmask();
> 
>  		gpr12 &= ~IMX6Q_GPR12_PCIE_CTL_2;
>  		writel(gpr12, imx6_pcie->iomuxc_gpr + IOMUXC_GPR12);
> --
> 2.5.0
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/6] i.MX6: pci: Replace magic number with a named constant

[Sascha Hauer]

On Thu, Jan 07, 2016 at 10:04:14AM +0100, Lucas Stach wrote:
> Am Donnerstag, den 31.12.2015, 21:58 -0800 schrieb Andrey Smirnov:
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> 
> Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

Applied this one and the other two pci patches.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox