Re: [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n
On 03.04.2019 14:11, Marek Vasut wrote: On 4/2/19 7:02 PM, Eugeniu Rosca wrote: On Tue, Apr 02, 2019 at 06:02:46PM +0200, Marek Vasut wrote: On 4/2/19 5:40 PM, Eugeniu Rosca wrote: On Tue, Apr 02, 2019 at 05:28:43PM +0200, Marek Vasut wrote: On 4/2/19 5:17 PM, Dirk Behme wrote: On 02.04.19 15:34, Marek Vasut wrote: On 4/2/19 3:18 PM, Eugeniu Rosca wrote: With CONFIG_PINCTRL_PFC=n, aarch64-linux-gnu-ld reports: -8<- LD u-boot drivers/gpio/built-in.o: In function `rcar_gpio_request': drivers/gpio/gpio-rcar.c:128: undefined reference to `sh_pfc_config_mux_for_gpio' -8<- [..] Does CONFIG_PINCTRL_PFC=n produce a bootable binary ? Why not? Main memory, boot device and UART are configured before U-Boot, no? It depends on what is running before U-Boot, so not necessarily. And speaking of boot device, consider the case where the system runs from eMMC and uses the HS200/HS400 modes, which need to switch bus mode using the pinmux driver. Is there a real-world use case where you would want to disable the pinmux driver ? And what is the benefit of that, except that it would cause all kinds of weird problems. My H3ULCB-KF boots just fine [1] with CONFIG_PINCTRL_PFC=n, but I personally don't have any use-case which I need to fulfill on a Renesas reference design by disabling PFC. And the eMMC and SDHI both work fine too in HS400/SDR104 modes ? They cannot, since you cannot switch the pinmux properties of the bus. What about the errors in the log below, they don't look quite fine. Rather, the motivation here is to ensure U-Boot builds fine with as many randconfig results as possible, which is a standard practice in Linux. I personally favor my solution, but I am also open minded if the linker error is avoided by introducing a direct/reverse dependency between PFC and another relevant R-Car3 Kconfig symbol. I am fine with fixing randconfig build errors. My question here is whether it makes sense to allow U-Boot build without PFC support, since that would cause all kinds of problems. I am banking toward playing it safe and not allowing such an option at all. Thoughts ? It looks like in Linux, PINCTRL is a fundamental feature selected (i.e. *cannot* be disabled by users) by ARCH_RENESAS since v4.5 commit https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=26a7e06dfee9 ("arm64: renesas: r8a7795: Add Renesas R8A7795 SoC support"). So, demanding a PFC-free U-Boot doesn't look reasonable to me. That's sensible. Should PINCTRL be selected by CONFIG_RCAR_GEN3 as it is done in Linux? One caveat is that PINCTRL currently depends on DM, so R-Car3 U-Boot would become dependent on DM too, i.e. users won't have the option of a legacy U-Boot anymore. Non-DM operation is not supported anyway, the direction is toward DM/DT support. Ultimately, it should be possible to have a single U-Boot binary and just exchange the DT to support different boards. My concern is with the size of the PFC tables, they are massive, sparse and keep growing, but that's a different topic. That said, what about making the GPIO driver depend on PFC driver and then have Gen3 select PFC by default in Kconfig ? Of course, you can add such a dependency in Kconfig. But that's not the question here and won't fix the issue: It won't fix the issue that we have code encapsulated with a CONFIG_* option and a caller which is not encapsulated with this. To fix this with your proposal, you need to merge CONFIG_PINCTRL_PFC and CONFIG_RCAR_GPIO to *one* CONFIG_RCAR_PINCTRL_PFC_GPIO (or whatever) to ensure that both, the function definition *and* the caller are encapsulated by the *same* CONFIG switch. But this sounds somehow quite strange to me ... Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n
On 02.04.19 17:40, Eugeniu Rosca wrote: On Tue, Apr 02, 2019 at 05:28:43PM +0200, Marek Vasut wrote: On 4/2/19 5:17 PM, Dirk Behme wrote: On 02.04.19 15:34, Marek Vasut wrote: On 4/2/19 3:18 PM, Eugeniu Rosca wrote: With CONFIG_PINCTRL_PFC=n, aarch64-linux-gnu-ld reports: -8<- LD u-boot drivers/gpio/built-in.o: In function `rcar_gpio_request': drivers/gpio/gpio-rcar.c:128: undefined reference to `sh_pfc_config_mux_for_gpio' -8<- Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. Some recent Linux commits sharing the same approach: - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce ("acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle") - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e ("spi: make OF helper available for others") - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") Reported-by: Dirk Behme Signed-off-by: Eugeniu Rosca Does CONFIG_PINCTRL_PFC=n produce a bootable binary ? Why not? Main memory, boot device and UART are configured before U-Boot, no? It depends on what is running before U-Boot, so not necessarily. And speaking of boot device, consider the case where the system runs from eMMC and uses the HS200/HS400 modes, which need to switch bus mode using the pinmux driver. Is there a real-world use case where you would want to disable the pinmux driver ? And what is the benefit of that, except that it would cause all kinds of weird problems. My H3ULCB-KF boots just fine [1] with CONFIG_PINCTRL_PFC=n, but I personally don't have any use-case which I need to fulfill on a Renesas reference design by disabling PFC. What's about people needing to do products based on these reference designs which have boot time and by this size requirements? And having functions which are build time encapsulated with CONFIG_* but not their callers I simply would consider as a bug which needs to be fixed. Like you have done here, citing several kernel examples for this :) Best regards Dirk Rather, the motivation here is to ensure U-Boot builds fine with as many randconfig results as possible, which is a standard practice in Linux. I personally favor my solution, but I am also open minded if the linker error is avoided by introducing a direct/reverse dependency between PFC and another relevant R-Car3 Kconfig symbol. [1] U-Boot 2019.04-rc4-00100-g03ece61db8 (Apr 02 2019 - 17:23:57 +0200) CPU: Renesas Electronics R8A7795 rev 2.0 Model: Renesas H3ULCB board based on r8a7795 ES2.0+ DRAM: 3.9 GiB MMC: gpio@e6055000: set_value: error: gpio gpio@e60550001 not reserved Can't set regulator : regulator-vccq-sdhi0 gpio to: 1 sd@ee10: 0, sd@ee14: 1 Loading Environment from MMC... OK In:serial@e6e88000 Out: serial@e6e88000 Err: serial@e6e88000 Net: gpio@e6052000: set_value: error: gpio gpio@e605200010 not reserved gpio@e6052000: set_value: error: gpio gpio@e605200010 not reserved eth0: ethernet@e680 Hit any key to stop autoboot: 0 -- Best regards, Marek Vasut Best regards, Eugeniu. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] pinctrl: renesas: Fix linker error when PINCTRL_PFC=n
On 02.04.19 15:34, Marek Vasut wrote: On 4/2/19 3:18 PM, Eugeniu Rosca wrote: With CONFIG_PINCTRL_PFC=n, aarch64-linux-gnu-ld reports: -8<- LD u-boot drivers/gpio/built-in.o: In function `rcar_gpio_request': drivers/gpio/gpio-rcar.c:128: undefined reference to `sh_pfc_config_mux_for_gpio' -8<- Fix it in the least intrusive way and *avoid* ifdefs in the *.c code. Some recent Linux commits sharing the same approach: - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=23222f8f8dce ("acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle") - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f143af7501e ("spi: make OF helper available for others") - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bccd06223f21 ("IB/uverbs: Add UVERBS_ATTR_FLAGS_IN to the specs language") Fixes: f6e545a73f88 ("pfc: rmobile: Add hook to configure pin as GPIO") Reported-by: Dirk Behme Signed-off-by: Eugeniu Rosca Does CONFIG_PINCTRL_PFC=n produce a bootable binary ? Why not? Main memory, boot device and UART are configured before U-Boot, no? Best regards Dirk What is the usecase of CONFIG_PINCTRL_PFC=n ? I suspect we should rather make sure CONFIG_PINCTRL_PFC=y . --- drivers/pinctrl/renesas/sh_pfc.h | 8 1 file changed, 8 insertions(+) diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h index b98c2f185d26..3d95d3c725cf 100644 --- a/drivers/pinctrl/renesas/sh_pfc.h +++ b/drivers/pinctrl/renesas/sh_pfc.h @@ -261,7 +261,15 @@ void sh_pfc_write(struct sh_pfc *pfc, u32 reg, u32 data); const struct pinmux_bias_reg * sh_pfc_pin_to_bias_reg(const struct sh_pfc *pfc, unsigned int pin, unsigned int *bit); +#if IS_ENABLED(CONFIG_PINCTRL_PFC) int sh_pfc_config_mux_for_gpio(struct udevice *dev, unsigned pin_selector); +#else +static inline +int sh_pfc_config_mux_for_gpio(struct udevice *dev, unsigned pin_selector) +{ + return -ENODEV; +} +#endif extern const struct sh_pfc_soc_info r8a7790_pinmux_info; extern const struct sh_pfc_soc_info r8a7791_pinmux_info; ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] usb: ehci: do not invalidate a NULL buffer
On 17.11.2017 16:04, Marek Vasut wrote: On 11/17/2017 03:28 PM, Dirk Behme wrote: Its a valid use case to call ehci_submit_async() with a NULL buffer with length 0. E.g. from usb_set_configuration(). As invalidate_dcache_range() isn't able to judge if the address NULL is valid or not (depending on the SoC hardware configuration it might be valid) do the check in ehci_submit_async() as here we know that we don't have to invalidate such a buffer. Signed-off-by: Dirk Behme <dirk.be...@de.bosch.com> Looks OK, Ok, thanks, I'll drop the RFC, then. what about the other cache ops in EHCI, are they also affected? This was found based on a MMU configuration excluding address 0x. Fixing this one location made a usb start and reading from an USB stick work. So either only this location is affected, or above use case doesn't cover all relevant path. In latter case I don't have enough USB knowledge to evaluate all path by review. Best regards Dirk --- Notes: This was found on an older vendor specific U-Boot on an ARMv8 SoC with a MMU configuration where address 0x is invalid. The callstack I've seen is usb_set_configuration(), calls -> usb_control_msg() with *data == NULL and size == 0 -> submit_control_msg() -> _ehci_submit_control_msg() -> ehci_submit_async() ehci_submit_async() called __asm_invalidate_dcache_range() from arch/arm/cpu/armv8/cache.S, then. Resulting in an exception trying to use address 0 in x0 (dc ivac, x0). I'm slightly unsure why this hasn't been catched or complained about previously, already. Maybe I missed anything while working with an older vendor U-Boot? Sorry in this case. Best regards Dirk --- drivers/usb/host/ehci-hcd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index be3e842dcc..32c661b37e 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -595,8 +595,9 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, * dangerous operation, it's responsibility of the calling * code to make sure enough space is reserved. */ - invalidate_dcache_range((unsigned long)buffer, - ALIGN((unsigned long)buffer + length, ARCH_DMA_MINALIGN)); + if (buffer != NULL && length > 0) + invalidate_dcache_range((unsigned long)buffer, + ALIGN((unsigned long)buffer + length, ARCH_DMA_MINALIGN)); /* Check that the TD processing happened */ if (QT_TOKEN_GET_STATUS(token) & QT_TOKEN_STATUS_ACTIVE) ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [RFC PATCH] usb: ehci: do not invalidate a NULL buffer
Its a valid use case to call ehci_submit_async() with a NULL buffer with length 0. E.g. from usb_set_configuration(). As invalidate_dcache_range() isn't able to judge if the address NULL is valid or not (depending on the SoC hardware configuration it might be valid) do the check in ehci_submit_async() as here we know that we don't have to invalidate such a buffer. Signed-off-by: Dirk Behme <dirk.be...@de.bosch.com> --- Notes: This was found on an older vendor specific U-Boot on an ARMv8 SoC with a MMU configuration where address 0x is invalid. The callstack I've seen is usb_set_configuration(), calls -> usb_control_msg() with *data == NULL and size == 0 -> submit_control_msg() -> _ehci_submit_control_msg() -> ehci_submit_async() ehci_submit_async() called __asm_invalidate_dcache_range() from arch/arm/cpu/armv8/cache.S, then. Resulting in an exception trying to use address 0 in x0 (dc ivac, x0). I'm slightly unsure why this hasn't been catched or complained about previously, already. Maybe I missed anything while working with an older vendor U-Boot? Sorry in this case. Best regards Dirk --- drivers/usb/host/ehci-hcd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index be3e842dcc..32c661b37e 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -595,8 +595,9 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, * dangerous operation, it's responsibility of the calling * code to make sure enough space is reserved. */ - invalidate_dcache_range((unsigned long)buffer, - ALIGN((unsigned long)buffer + length, ARCH_DMA_MINALIGN)); + if (buffer != NULL && length > 0) + invalidate_dcache_range((unsigned long)buffer, + ALIGN((unsigned long)buffer + length, ARCH_DMA_MINALIGN)); /* Check that the TD processing happened */ if (QT_TOKEN_GET_STATUS(token) & QT_TOKEN_STATUS_ACTIVE) -- 2.14.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v7 3/9] armv8: Add Secure Monitor/Hypervisor Call (SMC/HVC) infrastructure
On 25.06.2016 21:04, Mateusz Kulikowski wrote: Hi Dirk, On 23.06.2016 13:33, Dirk Behme wrote: [...] Idea: perhaps after this series is merged we can add 2 new commands to u-boot (SMC/HVC) to play with hypervisors/secure monitors (and perhaps use some simple functionality if needed). How this should look like? I thought of something like this (I did such code few times): u-boot> smc 42 42 42 42 42 42 ret => (0x1, 0x2, 0x3, 0x4) Could you share any (example?) code you have for such an smc/hvc U-Boot command? I'm afraid I don't have it anymore :( SMC call itself is trivial, you can use smc_call @ u-boot: arch/arm/cpu/armv8/fwcall.c (this is code for armv8 in 64-bit mode, but you can easily port it to armv7) As for adding custom commands - just use any existing as template (sleep may be a good idea :) ). Anything like below [1]? Best regards Dirk [1] From f5fb9bdab3054fdc37ca3fed44703f0dc5c8b083 Mon Sep 17 00:00:00 2001 From: Dirk Behme <dirk.be...@de.bosch.com> Date: Mon, 27 Jun 2016 12:39:35 +0200 Subject: [PATCH] common: Add ARMv8 smc command Add a command line interface to do ARMv8 secure monitor calls (smc). Signed-off-by: Dirk Behme <dirk.be...@de.bosch.com> --- common/Makefile |2 + common/cmd_armv8svchvc.c | 60 ++ 2 files changed, 62 insertions(+), 0 deletions(-) create mode 100644 common/cmd_armv8svchvc.c diff --git a/common/Makefile b/common/Makefile index 252fbf1..5a8dad0 100644 --- a/common/Makefile +++ b/common/Makefile @@ -191,6 +191,8 @@ obj-$(CONFIG_CMD_SPL) += cmd_spl.o obj-$(CONFIG_CMD_ZIP) += cmd_zip.o obj-$(CONFIG_CMD_ZFS) += cmd_zfs.o +obj-$(CONFIG_ARM64) += cmd_armv8svchvc.o + # others obj-$(CONFIG_BOOTSTAGE) += bootstage.o obj-$(CONFIG_CONSOLE_MUX) += iomux.o diff --git a/common/cmd_armv8svchvc.c b/common/cmd_armv8svchvc.c new file mode 100644 index 000..6491704 --- /dev/null +++ b/common/cmd_armv8svchvc.c @@ -0,0 +1,60 @@ +/* + * (C) Copyright 2016 + * Robert Bosch Car Multimedia GmbH + * Dirk Behme <dirk.be...@de.bosch.com> + * + * SPDX-License-Identifier:GPL-2.0+ + */ + +#include +#include + +static int do_smc(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + struct pt_regs regs; + + memset(, 0, sizeof(struct pt_regs)); + + switch (argc) { + case 9: + regs.regs[7] = simple_strtoul(argv[8], NULL, 16); + /* fall through */ + case 8: + regs.regs[6] = simple_strtoul(argv[7], NULL, 16); + /* fall through */ + case 7: + regs.regs[5] = simple_strtoul(argv[6], NULL, 16); + /* fall through */ + case 6: + regs.regs[4] = simple_strtoul(argv[5], NULL, 16); + /* fall through */ + case 5: + regs.regs[3] = simple_strtoul(argv[4], NULL, 16); + /* fall through */ + case 4: + regs.regs[2] = simple_strtoul(argv[3], NULL, 16); + /* fall through */ + case 3: + regs.regs[1] = simple_strtoul(argv[2], NULL, 16); + /* fall through */ + case 2: + regs.regs[0] = simple_strtoul(argv[1], NULL, 16); + break; + default: + return CMD_RET_USAGE; + } + + smc_call(); + + printf("ret: x0: 0x%016luX\n x1: 0x%016luX\n x2: 0x%016luX x3: 0x%016lX\n", + regs.regs[0], regs.regs[1], regs.regs[2], regs.regs[3]); + + return 0; +} + +U_BOOT_CMD( + smc ,9,0, do_smc, + "do ARMv8 hypervisor call (SMC)", + "x0 [x1 x2 x3 x4 x5 x6 x7]\n" + "- 8 SMC parameters for the registers x0 - x7" +); -- 1.6.6.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v7 3/9] armv8: Add Secure Monitor/Hypervisor Call (SMC/HVC) infrastructure
Hi Mateusz, On 07.01.2016 22:39, Mateusz Kulikowski wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 07.01.2016 16:06, Michal Simek wrote: On 6.1.2016 14:04, Mateusz Kulikowski wrote: On 14.10.2015 18:55, Sergey Temerkhanov wrote: [...] Idea: perhaps after this series is merged we can add 2 new commands to u-boot (SMC/HVC) to play with hypervisors/secure monitors (and perhaps use some simple functionality if needed). How this should look like? I thought of something like this (I did such code few times): u-boot> smc 42 42 42 42 42 42 ret => (0x1, 0x2, 0x3, 0x4) Could you share any (example?) code you have for such an smc/hvc U-Boot command? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] ARMv8: Use U-Boot to boot Xen?
On 29.01.2016 10:18, Ian Campbell wrote: On Fri, 2016-01-29 at 12:04 +0800, Peng Fan wrote: Hi Dirk, Cc Ian xen experts. On Thu, Jan 28, 2016 at 08:06:30PM +0100, Dirk Behme wrote: Hi, are there any U-Boot examples/patches to boot Xen on an ARMv8/aarch64 system? I've found http://lists.denx.de/pipermail/u-boot/2015-October/230077.html what might be helpful. But maybe I missed anything else? I guess you are asking steps how to boot xen using uboot? You may need to take this: http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions Indeed. In addition one AArch64 system which uses u-boot and which has concrete instructions is the APM X-Gene/Mustang: http://wiki.xen.org/wiki/Xen_ARM_with_Virtualization_Extensions/APMXGeneMustang which would likely be helpful. Yes, that page is very helpful, thanks! But I think it misses one information, or most probably the configuration on that board is different. Above page just do a 'run xen_run' which in the end does the necessary tftp loading and finally the bootm to start Xen. So I'd assume that on that board U-Boot runs at EL3 or EL2 (?). In my environment, U-Boot runs at EL1. I.e. after loading Xen by tftp I somehow need to switch to EL2 to be able to start Xen. And this is what I think I miss. How to switch from U-Boot running at EL1 to hypervisor mode EL2? Therefore I'd think that http://lists.denx.de/pipermail/u-boot/2015-October/230077.html might be helpful. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] ARMv8: Use U-Boot to boot Xen?
Hi, are there any U-Boot examples/patches to boot Xen on an ARMv8/aarch64 system? I've found http://lists.denx.de/pipermail/u-boot/2015-October/230077.html what might be helpful. But maybe I missed anything else? Many thanks Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] mx6: soc: Switch to cold reset
Disable the warm reset and enable the cold reset for a more reliable restart ('reset'). This is taken from the Linux kernel, see imx_src_init() in arch/arm/mach-imx/src.c. Signed-off-by: Dirk Behme dirk.be...@de.bosch.com --- arch/arm/cpu/armv7/mx6/soc.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c index 5f5f497..12be6ff 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -267,6 +267,22 @@ static void set_preclk_from_osc(void) } #endif +#define SRC_SCR_WARM_RESET_ENABLE 0 + +static void init_src(void) +{ + struct src *src_regs = (struct src *)SRC_BASE_ADDR; + u32 val; + + /* +* force warm reset sources to generate cold reset +* for a more reliable restart +*/ + val = readl(src_regs-scr); + val = ~(1 SRC_SCR_WARM_RESET_ENABLE); + writel(val, src_regs-scr); +} + int arch_cpu_init(void) { init_aips(); @@ -294,6 +310,8 @@ int arch_cpu_init(void) mxs_dma_init(); #endif + init_src(); + return 0; } -- 1.8.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] scripts: fix binutils-version.sh
On 25.12.2014 03:09, Masahiro Yamada wrote: The current binutils-version.sh expects the version string at the end of the first line. It turned out to not work with Linaro toolchain: It has Linaro 2014.09 at the back. To fix this issue, let's parse the word right after the close parenthesis. Signed-off-by: Masahiro Yamada yamad...@jp.panasonic.com Reported-by: York Sun york...@freescale.com Acked-by: Dirk Behme dirk.be...@gmail.com This fixes the issue [1] for me :) I'd propose to apply this to 2015.01-rc3. Thanks! Dirk [1] http://lists.denx.de/pipermail/u-boot/2014-December/199515.html --- scripts/binutils-version.sh | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/binutils-version.sh b/scripts/binutils-version.sh index d4d9eb4..0bc26cf 100755 --- a/scripts/binutils-version.sh +++ b/scripts/binutils-version.sh @@ -14,7 +14,9 @@ if [ ${#gas} -eq 0 ]; then exit 1 fi -MAJOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 1) -MINOR=$($gas --version | head -1 | awk '{print $NF}' | cut -d . -f 2) +version_string=$($gas --version | head -1 | sed -e 's/.*) *\([0-9.]*\).*/\1/' ) + +MAJOR=$(echo $version_string | cut -d . -f 1) +MINOR=$(echo $version_string | cut -d . -f 2) printf %02d%02d\\n $MAJOR $MINOR ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] buildman reports error of script/binutils-version.sh
On 23.11.2014 23:20, Simon Glass wrote: Hi, On 21 November 2014 at 16:36, York Sun york...@freescale.com wrote: Simon, Shall we consider host error to be an error reported by buildman? I happen to try a newer version of toolchain from Linaro. Buildman reports this error +../scripts/binutils-version.sh: line 20: printf: 09: invalid octal number The root cause is this version of AS reports version string differently GNU assembler (crosstool-NG linaro-1.13.1-4.9-2014.09 - Linaro GCC 4.9-2014.09) 2.24.0.20140829 Linaro 2014.09 It has Linaro 2014.09 at the back, so script/binutils-version.sh cannot parse it correctly (the error is not handling zero-leading numbers, but not in the scope of this discussion). Another angle of this question is, why do we need this script? It doesn't sound stable to me to parse the version string this way. That's another one in Masahiro's domain I think. Are there any news/answers/fixes for this? I'm getting above +../scripts/binutils-version.sh: line 20: printf: 09: invalid octal number too, with the Linaro 2014.09 tool chain. Thanks, Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] buildman: Fix some typos in README
Signed-off-by: Dirk Behme dirk.be...@gmail.com --- tools/buildman/README | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index bfb2f18..0f8ea20 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -42,7 +42,7 @@ Theory of Operation Buildman is a builder. It is not make, although it runs make. It does not produce any useful output on the terminal while building, except for progress information (except with -v, see below). All the output (errors, -warnings and binaries if you are ask for them) is stored in output +warnings and binaries if you ask for them) is stored in output directories, which you can look at while the build is progressing, or when it is finished. @@ -121,7 +121,7 @@ You can also use -x to specifically exclude some boards. For example: means to build all arm boards except nvidia, freescale and anything ending with 'ball'. -It is convenient to use the -n option to see whaat will be built based on +It is convenient to use the -n option to see what will be built based on the subset given. Buildman does not store intermediate object files. It optionally copies @@ -371,7 +371,7 @@ in an hour and 15 minutes. Use this time to buy a faster computer. To find out how the build went, ask for a summary with -s. You can do this -either before the build completes (presumably in another terminal) or or +either before the build completes (presumably in another terminal) or afterwards. Let's work through an example of how this is used: $ ./tools/buildman/buildman -b lcd9b -s @@ -439,7 +439,7 @@ again. At commit 16, the error moves - you can see that the old error at line 120 is fixed, but there is a new one at line 126. This is probably only because -we added some code and moved the broken line father down the file. +we added some code and moved the broken line further down the file. If many boards have the same error, then -e will display the error only once. This makes the output as concise as possible. To see which boards have @@ -647,8 +647,8 @@ This shows that commit 19 has increased text size for arm (although only one board was built) and by 96 bytes for powerpc. This increase was offset in both cases by reductions in rodata and data/bss. -Shown below the summary lines is the sizes for each board. Below each board -is the sizes for each function. This information starts with: +Shown below the summary lines are the sizes for each board. Below each board +are the sizes for each function. This information starts with: add - number of functions added / removed grow - number of functions which grew / shrunk @@ -817,7 +817,7 @@ TODO This has mostly be written in my spare time as a response to my difficulties in testing large series of patches. Apart from tidying up there is quite a bit of scope for improvement. Things like better error diffs and easier -access to log files. Also it would be nice it buildman could 'hunt' for +access to log files. Also it would be nice if buildman could 'hunt' for problems, perhaps by building a few boards for each arch, or checking commits for changed files and building only boards which use those files. -- 2.2.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v1 2/4] spi, spi_mxc: do not hang in spi_xchg_single
Am 28.05.2014 12:16, schrieb Heiko Schocher: if status register do never set MXC_CSPICTRL_TC, spi_xchg_single endless loops. Add a timeout here to prevent endless hang. As I've never seen this, yet: Any idea what goes wrong if this happens? Thanks Dirk Signed-off-by: Heiko Schocher h...@denx.de Cc: Dirk Behme dirk.be...@gmail.com Cc: Jagannadha Sutradharudu Teki jagannadh.t...@gmail.com --- drivers/spi/mxc_spi.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index f3f029d..3cd93cf 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -212,6 +212,7 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen, int nbytes = DIV_ROUND_UP(bitlen, 8); u32 data, cnt, i; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; + int timeout; debug(%s: bitlen %d dout 0x%x din 0x%x\n, __func__, bitlen, (u32)dout, (u32)din); @@ -272,9 +273,12 @@ int spi_xchg_single(struct spi_slave *slave, unsigned int bitlen, reg_write(regs-ctrl, mxcs-ctrl_reg | MXC_CSPICTRL_EN | MXC_CSPICTRL_XCH); + timeout = 1; /* Wait until the TC (Transfer completed) bit is set */ - while ((reg_read(regs-stat) MXC_CSPICTRL_TC) == 0) - ; + while (timeout ((reg_read(regs-stat) MXC_CSPICTRL_TC) == 0)) { + timeout--; + udelay(10); + } /* Transfer completed, clear any pending request */ reg_write(regs-stat, MXC_CSPICTRL_TC | MXC_CSPICTRL_RXOVF); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] ARM: Add workaround for Cortex-A9 errata 794072
On 02.04.2014 05:33, nitin.g...@freescale.com wrote: From: Nitin Garg nitin.g...@freescale.com A short loop including a DMB instruction might cause a denial of service on another processor which executes a CP15 broadcast operation. Exists on r1, r2, r3, r4 revisions. Signed-off-by: Nitin Garg nitin.g...@freescale.com --- README |1 + arch/arm/cpu/armv7/start.S |5 + 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/README b/README index 7cb7c4f..a496c65 100644 --- a/README +++ b/README @@ -566,6 +566,7 @@ The following options need to be configured: CONFIG_ARM_ERRATA_742230 CONFIG_ARM_ERRATA_743622 CONFIG_ARM_ERRATA_751472 + CONFIG_ARM_ERRATA_794072 If set, the workarounds for these ARM errata are applied early during U-Boot startup. Note that these options force the diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index ac1e55a..b87a378 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -222,6 +222,11 @@ ENTRY(cpu_init_cp15) orr r0, r0, #1 11 @ set bit #11 mcr p15, 0, r0, c15, c0, 1 @ write diagnostic register #endif +#ifdef CONFIG_ARM_ERRATA_794072 + mrc p15, 0, r0, c15, c0, 1 @ read diagnostic register + orr r0, r0, #1 4 @ set bit #4 + mcr p15, 0, r0, c15, c0, 1 @ write diagnostic register +#endif Where is the difference between the errata code for above new CONFIG_ARM_ERRATA_794072 and the existing #ifdef CONFIG_ARM_ERRATA_742230 mrc p15, 0, r0, c15, c0, 1 @ read diagnostic register orr r0, r0, #1 4 @ set bit #4 mcr p15, 0, r0, c15, c0, 1 @ write diagnostic register #endif ? Maybe we should just do a #if (defined(CONFIG_ARM_ERRATA_794072) || defined(CONFIG_ARM_ERRATA_742230)) ? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] ARM: Add workaround for Cortex-A9 errata 761320
On 02.04.2014 05:33, nitin.g...@freescale.com wrote: From: Nitin Garg nitin.g...@freescale.com Full cache line writes to the same memory region from at least two processors might deadlock the processor. Exists on r1, r2, r3 revisions. Signed-off-by: Nitin Garg nitin.g...@freescale.com --- README |1 + arch/arm/cpu/armv7/start.S |5 + 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/README b/README index a496c65..b7c0f68 100644 --- a/README +++ b/README @@ -567,6 +567,7 @@ The following options need to be configured: CONFIG_ARM_ERRATA_743622 CONFIG_ARM_ERRATA_751472 CONFIG_ARM_ERRATA_794072 + CONFIG_ARM_ERRATA_761320 If set, the workarounds for these ARM errata are applied early during U-Boot startup. Note that these options force the diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index b87a378..1229476 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -227,6 +227,11 @@ ENTRY(cpu_init_cp15) orr r0, r0, #1 4 @ set bit #4 mcr p15, 0, r0, c15, c0, 1 @ write diagnostic register #endif +#ifdef CONFIG_ARM_ERRATA_761320 + mrc p15, 0, r0, c15, c0, 1 @ read diagnostic register + orr r0, r0, #1 21 @ set bit #21 + mcr p15, 0, r0, c15, c0, 1 @ write diagnostic register +#endif Is there any reason why you dropped the check for r4p0 cmp r6, #0x40 @ present prior to r4p0 which you still had in http://www.spinics.net/lists/arm-kernel/msg319223.html ? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] ARM: Add workaround for Cortex-A9 errata 794072
On 02.04.2014 14:58, nitin.g...@freescale.com wrote: From: Nitin Garg nitin.g...@freescale.com A short loop including a DMB instruction might cause a denial of service on another processor which executes a CP15 broadcast operation. Exists on r1, r2, r3, r4 revisions. Signed-off-by: Nitin Garg nitin.g...@freescale.com --- README |1 + arch/arm/cpu/armv7/start.S |2 +- 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/README b/README index 7cb7c4f..a496c65 100644 --- a/README +++ b/README @@ -566,6 +566,7 @@ The following options need to be configured: CONFIG_ARM_ERRATA_742230 CONFIG_ARM_ERRATA_743622 CONFIG_ARM_ERRATA_751472 + CONFIG_ARM_ERRATA_794072 If set, the workarounds for these ARM errata are applied early during U-Boot startup. Note that these options force the diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index ac1e55a..f3830c8 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -205,7 +205,7 @@ ENTRY(cpu_init_cp15) mcr p15, 0, r0, c1, c0, 0 @ write system control register #endif -#ifdef CONFIG_ARM_ERRATA_742230 +#if (defined(CONFIG_ARM_ERRATA_742230) || defined(CONFIG_ARM_ERRATA_794072)) mrc p15, 0, r0, c15, c0, 1 @ read diagnostic register orr r0, r0, #1 4 @ set bit #4 mcr p15, 0, r0, c15, c0, 1 @ write diagnostic register Acked-by: Dirk Behme dirk.be...@de.bosch.com Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] mx6: Enable L2 cache support
Am 29.01.2014 20:39, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com Add L2 cache support and enable it by default. Configure the L2 cache in the same way as done by FSL kernel: http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx6/mm.c?h=imx_3.0.35_4.1.0 Signed-off-by: Fabio Estevam fabio.este...@freescale.com Acked-by: Dirk Behme dirk.be...@gmail.com Thanks Dirk --- Changes since v3: - Enable l2 cache using the same method as the kernel Changes since v2: - Add L2_PL310_BASE definition in imx_regs.h Changes since v1: - Fx typo in commit log arch/arm/cpu/armv7/mx6/soc.c | 58 arch/arm/include/asm/arch-mx6/imx-regs.h | 1 + arch/arm/include/asm/pl310.h | 21 include/configs/mx6_common.h | 5 +++ 4 files changed, 85 insertions(+) diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c index 0208cba..396bba5 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -8,6 +8,8 @@ */ #include common.h +#include asm/armv7.h +#include asm/pl310.h #include asm/errno.h #include asm/io.h #include asm/arch/imx-regs.h @@ -336,3 +338,59 @@ void imx_setup_hdmi(void) writel(reg, mxc_ccm-chsccdr); } #endif + +#ifndef CONFIG_SYS_L2CACHE_OFF +#define IOMUXC_GPR11_L2CACHE_AS_OCRAM 0x0002 +void v7_outer_cache_enable(void) +{ + struct pl310_regs *const pl310 = (struct pl310_regs *)L2_PL310_BASE; + unsigned int val; + +#if defined CONFIG_MX6SL + struct iomuxc *iomux = (struct iomuxc *)IOMUXC_BASE_ADDR; + val = readl(iomux-gpr[11]); + if (val IOMUXC_GPR11_L2CACHE_AS_OCRAM) { + /* L2 cache configured as OCRAM, reset it */ + val = ~IOMUXC_GPR11_L2CACHE_AS_OCRAM; + writel(val, iomux-gpr[11]); + } +#endif + + writel(0x132, pl310-pl310_tag_latency_ctrl); + writel(0x132, pl310-pl310_data_latency_ctrl); + + val = readl(pl310-pl310_prefetch_ctrl); + + /* Turn on the L2 I/D prefetch */ + val |= 0x3000; + + /* +* The L2 cache controller(PL310) version on the i.MX6D/Q is r3p1-50rel0 +* The L2 cache controller(PL310) version on the i.MX6DL/SOLO/SL is r3p2 +* But according to ARM PL310 errata: 752271 +* ID: 752271: Double linefill feature can cause data corruption +* Fault Status: Present in: r3p0, r3p1, r3p1-50rel0. Fixed in r3p2 +* Workaround: The only workaround to this erratum is to disable the +* double linefill feature. This is the default behavior. +*/ + +#ifndef CONFIG_MX6Q + val |= 0x4080; +#endif + writel(val, pl310-pl310_prefetch_ctrl); + + val = readl(pl310-pl310_power_ctrl); + val |= L2X0_DYNAMIC_CLK_GATING_EN; + val |= L2X0_STNDBY_MODE_EN; + writel(val, pl310-pl310_power_ctrl); + + setbits_le32(pl310-pl310_ctrl, L2X0_CTRL_EN); +} + +void v7_outer_cache_disable(void) +{ + struct pl310_regs *const pl310 = (struct pl310_regs *)L2_PL310_BASE; + + clrbits_le32(pl310-pl310_ctrl, L2X0_CTRL_EN); +} +#endif /* !CONFIG_SYS_L2CACHE_OFF */ diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index f2ad6e9..c2d210a 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -53,6 +53,7 @@ #define GLOBAL_TIMER_BASE_ADDR 0x00A00200 #define PRIVATE_TIMERS_WD_BASE_ADDR 0x00A00600 #define IC_DISTRIBUTOR_BASE_ADDR0x00A01000 +#define L2_PL310_BASE 0x00A02000 #define GPV0_BASE_ADDR 0x00B0 #define GPV1_BASE_ADDR 0x00C0 #define PCIE_ARB_BASE_ADDR 0x0100 diff --git a/arch/arm/include/asm/pl310.h b/arch/arm/include/asm/pl310.h index f41ad8c..d4526af 100644 --- a/arch/arm/include/asm/pl310.h +++ b/arch/arm/include/asm/pl310.h @@ -12,6 +12,9 @@ /* Register bit fields */ #define PL310_AUX_CTRL_ASSOCIATIVITY_MASK (1 16) +#define L2X0_DYNAMIC_CLK_GATING_EN (1 1) +#define L2X0_STNDBY_MODE_EN(1 0) +#define L2X0_CTRL_EN 1 struct pl310_regs { u32 pl310_cache_id; @@ -47,6 +50,24 @@ struct pl310_regs { u32 pad9[1]; u32 pl310_clean_inv_line_idx; u32 pl310_clean_inv_way; + u32 pad10[64]; + u32 pl310_lockdown_dbase; + u32 pl310_lockdown_ibase; + u32 pad11[190]; + u32 pl310_addr_filter_start; + u32 pl310_addr_filter_end; + u32 pad12[190]; + u32 pl310_test_operation; + u32 pad13[3]; + u32 pl310_line_data; + u32 pad14[7]; + u32 pl310_line_tag; + u32 pad15[3]; + u32 pl310_debug_ctrl; + u32 pad16[7]; + u32 pl310_prefetch_ctrl; + u32 pad17[7]; + u32 pl310_power_ctrl; }; void pl310_inval_all(void); diff --git
Re: [U-Boot] [PATCH v3] mx6: Enable L2 cache support
Hi Fabio, Am 28.01.2014 15:54, schrieb Fabio Estevam: Add L2 cache support and enable it by default. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v2: - Add L2_PL310_BASE definition in imx_regs.h Changes since v1: - Fx typo in commit log arch/arm/cpu/armv7/mx6/soc.c | 20 arch/arm/include/asm/arch-mx6/imx-regs.h | 1 + include/configs/mx6_common.h | 5 + 3 files changed, 26 insertions(+) diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c index 0208cba..b84de87 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -8,6 +8,8 @@ */ #include common.h +#include asm/armv7.h +#include asm/pl310.h #include asm/errno.h #include asm/io.h #include asm/arch/imx-regs.h @@ -336,3 +338,21 @@ void imx_setup_hdmi(void) writel(reg, mxc_ccm-chsccdr); } #endif + +#ifndef CONFIG_SYS_L2CACHE_OFF +#define L2CACHE1 +void v7_outer_cache_enable(void) +{ + struct pl310_regs *const pl310 = (struct pl310_regs *)L2_PL310_BASE; + + setbits_le32(pl310-pl310_ctrl, L2CACHE); + +} Just for better understanding: Do you want to keep this intentionally simple? Or is there any special reason why you don't set additional (performance) registers here? E.g. the L2 PREFETCH and POWER registers, and the tag and data latency settings? Like done in the kernel. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] configs: mx6_common: Restrict CONFIG_ARM_ERRATA_742230 for multi-core
Hi Fabio, On 08.01.2014 14:59, Fabio Estevam wrote: From: Fabio Estevam fabio.este...@freescale.com According to e9fd66defd (ARM: mx6: define CONFIG_ARM_ERRATA_742230), the CONFIG_ARM_ERRATA_742230 option should only be applied to multi-core variants, so restrict its usage for quad and dual-lite only. Just for my understanding: Is there a technical reason not to use this errata on single core (solo/sololite)? I.e. do you see any real issues using this errata on solo/sololite? Or is this patch just out of formal aspects? I.e. there are no positive/negative issues seen on solo/sololite, but the documentation tells that it shouldn't be used on solo/sololite, so disable it? Best regards Dirk Quoting Shawn Guo [2]: The sololite has the same core version as dual/quad - r2p10. The help text of erratum 742230 in kernel suggests that only version r1p0..r2p2 are affected. So it sounds like the erratum 742230 should be irrelevant to i.MX6 SoCs. However we were running into a reboot issue on multi-core i.MX6 SoCs. There was a quite long discussion [1] about it. Though we did not reach a conclusion in the thread, one ARM people sent me a private message, suggesting this should be an ARM core issue and workaround for erratum 742230 might help. And it turns out what he said is true. And that's why I came up with the commit e9fd66defd (ARM: mx6: define CONFIG_ARM_ERRATA_742230) to turn on the erratum for imx6 dual/quad. Shawn [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/thread.html#113096; [2] http://lists.denx.de/pipermail/u-boot/2014-January/170424.html Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v1: - Improve commit log include/configs/mx6_common.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h index 514d634..0b8db85 100644 --- a/include/configs/mx6_common.h +++ b/include/configs/mx6_common.h @@ -17,7 +17,9 @@ #ifndef __MX6_COMMON_H #define __MX6_COMMON_H +#if defined(CONFIG_MX6Q) || defined(CONFIG_MX6DL) #define CONFIG_ARM_ERRATA_742230 +#endif #define CONFIG_ARM_ERRATA_743622 #define CONFIG_ARM_ERRATA_751472 #define CONFIG_BOARD_POSTCLK_INIT ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Configuring U-Boot through device tree (was: Minutes from the U-Boot Mini Summit 2013)
Am 27.10.2013 18:07, schrieb Detlev Zundel: ... ** Configuring U-Boot through device tree - _What_ should be configured? - Google requires every new U-Boot driver to be configured through device tree in U-Boot - Configuring U-Boot through device trees shall aim for using the exact same tree from the Linux kernel. At ELCE, I attended the Barebox presentation. One Barebox feature presented there was Configure Barbox through device tree. So the Barebox people seem to have this already running for some selected boards. After their presentation I asked them how do you decide which parts are configured in the boot loader, and which in the kernel, then?. And got the quick answer in doubt, the configuration is done two times. Do we really want this? If we use the same device tree for U-Boot and the Linux kernel (which most probably makes sense) do we really want to do the initialization part we do already in U-Boot again in the kernel? I'm thinking about pin mux or clock settings described in the device tree, which are need to get the U-Boot drivers working. If these are done two times, then, what's about the risk of clock or pin glitches doing the same configuration in the kernel a second time? Do we need a mechanism to do some configuration only at one place? Or isn't there any risk and we can do the same configuration two times? What do you think? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Enabling L2 cache on mx53
Am 19.08.2013 15:55, schrieb Fabio Estevam: Hi, I notice slow tftp transfer on mx53qsb and I suspected it could be due to L2 cache being disabled. Tried enabling with: --- a/arch/arm/cpu/armv7/mx5/lowlevel_init.S +++ b/arch/arm/cpu/armv7/mx5/lowlevel_init.S @@ -45,6 +45,11 @@ #endif mcr 15, 1, r0, c9, c0, 2 + + /* enable L2 cache */ + mrc 15, 0, r0, c1, c0, 1 + orr r0, r0, #(1 1)/* enable l2 cache */ + mcr 15, 0, r0, c1, c0, 1 .endm /* init_l2cc */ /* AIPS setup - Only setup MPROTx registers. ,but still see the same low tftp throughput (720 kB/s - on mx28 I see the double rate). Any suggestions as to how properly enable L2 cache on mx53? Is the mx53 L2 cache the same like on mx6? If so, besides enabling it, it needs a proper configuration. Have a look to https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5a5ca56e057d206db13461b84a7da3a3543e1206 https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b3a9c315378ff811bf34393f2f0a6e8b9ffced3b Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] uboot optimize memmove
On 26.07.2013 15:42, Andy Green wrote: On 26 July 2013 20:58, Wolfgang Denk w...@denx.de wrote: ... you not make sure that you provide optimized implementations for such functions and consequently #define __HAVE_ARCH_MEMMOVE (and __HAVE_ARCH_MEMCPY) ? Yes I found these afterwards... performance is slightly better than memcpy() thanks to Nicolas Pitre it seems. The U-Boot config for the platform we have didn't know about them, it's much better with them. After I wrote this patch it was also pointed out by Will Newton at Linaro that we have Neon accelerated memcpy lying around with BSD license https://launchpad.net/cortex-strings however for my purposes NOR boot is working good enough with the ARCH versions. I've had a look to https://launchpad.net/cortex-strings and there downloaded https://launchpad.net/cortex-strings/trunk/2013.01/+download/cortex-strings-1.0-2013.01.tar.bz2 (from the green download button on the right side). The README mentions src/neon contains NEON based routines for AArch32. but the cortex-strings-1.0-2013.01.tar.bz2 seems to contain an empty src/neon directory. Is this intended? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] ddr cfg: DRAM_RESET needs 0x00020030
On 17.07.2013 21:46, Troy Kisky wrote: The old value of 0x000e0030 will cause ethernet timeout issues on the sabrelite and possibly other boards using the KSZ9021. I have no explanation as to why. But this is a correct change, the TRM will be updated to show that 00b is the only valid setting for bits 19-18 of DRAM_RESET. My thanks go to Liu Hui(Jason) for this information. Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com Acked-by: Dirk Behme dirk.be...@de.bosch.com Thanks Dirk --- board/boundary/nitrogen6x/ddr-setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/board/boundary/nitrogen6x/ddr-setup.cfg b/board/boundary/nitrogen6x/ddr-setup.cfg index c315812..e5f8add 100644 --- a/board/boundary/nitrogen6x/ddr-setup.cfg +++ b/board/boundary/nitrogen6x/ddr-setup.cfg @@ -74,7 +74,7 @@ DATA 4, MX6_IOM_DRAM_RAS, 0x00020030 DATA 4, MX6_IOM_DRAM_SDCLK_0, 0x00020030 DATA 4, MX6_IOM_DRAM_SDCLK_1, 0x00020030 -DATA 4, MX6_IOM_DRAM_RESET, 0x000e0030 +DATA 4, MX6_IOM_DRAM_RESET, 0x00020030 DATA 4, MX6_IOM_DRAM_SDCKE0, 0x3000 DATA 4, MX6_IOM_DRAM_SDCKE1, 0x3000 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] fsl_esdhc: Touch only relevant sys ctrl bits
Dealing with the sys ctrl register should touch only the relevant bits and not accidently the whole register. On i.MX6, the sys control register contains bits which shouldn't be reset to 0, e.g. SYS_CTRL[3-0] and IPP_RST_N (SYS_CTRL[23]). Do this by read/modify/write instead of just a 32bit write. Signed-off-by: Dirk Behme dirk.be...@de.bosch.com --- drivers/mmc/fsl_esdhc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 973b19f..431dac2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -470,7 +470,7 @@ static int esdhc_init(struct mmc *mmc) int timeout = 1000; /* Reset the entire host controller */ - esdhc_write32(regs-sysctl, SYSCTL_RSTA); + esdhc_setbits32(regs-sysctl, SYSCTL_RSTA); /* Wait until the controller is available */ while ((esdhc_read32(regs-sysctl) SYSCTL_RSTA) --timeout) @@ -481,7 +481,7 @@ static int esdhc_init(struct mmc *mmc) esdhc_write32(regs-scr, 0x0040); #endif - esdhc_write32(regs-sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN); + esdhc_setbits32(regs-sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN); /* Set the initial clock speed */ mmc_set_clock(mmc, 40); @@ -515,7 +515,7 @@ static void esdhc_reset(struct fsl_esdhc *regs) unsigned long timeout = 100; /* wait max 100 ms */ /* reset the controller */ - esdhc_write32(regs-sysctl, SYSCTL_RSTA); + esdhc_setbits32(regs-sysctl, SYSCTL_RSTA); /* hardware clears the bit when it is done */ while ((esdhc_read32(regs-sysctl) SYSCTL_RSTA) --timeout) -- 1.8.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] mxc_gpio: Correct the GPIO handling in gpio_direction_output()
Setting the direction and an output value should be done by First, set the desired output value. Then, switch to output. If this is done in the inverse order, like at the moment, there can be a glitch on the GPIO line while switching first the old output value and aftwards the new one. Fix this by inverting the order of the direction/set_value calls. Signed-off-by: Dirk Behme dirk.be...@de.bosch.com --- drivers/gpio/mxc_gpio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index a388064..1d820d4 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -143,10 +143,10 @@ int gpio_direction_input(unsigned gpio) int gpio_direction_output(unsigned gpio, int value) { - int ret = mxc_gpio_direction(gpio, MXC_GPIO_DIRECTION_OUT); + int ret = gpio_set_value(gpio, value); if (ret 0) return ret; - return gpio_set_value(gpio, value); + return mxc_gpio_direction(gpio, MXC_GPIO_DIRECTION_OUT); } -- 1.8.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v8 3/3] MIPS: Jz4740: Add qi_lb60 board support
Dear Wolfgang and Tom, Am 03.07.2013 11:54, schrieb Wolfgang Denk: Dear Xiangfu Liu, In message 4e95a3ba.8000...@pobox.com you wrote: Add support for the qi_lb60 (a.k.a QI Ben NanoNote) clamshell device from Qi hardware: http://en.qi-hardware.com/wiki/Ben_NanoNote http://en.qi-hardware.com/wiki/Main_Page http://en.wikipedia.org/wiki/Qi_hardware This Jz4740-based clamshell device does not use NOR flash to boot. The initial bring-up assumes that U-Boot is directly loaded into SDRAM using USB boot tool, and starts from 0x8010. ... MAINTAINERS |4 + MAKEALL |4 +- board/qi/qi_lb60/Makefile | 45 + board/qi/qi_lb60/config.mk | 31 +++ board/qi/qi_lb60/qi_lb60.c | 104 + board/qi/qi_lb60/u-boot.lds | 61 + boards.cfg |1 + include/configs/qi_lb60.h | 211 +++ 8 files changed, 460 insertions(+), 1 deletions(-) create mode 100644 board/qi/qi_lb60/Makefile create mode 100644 board/qi/qi_lb60/config.mk create mode 100644 board/qi/qi_lb60/qi_lb60.c create mode 100644 board/qi/qi_lb60/u-boot.lds create mode 100644 include/configs/qi_lb60.h It has been pointed out (see [1]) that the files board/qi/qi_lb60/qi_lb60.c include/configs/qi_lb60.h added by this patch are licensed as GPL version 3 of the License, or (at your option) any later version - however, this is incompatible with the GPLv2 and GPLv2+ licenses that cover the rest of U-Boot. Would you be willing to re-license these files under GPLv2+ (and submit apatch to do that) ? Otherwise we would probably be forced to remove the qi_lb60 board support to be legally clean. Sorry for the inconvenience, but obviously this issue slipped through at the initial review of the code... [1] http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/164965 No answer since one week. Should we revert this for v2013.07, then? Best regards Dirk [1] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commit;h=3c945542dad99b1ec4a324ad6b69b8de8829827b ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: Fix calculation of emi_slow clock rate
On 04.07.2013 13:27, Andrew Gabbasov wrote: This is porting of Freescale's patch from version imx_v2009.08_3.0.35_4.0.0, that fixes the obvious mistype of bits offset macro name (ACLK_EMI_PODF_OFFSET was used instead of ACLK_EMI_SLOW_PODF_OFFSET). Using the occasion, change the variable name 'emi_slow_pof' to more consistent 'emi_slow_podf'. Signed-off-by: Jason Liu r64...@freescale.com Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com Acked-by: Dirk Behme dirk.be...@de.bosch.com Thanks Dirk --- arch/arm/cpu/armv7/mx6/clock.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c index 3c0d908..064f8c8 100644 --- a/arch/arm/cpu/armv7/mx6/clock.c +++ b/arch/arm/cpu/armv7/mx6/clock.c @@ -244,13 +244,13 @@ static u32 get_axi_clk(void) static u32 get_emi_slow_clk(void) { - u32 emi_clk_sel, emi_slow_pof, cscmr1, root_freq = 0; + u32 emi_clk_sel, emi_slow_podf, cscmr1, root_freq = 0; cscmr1 = __raw_readl(imx_ccm-cscmr1); emi_clk_sel = cscmr1 MXC_CCM_CSCMR1_ACLK_EMI_SLOW_MASK; emi_clk_sel = MXC_CCM_CSCMR1_ACLK_EMI_SLOW_OFFSET; - emi_slow_pof = cscmr1 MXC_CCM_CSCMR1_ACLK_EMI_SLOW_PODF_MASK; - emi_slow_pof = MXC_CCM_CSCMR1_ACLK_EMI_PODF_OFFSET; + emi_slow_podf = cscmr1 MXC_CCM_CSCMR1_ACLK_EMI_SLOW_PODF_MASK; + emi_slow_podf = MXC_CCM_CSCMR1_ACLK_EMI_SLOW_PODF_OFFSET; switch (emi_clk_sel) { case 0: @@ -267,7 +267,7 @@ static u32 get_emi_slow_clk(void) break; } - return root_freq / (emi_slow_pof + 1); + return root_freq / (emi_slow_podf + 1); } #ifdef CONFIG_MX6SL ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] GPL v3 only code in U-Boot
Hi, hopefully ;) a short question: We found that the commit http://git.denx.de/?p=u-boot.git;a=commitdiff;h=3c945542dad99b1ec4a324ad6b69b8de8829827b contains two files board/qi/qi_lb60/qi_lb60.c include/configs/qi_lb60.h which seem to be GPL v3 *only*. Is this ok for U-Boot? Many thanks and best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [[PATCH]] imx6: fix GPR2 wrong definition
Am 19.06.2013 11:16, schrieb Pierre Aubert: Signed-off-by: Pierre Aubert p.aub...@staubli.com CC: Stefano Babic sba...@denx.de Acked-by: Dirk Behme dirk.be...@gmail.com Thanks Dirk --- arch/arm/include/asm/arch-mx6/imx-regs.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index 03abb2a..45824f9 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -364,7 +364,7 @@ struct iomuxc { #define IOMUXC_GPR2_MODE_DISABLED 0 #define IOMUXC_GPR2_MODE_ENABLED_DI0 1 -#define IOMUXC_GPR2_MODE_ENABLED_DI1 2 +#define IOMUXC_GPR2_MODE_ENABLED_DI1 3 #define IOMUXC_GPR2_LVDS_CH1_MODE_OFFSET 2 #define IOMUXC_GPR2_LVDS_CH1_MODE_MASK (3IOMUXC_GPR2_LVDS_CH1_MODE_OFFSET) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
On 10.06.2013 16:51, Gabbasov, Andrew wrote: Hi Dirk, From: Behme, Dirk - Bosch Sent: Monday, June 10, 2013 16:06 To: Gabbasov, Andrew Cc: u-boot@lists.denx.de; Stefano Babic; Fleming Andy-AFLEMING Subject: Re: [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop On 08.04.2013 11:06, Andrew Gabbasov wrote: Rework the waiting for transfer completion loop condition to continue waiting until both Transfer Complete and DMA End interrupts occur. Checking of DLA bit in Present State register looks not needed in addition to interrupts status checking, so it can be removed from the condition. Also, DMA Error condition is added to the list of data errors, checked in the loop. Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com --- drivers/mmc/fsl_esdhc.c |3 +-- include/fsl_esdhc.h |4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 54b5363..814bba4 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -400,8 +400,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) if (irqstat DATA_ERR) return COMM_ERR; - } while (!(irqstat IRQSTAT_TC) - (esdhc_read32(regs-prsstat) PRSSTAT_DLA)); + } while ((irqstat DATA_COMPLETE) != DATA_COMPLETE); #endif } diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h index 47d2fe4..ea0880b 100644 --- a/include/fsl_esdhc.h +++ b/include/fsl_esdhc.h @@ -63,7 +63,9 @@ #define IRQSTAT_CC (0x0001) #define CMD_ERR (IRQSTAT_CIE | IRQSTAT_CEBE | IRQSTAT_CCE) -#define DATA_ERR (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE) +#define DATA_ERR (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE | \ + IRQSTAT_DMAE) +#define DATA_COMPLETE(IRQSTAT_TC | IRQSTAT_DINT) #define IRQSTATEN 0x0002e034 #define IRQSTATEN_DMAE (0x1000) I haven't tested this myself, but I got the following issue report regarding this patch: Using a SANDISK ULTRA II 8GB card (or alternatively Transcend 16GB or 32GB cards) and trying an mmc write [1] into the upper area of the 8GB card makes the write hang in 9 of 10 cases. Sometimes even more. Reverting this patch make these writes work again. mmc read does work fine, though. Even newer SANDISK Extreme III or several micro SD cards are working fine. Any idea? Best regards Dirk So far the only idea that comes into my mind is that DMA for some reason completes its part of work too early so that the corresponding interrupt status bit appears and has already been cleared even before entering this loop. I will be trying to reproduce the issue. Meanwhile, is it possible to ask the reporter (who obviously can reproduce it) to try to add the debug print from the diff below and show what it prints when the write command hangs and when it succeeds? Thanks. Best regards, Andrew diff -u fsl_esdhc.c.orig fsl_esdhc.c --- fsl_esdhc.c.orig2013-05-30 03:48:26.0 -0500 +++ fsl_esdhc.c 2013-06-10 09:38:30.071905119 -0500 @@ -329,6 +329,7 @@ irqstat = esdhc_read32(regs-irqstat); esdhc_write32(regs-irqstat, irqstat); + printf(fsl_esdhc: irqstat = 0x%08x\n, irqstat); /* Reset CMD and DATA portions on error */ if (irqstat (CMD_ERR | IRQSTAT_CTOE)) { 1. Sandisk 8GB Ultra 2 class 4 SDHC [ 23.967081] MMC write: dev # 0, block # 740, count 1 ... fsl_esdhc: irqstat = 0x0001 [ 23.977473] fsl_esdhc: irqstat = 0x0009 = hang With fsl_esdhc: Fix DMA transfer completion waiting loop reverted: [ 41.769231] MMC write: dev # 0, block # 740, count 1 ... fsl_esdhc: irqstat = 0x0001 [ 41.779622] fsl_esdhc: irqstat = 0x0009 [ 41.798490] fsl_esdhc: irqstat = 0x0001 [ 41.802593] 1 blocks write: OK = work 2. Sandisk 16GB Extreme class 10 (30MB/s) [ 45.871140] MMC write: dev # 0, block # 740, count 1 ... fsl_esdhc: irqstat = 0x0001 [ 45.881528] fsl_esdhc: irqstat = 0x0001 [ 46.054409] fsl_esdhc: irqstat = 0x0001 [ 46.058513] 1 blocks write: OK = work With fsl_esdhc: Fix DMA transfer completion waiting loop reverted: [ 17.901514] MMC write: dev # 0, block # 740, count 1 ... fsl_esdhc: irqstat = 0x0001 [ 17.911901] fsl_esdhc: irqstat = 0x0001 [ 18.081153] fsl_esdhc: irqstat = 0x0001 [ 18.085256] 1 blocks write: OK = work Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fsl_esdhc: Do not clear interrupt status bits until data processed
On 11.06.2013 17:34, Andrew Gabbasov wrote: After waiting for the command completion event, the interrupt status bits, that occured to be set by that time, are cleared by writing them back. It is supposed, that it should be command related bits (command complete and may be command errors). However, in some cases the DMA already completes by that time before the full transaction completes. The corresponding DINT bit gets set and then cleared before even entering the loop, waiting for data part completion. That waiting loop never gets this bit set, causing the operation to hang. This is reported to happen, for example, for write operation of 1 sector to upper area (block #740) of SanDisk Ultra II 8GB card. The solution could be to explicitly clear only command related interrupt status bits. However, since subsequent processing does not rely on any command bits state, it could be easier just to remove clearing of any bits at that point, leaving them all until all data processing completes. After that the whole register will be cleared at once. Also, on occasion, interrupts masking moved to before writing the command, just for the case there should be no chance of interrupt between the first command and interrupts masking. Reported-by: Dirk Behme dirk.be...@de.bosch.com Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com Acked-by: Dirk Behme dirk.be...@de.bosch.com Thanks Dirk --- drivers/mmc/fsl_esdhc.c |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 861f4b9..b501b4d 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -310,6 +310,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) /* Figure out the transfer arguments */ xfertyp = esdhc_xfertyp(cmd, data); + /* Mask all irqs */ + esdhc_write32(regs-irqsigen, 0); + /* Send the command */ esdhc_write32(regs-cmdarg, cmd-cmdarg); #if defined(CONFIG_FSL_USDHC) @@ -320,15 +323,11 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) esdhc_write32(regs-xfertyp, xfertyp); #endif - /* Mask all irqs */ - esdhc_write32(regs-irqsigen, 0); - /* Wait for the command to complete */ while (!(esdhc_read32(regs-irqstat) (IRQSTAT_CC | IRQSTAT_CTOE))) ; irqstat = esdhc_read32(regs-irqstat); - esdhc_write32(regs-irqstat, irqstat); /* Reset CMD and DATA portions on error */ if (irqstat (CMD_ERR | IRQSTAT_CTOE)) { ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] spi: mxc_spi: Update pre and post divider algorithm
On 11.05.2013 07:25, Dirk Behme wrote: The spi clock divisor is of the form x * (2**y), or x y, where x is 1 to 16, and y is 0 to 15. Note the similarity with floating point numbers. Convert the desired divisor to the smallest number which is = desired divisor, and can be represented in this form. The previous algorithm chose a divisor which could be almost twice as large as needed. Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com Signed-off-by: Dirk Behme dirk.be...@gmail.com While we are talking about a first -rc now, could we get these two patches http://patchwork.ozlabs.org/patch/242709/ http://patchwork.ozlabs.org/patch/243113/ applied? Thanks Dirk --- Notes: - Changes in v2: Make the alogrithm simpler by removing the -1 as proposed by Troy. Make the pre_div and post_div u32. - This replaces v1 of this patch and depends on the previous sent patch http://patchwork.ozlabs.org/patch/242709/ drivers/spi/mxc_spi.c | 30 -- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 3e903b3..e87b899 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,8 +128,8 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); - s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; - u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; + s32 reg_ctrl, reg_config; + u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, pre_div = 0, post_div = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; if (max_hz == 0) { @@ -147,26 +147,20 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl |= MXC_CSPICTRL_EN; reg_write(regs-ctrl, reg_ctrl); - /* -* The following computation is taken directly from Freescale's code. -*/ if (clk_src max_hz) { - pre_div = DIV_ROUND_UP(clk_src, max_hz); - if (pre_div 16) { - post_div = pre_div / 16; - pre_div = 16; - } - if (post_div != 0) { - for (i = 0; i 16; i++) { - if ((1 i) = post_div) - break; - } - if (i == 16) { + pre_div = (clk_src - 1) / max_hz; + /* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ + post_div = fls(pre_div); + if (post_div 4) { + post_div -= 4; + if (post_div = 16) { printf(Error: no divider for the freq: %d\n, max_hz); return -1; } - post_div = i; + pre_div = post_div; + } else { + post_div = 0; } } @@ -174,7 +168,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_SELCHAN(3)) | MXC_CSPICTRL_SELCHAN(cs); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_PREDIV(0x0F)) | - MXC_CSPICTRL_PREDIV(pre_div - 1); + MXC_CSPICTRL_PREDIV(pre_div); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
On 08.04.2013 11:06, Andrew Gabbasov wrote: Rework the waiting for transfer completion loop condition to continue waiting until both Transfer Complete and DMA End interrupts occur. Checking of DLA bit in Present State register looks not needed in addition to interrupts status checking, so it can be removed from the condition. Also, DMA Error condition is added to the list of data errors, checked in the loop. Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com --- drivers/mmc/fsl_esdhc.c |3 +-- include/fsl_esdhc.h |4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 54b5363..814bba4 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -400,8 +400,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) if (irqstat DATA_ERR) return COMM_ERR; - } while (!(irqstat IRQSTAT_TC) - (esdhc_read32(regs-prsstat) PRSSTAT_DLA)); + } while ((irqstat DATA_COMPLETE) != DATA_COMPLETE); #endif } diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h index 47d2fe4..ea0880b 100644 --- a/include/fsl_esdhc.h +++ b/include/fsl_esdhc.h @@ -63,7 +63,9 @@ #define IRQSTAT_CC(0x0001) #define CMD_ERR (IRQSTAT_CIE | IRQSTAT_CEBE | IRQSTAT_CCE) -#define DATA_ERR (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE) +#define DATA_ERR (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE | \ + IRQSTAT_DMAE) +#define DATA_COMPLETE (IRQSTAT_TC | IRQSTAT_DINT) #define IRQSTATEN 0x0002e034 #define IRQSTATEN_DMAE(0x1000) I haven't tested this myself, but I got the following issue report regarding this patch: Using a SANDISK ULTRA II 8GB card (or alternatively Transcend 16GB or 32GB cards) and trying an mmc write [1] into the upper area of the 8GB card makes the write hang in 9 of 10 cases. Sometimes even more. Reverting this patch make these writes work again. mmc read does work fine, though. Even newer SANDISK Extreme III or several micro SD cards are working fine. Any idea? Best regards Dirk [1] mmc write 0x1080 70ea40 1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select
On 30.05.2013 12:02, Andrew Gabbasov wrote: The number of gpio signal is packed inside CONFIG_SF_DEFAULT_CS macro (shifted and or'ed with chip select), so it's incorrect to pass that macro directly as an argument to gpio_direction_output() call. The gpio number should be extracted (shifted back) before that. Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com --- board/boundary/nitrogen6x/nitrogen6x.c|2 +- board/freescale/mx6qsabrelite/mx6qsabrelite.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c index cc071d6..b588ac2 100644 --- a/board/boundary/nitrogen6x/nitrogen6x.c +++ b/board/boundary/nitrogen6x/nitrogen6x.c @@ -342,7 +342,7 @@ iomux_v3_cfg_t const ecspi1_pads[] = { void setup_spi(void) { - gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1); + gpio_direction_output(CONFIG_SF_DEFAULT_CS 8, 1); imx_iomux_v3_setup_multiple_pads(ecspi1_pads, ARRAY_SIZE(ecspi1_pads)); } diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index 9f9cac8..8b71e03 100644 --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c @@ -312,7 +312,7 @@ iomux_v3_cfg_t const ecspi1_pads[] = { void setup_spi(void) { - gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1); + gpio_direction_output(CONFIG_SF_DEFAULT_CS 8, 1); imx_iomux_v3_setup_multiple_pads(ecspi1_pads, ARRAY_SIZE(ecspi1_pads)); } To my understanding, above change is correct, but not complete ;) The question is why has it worked with the wrong setting and nobody ever noticed that its wrong? To my understanding the answer is because the SPI driver does it correctly: http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/spi/mxc_spi.c;h=5bed858787f610a9c9a46bb2214665a51d60a9e9;hb=refs/heads/master#l376 So IMHO the gpio_direction_output() above can be removed completely. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select
On 30.05.2013 13:32, Gabbasov, Andrew wrote: Hi Dirk, From: Behme, Dirk - Bosch Sent: Thursday, May 30, 2013 14:50 To: Gabbasov, Andrew Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Fix use of gpio number in SF chip select [skipped] To my understanding, above change is correct, but not complete ;) The question is why has it worked with the wrong setting and nobody ever noticed that its wrong? To my understanding the answer is because the SPI driver does it correctly: http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/spi/mxc_spi.c;h=5bed858787f610a9c9a46bb2214665a51d60a9e9;hb=refs/heads/master#l376 So IMHO the gpio_direction_output() above can be removed completely. Best regards Dirk Yes, the SPI driver correctly activates and deactivates the CS signal. But before the first activation it relies on what signal state was set before that. Setting it early at startup just adds some confidence that we have correct (inactive) chip select state before the first activation by SPI driver. Otherwise we have to rely on particular pad configuration (e.g. EIM_D19). I understand, that we set its configuration to pull-up (and this is also the reset default), and if we do nothing here, it will be recognized as high. But in order to make sure, it's more safe to explicitly set the signal to 1. Hmm, what's 'unsure' in the time between calling setup_spi() the first time and calling spi_setup_slave() the first time? Are they even called in this order? How long is the time between these two calls? What's 'unsafe' in this time frame? Why isn't it unsafe _until_ setup_spi() is called, then? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: mx6qsabrelite/nitrogen6x: Remove incorrect setting of gpio CS signal
On 30.05.2013 16:47, Andrew Gabbasov wrote: The number of gpio signal is packed inside CONFIG_SF_DEFAULT_CS macro (shifted and or'ed with chip select), so it's incorrect to pass that macro directly as an argument to gpio_direction_output() call. Also, SPI driver sets the direction and initial value of a gpio, used as a chip select signal, before any actual activity happens on the bus. So, it is safe to just remove the gpio_direction_output call, that works incorrectly, thus making no effect, anyway. Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com --- board/boundary/nitrogen6x/nitrogen6x.c|1 - board/freescale/mx6qsabrelite/mx6qsabrelite.c |1 - 2 files changed, 2 deletions(-) diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c index cc071d6..735fd76 100644 --- a/board/boundary/nitrogen6x/nitrogen6x.c +++ b/board/boundary/nitrogen6x/nitrogen6x.c @@ -342,7 +342,6 @@ iomux_v3_cfg_t const ecspi1_pads[] = { void setup_spi(void) { - gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1); imx_iomux_v3_setup_multiple_pads(ecspi1_pads, ARRAY_SIZE(ecspi1_pads)); } diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index 9f9cac8..29815d7 100644 --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c @@ -312,7 +312,6 @@ iomux_v3_cfg_t const ecspi1_pads[] = { void setup_spi(void) { - gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1); imx_iomux_v3_setup_multiple_pads(ecspi1_pads, ARRAY_SIZE(ecspi1_pads)); } Acked-by: Dirk Behme dirk.be...@de.bosch.com Thanks Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC PATCH] ARM: byteorder: add optimized swab16 and swab32
Use the specialized ARM instructions for swapping 16 and 32 bit values instead of using the generic ones from include/linux/byteorder/swab.h. The x86 version in arch/x86/include/asm/byteorder.h was taken as an example for this. E.g. for the mx6qsabrelite target this results in ~4k less code. Signed-off-by: Dirk Behme dirk.be...@gmail.com --- This patch is marked as RFC due to two questions: 1) These ARM instructions are available in ARMv6 and above. Do we have to limit this code somehow to = ARMv6? I couldn't find any #define to do this in U-Boot (?) 2) I'm not sure about the rev16. The rev16 instruction swaps *both* half words, the lower _and_ the upper one: REV16 Rd, Rm = Rd[15:8] := Rm[7:0], Rd[7:0] := Rm[15:8], Rd[31:24] := Rm[23:16], Rd[23:16] := Rm[31:24] I'm not sure if we want this? Most probably we only want to swap the lower half word with swap16()? arch/arm/include/asm/byteorder.h | 17 + 1 file changed, 17 insertions(+) diff --git a/arch/arm/include/asm/byteorder.h b/arch/arm/include/asm/byteorder.h index c3489f1..f21baf0 100644 --- a/arch/arm/include/asm/byteorder.h +++ b/arch/arm/include/asm/byteorder.h @@ -15,6 +15,23 @@ #ifndef __ASM_ARM_BYTEORDER_H #define __ASM_ARM_BYTEORDER_H +static __inline__ __u32 ___arch__swab32(__u32 x) +{ + __asm__(rev %0,%1 /* swap bytes */ \ + : =r (x) \ + : 0 (x)); \ + return x; +} +#define __arch__swab32(x) ___arch__swab32(x) + +static __inline__ __u16 ___arch__swab16(__u16 x) +{ + __asm__(rev16 %0,%1 /* swap bytes */ \ + : =r (x) \ + : 0 (x)); \ + return x; +} +#define __arch__swab16(x) ___arch__swab16(x) #include asm/types.h -- 1.7.10.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] ARM: byteorder: add optimized swab16 and swab32
Am 10.05.2013 18:56, schrieb Måns Rullgård: Dirk Behme dirk.be...@gmail.com writes: Use the specialized ARM instructions for swapping 16 and 32 bit values instead of using the generic ones from include/linux/byteorder/swab.h. The x86 version in arch/x86/include/asm/byteorder.h was taken as an example for this. E.g. for the mx6qsabrelite target this results in ~4k less code. Which compiler are you using? An older gcc 4.4.4 version. GCC 4.5 and later recognises the byteswap pattern and emits these instructions automatically. This gives better code than the inline asm since it allows the compiler to do proper instruction scheduling, and in the 16-bit case it avoids extraneous masking. Thanks, Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] spi: mxc_spi: Update pre and post divider algorithm
Am 10.05.2013 20:44, schrieb Troy Kisky: On 5/9/2013 10:34 PM, Dirk Behme wrote: Am 09.05.2013 20:00, schrieb Troy Kisky: On 5/8/2013 10:19 PM, Dirk Behme wrote: The spi clock divisor is of the form x * (2**y), or x y, where x is 1 to 16, and y is 0 to 15. Note the similarity with floating point numbers. Convert the desired divisor to the smallest number which is = desired divisor, and can be represented in this form. The previous algorithm chose a divisor which could be almost twice as large as needed. Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com Signed-off-by: Dirk Behme dirk.be...@gmail.com --- drivers/spi/mxc_spi.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 3e903b3..66c2ad8 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,7 +128,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); -s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; +s32 pre_div = 1, post_div = 0, reg_ctrl, reg_config; First, I'm totally fine with the patch as it is. I'm just going to point out things you may want to change, or send a follow-up patch. Here, no need to initialize pre_div, post_div, if you delete the if (clk_src max_hz) below which is not needed. Hmm, why is it not needed? If you remove the if, you *always* do at least the computation pre_div = DIV_ROUND_UP(clk_src, max_hz); post_div = fls(pre_div - 1); if (post_div 4) { I would think that doing the initialization and the if is much cheaper than always doing above computation, even if its not needed? I would keep the if. u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; @@ -147,27 +147,24 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl |= MXC_CSPICTRL_EN; reg_write(regs-ctrl, reg_ctrl); -/* - * The following computation is taken directly from Freescale's code. - */ if (clk_src max_hz) { This if can be removed. pre_div = DIV_ROUND_UP(clk_src, max_hz); If you subtract -1 here instead of when you set the divisor register, the logic becomes simpler pre_div = DIV_ROUND_UP(clk_src, max_hz) - 1; or just pre_div = (clk_src - 1) / max_hz; -if (pre_div 16) { -post_div = pre_div / 16; -pre_div = 16; -} -if (post_div != 0) { -for (i = 0; i 16; i++) { -if ((1 i) = post_div) -break; -} -if (i == 16) { +/* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ +post_div = fls(pre_div - 1); +if (post_div 4) { +post_div -= 4; + +if (post_div = 16) { printf(Error: no divider for the freq: %d\n, max_hz); return -1; } -post_div = i; +pre_div = (pre_div + (1 post_div) - 1) post_div; This would become pre_div = post_div; + +} else { +post_div = 0; } + } debug(pre_div = %d, post_div=%d\n, pre_div, post_div); And MXC_CSPICTRL_PREDIV(pre_div - 1); would return to MXC_CSPICTRL_PREDIV(pre_div); Well, where to do the -1 mainly depends on how we want to interpret the output of debug(pre_div = %d, post_div=%d\n, pre_div, post_div); I'd change to debug(actual div = %d, pre_div = %d, post_div=%d\n, (pre_div + 1) post_div, pre_div, post_div); to eliminate confusion. There are two options how to understand this, at least the pre_div = %d part: a) print the pre_div as the real divider used in a human readable format. I.e. if we divide by /15 then print 15 or b) print the pre_div value we write to the register. I.e. if we divide by /15 then print 14 Up to now we are doing (a) and calculate the register value after the debug print. Your proposal would switch this to (b). Anyway, if it makes the algorithm simpler I'd agree that it's fine to switch to (b). To summarize, this would become then: s32 pre_div = 1, post_div = 0, reg_ctrl, reg_config; ... If you keep the if, pre_div = 0 Ah, yes, thanks. Also, I'd change s32 to unsigned or u32. But usually the if will be true, so why keep it? For cases where clk_src = max_hz. Then don't do the calculation because it's unnecessary. It is just code bloat and prone to errors like yours above. The only logic difference between keeping/killing the if is when clk_src == 0. Keeping will give a divide by 1. Killing will give an error. I'd argue that an error is more appropriate. if (clk_src max_hz) { pre_div = (clk_src - 1) / max_hz; /* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ post_div = fls(pre_div); if (post_div 4) { post_div -= 4; if (post_div
[U-Boot] [PATCH v2] spi: mxc_spi: Update pre and post divider algorithm
The spi clock divisor is of the form x * (2**y), or x y, where x is 1 to 16, and y is 0 to 15. Note the similarity with floating point numbers. Convert the desired divisor to the smallest number which is = desired divisor, and can be represented in this form. The previous algorithm chose a divisor which could be almost twice as large as needed. Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com Signed-off-by: Dirk Behme dirk.be...@gmail.com --- Notes: - Changes in v2: Make the alogrithm simpler by removing the -1 as proposed by Troy. Make the pre_div and post_div u32. - This replaces v1 of this patch and depends on the previous sent patch http://patchwork.ozlabs.org/patch/242709/ drivers/spi/mxc_spi.c | 30 -- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 3e903b3..e87b899 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,8 +128,8 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); - s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; - u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; + s32 reg_ctrl, reg_config; + u32 ss_pol = 0, sclkpol = 0, sclkpha = 0, pre_div = 0, post_div = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; if (max_hz == 0) { @@ -147,26 +147,20 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl |= MXC_CSPICTRL_EN; reg_write(regs-ctrl, reg_ctrl); - /* -* The following computation is taken directly from Freescale's code. -*/ if (clk_src max_hz) { - pre_div = DIV_ROUND_UP(clk_src, max_hz); - if (pre_div 16) { - post_div = pre_div / 16; - pre_div = 16; - } - if (post_div != 0) { - for (i = 0; i 16; i++) { - if ((1 i) = post_div) - break; - } - if (i == 16) { + pre_div = (clk_src - 1) / max_hz; + /* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ + post_div = fls(pre_div); + if (post_div 4) { + post_div -= 4; + if (post_div = 16) { printf(Error: no divider for the freq: %d\n, max_hz); return -1; } - post_div = i; + pre_div = post_div; + } else { + post_div = 0; } } @@ -174,7 +168,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_SELCHAN(3)) | MXC_CSPICTRL_SELCHAN(cs); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_PREDIV(0x0F)) | - MXC_CSPICTRL_PREDIV(pre_div - 1); + MXC_CSPICTRL_PREDIV(pre_div); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); -- 1.7.10.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] spi: mxc_spi: Update pre and post divider algorithm
Am 09.05.2013 20:00, schrieb Troy Kisky: On 5/8/2013 10:19 PM, Dirk Behme wrote: The spi clock divisor is of the form x * (2**y), or x y, where x is 1 to 16, and y is 0 to 15. Note the similarity with floating point numbers. Convert the desired divisor to the smallest number which is = desired divisor, and can be represented in this form. The previous algorithm chose a divisor which could be almost twice as large as needed. Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com Signed-off-by: Dirk Behme dirk.be...@gmail.com --- drivers/spi/mxc_spi.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 3e903b3..66c2ad8 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,7 +128,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); -s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; +s32 pre_div = 1, post_div = 0, reg_ctrl, reg_config; First, I'm totally fine with the patch as it is. I'm just going to point out things you may want to change, or send a follow-up patch. Here, no need to initialize pre_div, post_div, if you delete the if (clk_src max_hz) below which is not needed. Hmm, why is it not needed? If you remove the if, you *always* do at least the computation pre_div = DIV_ROUND_UP(clk_src, max_hz); post_div = fls(pre_div - 1); if (post_div 4) { I would think that doing the initialization and the if is much cheaper than always doing above computation, even if its not needed? I would keep the if. u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; @@ -147,27 +147,24 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl |= MXC_CSPICTRL_EN; reg_write(regs-ctrl, reg_ctrl); -/* - * The following computation is taken directly from Freescale's code. - */ if (clk_src max_hz) { This if can be removed. pre_div = DIV_ROUND_UP(clk_src, max_hz); If you subtract -1 here instead of when you set the divisor register, the logic becomes simpler pre_div = DIV_ROUND_UP(clk_src, max_hz) - 1; or just pre_div = (clk_src - 1) / max_hz; -if (pre_div 16) { -post_div = pre_div / 16; -pre_div = 16; -} -if (post_div != 0) { -for (i = 0; i 16; i++) { -if ((1 i) = post_div) -break; -} -if (i == 16) { +/* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ +post_div = fls(pre_div - 1); +if (post_div 4) { +post_div -= 4; + +if (post_div = 16) { printf(Error: no divider for the freq: %d\n, max_hz); return -1; } -post_div = i; +pre_div = (pre_div + (1 post_div) - 1) post_div; This would become pre_div = post_div; + +} else { +post_div = 0; } + } debug(pre_div = %d, post_div=%d\n, pre_div, post_div); And MXC_CSPICTRL_PREDIV(pre_div - 1); would return to MXC_CSPICTRL_PREDIV(pre_div); Well, where to do the -1 mainly depends on how we want to interpret the output of debug(pre_div = %d, post_div=%d\n, pre_div, post_div); There are two options how to understand this, at least the pre_div = %d part: a) print the pre_div as the real divider used in a human readable format. I.e. if we divide by /15 then print 15 or b) print the pre_div value we write to the register. I.e. if we divide by /15 then print 14 Up to now we are doing (a) and calculate the register value after the debug print. Your proposal would switch this to (b). Anyway, if it makes the algorithm simpler I'd agree that it's fine to switch to (b). To summarize, this would become then: s32 pre_div = 1, post_div = 0, reg_ctrl, reg_config; ... if (clk_src max_hz) { pre_div = (clk_src - 1) / max_hz; /* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ post_div = fls(pre_div); if (post_div 4) { post_div -= 4; if (post_div = 16) { printf(Error: no divider for the freq: %d\n, max_hz); return -1; } pre_div = post_div; } else { post_div = 0; } } ... MXC_CSPICTRL_PREDIV(pre_div); Is this correct? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 1/2] spi: mxc_spi: Fix pre and post divider calculation
Fix two issues with the calculation of pre_div and post_div: 1. pre_div: While the calculation of pre_div looks correct, to set the CONREG[15-12] bits pre_div needs to be decremented by 1: The i.MX 6Dual/6Quad Applications Processor Reference Manual (IMX6DQRM Rev. 0, 11/2012) states: CONREG[15-12]: PRE_DIVIDER Divide by 1 0001 Divide by 2 0010 Divide by 3 ... 1101 Divide by 14 1110 Divide by 15 Divide by 16 I.e. if we want to divide by 2, we have to write 1 to CONREG[15-12]. 2. In case the post divider becomes necessary, pre_div will be divided by 16. So set pre_div to 16, too. And not 15. Both issues above are tested using the following examples: clk_src = 6000 (60MHz, default i.MX6 ECSPI clock) a) max_hz == 2300 (23MHz, max i.MX6 ECSPI read clock) - pre_div = 3 (divide by 3 = CONREG[15-12] == 2) - post_div = 0 (divide by 1 = CONREG[11- 8] == 0) = 60MHz / 3 = 20MHz SPI clock b) max_hz == 200 (2MHz) - pre_div = 16 (divide by 16 = CONREG[15-12] == 15) - post_div = 1 (divide by 2 = CONREG[11- 8] == 1) = 60MHz / 32 = 1.875MHz SPI clock c) max_hz == 100 (1MHz) - pre_div = 16 (divide by 16 = CONREG[15-12] == 15) - post_div = 2 (divide by 4 = CONREG[11- 8] == 2) = 60MHz / 64 = 937.5kHz SPI clock d) max_hz == 50 (500kHz) - pre_div = 16 (divide by 16 = CONREG[15-12] == 15) - post_div = 3 (divide by 8 = CONREG[11- 8] == 3) = 60MHz / 128 = 468.75kHz SPI clock Signed-off-by: Dirk Behme dirk.be...@gmail.com --- Note: Changes in v2: Use pre_div divider /16 instead of /15 in the first version of this patch. drivers/spi/mxc_spi.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 843a1f2..3e903b3 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,7 +128,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); - s32 pre_div = 0, post_div = 0, i, reg_ctrl, reg_config; + s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; @@ -154,7 +154,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, pre_div = DIV_ROUND_UP(clk_src, max_hz); if (pre_div 16) { post_div = pre_div / 16; - pre_div = 15; + pre_div = 16; } if (post_div != 0) { for (i = 0; i 16; i++) { @@ -174,7 +174,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_SELCHAN(3)) | MXC_CSPICTRL_SELCHAN(cs); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_PREDIV(0x0F)) | - MXC_CSPICTRL_PREDIV(pre_div); + MXC_CSPICTRL_PREDIV(pre_div - 1); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); -- 1.7.10.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/2] spi: mxc_spi: Update pre and post divider algorithm
The spi clock divisor is of the form x * (2**y), or x y, where x is 1 to 16, and y is 0 to 15. Note the similarity with floating point numbers. Convert the desired divisor to the smallest number which is = desired divisor, and can be represented in this form. The previous algorithm chose a divisor which could be almost twice as large as needed. Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com Signed-off-by: Dirk Behme dirk.be...@gmail.com --- drivers/spi/mxc_spi.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 3e903b3..66c2ad8 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,7 +128,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); - s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; + s32 pre_div = 1, post_div = 0, reg_ctrl, reg_config; u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; @@ -147,27 +147,24 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl |= MXC_CSPICTRL_EN; reg_write(regs-ctrl, reg_ctrl); - /* -* The following computation is taken directly from Freescale's code. -*/ if (clk_src max_hz) { pre_div = DIV_ROUND_UP(clk_src, max_hz); - if (pre_div 16) { - post_div = pre_div / 16; - pre_div = 16; - } - if (post_div != 0) { - for (i = 0; i 16; i++) { - if ((1 i) = post_div) - break; - } - if (i == 16) { + /* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ + post_div = fls(pre_div - 1); + if (post_div 4) { + post_div -= 4; + + if (post_div = 16) { printf(Error: no divider for the freq: %d\n, max_hz); return -1; } - post_div = i; + pre_div = (pre_div + (1 post_div) - 1) post_div; + + } else { + post_div = 0; } + } debug(pre_div = %d, post_div=%d\n, pre_div, post_div); -- 1.7.10.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] spi: mxc_spi: Fix pre and post divider calculation
Am 03.05.2013 22:47, schrieb Troy Kisky: On 5/2/2013 10:58 PM, Dirk Behme wrote: Do you want to say you propose post_div = pre_div / 16; pre_div = 16; ? yes, that's what I said If so: First, I agree that we have to use the same dividers in both lines. But, second, this would mean that you use /16 as max pre_div. For the i.MX6 case where clk_src is 60MHz this would result in a pre-divided clock of 3.75Mhz (instead of 4MHz with /15). That does sound better for i.MX6, what about other processors using this file? So using /15 or /16 is just a decision of which end clocks most probably are needed. If you want to be able to configure 4MHz, 2MHz, 1MHz, 500kHz etc then /15 is the better choice. If you want to be able to configure 3.75Mhz, 1.875MHz, 937.5kHz, 468.75kHz etc then /16 is the better choice. I vote for /15 as done by my patch. Thanks for explaining. The downside of using /15 is that you can't get the slowest clock possible. Yes. I was looking for the _highest_ clock possible, though ;) And this isn't correctly done by the recent code. This is why I was looking into it ... How about restructuring the code to improve both. Calculate post_div first. pre_div = DIV_ROUND_UP(clk_src, max_hz); /* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ post_div = fls(pre_div - 1); if (post_div 4) post_div -= 4; else post_div = 0; if (post_div = 16) { printf(Error: no divider for the freq: %d\n, max_hz); return -1; } pre_div = (pre_div + (1 post_div) - 1) post_div; Using my test code gives the correct values using this algorithm. So yes, sounds good. Just a small note: Wouldn't it be better to put the printf and the last line with the pre_div calculation into the if(post_div 4) part? I.e. pre_div = DIV_ROUND_UP(clk_src, max_hz); /* fls(1) = 1, fls(0x8000) = 32, fls(16) = 5 */ post_div = fls(pre_div - 1); if (post_div 4) { post_div -= 4; if (post_div = 16) { printf(Error: no divider for the freq: %d\n, max_hz); return -1; } pre_div = (pre_div + (1 post_div) - 1) post_div; } else post_div = 0; ? In case we agree on this, I'm thinking about doing 2 patches to make clear what we are doing: 1. Re-doing my initial patch with post_div = pre_div / 16; pre_div = 16; This would be the fix the issues in the existing (non-optimal) algorithm but keep that patch. 2. Replace the existing algorithm with your above version. This would be the improve the algorithm patch. What do you think? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] spi: mxc_spi: Fix pre and post divider calculation
From: Dirk Behme dirk.be...@gmail.com Fix two issues with the calculation of pre_div and post_div: 1. pre_div: While the calculation of pre_div looks correct, to set the CONREG[15-12] bits pre_div needs to be decremented by 1: The i.MX 6Dual/6Quad Applications Processor Reference Manual (IMX6DQRM Rev. 0, 11/2012) states: CONREG[15-12]: PRE_DIVIDER Divide by 1 0001 Divide by 2 0010 Divide by 3 ... 1101 Divide by 14 1110 Divide by 15 Divide by 16 I.e. if we want to divide by 2, we have to write 1 to CONREG[15-12]. 2. In case the post divider becomes necessary, pre_div will be set to 15. To calculate the post divider, divide by 15, too. And not 16. Both issues above are tested using the following examples: clk_src = 6000 (60MHz, default i.MX6 ECSPI clock) a) max_hz == 2300 (23MHz, max i.MX6 ECSPI read clock) - pre_div = 3 (divide by 3 = CONREG[15-12] == 2) - post_div = 0 (divide by 1 = CONREG[11- 8] == 0) = 60MHz / 3 = 20MHz SPI clock b) max_hz == 200 (2MHz) - pre_div = 15 (divide by 15 = CONREG[15-12] == 14) - post_div = 1 (divide by 2 = CONREG[11- 8] == 1) = 60MHz / 30 = 2MHz SPI clock c) max_hz == 100 (1MHz) - pre_div = 15 (divide by 15 = CONREG[15-12] == 14) - post_div = 2 (divide by 4 = CONREG[11- 8] == 2) = 60MHz / 60 = 1MHz SPI clock d) max_hz == 50 (500kHz) - pre_div = 15 (divide by 15 = CONREG[15-12] == 14) - post_div = 3 (divide by 8 = CONREG[11- 8] == 3) = 60MHz / 120 = 500kHz SPI clock Signed-off-by: Dirk Behme dirk.be...@gmail.com --- drivers/spi/mxc_spi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 5bed858..8630bbd 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,7 +128,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); - s32 pre_div = 0, post_div = 0, i, reg_ctrl, reg_config; + s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; @@ -153,7 +153,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, if (clk_src max_hz) { pre_div = DIV_ROUND_UP(clk_src, max_hz); if (pre_div 16) { - post_div = pre_div / 16; + post_div = pre_div / 15; pre_div = 15; } if (post_div != 0) { @@ -174,7 +174,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_SELCHAN(3)) | MXC_CSPICTRL_SELCHAN(cs); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_PREDIV(0x0F)) | - MXC_CSPICTRL_PREDIV(pre_div); + MXC_CSPICTRL_PREDIV(pre_div - 1); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); -- 1.8.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] spi: mxc_spi: Fix pre and post divider calculation
On 02.05.2013 20:38, Troy Kisky wrote: On 5/2/2013 3:59 AM, Dirk Behme wrote: From: Dirk Behme dirk.be...@gmail.com Fix two issues with the calculation of pre_div and post_div: 1. pre_div: While the calculation of pre_div looks correct, to set the CONREG[15-12] bits pre_div needs to be decremented by 1: The i.MX 6Dual/6Quad Applications Processor Reference Manual (IMX6DQRM Rev. 0, 11/2012) states: CONREG[15-12]: PRE_DIVIDER Divide by 1 0001 Divide by 2 0010 Divide by 3 ... 1101 Divide by 14 1110 Divide by 15 Divide by 16 I.e. if we want to divide by 2, we have to write 1 to CONREG[15-12]. 2. In case the post divider becomes necessary, pre_div will be set to 15. To calculate the post divider, divide by 15, too. And not 16. Both issues above are tested using the following examples: clk_src = 6000 (60MHz, default i.MX6 ECSPI clock) a) max_hz == 2300 (23MHz, max i.MX6 ECSPI read clock) - pre_div = 3 (divide by 3 = CONREG[15-12] == 2) - post_div = 0 (divide by 1 = CONREG[11- 8] == 0) = 60MHz / 3 = 20MHz SPI clock b) max_hz == 200 (2MHz) - pre_div = 15 (divide by 15 = CONREG[15-12] == 14) - post_div = 1 (divide by 2 = CONREG[11- 8] == 1) = 60MHz / 30 = 2MHz SPI clock c) max_hz == 100 (1MHz) - pre_div = 15 (divide by 15 = CONREG[15-12] == 14) - post_div = 2 (divide by 4 = CONREG[11- 8] == 2) = 60MHz / 60 = 1MHz SPI clock d) max_hz == 50 (500kHz) - pre_div = 15 (divide by 15 = CONREG[15-12] == 14) - post_div = 3 (divide by 8 = CONREG[11- 8] == 3) = 60MHz / 120 = 500kHz SPI clock Signed-off-by: Dirk Behme dirk.be...@gmail.com --- drivers/spi/mxc_spi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 5bed858..8630bbd 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -128,7 +128,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, unsigned int max_hz, unsigned int mode) { u32 clk_src = mxc_get_clock(MXC_CSPI_CLK); - s32 pre_div = 0, post_div = 0, i, reg_ctrl, reg_config; + s32 pre_div = 1, post_div = 0, i, reg_ctrl, reg_config; u32 ss_pol = 0, sclkpol = 0, sclkpha = 0; struct cspi_regs *regs = (struct cspi_regs *)mxcs-base; @@ -153,7 +153,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, if (clk_src max_hz) { pre_div = DIV_ROUND_UP(clk_src, max_hz); if (pre_div 16) { - post_div = pre_div / 16; + post_div = pre_div / 15; I think this should not change Hmm, why? You get wrong post_div dividers if you run with 'pre_div = 15' and '/16'. Hmm, reading your below comment do you want to say I agree that both lines post_div = pre_div / 16; pre_div = 15; have to use the same value, but instead of using 15, use 16 ? pre_div = 15; and this should be = 16, because you now subtract 1 below Do you want to say you propose post_div = pre_div / 16; pre_div = 16; ? If so: First, I agree that we have to use the same dividers in both lines. But, second, this would mean that you use /16 as max pre_div. For the i.MX6 case where clk_src is 60MHz this would result in a pre-divided clock of 3.75Mhz (instead of 4MHz with /15). So using /15 or /16 is just a decision of which end clocks most probably are needed. If you want to be able to configure 4MHz, 2MHz, 1MHz, 500kHz etc then /15 is the better choice. If you want to be able to configure 3.75Mhz, 1.875MHz, 937.5kHz, 468.75kHz etc then /16 is the better choice. I vote for /15 as done by my patch. Best regards Dirk } if (post_div != 0) { @@ -174,7 +174,7 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_SELCHAN(3)) | MXC_CSPICTRL_SELCHAN(cs); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_PREDIV(0x0F)) | - MXC_CSPICTRL_PREDIV(pre_div); + MXC_CSPICTRL_PREDIV(pre_div - 1); reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot -- == Dirk Behme Robert Bosch Car Multimedia GmbH CM-AI/PJ-CF32 Phone: +49 5121 49-3274 Dirk Behme Fax: +49 711 811 5053274 PO Box 77 77 77 mailto:dirk.be...@de.bosch.com D-31132 Hildesheim - Germany Bosch Group, Car Multimedia (CM) Automotive Navigation and Infotainment Systems (AI) ProJect - CoreFunctions (PJ-CF) Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe Sitz: Hildesheim Registergericht
Re: [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels
Am 14.04.2013 09:08, schrieb Dirk Behme: Am 10.04.2013 01:06, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com The glitch in the SPI clock line, which commit 3cea335c34 (spi: mxc_spi: Fix spi clock glitch durant reset) solved, is back now and itwas re-introduced by commit d36b39bf0d (spi: mxc_spi: Fix ECSPI reset handling). Actually the glitch is happening due to always toggling between slave mode and master mode by configuring the CHANNEL_MODE bits in this reset function. Since the spi driver only supports master mode, set the mode for all channels always to master mode in order to have a stable, glitch-free SPI clock line. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v1: - Introduce MXC_CSPICTRL_MODE_MASK definition - Remove additional read of reg_ctrl arch/arm/include/asm/arch-mx5/imx-regs.h |1 + arch/arm/include/asm/arch-mx6/imx-regs.h |1 + drivers/spi/mxc_spi.c| 17 + 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h index 249d15a..a71cc13 100644 --- a/arch/arm/include/asm/arch-mx5/imx-regs.h +++ b/arch/arm/include/asm/arch-mx5/imx-regs.h @@ -230,6 +230,7 @@ #define MXC_CSPICTRL_EN(1 0) #define MXC_CSPICTRL_MODE(1 1) #define MXC_CSPICTRL_XCH(1 2) +#define MXC_CSPICTRL_MODE_MASK(0xf 4) #define MXC_CSPICTRL_CHIPSELECT(x)(((x) 0x3) 12) #define MXC_CSPICTRL_BITCOUNT(x)(((x) 0xfff) 20) #define MXC_CSPICTRL_PREDIV(x)(((x) 0xF) 12) diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index eaa7439..d79ab2f 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -346,6 +346,7 @@ struct cspi_regs { #define MXC_CSPICTRL_EN(1 0) #define MXC_CSPICTRL_MODE(1 1) #define MXC_CSPICTRL_XCH(1 2) +#define MXC_CSPICTRL_MODE_MASK (0xf 4) #define MXC_CSPICTRL_CHIPSELECT(x)(((x) 0x3) 12) #define MXC_CSPICTRL_BITCOUNT(x)(((x) 0xfff) 20) #define MXC_CSPICTRL_PREDIV(x)(((x) 0xF) 12) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 4c19e0b..20419e6 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -137,11 +137,15 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, return -1; } -/* Reset spi */ -reg_write(regs-ctrl, 0); -reg_write(regs-ctrl, MXC_CSPICTRL_EN); - -reg_ctrl = reg_read(regs-ctrl); +/* + * Reset SPI and set all CSs to master mode, if toggling + * between slave and master mode we might see a glitch + * on the clock line + */ +reg_ctrl = MXC_CSPICTRL_MODE_MASK; I was offline some days, giving me some time to think about this ;) Most probably it does no harm, but I somehow feel uncomfortable with setting *all* CSs to master mode. Just because there might be already (one, several?) CS at master mode and switching it back to the (reset default) slave will give a glitch on the clock line. Wouldn't it be cleaner to keep the master mode only for the CS which are already in master mode before? E.g. instead of reg_ctrl = MXC_CSPICTRL_MODE_MASK; from above something like reg_ctrl = reg_read(regs-ctrl) MXC_CSPICTRL_MODE_MASK; ? And then ... +reg_write(regs-ctrl, reg_ctrl); +reg_ctrl |= MXC_CSPICTRL_EN; +reg_write(regs-ctrl, reg_ctrl); /* * The following computation is taken directly from Freescale's code. @@ -174,9 +178,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); -/* always set to master mode */ -reg_ctrl |= 1 (cs + 4); ... keeping thins line? What's about anything like below? Best regards Dirk From: Dirk Behme dirk.be...@gmail.com Date: Wed, 1 May 2013 07:28:38 +0200 Subject: [PATCH] spi: mxc_spi: Keep master mode only for configured channels To avoid a glitch on the clock line while resetting the SPI controller, the commit 0f1411bc8d (spi: mxc_spi: Set master mode for all channels) enables the master mode for all channels. Instead of enabling the master mode for all channels, keep only the channels which are already configured in master mode in this mode. To be able to switch additional channels to master mode, re-introduce the master mode enable which was removed by above commit, then. Signed-off-by: Dirk Behme dirk.be...@gmail.com --- drivers/spi/mxc_spi.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 5bed858..843a1f2 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -138,11 +138,11 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, } /* -* Reset SPI and set all CSs to master mode
Re: [U-Boot] [PATCH v2] spi: mxc_spi: Set master mode for all channels
Am 10.04.2013 01:06, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com The glitch in the SPI clock line, which commit 3cea335c34 (spi: mxc_spi: Fix spi clock glitch durant reset) solved, is back now and itwas re-introduced by commit d36b39bf0d (spi: mxc_spi: Fix ECSPI reset handling). Actually the glitch is happening due to always toggling between slave mode and master mode by configuring the CHANNEL_MODE bits in this reset function. Since the spi driver only supports master mode, set the mode for all channels always to master mode in order to have a stable, glitch-free SPI clock line. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v1: - Introduce MXC_CSPICTRL_MODE_MASK definition - Remove additional read of reg_ctrl arch/arm/include/asm/arch-mx5/imx-regs.h |1 + arch/arm/include/asm/arch-mx6/imx-regs.h |1 + drivers/spi/mxc_spi.c| 17 + 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h index 249d15a..a71cc13 100644 --- a/arch/arm/include/asm/arch-mx5/imx-regs.h +++ b/arch/arm/include/asm/arch-mx5/imx-regs.h @@ -230,6 +230,7 @@ #define MXC_CSPICTRL_EN (1 0) #define MXC_CSPICTRL_MODE (1 1) #define MXC_CSPICTRL_XCH (1 2) +#define MXC_CSPICTRL_MODE_MASK (0xf 4) #define MXC_CSPICTRL_CHIPSELECT(x)(((x) 0x3) 12) #define MXC_CSPICTRL_BITCOUNT(x) (((x) 0xfff) 20) #define MXC_CSPICTRL_PREDIV(x)(((x) 0xF) 12) diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h index eaa7439..d79ab2f 100644 --- a/arch/arm/include/asm/arch-mx6/imx-regs.h +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h @@ -346,6 +346,7 @@ struct cspi_regs { #define MXC_CSPICTRL_EN (1 0) #define MXC_CSPICTRL_MODE (1 1) #define MXC_CSPICTRL_XCH (1 2) +#define MXC_CSPICTRL_MODE_MASK (0xf 4) #define MXC_CSPICTRL_CHIPSELECT(x)(((x) 0x3) 12) #define MXC_CSPICTRL_BITCOUNT(x) (((x) 0xfff) 20) #define MXC_CSPICTRL_PREDIV(x)(((x) 0xF) 12) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 4c19e0b..20419e6 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -137,11 +137,15 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, return -1; } - /* Reset spi */ - reg_write(regs-ctrl, 0); - reg_write(regs-ctrl, MXC_CSPICTRL_EN); - - reg_ctrl = reg_read(regs-ctrl); + /* +* Reset SPI and set all CSs to master mode, if toggling +* between slave and master mode we might see a glitch +* on the clock line +*/ + reg_ctrl = MXC_CSPICTRL_MODE_MASK; I was offline some days, giving me some time to think about this ;) Most probably it does no harm, but I somehow feel uncomfortable with setting *all* CSs to master mode. Just because there might be already (one, several?) CS at master mode and switching it back to the (reset default) slave will give a glitch on the clock line. Wouldn't it be cleaner to keep the master mode only for the CS which are already in master mode before? E.g. instead of reg_ctrl = MXC_CSPICTRL_MODE_MASK; from above something like reg_ctrl = reg_read(regs-ctrl) MXC_CSPICTRL_MODE_MASK; ? And then ... + reg_write(regs-ctrl, reg_ctrl); + reg_ctrl |= MXC_CSPICTRL_EN; + reg_write(regs-ctrl, reg_ctrl); /* * The following computation is taken directly from Freescale's code. @@ -174,9 +178,6 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_POSTDIV(0x0F)) | MXC_CSPICTRL_POSTDIV(post_div); - /* always set to master mode */ - reg_ctrl |= 1 (cs + 4); ... keeping thins line? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
Am 08.04.2013 11:06, schrieb Andrew Gabbasov: Rework the waiting for transfer completion loop condition to continue waiting until both Transfer Complete and DMA End interrupts occur. Checking of DLA bit in Present State register looks not needed in addition to interrupts status checking, so it can be removed from the condition. Also, DMA Error condition is added to the list of data errors, checked in the loop. Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com --- drivers/mmc/fsl_esdhc.c |3 +-- include/fsl_esdhc.h |4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 54b5363..814bba4 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -400,8 +400,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) if (irqstat DATA_ERR) return COMM_ERR; - } while (!(irqstat IRQSTAT_TC) - (esdhc_read32(regs-prsstat) PRSSTAT_DLA)); + } while ((irqstat DATA_COMPLETE) != DATA_COMPLETE); #endif } diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h index 47d2fe4..ea0880b 100644 --- a/include/fsl_esdhc.h +++ b/include/fsl_esdhc.h @@ -63,7 +63,9 @@ #define IRQSTAT_CC(0x0001) #define CMD_ERR (IRQSTAT_CIE | IRQSTAT_CEBE | IRQSTAT_CCE) -#define DATA_ERR (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE) +#define DATA_ERR (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE | \ + IRQSTAT_DMAE) +#define DATA_COMPLETE (IRQSTAT_TC | IRQSTAT_DINT) #define IRQSTATEN 0x0002e034 #define IRQSTATEN_DMAE(0x1000) I can't say anything to the content of the patches ;) But are this patch and the patch from Eric http://patchwork.ozlabs.org/patch/233595/ fine and should be applied together, now? Thanks Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] mx6sl: Add initial support for mx6slevk board
Am 06.04.2013 02:55, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com mx6slevk board is a development board from Freescale based on the mx6 solo-lite processor. For details about mx6slevk, please refer to: http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=IMX6SLEVKparentCode=i.MX6SLfpsp=1 Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- diff --git a/board/freescale/mx6slevk/imximage.cfg b/board/freescale/mx6slevk/imximage.cfg new file mode 100644 index 000..df39a16 --- /dev/null +++ b/board/freescale/mx6slevk/imximage.cfg @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License or (at your option) any later version. + * + * Refer docs/README.imxmage for more details about how-to configure + * and create imximage boot image + * + * The syntax is taken as close as possible with the kwbimage + */ + +/* image version */ + +IMAGE_VERSION 2 + +/* + * Boot Device : one of + * spi, sd (the board has no nand neither onenand) + */ + +BOOT_FROM sd + +/* + * Device Configuration Data (DCD) + * + * Each entry must have the format: + * Addr-type AddressValue + * + * where: + * Addr-type register length (1,2 or 4 bytes) + * Address absolute address of the register + * value value to be stored in the register + */ +DATA 4 0x020c4018 0x00260324 + +DATA 4 0x020c4068 0x +DATA 4 0x020c406c 0x +DATA 4 0x020c4070 0x +DATA 4 0x020c4074 0x +DATA 4 0x020c4078 0x +DATA 4 0x020c407c 0x +DATA 4 0x020c4080 0x Again this default stuff? ;) Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] spi: mxc_spi: Set master mode for all channels
Am 06.04.2013 15:15, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com The glitch in the SPI clock line, which commit 3cea335c34 (spi: mxc_spi: Fix spi clock glitch durant reset) solved, is back now and itwas re-introduced by commit d36b39bf0d (spi: mxc_spi: Fix ECSPI reset handling). Actually the glitch is happening due to always toggling between slave mode and master mode by configuring the CHANNEL_MODE bits in this reset function. Since the spi driver only supports master mode, set the mode for all channels always to master mode in order to have a stable, glitch-free SPI clock line. Ok, thanks, I wasn't aware of this. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- drivers/spi/mxc_spi.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index 4c19e0b..9eb2bce 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -137,9 +137,14 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, return -1; } - /* Reset spi */ - reg_write(regs-ctrl, 0); - reg_write(regs-ctrl, MXC_CSPICTRL_EN); + /* +* Reset SPI and set all CSs to master mode, I haven't checked the rest of the driver, but do we touch the CSs bits anywhere else, too? Could that parts dropped with this patch, then. I.e. never touch the bits regs-ctrl[7-4] again? if toggling +* between slave and master mode we might see a glitch +* on the clock line +*/ + reg_write(regs-ctrl, 0x00F0); Do we have a macro for the 0xF? + reg_ctrl = reg_read(regs-ctrl); Do we expect reg_ctrl to be 0x00F0 then? If yes, do we have to really read it here? If we read it here, should we drop the read below, then? + reg_write(regs-ctrl, reg_ctrl | MXC_CSPICTRL_EN); reg_ctrl = reg_read(regs-ctrl); So, as mentioned by Stefano before, I'd propose to either a) do the reg_read(regs-ctrl); only _once_. E.g. /* * Reset SPI and set all CSs to master mode, if toggling * between slave and master mode we might see a glitch * on the clock line */ reg_write(regs-ctrl, 0x00F0); /* FIXME: use a macro here*/ reg_ctrl = reg_read(regs-ctrl) | MXC_CSPICTRL_EN; /* Reading back the register content is done because we don't get back the value written above */ reg_write(regs-ctrl, reg_ctrl); = no additional read of reg_ctrl needed here as reg_ctrl contains already the recent register content = or b) _never_ read back the register. E.g. /* * Reset SPI and set all CSs to master mode, if toggling * between slave and master mode we might see a glitch * on the clock line */ reg_ctrl = 0x00F0; /* FIXME: use a macro here*/ reg_write(regs-ctrl, reg_ctrl); reg_ctrl |= MXC_CSPICTRL_EN; reg_write(regs-ctrl, reg_ctrl); = no additional read of reg_ctrl needed here as reg_ctrl contains already the recent register content = Would one of these options work? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] mx6sl: Add initial support for mx6slevk board
Am 06.04.2013 14:47, schrieb Fabio Estevam: Hi Dirk, On Sat, Apr 6, 2013 at 4:27 AM, Dirk Behme dirk.be...@gmail.com wrote: +DATA 4 0x020c4068 0x +DATA 4 0x020c406c 0x +DATA 4 0x020c4070 0x +DATA 4 0x020c4074 0x +DATA 4 0x020c4078 0x +DATA 4 0x020c407c 0x +DATA 4 0x020c4080 0x Again this default stuff? ;) I should have replied to your previous comment. I tried removing this block as you suggest and the board does not boot. Then I read the following from the FSL U-boot: /* Enable all clocks (they are disabled by ROM code)*/ That's why I am keeping them. Most probably it would be sufficient to enable only the clocks needed for booting ;) And not all clocks. On the other boards we do /* set the default clock gate to save power */ DATA 4 0x020c4068 0x00C03F3F DATA 4 0x020c406c 0x0030FC03 DATA 4 0x020c4070 0x0FFFC000 DATA 4 0x020c4074 0x3FF0 DATA 4 0x020c4078 0x00FFF300 DATA 4 0x020c407c 0x0FC3 DATA 4 0x020c4080 0x03FF http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg;h=f4cae5eeb9899ab4ba937ae286e004d6861f1d43;hb=refs/heads/master#l161 Do you like to try anything similar here, too? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] mx6sl: Add initial support for mx6slevk board
Am 06.04.2013 16:15, schrieb Fabio Estevam: On Sat, Apr 6, 2013 at 10:52 AM, Dirk Behme dirk.be...@gmail.com wrote: Most probably it would be sufficient to enable only the clocks needed for booting ;) And not all clocks. On the other boards we do /* set the default clock gate to save power */ DATA 4 0x020c4068 0x00C03F3F DATA 4 0x020c406c 0x0030FC03 DATA 4 0x020c4070 0x0FFFC000 DATA 4 0x020c4074 0x3FF0 DATA 4 0x020c4078 0x00FFF300 DATA 4 0x020c407c 0x0FC3 DATA 4 0x020c4080 0x03FF http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg;h=f4cae5eeb9899ab4ba937ae286e004d6861f1d43;hb=refs/heads/master#l161 Do you like to try anything similar here, too? No, sorry. It is possible to turn some clocks off, for sure, but I do not see real benefit by doing this on this solo-lite board. U-boot runs only for few seconds (or ms), so not much of power savings we can do in the bootloader. To my understanding what we do above is not about saving power in the bootloader ... I am just setting the clocks to the default states, so it is up to the kernel to manage the clocks as needed. ... but saving power over the whole (kernel) runtime. I might be wrong, but to my understanding the kernel doesn't *disable* unneeded clocks? So it's up to the bootloader to enable only the peripherals (clocks) really needed for booting. And then it's up to the kernel's driver init functions to enable the needed clocks for the subsystem once it's needed. But as I won't use this board I won't care about the power consumption here. If you think it's fine for the use cases of this board to enable all clocks by default, then this shouldn't stop applying this patch. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/3] mx6sl: Add initial support for mx6slevk board
Am 06.04.2013 18:40, schrieb Fabio Estevam: Hi Dirk, On Sat, Apr 6, 2013 at 1:30 PM, Dirk Behme dirk.be...@gmail.com wrote: ... but saving power over the whole (kernel) runtime. I might be wrong, but to my understanding the kernel doesn't *disable* unneeded clocks? So it's up to the bootloader to enable only the peripherals (clocks) really needed for booting. And then it's up to the kernel's driver init functions to enable the needed clocks for the subsystem once it's needed. But as I won't use this board I won't care about the power consumption here. If you think it's fine for the use cases of this board to enable all clocks by default, then this shouldn't stop applying this patch. Ok, understood. I will go through each one of the CCM registers and check what clocks can be turned off. This may take some time though as it needs proper testing. I suggest that we keep the current clock settings as is, so that the board support can reach U-boot, then later I can submit a patch for turning off the unneeded clocks. Sounds like a good plan :) Thanks Dirk P.S.: I would think that the clock settings of the other boards should be a good starting point to minimize the work. At least if the SoloLite clocks are not completely different ;) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mmc: mx6qsabrelite: fsl_esdhc: Define maximum bus width supported by SabreLite board
On 03.04.2013 11:06, Stefano Babic wrote: On 21/03/2013 12:00, Abbas Raza wrote: From: Abbas Raza abbas_r...@mentor.com Maximum bus width supported by SabreLite board is not 8bit like all other mx6q specific boards. In case where both host controller and card support 8bit transfers, they agree to communicate on 8bit interface while boards like the SabreLite support only 4bit interface. Due to this reason the mmc 8bit default mode fails on the SabreLite. To rectify this, define maximum bus width supported by this board (4bit). If max_bus_width is not defined, it is 0 by default and 8bit width support will be enabled in host capabilities otherwise host capabilities are modified accordingly. It is tested with a MMCplus card. Signed-off-by: Abbas Raza abbas_r...@mentor.com cc: stefano Babic sba...@denx.de cc: Andy Fleming aflem...@gmail.com Acked-by: Dirk Behme dirk.be...@de.bosch.com Acked-by: Andrew Gabbasov andrew_gabba...@mentor.com --- Applied to u-boot-imx, thanks. FYI: of course, I will merge the same fix for max_bus_width for Nitrogen6x and Wandboard if posted. Have you noticed v2 of this patch http://lists.denx.de/pipermail/u-boot/2013-March/150119.html ? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] spi: mxc_spi: Fix ECSPI reset handling
Am 03.04.2013 11:12, schrieb Stefano Babic: On 21/03/2013 09:03, Dirk Behme wrote: Reviewing the ECSPI reset handling shows two issues: Hi Dirk, agree completely, only a very minor question.. + + reg_ctrl = reg_read(regs-ctrl); As you says, it makes no sense to read back the value of the register, also because reg_ctrl is overwritten some lines later ;-) Hmm, sorry if I overlooked something, but we have to initialize the variable 'reg_ctrl' with the recent register content because it is first used and _then_ overwritten in the next step: reg_ctrl = (reg_ctrl ~MXC_CSPICTRL_SELCHAN(3)) | MXC_CSPICTRL_SELCHAN(cs); http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=drivers/spi/mxc_spi.c;h=d792d8d493c13c475ec8ca03694f4efd8fde0e7f;hb=HEAD#l170 (?) Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: fsl_esdhc: Fix waiting for DMA operation completion
Am 02.04.2013 17:49, schrieb Eric Nelson: Thanks Andrew, On 04/02/2013 03:04 AM, Andrew Gabbasov wrote: On iMX6 sometimes the Transfer Complete interrupt occurs earlier than the DMA part completes its operation. If immediately after that the read data is used for some data verification, those obtained data may be incomplete, which causes intermittent verification failures. Can you describe how to repeat this? For example, when the default environment command tries to load and run boot script from FAT partition on SD/MMC card, it sometimes fails, reporting invalid partition table, or unknown partition type, or something else of that kind. Such errors disappear if the build configuration has CONFIG_SYS_FSL_ESDHC_USE_PIO, or if some delay is added after transfer completion. We do this on every boot on SABRE Lite and Nitrogen6x boards, and haven't seen an issue. What board are you testing on? Do you have cache enabled? Is this with an SD card or eMMC? Andrew will have the details, but to my understanding this implements in U-Boot what the Freescale kernel has in http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/mmc/host/sdhci-esdhc-imx.c?h=imx_3.0.35_12.09.01#n234 for TO 1.0. Best regards Dirk Adding extra waiting for DMA completion after Transfer Complete event fixes this issue. Signed-off-by: Andrew Gabbasov andrew_gabba...@mentor.com --- drivers/mmc/fsl_esdhc.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index d2a505e..806c6dd 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -402,6 +402,12 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) return COMM_ERR; } while (!(irqstat IRQSTAT_TC) (esdhc_read32(regs-prsstat) PRSSTAT_DLA)); +#ifdef CONFIG_MX6 +/* In imx6 TC (data end) interrupt sometimes occur earlier + than DMA completes. In this case just wait a little more. */ +while (!(irqstat (IRQSTAT_DINT | IRQSTAT_DMAE))) +irqstat = esdhc_read32(regs-irqstat); +#endif #endif } ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add initial support for mx6slevk board
Hi Fabio, Am 30.03.2013 22:08, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com mx6slevk board is a development board from Freescale based on the mx6 solo lite processor. For details about mx6slevk, please refer to: http://www.freescale.com/webapp/sps/site/prod_summary.jsp?code=IMX6SLEVKparentCode=i.MX6SLfpsp=1 Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- MAINTAINERS|1 + arch/arm/cpu/armv7/mx6/clock.c | 38 +- arch/arm/include/asm/arch-mx6/crm_regs.h |1 + arch/arm/include/asm/arch-mx6/imx-regs.h | 63 +- arch/arm/include/asm/arch-mx6/mx6-pins.h |4 + arch/arm/include/asm/arch-mx6/mx6sl_pins.h | 1374 Would it make sense to split this patch into two parts? One general add i.MX6 SoloLite support and then the add mx6slevk board board support? board/freescale/mx6slevk/Makefile | 28 + board/freescale/mx6slevk/imximage.cfg | 118 +++ board/freescale/mx6slevk/mx6slevk.c| 102 +++ boards.cfg |1 + include/configs/mx6slevk.h | 189 11 files changed, 1916 insertions(+), 3 deletions(-) create mode 100644 arch/arm/include/asm/arch-mx6/mx6sl_pins.h create mode 100644 board/freescale/mx6slevk/Makefile create mode 100644 board/freescale/mx6slevk/imximage.cfg create mode 100644 board/freescale/mx6slevk/mx6slevk.c create mode 100644 include/configs/mx6slevk.h ... +# diff --git a/board/freescale/mx6slevk/imximage.cfg b/board/freescale/mx6slevk/imximage.cfg new file mode 100644 index 000..df39a16 --- /dev/null +++ b/board/freescale/mx6slevk/imximage.cfg @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License or (at your option) any later version. + * + * Refer docs/README.imxmage for more details about how-to configure + * and create imximage boot image + * + * The syntax is taken as close as possible with the kwbimage + */ + +/* image version */ + +IMAGE_VERSION 2 + +/* + * Boot Device : one of + * spi, sd (the board has no nand neither onenand) + */ + +BOOT_FROM sd + +/* + * Device Configuration Data (DCD) + * + * Each entry must have the format: + * Addr-type AddressValue + * + * where: + * Addr-type register length (1,2 or 4 bytes) + * Address absolute address of the register + * value value to be stored in the register + */ +DATA 4 0x020c4018 0x00260324 + +DATA 4 0x020c4068 0x +DATA 4 0x020c406c 0x +DATA 4 0x020c4070 0x +DATA 4 0x020c4074 0x +DATA 4 0x020c4078 0x +DATA 4 0x020c407c 0x +DATA 4 0x020c4080 0x I haven't looked into the manual, but are these the clock registers? If so, is it necessary to set them to 0x? Or are these already the default reset values? ... diff --git a/include/configs/mx6slevk.h b/include/configs/mx6slevk.h new file mode 100644 index 000..67b63f4 --- /dev/null +++ b/include/configs/mx6slevk.h @@ -0,0 +1,189 @@ +/* + * Copyright 2013 Freescale Semiconductor, Inc. + * + * Configuration settings for the Freescale i.MX6SL EVK board. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + */ + +#ifndef __CONFIG_H +#define __CONFIG_H + +#include asm/arch/imx-regs.h +#include asm/sizes.h + +#define CONFIG_MX6 +#define CONFIG_DISPLAY_CPUINFO +#define CONFIG_DISPLAY_BOARDINFO + +#define MACH_TYPE_MX6SLEVK 4307 Below, you seem to boot with device tree. Is this necessary, then? +#define CONFIG_MACH_TYPE MACH_TYPE_MX6SLEVK + +#define CONFIG_CMDLINE_TAG +#define CONFIG_SETUP_MEMORY_TAGS +#define CONFIG_INITRD_TAG +#define CONFIG_REVISION_TAG + +/* Size of malloc() pool */ +#define CONFIG_SYS_MALLOC_LEN (3 * SZ_1M) + +#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_MXC_GPIO + +#define CONFIG_MXC_UART +#define CONFIG_MXC_UART_BASE UART1_IPS_BASE_ADDR + +/* MMC Configs */ +#define CONFIG_FSL_ESDHC +#define CONFIG_FSL_USDHC +#define CONFIG_SYS_FSL_ESDHC_ADDR 0 + +#define CONFIG_MMC +#define CONFIG_CMD_MMC +#define CONFIG_GENERIC_MMC +#define CONFIG_CMD_FAT +#define CONFIG_DOS_PARTITION + +/* allow to overwrite serial and ethaddr */ +#define CONFIG_ENV_OVERWRITE +#define CONFIG_CONS_INDEX 1 +#define CONFIG_BAUDRATE115200 + +/* Command definition */ +#include config_cmd_default.h + +#undef CONFIG_CMD_IMLS + +#define CONFIG_BOOTDELAY
Re: [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
On 27.03.2013 18:36, Fabio Estevam wrote: When booting a Freescale kernel 3.0.35 on a Wandboard solo, the get_board_rev() returns 0x62xxx, which is not a value understood by the VPU (Video Processing Unit) library in the kernel and causes the video playback to fail. The expected values for get_board_rev are: 0x63xxx: For mx6quad/dual 0x61xxx: For mx6dual-lite/solo So adjust get_board_rev() accordingly and make it as weak function, so that we do not need to define it in every mx6 board file. Signed-off-by: Fabio Estevam fabio.este...@freescale.com Acked-by: Dirk Behme dirk.be...@de.bosch.com Thanks Dirk --- Changes since v1: - Avoid extra call to get_cpu_rev() arch/arm/cpu/armv7/mx6/soc.c | 12 board/boundary/nitrogen6x/nitrogen6x.c|5 - board/freescale/mx6qsabrelite/mx6qsabrelite.c |5 - board/freescale/mx6qsabresd/mx6qsabresd.c |5 - board/wandboard/wandboard.c |5 - 5 files changed, 12 insertions(+), 20 deletions(-) diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c index 193ba12..4f3cd14 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -61,6 +61,18 @@ u32 get_cpu_rev(void) return (type 12) | (reg + 0x10); } +#ifdef CONFIG_REVISION_TAG +u32 __weak get_board_rev(void) +{ + u32 cpurev = get_cpu_rev(); + u32 type = ((cpurev 12) 0xff); + if (type == MXC_CPU_MX6SOLO) + cpurev = (MXC_CPU_MX6DL) 12 | (cpurev 0xFFF); + + return cpurev; +} +#endif + void init_aips(void) { struct aipstz_regs *aips1, *aips2; diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c index 229c237..1b1bedf 100644 --- a/board/boundary/nitrogen6x/nitrogen6x.c +++ b/board/boundary/nitrogen6x/nitrogen6x.c @@ -328,11 +328,6 @@ int board_mmc_init(bd_t *bis) } #endif -u32 get_board_rev(void) -{ - return 0x63000; -} - #ifdef CONFIG_MXC_SPI iomux_v3_cfg_t const ecspi1_pads[] = { /* SS1 */ diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index 5b69a6d..782e5ba 100644 --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c @@ -298,11 +298,6 @@ int board_mmc_init(bd_t *bis) } #endif -u32 get_board_rev(void) -{ - return 0x63000 ; -} - #ifdef CONFIG_MXC_SPI iomux_v3_cfg_t const ecspi1_pads[] = { /* SS1 */ diff --git a/board/freescale/mx6qsabresd/mx6qsabresd.c b/board/freescale/mx6qsabresd/mx6qsabresd.c index 2b3926a..806769f 100644 --- a/board/freescale/mx6qsabresd/mx6qsabresd.c +++ b/board/freescale/mx6qsabresd/mx6qsabresd.c @@ -239,11 +239,6 @@ int board_eth_init(bd_t *bis) return 0; } -u32 get_board_rev(void) -{ - return 0x63000; -} - int board_early_init_f(void) { setup_iomux_uart(); diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c index d95189f..1e379fb 100644 --- a/board/wandboard/wandboard.c +++ b/board/wandboard/wandboard.c @@ -168,11 +168,6 @@ int board_init(void) return 0; } -u32 get_board_rev(void) -{ - return get_cpu_rev(); -} - int checkboard(void) { puts(Board: Wandboard\n); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: Fix the reading of CPU revision
On 26.03.2013 18:04, Fabio Estevam wrote: Hi Dirk, On Tue, Mar 26, 2013 at 12:43 PM, Dirk Behme dirk.be...@de.bosch.com wrote: Hi Fabio, On 26.03.2013 13:54, Fabio Estevam wrote: Currently when booting a mx6 solo processor get_cpu_rev() returns 0x62xxx, which is an invalid mx6 CPU revision. Do you have somewhere a list of valid CPU revisions? From two points of view: a) the i.MX6 hardware spec b) the VPU library Sorry, I don't. I am basing the CPU revision numbers from FSL U-boot: http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/board/freescale/common/fsl_sys_rev.c?h=imx_v2009.08_3.0.0 Adding Jason, in case he could clarify it. You remove Troy's code here introduced with http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=20332a066aff98f39419495821e14edd10b2a3f8 Troy's detection you remove here intentionally distinguishes between DualLite and Solo. You now re-introduce a common DL_S, again. Additionally, you completely seem to drop checking for scu-config. I've already seen some (broken?) i.MX6Solo where this check was essential. I can't talk about the problems when trying to use VPU library in the kernel (btw, which problems?) and the invalid 0x62xxx, but we used Troy's version of the detection successfully. Passing 0x62xxx as cpu_rev on a mx6solo caused the VPU issues described here: https://community.freescale.com/thread/305396 Which cpu_rev value is returned with your mx6solo? Are you able to use VPU lib? I'll check this. Rethinking about the issue here, my recent understanding is: a) We have a VPU library which only understands 0x63 (Quad) and 0x61 (DualLite/Solo) b) We have Troy's existing get_cpu_rev() [1] which seems to correctly decode the CPU revision (at least this is my impression from testing ;) ). But reports 0x62 for the Solo which then isn't understood by the VPU library (to be checked). I wonder if we could find a way to combine both parts without breaking the other? I.e. using Troy's get_cpu_rev() to correctly report the CPU revision (in U-Boot), but let the VPU library get the revision it understands? Best regards Dirk [1] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=20332a066aff98f39419495821e14edd10b2a3f8 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: Fix the reading of CPU revision
On 27.03.2013 09:02, Dirk Behme wrote: On 26.03.2013 18:04, Fabio Estevam wrote: Hi Dirk, On Tue, Mar 26, 2013 at 12:43 PM, Dirk Behme dirk.be...@de.bosch.com wrote: Hi Fabio, On 26.03.2013 13:54, Fabio Estevam wrote: Currently when booting a mx6 solo processor get_cpu_rev() returns 0x62xxx, which is an invalid mx6 CPU revision. Do you have somewhere a list of valid CPU revisions? From two points of view: a) the i.MX6 hardware spec b) the VPU library Sorry, I don't. I am basing the CPU revision numbers from FSL U-boot: http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/board/freescale/common/fsl_sys_rev.c?h=imx_v2009.08_3.0.0 Adding Jason, in case he could clarify it. You remove Troy's code here introduced with http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=20332a066aff98f39419495821e14edd10b2a3f8 Troy's detection you remove here intentionally distinguishes between DualLite and Solo. You now re-introduce a common DL_S, again. Additionally, you completely seem to drop checking for scu-config. I've already seen some (broken?) i.MX6Solo where this check was essential. I can't talk about the problems when trying to use VPU library in the kernel (btw, which problems?) and the invalid 0x62xxx, but we used Troy's version of the detection successfully. Passing 0x62xxx as cpu_rev on a mx6solo caused the VPU issues described here: https://community.freescale.com/thread/305396 Which cpu_rev value is returned with your mx6solo? Are you able to use VPU lib? I'll check this. Rethinking about the issue here, my recent understanding is: a) We have a VPU library which only understands 0x63 (Quad) and 0x61 (DualLite/Solo) b) We have Troy's existing get_cpu_rev() [1] which seems to correctly decode the CPU revision (at least this is my impression from testing ;) ). But reports 0x62 for the Solo which then isn't understood by the VPU library (to be checked). Some additional rethinking: I missed that we have a Linux kernel, too ;) c) It's the job of the Linux kernel to export the CPU revision to the VPU library. In case the Linux kernel completely ignores what we are doing in U-Boot and calculates the CPU revision itself (*), e.g. by something like http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx6/mm.c?h=imx_3.0.35_1.1.0id=207f80453c77bc32e04b5fef863f6fe50a7fd1a8#n60 we can do anything in U-Boot. Independent of the VPU library. In this case I'd propose to just keep Troy's version of get_cpu_rev() as it is [1]. Sorry for the confusion, hopefully this is correct now ;) Best regards Dirk (*) There might be U-Boot/Kernel combinations out there, where U-Boot exports the CPU revision via ATAGs to the kernel. But hopefully this doesn't affect us here (?) I wonder if we could find a way to combine both parts without breaking the other? I.e. using Troy's get_cpu_rev() to correctly report the CPU revision (in U-Boot), but let the VPU library get the revision it understands? Best regards Dirk [1] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=20332a066aff98f39419495821e14edd10b2a3f8 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: Fix the reading of CPU revision
On 27.03.2013 14:37, Fabio Estevam wrote: On Wed, Mar 27, 2013 at 5:57 AM, Dirk Behme dirk.be...@de.bosch.com wrote: Some additional rethinking: I missed that we have a Linux kernel, too ;) c) It's the job of the Linux kernel to export the CPU revision to the VPU library. In case the Linux kernel completely ignores what we are doing in U-Boot and calculates the CPU revision itself (*), e.g. by something like http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx6/mm.c?h=imx_3.0.35_1.1.0id=207f80453c77bc32e04b5fef863f6fe50a7fd1a8#n60 we can do anything in U-Boot. Independent of the VPU library. Unfortunately VPU library relies on the bootloader to pass the correct silicon revision. I don't think so, at least this depends on the kernel. To my understanding the VPU library relies on the kernel to pass the correct silicon revision. And where the kernel gets this information from seems to depend on the kernel version: a) get it from U-Boot via ATAGs b) calculate the value in the kernel independent of U-Boot About which kernel are you talking? To my understanding, using a recent kernel with device tree there is no revision information passed from the boot loader to the kernel? Instead of hacking U-Boot for these (old) kernels, I'd propose to fix the kernel to pass the 0x61/0x63 correctly to the VPU library. Or as Wolfgang mentioned, even better, fix the VPU library. I really like Troys existing code and see no reason to change it just due to the VPU library. Do the change in the kernel or the VPU library. Best regards Dirk Eric's tested passing 0 as get_cpu_rev and showed that VPU simply cannot work on this case. In this case I'd propose to just keep Troy's version of get_cpu_rev() as it is [1]. This is proven to not to work with mx6solo and VPU, so we need the fix I proposed. Here is what I am planning to do: 1. Send a v2 of this patch with the small correction pointed out by Eric 2. Include a weak function to pass get_cpu_rev in common mx6 code Then on top of that, one can send a patch that prints the mx6 silicon strings by differentiating between a mx6dual-lite and mx6solo, if it is worth. Regards, Fabio Estevam -- == Dirk Behme Robert Bosch Car Multimedia GmbH CM-AI/PJ-CF32 Phone: +49 5121 49-3274 Dirk Behme Fax: +49 711 811 5053274 PO Box 77 77 77 mailto:dirk.be...@de.bosch.com D-31132 Hildesheim - Germany Bosch Group, Car Multimedia (CM) Automotive Navigation and Infotainment Systems (AI) ProJect - CoreFunctions (PJ-CF) Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe Sitz: Hildesheim Registergericht: Amtsgericht Hildesheim HRB 201334 Aufsichtsratsvorsitzender: Volkmar Denner Geschäftsführung: Uwe Thomas, Michael Bolle, Robby Drave, Egbert Hellwig == ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: Fix the reading of CPU revision
Hi Eric, On 27.03.2013 15:00, Eric Nelson wrote: Hi Fabio, On 03/27/2013 06:37 AM, Fabio Estevam wrote: On Wed, Mar 27, 2013 at 5:57 AM, Dirk Behme dirk.be...@de.bosch.com wrote: Some additional rethinking: I missed that we have a Linux kernel, too ;) c) It's the job of the Linux kernel to export the CPU revision to the VPU library. In case the Linux kernel completely ignores what we are doing in U-Boot and calculates the CPU revision itself (*), e.g. by something like http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx6/mm.c?h=imx_3.0.35_1.1.0id=207f80453c77bc32e04b5fef863f6fe50a7fd1a8#n60 we can do anything in U-Boot. Independent of the VPU library. Unfortunately VPU library relies on the bootloader to pass the correct silicon revision. The VPU library relies on the output of /proc/cpuinfo (specifically the line beginning with Revision. The snippet (from vpu_io.h) is: tmp = strstr(buf, Revision); if (tmp != NULL) { rev = index(tmp, ':'); if (rev != NULL) { rev++; system_rev = strtoul(rev, NULL, 16); ret = 0; } } This code should really be changed, Yes :) so we don't have to carry this data all the way from boot loader to /proc/cpuinfo. As mentioned in my previous mail, I have some doubts that *all* kernels pick the version from the boot loader. It's my understanding that this strongly depends on the kernel? I.e. there are kernels which get the version from the boot loader, e.g. via ATAGs. But there are kernels which are independent from the boot loader and calculate it on their own? E.g. http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx6/mm.c?h=imx_3.0.35_1.1.0id=207f80453c77bc32e04b5fef863f6fe50a7fd1a8#n60 ? Best regards Dirk Similar (but different) code is present in mxc_ipu_hl_lib.c for the IPU. In the case of the VPU library, it seems more sane to have the VPU driver expose the particular IP revision present on the system. Eric's tested passing 0 as get_cpu_rev and showed that VPU simply cannot work on this case. In this case I'd propose to just keep Troy's version of get_cpu_rev() as it is [1]. This is proven to not to work with mx6solo and VPU, so we need the fix I proposed. Here is what I am planning to do: 1. Send a v2 of this patch with the small correction pointed out by Eric 2. Include a weak function to pass get_cpu_rev in common mx6 code Then on top of that, one can send a patch that prints the mx6 silicon strings by differentiating between a mx6dual-lite and mx6solo, if it is worth. It seems a reasonable interim solution to provide backward compatibility until the kernel driver(s) and userspace can be fixed. Another way of doing this that prevents get_cpu_rev() from hiding the precise CPU is to do this in the weak version of get_board_rev(). Regards, ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
Hi Fabio, On 27.03.2013 18:36, Fabio Estevam wrote: When booting a Freescale kernel 3.0.35 on a Wandboard solo, the get_board_rev() returns 0x62xxx, which is not a value understood by the VPU (Video Processing Unit) library in the kernel and causes the video playback to fail. The expected values for get_board_rev are: 0x63xxx: For mx6quad/dual 0x61xxx: For mx6dual-lite/solo So adjust get_board_rev() accordingly and make it as weak function, so that we do not need to define it in every mx6 board file. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v1: - Avoid extra call to get_cpu_rev() arch/arm/cpu/armv7/mx6/soc.c | 12 board/boundary/nitrogen6x/nitrogen6x.c|5 - board/freescale/mx6qsabrelite/mx6qsabrelite.c |5 - board/freescale/mx6qsabresd/mx6qsabresd.c |5 - board/wandboard/wandboard.c |5 - 5 files changed, 12 insertions(+), 20 deletions(-) diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c index 193ba12..4f3cd14 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -61,6 +61,18 @@ u32 get_cpu_rev(void) return (type 12) | (reg + 0x10); } +#ifdef CONFIG_REVISION_TAG +u32 __weak get_board_rev(void) +{ + u32 cpurev = get_cpu_rev(); + u32 type = ((cpurev 12) 0xff); + if (type == MXC_CPU_MX6SOLO) + cpurev = (MXC_CPU_MX6DL) 12 | (cpurev 0xFFF); + + return cpurev; +} +#endif This is much better than changing Troy's get_cpu_rev(). Thanks :) But what's about to add this code to http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/kernel/setup.c?h=imx_3.0.35_1.1.0#n655 ? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
Am 27.03.2013 20:36, schrieb Fabio Estevam: Hi Dirk, On Wed, Mar 27, 2013 at 3:56 PM, Dirk Behme dirk.be...@gmail.com wrote: This is much better than changing Troy's get_cpu_rev(). Thanks :) I am glad you like it :-) But what's about to add this code to http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/kernel/setup.c?h=imx_3.0.35_1.1.0#n655 Because I cannot post a patch in the U-boot list to fix an internal FSL kernel :-) I can certainly suggest this (and yes, I fully agree that the kernel should handle the CPU detection without relying at the bootloader at all), but from a U-boot perspective we cannot really know which kernel version the user will deploy. If we want mainline U-boot to work with this particular kernel version on mx6 solo, then we need to adjust ATAGS as proposed by my patch. Ok, sounds reasonable, agreed. Even if someone might complain that this is off-topic here, again ( ;) ): Do you like to check if the recent _mainline_ kernel wants something similar? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: Fix the reading of CPU revision
_SYS_PROTO_H_ -#define MXC_CPU_MX51 0x51 -#define MXC_CPU_MX53 0x53 -#define MXC_CPU_MX6SL 0x60 -#define MXC_CPU_MX6DL 0x61 -#define MXC_CPU_MX6SOLO0x62 -#define MXC_CPU_MX6Q 0x63 - #define is_soc_rev(rev)((get_cpu_rev() 0xFF) - rev) u32 get_cpu_rev(void); const char *get_imx_type(u32 imxtype); -- == Dirk Behme Robert Bosch Car Multimedia GmbH CM-AI/PJ-CF32 Phone: +49 5121 49-3274 Dirk Behme Fax: +49 711 811 5053274 PO Box 77 77 77 mailto:dirk.be...@de.bosch.com D-31132 Hildesheim - Germany Bosch Group, Car Multimedia (CM) Automotive Navigation and Infotainment Systems (AI) ProJect - CoreFunctions (PJ-CF) Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe Sitz: Hildesheim Registergericht: Amtsgericht Hildesheim HRB 201334 Aufsichtsratsvorsitzender: Volkmar Denner Geschäftsführung: Uwe Thomas, Michael Bolle, Robby Drave, Egbert Hellwig == ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] spi: mxc_spi: Fix ECSPI reset handling
Reviewing the ECSPI reset handling shows two issues: 1. For the enable/reset bit (MXC_CSPICTRL_EN) in the control reg (ECSPIx_CONGREG) the i.MX6 technical reference manual states: -- cut -- ECSPIx_CONREG[0]: EN: Writing zero to this bit disables the block and resets the internal logic with the exception of the ECSPI_CONREG. -- cut -- Note the exception mentioned: The CONREG itself isn't reset. Fix this by manually writing the reset value 0 to the whole register. This sets the EN bit to zero, too (i.e. includes the old ~MXC_CSPICTRL_EN). 2. We want to reset the whole SPI block here. So it makes no sense to first read the old value of the CONREG and write it back, later. This will give us the old (historic/random) value of the CONREG back. And doesn't reset the CONREG. To get a clean CONREG after the reset of the block, too, don't use the old (historic/random) value of the CONREG while doing the reset. And read the clean CONREG after the reset. This was found while working on a SPI boot device where the i.MX6 boot ROM has already initialized the SPI block. The initialization by the boot ROM might be different to what the U-Boot driver wants to configure. I.e. we need a clean reset of SPI block, including the CONREG. Signed-off-by: Dirk Behme dirk.be...@de.bosch.com CC: Stefano Babic sba...@denx.de CC: Fabio Estevam fabio.este...@freescale.com --- drivers/spi/mxc_spi.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/spi/mxc_spi.c b/drivers/spi/mxc_spi.c index d792d8d..cb48019 100644 --- a/drivers/spi/mxc_spi.c +++ b/drivers/spi/mxc_spi.c @@ -137,11 +137,11 @@ static s32 spi_cfg_mxc(struct mxc_spi_slave *mxcs, unsigned int cs, return -1; } - reg_ctrl = reg_read(regs-ctrl); - /* Reset spi */ - reg_write(regs-ctrl, (reg_ctrl ~MXC_CSPICTRL_EN)); - reg_write(regs-ctrl, (reg_ctrl | MXC_CSPICTRL_EN)); + reg_write(regs-ctrl, 0); + reg_write(regs-ctrl, MXC_CSPICTRL_EN); + + reg_ctrl = reg_read(regs-ctrl); /* * The following computation is taken directly from Freescale's code. -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision
Am 15.03.2013 22:06, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com Instead of hardcoding the CPU revision, it is better to use get_cpu_rev(). I think to remember that there is a reason why it is hard coded this way. Have you tested this with the Vivante GPU driver? If I remember correctly they check for exactly the 0x63000 and if it's not there, it won't work (?). Best regards Dirk Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- board/freescale/mx6qsabrelite/mx6qsabrelite.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index 5b69a6d..9bd444e 100644 --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c @@ -26,6 +26,7 @@ #include asm/arch/imx-regs.h #include asm/arch/iomux.h #include asm/arch/mx6q_pins.h +#include asm/arch/sys_proto.h #include asm/errno.h #include asm/gpio.h #include asm/imx-common/iomux-v3.h @@ -300,7 +301,7 @@ int board_mmc_init(bd_t *bis) u32 get_board_rev(void) { - return 0x63000 ; + return get_cpu_rev(); } #ifdef CONFIG_MXC_SPI ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel
Am 16.03.2013 17:13, schrieb Eric Nelson: On 03/16/2013 07:58 AM, Fabio Estevam wrote: Hi Eric, On Fri, Mar 15, 2013 at 9:20 PM, Eric Nelson eric.nel...@boundarydevices.com wrote: This is the **board** revision, right? At first glance, the kernel seems to be getting the silicon revision from the same place as get_cpu_rev(): https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_1.1.1/arch/arm/mach-mx6/cpu.c#L51 http://git.denx.de/u-boot.git/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/mx6/soc.c;h=a8aad5dd0a6c8548277021ebe8f6e159dbf31b9b;hb=HEAD#l42 Is there a reference to the ATAG that I'm not seeing somewhere? Ok, so 3.0.35 treats cpu_rev correctly and do not assume this info to be passed from the bootloader. I was confused with 2.6.35, where I had issues with this on mx53. So, it seems that nitrogen does not need to pass get_board_rev() at all then? At the moment, it doesn't. I would really like to see us (the i.MX6 community) standardize the use of some fuses to specifically mean board revision. We're contemplating some board changes such as switching the ethernet PHY and having a convention for the use of a few bits in OTP would allow us to implement get_board_rev() once in a common place. Over the lifetime of most boards, it's likely that at least one board revision will have software implications and having a common way to express/detect this could prevent some churn in board-specific files. Such a convention would need to have broad sign off though. Let me know your thoughts on the subject. I think the OMAP/Beagle community introduced serial EEPROMs to identify their (add on) boards. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] imx-common: timer: fix 32-bit overflow
On 04.03.2013 15:16, Dirk Behme wrote: From: Knut Wohlrab knut.wohl...@de.bosch.com The i.MX6 common timer uses the 32-bit variable tbl (time base lower) to record the overflow of the 32-bit counter. I.e. if the counter overflows, the variable tbl does overflow, too. To capture this overflow, use the variable tbu (time base upper), too. Return the combined value of tbl and tbu. lastinc is unused then, remove it. Signed-off-by: Knut Wohlrab knut.wohl...@de.bosch.com Signed-off-by: Dirk Behme dirk.be...@de.bosch.com --- Note: This replaces the patch http://patchwork.ozlabs.org/patch/224646/ arch/arm/imx-common/timer.c | 26 +++--- 1 file changed, 7 insertions(+), 19 deletions(-) Index: freescale-u-boot-imx.git/arch/arm/imx-common/timer.c === --- freescale-u-boot-imx.git.orig/arch/arm/imx-common/timer.c +++ freescale-u-boot-imx.git/arch/arm/imx-common/timer.c @@ -48,9 +48,6 @@ static struct mxc_gpt *cur_gpt = (struct DECLARE_GLOBAL_DATA_PTR; -#define timestamp (gd-arch.tbl) -#define lastinc (gd-arch.lastinc) - static inline unsigned long long tick_to_time(unsigned long long tick) { tick *= CONFIG_SYS_HZ; @@ -70,7 +67,6 @@ static inline unsigned long long us_to_t int timer_init(void) { int i; - ulong val; /* setup GP Timer 1 */ __raw_writel(GPTCR_SWR, cur_gpt-control); @@ -85,9 +81,8 @@ int timer_init(void) i = __raw_readl(cur_gpt-control); __raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, cur_gpt-control); - val = __raw_readl(cur_gpt-counter); - lastinc = val / (MXC_CLK32 / CONFIG_SYS_HZ); - timestamp = 0; + gd-arch.tbl = __raw_readl(cur_gpt-counter); + gd-arch.tbu = 0; return 0; } @@ -96,18 +91,11 @@ unsigned long long get_ticks(void) { ulong now = __raw_readl(cur_gpt-counter); /* current tick value */ - if (now = lastinc) { - /* -* normal mode (non roll) -* move stamp forward with absolut diff ticks -*/ - timestamp += (now - lastinc); - } else { - /* we have rollover of incrementer */ - timestamp += (0x - lastinc) + now; - } - lastinc = now; - return timestamp; + /* increment tbu if tbl has rolled over */ + if (now gd-arch.tbl) + gd-arch.tbu++; + gd-arch.tbl = now; + return (((unsigned long long)gd-arch.tbu) 32) | gd-arch.tbl; } ulong get_timer_masked(void) Any comments on this? Many thanks Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] ARM: global_data: make tbl long long
From: Dirk Behme dirk.be...@de.bosch.com Several ARM timer implementations use gd-arch.tbl to record the absolute tick count of 32-bit counters, including timer overflows. For example arch/arm/imx-common/timer.c does: ulong lastinc; ulong now = counter value; if (no overflow) { ... } else { /* counter overflow */ gd-arch.tbl += (0x - lastinc) + now; } lastinc = now; As we use a 32-bit counter and the two ulong (32-bit) variables 'lastinc' and 'now' here, gd-arch.tbl should be long long (64-bit) to not overflow at the same time, too. Signed-off-by: Knut Wohlrab knut.wohl...@de.bosch.com Signed-off-by: Dirk Behme dirk.be...@de.bosch.com --- arch/arm/include/asm/global_data.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 37ac0da..1099af9 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -41,7 +41,7 @@ struct arch_global_data { /* static data needed by most of timer.c on ARM platforms */ unsigned long timer_rate_hz; unsigned long tbu; - unsigned long tbl; + unsigned long long tbl; unsigned long lastinc; unsigned long long timer_reset_value; #ifdef CONFIG_IXP425 -- 1.7.10.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] imx-common: timer: fix 32-bit overflow
From: Knut Wohlrab knut.wohl...@de.bosch.com The i.MX6 common timer uses the 32-bit variable tbl (time base lower) to record the overflow of the 32-bit counter. I.e. if the counter overflows, the variable tbl does overflow, too. To capture this overflow, use the variable tbu (time base upper), too. Return the combined value of tbl and tbu. lastinc is unused then, remove it. Signed-off-by: Knut Wohlrab knut.wohl...@de.bosch.com Signed-off-by: Dirk Behme dirk.be...@de.bosch.com --- Note: This replaces the patch http://patchwork.ozlabs.org/patch/224646/ arch/arm/imx-common/timer.c | 26 +++--- 1 file changed, 7 insertions(+), 19 deletions(-) Index: freescale-u-boot-imx.git/arch/arm/imx-common/timer.c === --- freescale-u-boot-imx.git.orig/arch/arm/imx-common/timer.c +++ freescale-u-boot-imx.git/arch/arm/imx-common/timer.c @@ -48,9 +48,6 @@ static struct mxc_gpt *cur_gpt = (struct DECLARE_GLOBAL_DATA_PTR; -#define timestamp (gd-arch.tbl) -#define lastinc (gd-arch.lastinc) - static inline unsigned long long tick_to_time(unsigned long long tick) { tick *= CONFIG_SYS_HZ; @@ -70,7 +67,6 @@ static inline unsigned long long us_to_t int timer_init(void) { int i; - ulong val; /* setup GP Timer 1 */ __raw_writel(GPTCR_SWR, cur_gpt-control); @@ -85,9 +81,8 @@ int timer_init(void) i = __raw_readl(cur_gpt-control); __raw_writel(i | GPTCR_CLKSOURCE_32 | GPTCR_TEN, cur_gpt-control); - val = __raw_readl(cur_gpt-counter); - lastinc = val / (MXC_CLK32 / CONFIG_SYS_HZ); - timestamp = 0; + gd-arch.tbl = __raw_readl(cur_gpt-counter); + gd-arch.tbu = 0; return 0; } @@ -96,18 +91,11 @@ unsigned long long get_ticks(void) { ulong now = __raw_readl(cur_gpt-counter); /* current tick value */ - if (now = lastinc) { - /* -* normal mode (non roll) -* move stamp forward with absolut diff ticks -*/ - timestamp += (now - lastinc); - } else { - /* we have rollover of incrementer */ - timestamp += (0x - lastinc) + now; - } - lastinc = now; - return timestamp; + /* increment tbu if tbl has rolled over */ + if (now gd-arch.tbl) + gd-arch.tbu++; + gd-arch.tbl = now; + return (((unsigned long long)gd-arch.tbu) 32) | gd-arch.tbl; } ulong get_timer_masked(void) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM: global_data: make tbl long long
Dear Wolfgang, On 04.03.2013 12:10, Wolfgang Denk wrote: Dear Dirk Behme, In message 1362387637-32334-1-git-send-email-dirk.be...@de.bosch.com you wrote: From: Dirk Behme dirk.be...@de.bosch.com Several ARM timer implementations use gd-arch.tbl to record the absolute tick count of 32-bit counters, including timer overflows. For example arch/arm/imx-common/timer.c does: ulong lastinc; ulong now = counter value; if (no overflow) { ... } else { /* counter overflow */ gd-arch.tbl += (0x - lastinc) + now; } lastinc = now; As we use a 32-bit counter and the two ulong (32-bit) variables 'lastinc' and 'now' here, gd-arch.tbl should be long long (64-bit) to not overflow at the same time, too. I think this is wrong. tbl means time base, lower 32 bits and is complemented by tbu (time base, upper 32 bits) to form a 64 bit time base counter. If you need the full 64 bit precision, then please either maintain the carry manually, or use a proper union or similar. Many thanks for this explanation! This patch is obsolete now, replaced by http://patchwork.ozlabs.org/patch/224740/ Thanks Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] imx6, spl and falcon boot
Am 15.02.2013 21:45, schrieb Chaves, Kevin: Hi, First off, sorry I'm not used to using mailing lists. I'm not sure if this is the best way to dig for information. We've recently switched to uboot/linux from wince and I'm trying to understand how configuring uboot works. I'm also trying to find out how to use the SPL framework and possibly falcon mode with a i.mx6 nitrogen6x board. I'd be delighted if any one could help! I haven't used spl and falcon mode, so I can't help with these. Regarding the general support for the i.mx6 nitrogen6x this isn't in U-Boot mainline, yet. Try to git pull the most recent U-Boot mainline from git://git.denx.de/u-boot.git [1] and apply the patch http://patchwork.ozlabs.org/patch/216991/ You should then be able to build one of the supported boards nitrogen6q nitrogen6dl nitrogen6s nitrogen6q2g nitrogen6dl2g nitrogen6s1g by make nitrogen6xxx_config make (replace the 'xxx' with the ending matching your board, e.g. 'q' if you have a Quad board). Best regards Dirk [1] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=summary ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2] mx6qsabreauto: enable USB host interface
From: Knut Wohlrab knut.wohl...@de.bosch.com The USB host interface is routed to plug USB1/J30 on the mother board. Signed-off-by: Knut Wohlrab knut.wohl...@de.bosch.com --- Changes in v2: - Don't add an empty board_ehci_hcd_init() to mx6qsabreauto.c. It's not needed because board_ehci_hcd_init() is declared as weak. include/configs/mx6qsabreauto.h | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/include/configs/mx6qsabreauto.h b/include/configs/mx6qsabreauto.h index f4a082a..f2ff3e1 100644 --- a/include/configs/mx6qsabreauto.h +++ b/include/configs/mx6qsabreauto.h @@ -19,6 +19,17 @@ #define CONFIG_MMCROOT /dev/mmcblk0p2 #define PHYS_SDRAM_SIZE(2u * 1024 * 1024 * 1024) +/* USB Configs */ +#define CONFIG_CMD_USB +#define CONFIG_USB_EHCI +#define CONFIG_USB_EHCI_MX6 +#define CONFIG_USB_STORAGE +#define CONFIG_USB_HOST_ETHER +#define CONFIG_USB_ETHER_ASIX +#define CONFIG_MXC_USB_PORT1 +#define CONFIG_MXC_USB_PORTSC (PORT_PTS_UTMI | PORT_PTS_PTW) +#define CONFIG_MXC_USB_FLAGS 0 + #include mx6qsabre_common.h #define CONFIG_SYS_FSL_USDHC_NUM 2 -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] U-Boot Graphics Library?
On 16.01.2013 23:54, Simon Glass wrote: Hi Wolfgang, On Wed, Jan 16, 2013 at 2:40 PM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message CAPnjgZ3rNhvL98JEOPV=90u-Wr3iEqs8QgqYaz=boohrfrj...@mail.gmail.com you wrote: The problem is that a major purpose of the GUIs is to allow installing an OS from a USB stick/SD card, since the OS may have become corrupted and unbootable. We have looked at keeping around a separate OS image for this (e.g. in an available disk partition), but so far that hasn't proved practical since that itself can be fairly easily overwritten and we then have a doorstop. The less fancy, the better, but consumers have certain expectations that force us to make some efforts...there is already a single font in U-Boot, but it's not good enough for multi-language pretty displays unfortunately. Why don't you load and start a small recovery system from the USB stick then? I don't see why you need a GUI to do that. This could be mostly made automatic. That's exactly what we do, yes! But the user has to be told that there is a problem, and be able to select their language so they can understand the instructions. The USB image needs to be downloaded, and the stick inserted. Also we require that the stick be inserted while we are watching, since this shows that the user is actually present, and it is not a remote attacker causing recovery. All of this means instructions are needed, hence the bitmaps and text. Yes, we were thinking about the same use case: An U-Boot based device where U-Boot is responsible for the whole system update, including the OS. The user has to insert an USB stick or SD card containing the files to be updated. Then U-Boot has to copy these files to the NOR, NAND or whatever the system boots from. The only user interface is a display, no serial console. On the display something like Please insert the USB stick, Update in progress, please wait, Update successfully finished, Update error xx has to be shown, then. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] mx6qsabreauto: enable USB host interface
From: Knut Wohlrab knut.wohl...@de.bosch.com The USB host interface is routed to plug USB1/J30 on the mother board. Signed-off-by: Knut Wohlrab knut.wohl...@de.bosch.com --- board/freescale/mx6qsabreauto/mx6qsabreauto.c |7 +++ include/configs/mx6qsabreauto.h | 11 +++ 2 files changed, 18 insertions(+), 0 deletions(-) diff --git a/board/freescale/mx6qsabreauto/mx6qsabreauto.c b/board/freescale/mx6qsabreauto/mx6qsabreauto.c index 9e3700e..696ddb4 100644 --- a/board/freescale/mx6qsabreauto/mx6qsabreauto.c +++ b/board/freescale/mx6qsabreauto/mx6qsabreauto.c @@ -101,6 +101,13 @@ static void setup_iomux_uart(void) imx_iomux_v3_setup_multiple_pads(uart4_pads, ARRAY_SIZE(uart4_pads)); } +#ifdef CONFIG_USB_EHCI_MX6 +int board_ehci_hcd_init(int port) +{ + return 0; +} +#endif + #ifdef CONFIG_FSL_ESDHC struct fsl_esdhc_cfg usdhc_cfg[1] = { {USDHC3_BASE_ADDR}, diff --git a/include/configs/mx6qsabreauto.h b/include/configs/mx6qsabreauto.h index f1ff201..52d1a98 100644 --- a/include/configs/mx6qsabreauto.h +++ b/include/configs/mx6qsabreauto.h @@ -18,6 +18,17 @@ #define CONFIG_MMCROOT /dev/mmcblk0p2 #define PHYS_SDRAM_SIZE(2u * 1024 * 1024 * 1024) +/* USB Configs */ +#define CONFIG_CMD_USB +#define CONFIG_USB_EHCI +#define CONFIG_USB_EHCI_MX6 +#define CONFIG_USB_STORAGE +#define CONFIG_USB_HOST_ETHER +#define CONFIG_USB_ETHER_ASIX +#define CONFIG_MXC_USB_PORT1 +#define CONFIG_MXC_USB_PORTSC (PORT_PTS_UTMI | PORT_PTS_PTW) +#define CONFIG_MXC_USB_FLAGS 0 + #include mx6qsabre_common.h #define CONFIG_SYS_FSL_USDHC_NUM 2 -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] tools: imximage: Load a size that is multiple of 512
On 03.01.2013 19:24, Fabio Estevam wrote: In order to mx53 ROM to properly load the U-boot image, its header size should be multiple of 512 bytes. ... Signed-off-by: Troy Kisky troy.ki...@boundarydevices.com Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- Changes since v1: - Improvec commit log include/image.h |3 +++ tools/imximage.c |9 - 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/image.h b/include/image.h index f54d983..e1e83b4 100644 --- a/include/image.h +++ b/include/image.h @@ -179,6 +179,9 @@ #define IH_MAGIC 0x27051956 /* Image Magic Number */ #define IH_NMLEN 32 /* Image Name Length*/ +/* Reused from common.h */ +#define ROUND(a, b)(((a) + (b) - 1) ~((b) - 1)) + /* * Legacy format image header, * all data in network byte order (aka natural aka bigendian). diff --git a/tools/imximage.c b/tools/imximage.c index 63f88b6..a93d7eb 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -515,7 +515,14 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, /* Set the imx header */ (*set_imx_hdr)(imxhdr, dcd_len, params-ep, imxhdr-flash_offset); - *header_size_ptr = sbuf-st_size + imxhdr-flash_offset; + + /* +* ROM bug alert +* mx53 only loads 512 byte multiples. Is this i.MX53 specific or is this valid for i.MX6, too? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/5] Add fuse API and commands
On 26.11.2012 17:03, Benoît Thébaudeau wrote: Hi Eric, all, On Thursday, August 23, 2012 3:23:20 PM, Eric Nelson wrote: On 08/23/2012 03:31 AM, Stefano Babic wrote: On 22/08/2012 12:43, Dirk Behme wrote: On 14.08.2012 14:52, Benoît Thébaudeau wrote: This can be useful for fuse-like hardware, OTP SoC options, etc. For i.MX6, I have a port of the OTP support from Freescale's U-Boot to our mainline U-Boot in the queue [1]. As I don't have the overview over the various i.MXxx SoCs and don't understand much of this patch below: Should this implement the same functionality like my patch [1] for i.MX6? I have not checked the details. but seeing the code it looks that the procedure to read / write are different. In this case, a further driver is ok. Anyway, you should take a look if your patches can be used on a mxs (MX28) device, because they should be closer. And then I will not like to have a driver for each SOC. Generally, I think we should use the approach of the common command and a specific fuse implementation. Then this API should be used by your patches as well. I agree. The use of the fuse API will likely result in more code than the imxotp implementation, and more importantly, it will make the usage more confusing by introducing terms bank and row. Reading and writing fuses is probably not an area that we want confusion. I am looking again into this question now that I have the i.MX6 reference manual. I don't see any difference between IIM and OCOTP features: - There are still banks: They exist as seen from the controller, even if they don't exist physically in the efusebox. See 46.2.1 and OCOTP memory map (Value of OTP Bankx Wordy shadow registers). - Rows are named words. - The read operations are read accesses to the shadow registers. - The sense operations are direct fuse reads (shadow reload + read), as explained by the steps in 46.2.1.2. - The prog operations are the programming of the fuses, as explained by the steps in 46.2.1.3. - The override operations are simple write accesses to the shadow registers, as explained in 46.2.1.3. As to the vocabulary used, the differences are: - row - word. - sense - direct read. Hence, the fuse API applies very well in this case too. Do you like to update/rebase your patches to the latest U-Boot and re-send? Maybe it makes sense to discuss your patches again, then? Eric: What do you think? Thanks Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: clock: Only show CSPI clock if CSPI is enabled
Am 16.11.2012 12:30, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com If a board does not enable CSPI, there is no need to show the CSPI clock frequency as part of the 'clock' command. Reported-by: Dirk Behme dirk.be...@gmail.com Signed-off-by: Fabio Estevam fabio.este...@freescale.com Acked-by: Dirk Behme dirk.be...@gmail.com Thanks Dirk --- arch/arm/cpu/armv7/mx6/clock.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c index a01d96f..a50db70 100644 --- a/arch/arm/cpu/armv7/mx6/clock.c +++ b/arch/arm/cpu/armv7/mx6/clock.c @@ -404,7 +404,9 @@ int do_mx6_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) printf(\n); printf(IPG%8d kHz\n, mxc_get_clock(MXC_IPG_CLK) / 1000); printf(UART %8d kHz\n, mxc_get_clock(MXC_UART_CLK) / 1000); +#ifdef CONFIG_MXC_SPI printf(CSPI %8d kHz\n, mxc_get_clock(MXC_CSPI_CLK) / 1000); +#endif printf(AHB%8d kHz\n, mxc_get_clock(MXC_AHB_CLK) / 1000); printf(AXI%8d kHz\n, mxc_get_clock(MXC_AXI_CLK) / 1000); printf(DDR%8d kHz\n, mxc_get_clock(MXC_DDR_CLK) / 1000); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/4] mx5: Print CSPI clock in 'clock' command
Am 15.11.2012 22:23, schrieb Fabio Estevam: From: Fabio Estevam fabio.este...@freescale.com Print CSPI clock in 'clock' command. Signed-off-by: Fabio Estevam feste...@gmail.com --- arch/arm/cpu/armv7/mx5/clock.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/arm/cpu/armv7/mx5/clock.c b/arch/arm/cpu/armv7/mx5/clock.c index 1c9223f..76c2c52 100644 --- a/arch/arm/cpu/armv7/mx5/clock.c +++ b/arch/arm/cpu/armv7/mx5/clock.c @@ -928,7 +928,9 @@ int do_mx5_showclocks(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) printf(IPG%8d kHz\n, mxc_get_clock(MXC_IPG_CLK) / 1000); printf(IPG PERCLK %8d kHz\n, mxc_get_clock(MXC_IPG_PERCLK) / 1000); printf(DDR%8d kHz\n, mxc_get_clock(MXC_DDR_CLK) / 1000); - +#ifdef CONFIG_MXC_SPI + printf(CSPI %8d kHz\n, mxc_get_clock(MXC_CSPI_CLK) / 1000); +#endif I wondered if we want something similar for i.MX6, too. And found that we have this on i.MX6, already. But without #ifdef. http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/mx6/clock.c;h=a01d96f48e04377d705b7a587ec7b3ea59aee283;hb=HEAD#l407 Do we want to add the #ifdef for i.MX6, too? Or would it be nicer to drop the #ifdef in this patch to make i.MX6 and i.MX5 equal? Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Makefile: silence 'make clean'
On 13.11.2012 17:31, Andreas Bießmann wrote: Signed-off-by: Andreas Bießmann andreas.de...@googlemail.com Cc: Marek Vasut ma...@denx.de Cc: Wolfgang Denk w...@denx.de Tested-by: Dirk Behme dirk.be...@de.bosch.com Thanks Dirk --- Makefile |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9dc89f9..5fa2fba 100644 --- a/Makefile +++ b/Makefile @@ -811,7 +811,7 @@ clean: @rm -f $(obj)include/generated/asm-offsets.h @rm -f $(obj)$(CPUDIR)/$(SOC)/asm-offsets.s @rm -f $(TIMESTAMP_FILE) $(VERSION_FILE) - @$(MAKE) -C doc/DocBook/ cleandocs + @$(MAKE) -s -C doc/DocBook/ cleandocs @find $(OBJTREE) -type f \ \( -name 'core' -o -name '*.bak' -o -name '*~' -o -name '*.su' \ -o -name '*.o' -o -name '*.a' -o -name '*.exe' \) -print \ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] i.MX: mxc_ipuv3_fb: add ipuv3_fb_shutdown() routine to stop IPU before bootm
On 23.09.2012 17:56, Stefano Babic wrote: On 22/09/2012 16:37, Fabio Estevam wrote: On Sat, Sep 22, 2012 at 10:42 AM, Otavio Salvador ota...@ossystems.com.br wrote: Hello Eric, On Fri, Sep 21, 2012 at 5:36 PM, Eric Nelson eric.nel...@boundarydevices.com wrote: Signed-off-by: Eric Nelsoneric.nel...@boundarydevices.com Did you test it in mx5 too? We seem to need to handle it in mx5 too as we had hungs in FSL kernel when using framebuffer in U-Boot. We're using a patch in kernel for workaround it but it seems your fix does what is need. I have just tested Eric's series on a mx53loco and it does fix the kernel hang issue. I made some comments on this series and hopefully Eric's v2 can get into 2012.10, since this is a bug fix. Ok, I am waiting for V2 and I will push it. Anyway, a question about the issue. It seems to me that it is not possible with IPUV3 (I have not tested myself, so my question) to get the u-boot splashscreen displayed on the LCD until the kernel has finished to boot. This could be possible (and it is possible on other SOC) if the IPU is loaded as module instead of statically linked to the kernel, and if the kernel does not touch the IPU setup. This means also that it should not disable the clocks used by U-Boot for the IPU. But I understand from your patch that this way is not possible on iMX53/MX6, and IPU must be always disabled. Is this correct ? I'm no expert on this, so I might be wrong and people will correct me: No, not always. It's my understanding that at the *moment* we have to disable the IPU in U-Boot before jumping into the kernel due to a bug in Sascha's kernel IPU driver: http://lists.infradead.org/pipermail/linux-arm-kernel/2012-September/121980.html http://lists.infradead.org/pipermail/linux-arm-kernel/2012-September/122100.html So from my understanding, this isn't a SOC limitation. But we have to do it as long as the kernel's IPU driver isn't fixed. Sorry if this is wrong ;) Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 00/21] Add mx6solo/mx6duallite support
Hi Troy, On 22.09.2012 04:38, Troy Kisky wrote: After this series the same binary will run on a Saberlite board using any of the pin compatible processors mx6 quad, mx6 duallite, or mx6 solo. This is accomplished using a plugin and a table built by mkimage. Could you briefly explain or give a link to some documentation how this does work? It sounds to me that the plugin concept is something the boot ROM has to understand? Or in other words: How does the boot ROM decide on which processor it runs and which DCD table to execute then? Sorry if I misunderstood something, I'm not so familiar with the boot ROM options. Many thanks Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6: Remove lowlevel_init.S
On 17.09.2012 18:34, Fabio Estevam wrote: lowlevel_init.S is not used on mx6, Yes, but ... We use lowlevel_init.S on a not yet public custom board to do some early, custom specific initialization. So I would vote to keep this. But most probably non-mainline code isn't a reason to keep this? Best regards Dirk so remove the file and the associated calls. Signed-off-by: Fabio Estevam fabio.este...@freescale.com --- arch/arm/cpu/armv7/mx6/Makefile|1 - arch/arm/cpu/armv7/mx6/lowlevel_init.S | 25 - arch/arm/cpu/armv7/start.S | 24 3 files changed, 50 deletions(-) delete mode 100644 arch/arm/cpu/armv7/mx6/lowlevel_init.S diff --git a/arch/arm/cpu/armv7/mx6/Makefile b/arch/arm/cpu/armv7/mx6/Makefile index cbce411..4f9ca68 100644 --- a/arch/arm/cpu/armv7/mx6/Makefile +++ b/arch/arm/cpu/armv7/mx6/Makefile @@ -28,7 +28,6 @@ include $(TOPDIR)/config.mk LIB= $(obj)lib$(SOC).o COBJS = soc.o clock.o -SOBJS = lowlevel_init.o SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS)) diff --git a/arch/arm/cpu/armv7/mx6/lowlevel_init.S b/arch/arm/cpu/armv7/mx6/lowlevel_init.S deleted file mode 100644 index acadef2..000 --- a/arch/arm/cpu/armv7/mx6/lowlevel_init.S +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License as - * published by the Free Software Foundation; either version 2 of - * the License, or (at your option) any later version. - * - * 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. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, - * MA 02111-1307 USA - */ -.section .text.init, x - -#include linux/linkage.h - -ENTRY(lowlevel_init) - mov pc, lr -ENDPROC(lowlevel_init) diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 32658eb..4c7c385 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -152,7 +152,6 @@ reset: /* the mask ROM code should have PLL and others stable */ #ifndef CONFIG_SKIP_LOWLEVEL_INIT bl cpu_init_cp15 - bl cpu_init_crit #endif /* Set stackpointer in internal RAM to call board_init_f */ @@ -353,29 +352,6 @@ ENTRY(cpu_init_cp15) mov pc, lr @ back to my caller ENDPROC(cpu_init_cp15) -#ifndef CONFIG_SKIP_LOWLEVEL_INIT -/* - * - * CPU_init_critical registers - * - * setup important registers - * setup memory timing - * - */ -ENTRY(cpu_init_crit) - /* -* Jump to board specific initialization... -* The Mask ROM will have already initialized -* basic memory. Go here to bump up clock rate and handle -* wake up conditions. -*/ - mov ip, lr @ persevere link reg across call - bl lowlevel_init @ go setup pll,mux,memory - mov lr, ip @ restore link - mov pc, lr @ back to my caller -ENDPROC(cpu_init_crit) -#endif - #ifndef CONFIG_SPL_BUILD /* * ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] AR8031 Ethernet on mx6
On 15.09.2012 05:06, Fabio Estevam wrote: Hi Jason, I don't have a mx6qarm2 and would like to ask you if you could please confirm that Ethernet is functional on mx6qarm2 running the latest U-boot. I am trying to get Ethernet working on mx6qsabresd, which also uses the same AR8031 PHY, but it is not working yet. AR8031 is supposed to output a 125MHz clock to mx6q, but I see a 25MHz clock instead. I do the same PHY init as in mx6qarm2. Any ideas/suggestions are appreciated. Trying the recent mainline U-Boot (a6f0c4faa4c65a7b7048b) on an ARM2 board we get [1] after manually setting an ethaddr and the IP addresses. Sometimes U-Boot seems to hang after 'MMC: FSL_SDHC: 0, FSL_SDHC: 1' but this might be an other issue. Best regards Dirk [1] U-Boot 2012.07-00490-ga6f0c4f (Sep 17 2012 - 10:01:46) CPU: Freescale i.MX6Q rev1.0 at 792 MHz Reset cause: POR Board: MX6Q-Armadillo2 DRAM: 2 GiB WARNING: Caches not enabled MMC: FSL_SDHC: 0, FSL_SDHC: 1 In:serial Out: serial Err: serial Net: FEC Hit any key to stop autoboot: 0 MX6QARM2 U-Boot printenv baudrate=115200 bootargs=console=ttymxc3,115200 root=/dev/nfs ip=dhcp nfsroot=:,v3,tcp bootcmd=mmc dev ${mmcdev};if mmc rescan ${mmcdev}; then if run loadbootscript; then run bootscript; else if run loi bootdelay=3 bootscript=echo Running bootscript from mmc ...; source console=ttymxc3 ethact=FEC ethaddr=00:19:B8:00:E5:4E fdt_high=0x initrd_high=0x ipaddr=172.17.0.1 loadaddr=0x1080 loadbootscript=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script}; loaduimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage} mmcargs=setenv bootargs console=${console},${baudrate} root=${mmcroot} mmcboot=echo Booting from mmc ...; run mmcargs; bootm mmcdev=1 mmcpart=2 mmcroot=/dev/mmcblk0p3 rootwait rw netargs=setenv bootargs console=${console},${baudrate} root=/dev/nfs ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp netboot=echo Booting from net ...; run netargs; dhcp ${uimage}; bootm script=boot.scr serverip=172.17.0.6 stderr=serial stdin=serial stdout=serial uimage=uImage Environment size: 1125/8188 bytes MX6QARM2 U-Boot tftpboot ${loadaddr} uImage Using FEC device TFTP from server 172.17.0.6; our IP address is 172.17.0.1 Filename 'uImage'. Load address: 0x1080 Loading: # # # # done Bytes transferred = 3756664 (395278 hex) MX6QARM2 U-Boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] AR8031 Ethernet on mx6
On 15.09.2012 05:06, Fabio Estevam wrote: Hi Jason, I don't have a mx6qarm2 and would like to ask you if you could please confirm that Ethernet is functional on mx6qarm2 running the latest U-boot. I am trying to get Ethernet working on mx6qsabresd, which also uses the same AR8031 PHY, but it is not working yet. AR8031 is supposed to output a 125MHz clock to mx6q, but I see a 25MHz clock instead. I do the same PHY init as in mx6qarm2. Any ideas/suggestions are appreciated. I think a coworker has an ARM2, so I'll try to ask him on Monday. But while talking about ethernet on MX6: There seems to be a lot of issues with 1GB ethernet. Or is it just confusion? As I'm no expert on this, I'm really confused. That means that I don't know if we still really have some issues with it or not :( The initial SabreLite boards (revision from September/Octorber last year?) had a hardware issue with 1GB ethernet. I was told that this hardware issue is fixed in the latest SabreLite boards with hardware revision D. But a coworker reports that it looks like GB ethernet even doesn't work with the U-Boot the SabreLite revision D ships with (?). Nor a more recent (patched?) U-Boot. I'm not sure if the recent (unpatched) mainline was tested, though. So this might be an issue of an U-Boot version which has SW patches for the old, broken ethernet applied, which now breaks the fixed hardware? Or is there still an hardware issue? I don't know. In parallel, there is some rumor that on a Freescale board (SabreAuto? Have to check on Monday) 1GB doesn't work, too. As mentioned, this are all reports I got, I haven't done any testing myself. If anybody could clarify on this, this would be nice. Many thanks and best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] More problems booting an i.MX6 from NOR flash
On 11.09.2012 18:32, Carolyn Smith wrote: Thanks to Dirk for your previous advice. We had a pullup on one of the reserved BOOT_CFG pins that caused problems accessing NOR flash. Now I can put code in the NOR flash. If the code is incorrect in some way (not sure how yet), I end up with a board that I can't get control of with the BDI. I get - TARGET: processing reset request - TARGET: BDI executes scan chain init string - TARGET: Bypass check 0x0001 = 0x0004 - TARGET: JTAG exists check passed - Core#0: ID code is 0x4BA00477 - Core#0: DP-CSW is 0xF000 - Core#0: DBG-AP at 0x8215 - Core#0: DIDRis 0x3513702A - TARGET: Reset sequence passed - TARGET: resetting target passed - TARGET: processing target startup # TARGET: core #0 startup failed # TARGET: timeout while waiting for debug mode # TARGET: processing target startup failed # TARGET: timeout while waiting for debug mode I've tried different STARTUP modes, adding WAKEUP delays, different RESET modes all of which don't help. Any suggestions? Well, this depends on your question ;) If your question is how to re-enable a board which has wrong code in the NOR flash and therefore doesn't boot and I can't access the NOR flash any more to correct it: Then you have to re-solder the boot mode resistors so that a different boot mode is used (e.g. from SD card). This will disable the boot from the broken NOR and prevent the boot ROM from crashing. If your question is what's wrong with the content of my boot ROM?: I would propose to use a quite simple image in the NOR: No DCD Data, as user code just one assembly instructions like an infinite loop and as execution address of that loop the internal OCRAM. You could use an existing U-Boot i.MX6 board and strip it down. This should bring you to an image which let the boot ROM jump to the internal OCRAM and wait there in the infinite loop. You are successful with this if you attach with your debugger and can see the infinite loop running in the internal OCRAM, then. If your question is why can't I attach with the debugger any more?: Well, maybe because your code in the NOR flash confuses the boot ROM so much that it crashes in a way that stops the access with the JTAG. I saw this in the past with corrupted NOR flash images, too. The only way I could find out of this was to switch the boot mode by re-soldering, see my answer above. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Problem booting i.MX6 from parallel NOR flash
On 07.09.2012 20:41, Carolyn Smith wrote: Hello, I have a custom i.MX6 board that is configured to boot from 16-bit parallel NOR flash using non-multiplexed I/O with the data on the upper half of the data bus. I.e. chip select 0 of the EIM bus should be configured so that MUM = 0 and DSZ = 010b. However, the board is coming out of reset (using a BDI3000) with MUM = 1 and DSZ = 001b so multiplexed I/O with the data on the lower half of the data bus. According to the EIM multiplexing table, this isn't even a valid configuration. Check the BOOT_MODE and BOOT_CFG of your board. A valid boot configuration to boot from parallel NOR on your board should look like this: SRC_SBMR1: 0x02044000 Further, if I change the EIM_CS0GCR1 register (using the BDI) to force MUM = 0 and DSZ = 010b, then read from the flash, I can see the correct data on the bus using a logic analyzer but the BDI always returns 0. Check the pin multiplex and EIM configuration. An example can be found in u-boot-2009.08/board/freescale/mx6q_sabreauto/mx6q_sabreauto.c in Freescale's latest U-Boot ER release for i.MX6. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6qsabresd: Add basic support
On 11.09.2012 05:56, Fabio Estevam wrote: Hi Stefano, On Thu, Apr 12, 2012 at 7:52 AM, Stefano Babic sba...@denx.de wrote: This file is identical to imximage.cfg for the mx6qsabrelite board. I can imagine this is derived board. Why cannot we implement it as a variant of the original one ? We have several example in u-boot, for example the efika (MX51), or the TAM3517 (ok, I admit I know this very well because I did it...), or imx27-lite /magnesium, or After a long time, I am returning on adding support to mx6qsabresd. I have been comparing mx6qsabrelite against mx6qsabresd and I have started to do as you suggested: unify the 2 boards into mx6qsabrelite.c. What I realize is that the differences are relevant: UART1 pin muxing, SDHC ports, SDHC card detect GPIO, USB Host enable port, I2C devices, Ethernet PHY, etc. It seems to me that the code is becoming polluted by all the ifdef's I need to place in order to handle both boards, and I am starting to think if it wouldn't be better to follow with the original approach of adding a board/freescale/mx6qsabresd directory. After I finish mx6qsabresd, I also plan to add one more mx6q board, and this would mean even more ifdefs, which would make the code even harder to read. Please let me know what you think. I don't know all the boards close enough, but if we could somehow find an unique identifier to be able to auto-detect the board type at runtime and read this early in the boot phase, we could try to have one U-Boot binary for different boards which configures itself correctly at runtime. Just an idea ... Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/5] Add fuse API and commands
On 22.08.2012 13:11, Benoît Thébaudeau wrote: Hi Dirk, On Wednesday, August 22, 2012 12:43:05 PM, Dirk Behme wrote: On 14.08.2012 14:52, Benoît Thébaudeau wrote: This can be useful for fuse-like hardware, OTP SoC options, etc. For i.MX6, I have a port of the OTP support from Freescale's U-Boot to our mainline U-Boot in the queue [1]. As I don't have the overview over the various i.MXxx SoCs and don't understand much of this patch below: Should this implement the same functionality like my patch [1] for i.MX6? Or shall I send my patch [1] to this mailing list for official review because the functionality here and there is orthogonal? Thanks Dirk [1] https://github.com/dirkbehme/u-boot-imx6/commit/da718b338a79af160f7b7e542fe97b24edbfc36a This OTP IP is different from the IIM IP of other i.MXs, so having a different driver for it is fine. What you can do is implement in your driver the fuse API You mean http://lists.denx.de/pipermail/u-boot/2012-August/130904.html here, correct? that I've defined in this series. In that way, you could drop your common/cmd_imxotp.c and use the commands from this series. Let's see how this fits to i.MX6: My understanding is that the i.MX6 has 128 fuse 'registers' (#define IMX_OTP_ADDR_MAX 0x7F), each containing 32 fuses/bits. These fuses/bits can be written from 0 - 1 once. Looking at your API [2] we could set 'bank' to 0 and interpret 'row' as the 'register' number. That might fit. Doing this, fuse_read_bit/row() and fuse_prog_bit/row() could be used for i.MX6. From i.MX6 point of view, as I don't know your use case, I'm not sure what fuse_sense_bit/row() and fuse_override_bit/row() might be good for? Anybody else: Does this make sense? Or should we keep the i.MX6 specific common/cmd_imxotp.c (see [1] above)?. Best regards Dirk [2] + * Read/Sense/Program/Override interface: + * bank:Fuse bank + * row: Fuse row within the bank + * bit: Fuse bit within the row + * val: Value to read/write + * + * Returns: 0 on success, not 0 on failure + */ +int fuse_read_bit(u32 bank, u32 row, u32 bit, u32 *val); +int fuse_read_row(u32 bank, u32 row, u32 *val); +int fuse_sense_bit(u32 bank, u32 row, u32 bit, u32 *val); +int fuse_sense_row(u32 bank, u32 row, u32 *val); +int fuse_prog_bit(u32 bank, u32 row, u32 bit); +int fuse_prog_row(u32 bank, u32 row, u32 val); +int fuse_override_bit(u32 bank, u32 row, u32 bit, u32 val); +int fuse_override_row(u32 bank, u32 row, u32 val); ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 3/5] Add fuse API and commands
-boot-4d3c95f/include/fuse.h u-boot-4d3c95f/include/fuse.h new file mode 100644 index 000..baefefe --- /dev/null +++ u-boot-4d3c95f/include/fuse.h @@ -0,0 +1,49 @@ +/* + * (C) Copyright 2009-2012 ADVANSEE + * Benoît Thébaudeau benoit.thebaud...@advansee.com + * + * Based on the mpc512x iim code: + * Copyright 2008 Silicon Turnkey Express, Inc. + * Martha Marx mm...@silicontkx.com + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#ifndef _FUSE_H_ +#define _FUSE_H_ + +/* + * Read/Sense/Program/Override interface: + * bank:Fuse bank + * row: Fuse row within the bank + * bit: Fuse bit within the row + * val: Value to read/write + * + * Returns: 0 on success, not 0 on failure + */ +int fuse_read_bit(u32 bank, u32 row, u32 bit, u32 *val); +int fuse_read_row(u32 bank, u32 row, u32 *val); +int fuse_sense_bit(u32 bank, u32 row, u32 bit, u32 *val); +int fuse_sense_row(u32 bank, u32 row, u32 *val); +int fuse_prog_bit(u32 bank, u32 row, u32 bit); +int fuse_prog_row(u32 bank, u32 row, u32 val); +int fuse_override_bit(u32 bank, u32 row, u32 bit, u32 val); +int fuse_override_row(u32 bank, u32 row, u32 val); + +#endif /* _FUSE_H_ */ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot -- == Dirk Behme Robert Bosch Car Multimedia GmbH CM-AI/PJ-CF32 Phone: +49 5121 49-3274 Dirk Behme Fax: +49 711 811 5053274 PO Box 77 77 77 mailto:dirk.be...@de.bosch.com D-31132 Hildesheim - Germany Bosch Group, Car Multimedia (CM) Automotive Navigation and Infotainment Systems (AI) ProJect - CoreFunctions (PJ-CF) Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe Sitz: Hildesheim Registergericht: Amtsgericht Hildesheim HRB 201334 Aufsichtsratsvorsitzender: Volkmar Denner Geschäftsführung: Uwe Thomas, Michael Bolle, Robby Drave, Egbert Hellwig == ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] mx6qsabrelite: enable DCache and MMC bounce buffer
The recent U-Boot version 2012.07 has improved drivers (e.g. MMC and network/FEC) regarding DCache handling. So it should be safe to use the DCache on the i.MX6, now. Signed-off-by: Dirk Behme dirk.be...@de.bosch.com --- include/configs/mx6qsabrelite.h |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 0d376ba..995539c 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -73,6 +73,7 @@ #define CONFIG_MMC #define CONFIG_CMD_MMC #define CONFIG_GENERIC_MMC +#define CONFIG_MMC_BOUNCE_BUFFER #define CONFIG_CMD_EXT2 #define CONFIG_CMD_FAT #define CONFIG_DOS_PARTITION @@ -233,8 +234,6 @@ #define CONFIG_OF_LIBFDT #define CONFIG_CMD_BOOTZ -#define CONFIG_SYS_DCACHE_OFF - #ifndef CONFIG_SYS_DCACHE_OFF #define CONFIG_CMD_CACHE #endif -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6q: mx6qsabrelite: add GPIO_0__CCM_CLKO and GPIO_3__CCM_CLKO2 pin mux
On 01.08.2012 13:05, Dirk Behme wrote: A recent Linux kernel (= 3.5) has support for the SGTL 5000 sound on the SabreLite board. To make this work, U-Boot has to configure the pin mux for PAD_GPIO_0__CCM_CLKO and PAD_GPIO_3__CCM_CLKO2 correctly. Taken from Freescale's ER5 U-Boot for the SabreLite. Signed-off-by: Dirk Behme dirk.be...@de.bosch.com CC: Troy Kisky troy.ki...@boundarydevices.com CC: Stefano Babic sba...@denx.de --- board/freescale/mx6qsabrelite/imximage.cfg |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/board/freescale/mx6qsabrelite/imximage.cfg b/board/freescale/mx6qsabrelite/imximage.cfg index 62498ab..2d7825a 100644 --- a/board/freescale/mx6qsabrelite/imximage.cfg +++ b/board/freescale/mx6qsabrelite/imximage.cfg @@ -168,3 +168,6 @@ DATA 4 0x020e0010 0xF0CF # set IPU AXI-id0 Qos=0xf(bypass) AXI-id1 Qos=0x7 DATA 4 0x020e0018 0x007F007F DATA 4 0x020e001c 0x007F007F +# set PAD_GPIO_0__CCM_CLKO and PAD_GPIO_3__CCM_CLKO2 +DATA 4 0x020e0220 0x +DATA 4 0x020e022c 0x0004 \ No newline at end of file This is replaced with a kernel patch http://www.spinics.net/lists/arm-kernel/msg187222.html and therefore this patch is obsolete now. Thanks Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] mx6q: mx6qsabrelite: add GPIO_0__CCM_CLKO and GPIO_3__CCM_CLKO2 pin mux
A recent Linux kernel (= 3.5) has support for the SGTL 5000 sound on the SabreLite board. To make this work, U-Boot has to configure the pin mux for PAD_GPIO_0__CCM_CLKO and PAD_GPIO_3__CCM_CLKO2 correctly. Taken from Freescale's ER5 U-Boot for the SabreLite. Signed-off-by: Dirk Behme dirk.be...@de.bosch.com CC: Troy Kisky troy.ki...@boundarydevices.com CC: Stefano Babic sba...@denx.de --- board/freescale/mx6qsabrelite/imximage.cfg |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/board/freescale/mx6qsabrelite/imximage.cfg b/board/freescale/mx6qsabrelite/imximage.cfg index 62498ab..2d7825a 100644 --- a/board/freescale/mx6qsabrelite/imximage.cfg +++ b/board/freescale/mx6qsabrelite/imximage.cfg @@ -168,3 +168,6 @@ DATA 4 0x020e0010 0xF0CF # set IPU AXI-id0 Qos=0xf(bypass) AXI-id1 Qos=0x7 DATA 4 0x020e0018 0x007F007F DATA 4 0x020e001c 0x007F007F +# set PAD_GPIO_0__CCM_CLKO and PAD_GPIO_3__CCM_CLKO2 +DATA 4 0x020e0220 0x +DATA 4 0x020e022c 0x0004 \ No newline at end of file -- 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6q: mx6qsabrelite: add GPIO_0__CCM_CLKO and GPIO_3__CCM_CLKO2 pin mux
On 01.08.2012 13:26, Liu Hui-R64343 wrote: -Original Message- From: u-boot-boun...@lists.denx.de [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Dirk Behme Sent: Wednesday, August 01, 2012 7:06 PM To: u-boot@lists.denx.de Cc: Dirk Behme Subject: [U-Boot] [PATCH] mx6q: mx6qsabrelite: add GPIO_0__CCM_CLKO and GPIO_3__CCM_CLKO2 pin mux A recent Linux kernel (= 3.5) has support for the SGTL 5000 sound on the SabreLite board. To make this work, U-Boot has to configure the pin mux for PAD_GPIO_0__CCM_CLKO and PAD_GPIO_3__CCM_CLKO2 correctly. Why this can't be set in the kernel but relies on u-boot to configure it? I don't know :( It took me days to find this U-Boot dependency, thanks to Troy helping with this! I enabled SGTL5000 sound in the kernel and it didn't work. Until I found that it works with the ER5 Freescale U-Boot, but not with the recent mainline one. It seems to me that the SGTL5000 kernel feature for the SabreLite was developed with a Freescale U-Boot (patching the kernel with DT append) and not tested with the mainline U-Boot. Best regards Dirk Jason Taken from Freescale's ER5 U-Boot for the SabreLite. Signed-off-by: Dirk Behme dirk.be...@de.bosch.com CC: Troy Kisky troy.ki...@boundarydevices.com CC: Stefano Babic sba...@denx.de --- board/freescale/mx6qsabrelite/imximage.cfg |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) 1.7.0.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot -- == Dirk Behme Robert Bosch Car Multimedia GmbH CM-AI/PJ-CF32 Phone: +49 5121 49-3274 Dirk Behme Fax: +49 711 811 5053274 PO Box 77 77 77 mailto:dirk.be...@de.bosch.com D-31132 Hildesheim - Germany Bosch Group, Car Multimedia (CM) Automotive Navigation and Infotainment Systems (AI) ProJect - CoreFunctions (PJ-CF) Robert Bosch Car Multimedia GmbH - Ein Unternehmen der Bosch Gruppe Sitz: Hildesheim Registergericht: Amtsgericht Hildesheim HRB 201334 Aufsichtsratsvorsitzender: Volkmar Denner Geschäftsführung: Uwe Thomas, Michael Bolle, Robby Drave, Egbert Hellwig == ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6q: mx6qsabrelite: add GPIO_0__CCM_CLKO and GPIO_3__CCM_CLKO2 pin mux
On 01.08.2012 14:55, Wolfgang Denk wrote: Dear Dirk Behme, In message 5019180c.4060...@de.bosch.com you wrote: It seems to me that the SGTL5000 kernel feature for the SabreLite was developed with a Freescale U-Boot (patching the kernel with DT append) and not tested with the mainline U-Boot. Probably. But the question is still why this should be changed in U-Boot. Why doesn't the Linux driver set the pin mux configuration it needs? Sorry, I don't know. The Linux driver developers told me there is no U-Boot dependency. Maybe they could answer this question? CCed. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] mx6q: mx6qsabrelite: add GPIO_0__CCM_CLKO and GPIO_3__CCM_CLKO2 pin mux
On 01.08.2012 15:33, Thomas Petazzoni wrote: Le Wed, 1 Aug 2012 15:22:30 +0200, Dirk Behme dirk.be...@de.bosch.com a écrit : Probably. But the question is still why this should be changed in U-Boot. Why doesn't the Linux driver set the pin mux configuration it needs? Sorry, I don't know. The Linux driver developers told me there is no U-Boot dependency. Maybe they could answer this question? CCed. The kernel has a pinctrl driver for i.MX 6, so I would rather suggest to fix your imx6q-sabrelite.dts Device Tree source file so that the audio device is properly associated with the correct pinmux configuration. I'm using http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=b7879fe6dad97ce08e8df0bf8d408942c436d358 Each device can have a pinctrl-0 DT property, which points to one or more pinmux configurations that are defined in imx6q.dtsi file. You can add additional pinmux configurations here if needed. Sounds like the above is incomplete, then? But yeah, definitely, the kernel shouldn't rely too much on U-Boot having set the right pinmux configuration. Best regards Dirk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] i.MX(6) dcache status?
Hi, now, after U-Boot v2012.07 is released, I'd like to ask what's the status the dcache support for i.MX(6)? Is it safe to enable CONFIG_SYS_DCACHE_OFF now? E.g. for the SabreLite [1]? And if so, do we have to enable CONFIG_MMC_BOUNCE_BUFFER, too? Opinions? Many thanks and best regards Dirk [1] http://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/mx6qsabrelite.h;h=e42fe6b00b445e2ea0623fb682cb758fdf09c586;hb=HEAD#l238 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot