Re: [U-Boot] [PATCH V2] exynos: spl: Add a custom spi copy function
Hi Jagan, Simon had sent a V2 by rebasing the patch on MMC series submitted by Amar, but there are no comments on those MMC patches and are not yet merged. I incorporated the review comments given by Wolfgang Denk on top of V1 and sent a V2 patch. Regards, Rajeshwari Shinde. On Mon, Jun 3, 2013 at 12:36 AM, Jagan Teki jagannadh.t...@gmail.com wrote: Hi, I think this needs to send as v3, as Simon sent v2 for this. http://patchwork.ozlabs.org/patch/243139/ -- Thanks, Jagan. On Thu, May 30, 2013 at 12:01 PM, Rajeshwari Shinde rajeshwar...@samsung.com wrote: This patch implements a custom spi_copy funtion to copy u-boot from SF to RAM. This is faster then iROM spi_copy funtion as this runs spi at 50Mhz and also in WORD mode of operation. Changed a printf in pinmux.c to debug just to avoid the compilation error in SPL. Removed enum for boot mode from spl_boot.c as it was already define in spl.h Based on [PATCH V2] spi: exynos: Support word transfers Signed-off-by: Alim Akhtar alim.akh...@samsung.com Signed-off-by: Tom Wai-Hong Tam waih...@chromium.org Signed-off-by: Rajeshwari Shinde rajeshwar...@samsung.com --- Changes in V2: - Corrected the commit message. - Added a SPI timeout check. - Corrected the comments. arch/arm/cpu/armv7/exynos/pinmux.c |2 +- arch/arm/include/asm/arch-exynos/spi.h |2 + board/samsung/smdk5250/spl_boot.c | 136 --- include/configs/exynos5250-dt.h|3 + spl/Makefile |4 + 5 files changed, 132 insertions(+), 15 deletions(-) diff --git a/arch/arm/cpu/armv7/exynos/pinmux.c b/arch/arm/cpu/armv7/exynos/pinmux.c index bd499b4..c484a86 100644 --- a/arch/arm/cpu/armv7/exynos/pinmux.c +++ b/arch/arm/cpu/armv7/exynos/pinmux.c @@ -427,7 +427,7 @@ static int exynos4_pinmux_config(int peripheral, int flags) case PERIPH_ID_SDMMC1: case PERIPH_ID_SDMMC3: case PERIPH_ID_SDMMC4: - printf(SDMMC device %d not implemented\n, peripheral); + debug(SDMMC device %d not implemented\n, peripheral); return -1; default: debug(%s: invalid peripheral %d, __func__, peripheral); diff --git a/arch/arm/include/asm/arch-exynos/spi.h b/arch/arm/include/asm/arch-exynos/spi.h index e67ad27..3430ac1 100644 --- a/arch/arm/include/asm/arch-exynos/spi.h +++ b/arch/arm/include/asm/arch-exynos/spi.h @@ -43,6 +43,8 @@ struct exynos_spi { #define SPI_TIMEOUT_MS 10 +#define SF_READ_DATA_CMD 0x3 + /* SPI_CHCFG */ #define SPI_CH_HS_EN (1 6) #define SPI_CH_RST (1 5) diff --git a/board/samsung/smdk5250/spl_boot.c b/board/samsung/smdk5250/spl_boot.c index c0bcf46..85a5d68 100644 --- a/board/samsung/smdk5250/spl_boot.c +++ b/board/samsung/smdk5250/spl_boot.c @@ -22,17 +22,13 @@ #includecommon.h #includeconfig.h +#include asm/arch/clk.h +#include asm/arch/spi.h +#include asm/arch/pinmux.h +#include asm/arch/periph.h +#include asm/arch/spl.h -enum boot_mode { - BOOT_MODE_MMC = 4, - BOOT_MODE_SERIAL = 20, - /* Boot based on Operating Mode pin settings */ - BOOT_MODE_OM = 32, - BOOT_MODE_USB, /* Boot using USB download */ -}; - - typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst); - typedef u32 (*usb_copy_func_t)(void); +typedef u32 (*usb_copy_func_t)(void); /* * Set/clear program flow prediction and return the previous state. @@ -48,6 +44,119 @@ static int config_branch_prediction(int set_cr_z) return cr CR_Z; } +static void spi_rx_tx(struct exynos_spi *regs, int todo, + void *dinp, void const *doutp, int i) +{ + uint *rxp = (uint *)(dinp + (i * (32 * 1024))); + int rx_lvl, tx_lvl; + uint out_bytes, in_bytes; + + out_bytes = todo; + in_bytes = todo; + setbits_le32(regs-ch_cfg, SPI_CH_RST); + clrbits_le32(regs-ch_cfg, SPI_CH_RST); + writel(((todo * 8) / 32) | SPI_PACKET_CNT_EN, regs-pkt_cnt); + + while (in_bytes) { + uint32_t spi_sts; + int temp; + + spi_sts = readl(regs-spi_sts); + rx_lvl = ((spi_sts 15) 0x7f); + tx_lvl = ((spi_sts 6) 0x7f); + while (tx_lvl 32 out_bytes) { + temp = 0x; + writel(temp, regs-tx_data); + out_bytes -= 4; + tx_lvl += 4; + } + while (rx_lvl = 4 in_bytes) { + temp = readl(regs-rx_data); + if (rxp) + *rxp++ = temp; + in_bytes -= 4; + rx_lvl -= 4; + } + } +} + +/* +* Copy uboot from spi flash to RAM +* +* @parma uboot_sizesize of
Re: [U-Boot] [PATCH V2] exynos: spl: Add a custom spi copy function
Hi, I think this needs to send as v3, as Simon sent v2 for this. http://patchwork.ozlabs.org/patch/243139/ -- Thanks, Jagan. On Thu, May 30, 2013 at 12:01 PM, Rajeshwari Shinde rajeshwar...@samsung.com wrote: This patch implements a custom spi_copy funtion to copy u-boot from SF to RAM. This is faster then iROM spi_copy funtion as this runs spi at 50Mhz and also in WORD mode of operation. Changed a printf in pinmux.c to debug just to avoid the compilation error in SPL. Removed enum for boot mode from spl_boot.c as it was already define in spl.h Based on [PATCH V2] spi: exynos: Support word transfers Signed-off-by: Alim Akhtar alim.akh...@samsung.com Signed-off-by: Tom Wai-Hong Tam waih...@chromium.org Signed-off-by: Rajeshwari Shinde rajeshwar...@samsung.com --- Changes in V2: - Corrected the commit message. - Added a SPI timeout check. - Corrected the comments. arch/arm/cpu/armv7/exynos/pinmux.c |2 +- arch/arm/include/asm/arch-exynos/spi.h |2 + board/samsung/smdk5250/spl_boot.c | 136 --- include/configs/exynos5250-dt.h|3 + spl/Makefile |4 + 5 files changed, 132 insertions(+), 15 deletions(-) diff --git a/arch/arm/cpu/armv7/exynos/pinmux.c b/arch/arm/cpu/armv7/exynos/pinmux.c index bd499b4..c484a86 100644 --- a/arch/arm/cpu/armv7/exynos/pinmux.c +++ b/arch/arm/cpu/armv7/exynos/pinmux.c @@ -427,7 +427,7 @@ static int exynos4_pinmux_config(int peripheral, int flags) case PERIPH_ID_SDMMC1: case PERIPH_ID_SDMMC3: case PERIPH_ID_SDMMC4: - printf(SDMMC device %d not implemented\n, peripheral); + debug(SDMMC device %d not implemented\n, peripheral); return -1; default: debug(%s: invalid peripheral %d, __func__, peripheral); diff --git a/arch/arm/include/asm/arch-exynos/spi.h b/arch/arm/include/asm/arch-exynos/spi.h index e67ad27..3430ac1 100644 --- a/arch/arm/include/asm/arch-exynos/spi.h +++ b/arch/arm/include/asm/arch-exynos/spi.h @@ -43,6 +43,8 @@ struct exynos_spi { #define SPI_TIMEOUT_MS 10 +#define SF_READ_DATA_CMD 0x3 + /* SPI_CHCFG */ #define SPI_CH_HS_EN (1 6) #define SPI_CH_RST (1 5) diff --git a/board/samsung/smdk5250/spl_boot.c b/board/samsung/smdk5250/spl_boot.c index c0bcf46..85a5d68 100644 --- a/board/samsung/smdk5250/spl_boot.c +++ b/board/samsung/smdk5250/spl_boot.c @@ -22,17 +22,13 @@ #includecommon.h #includeconfig.h +#include asm/arch/clk.h +#include asm/arch/spi.h +#include asm/arch/pinmux.h +#include asm/arch/periph.h +#include asm/arch/spl.h -enum boot_mode { - BOOT_MODE_MMC = 4, - BOOT_MODE_SERIAL = 20, - /* Boot based on Operating Mode pin settings */ - BOOT_MODE_OM = 32, - BOOT_MODE_USB, /* Boot using USB download */ -}; - - typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst); - typedef u32 (*usb_copy_func_t)(void); +typedef u32 (*usb_copy_func_t)(void); /* * Set/clear program flow prediction and return the previous state. @@ -48,6 +44,119 @@ static int config_branch_prediction(int set_cr_z) return cr CR_Z; } +static void spi_rx_tx(struct exynos_spi *regs, int todo, + void *dinp, void const *doutp, int i) +{ + uint *rxp = (uint *)(dinp + (i * (32 * 1024))); + int rx_lvl, tx_lvl; + uint out_bytes, in_bytes; + + out_bytes = todo; + in_bytes = todo; + setbits_le32(regs-ch_cfg, SPI_CH_RST); + clrbits_le32(regs-ch_cfg, SPI_CH_RST); + writel(((todo * 8) / 32) | SPI_PACKET_CNT_EN, regs-pkt_cnt); + + while (in_bytes) { + uint32_t spi_sts; + int temp; + + spi_sts = readl(regs-spi_sts); + rx_lvl = ((spi_sts 15) 0x7f); + tx_lvl = ((spi_sts 6) 0x7f); + while (tx_lvl 32 out_bytes) { + temp = 0x; + writel(temp, regs-tx_data); + out_bytes -= 4; + tx_lvl += 4; + } + while (rx_lvl = 4 in_bytes) { + temp = readl(regs-rx_data); + if (rxp) + *rxp++ = temp; + in_bytes -= 4; + rx_lvl -= 4; + } + } +} + +/* +* Copy uboot from spi flash to RAM +* +* @parma uboot_sizesize of u-boot to copy +* @param uboot_addraddress in u-boot to copy +*/ +static void exynos_spi_copy(unsigned int uboot_size, unsigned int uboot_addr) +{ + int upto, todo; + int i, timeout = 100; + struct exynos_spi *regs = (struct exynos_spi *)CONFIG_ENV_SPI_BASE; + + set_spi_clk(PERIPH_ID_SPI1, 5000); /* set spi clock to 50Mhz */ +
Re: [U-Boot] [PATCH v2] EXYNOS: SPL: Add a custom spi copy function
Hi Wolfgang Denk, Thank you for review comments. On Sun, May 12, 2013 at 2:01 AM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message 1368285471-11039-1-git-send-email-...@chromium.org you wrote: From: Rajeshwari Shinde rajeshwar...@samsung.com This CL implements a custom spi_copy funtion to copy u-boot from SF to What is a CL ? Changed a printf in pimux.c to debug just to avoid the the compilation s/the the/the/ ?? Removed the enum for boot mode from spl_boot.c as it was already define in spl.h s/define/defined/ Line length in commit messages should be not more than 72 characters. Will correct these issues and resubmit the patch set soon. +/** + * Copy uboot from spi flash to RAM + * + * @parma uboot_size size of u-boot to copy + * @param uboot_addr address of u-boot to copy + */ Incorrect multiline comment style. Will correct these +static void exynos_spi_copy(unsigned int uboot_size, unsigned int uboot_addr) +{ + int upto, todo; + int i; + struct exynos_spi *regs = (struct exynos_spi *)CONFIG_ENV_SPI_BASE; + + set_spi_clk(PERIPH_ID_SPI1, 5000); /* set spi clock to 50Mhz */ + /* set the spi1 GPIO */ + exynos_pinmux_config(PERIPH_ID_SPI1, PINMUX_FLAG_NONE); + + /* set pktcnt and enable it */ + writel(4 | SPI_PACKET_CNT_EN, regs-pkt_cnt); + /* set FB_CLK_SEL */ + writel(SPI_FB_DELAY_180, regs-fb_clk); + /* set CH_WIDTH and BUS_WIDTH as word */ + setbits_le32(regs-mode_cfg, SPI_MODE_CH_WIDTH_WORD | + SPI_MODE_BUS_WIDTH_WORD); + clrbits_le32(regs-ch_cfg, SPI_CH_CPOL_L); /* CPOL: active high */ + + /* clear rx and tx channel if set priveously */ + clrbits_le32(regs-ch_cfg, SPI_RX_CH_ON | SPI_TX_CH_ON); + + typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst); We do not allow typedefs or variable declarations etc. right in the middle of the code. I'm surprised that checkpatch does not complain here about not to add new typedefs ?? Basically typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst); was removed in my original patch. Will check and remove this from the version of patch set I submit. + /* waiting for TX done */ + while (!(readl(regs-spi_sts) SPI_ST_TX_DONE)) + ; Potentially infinite loop. This (and similar code) should always have a timeout and appropriate error handling. Will add a timeout check here + /* let us our own function to copy u-boot from SF */ Please fix the wording of this comment. Will correct this. diff --git a/spl/Makefile b/spl/Makefile index b5a8de7..5e7816a 100644 --- a/spl/Makefile +++ b/spl/Makefile @@ -94,6 +94,10 @@ LIBS-y += arch/$(ARCH)/cpu/tegra-common/libcputegra-common.o LIBS-y += $(CPUDIR)/tegra-common/libtegra-common.o endif +ifneq ($(CONFIG_EXYNOS4)$(CONFIG_EXYNOS5),) +LIBS-y += $(CPUDIR)/s5p-common/libs5p-common.o +endif This should be done without the ifneq. This is specific to samsung and currently used only by EXYNOS hence it was added with a ifneq Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Love all, trust a few. - William Shakespeare ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot -- Regards, Rajeshwari Shinde ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] EXYNOS: SPL: Add a custom spi copy function
Hi Rajeshwari, On Sat, May 11, 2013 at 9:17 AM, Simon Glass s...@chromium.org wrote: From: Rajeshwari Shinde rajeshwar...@samsung.com This CL implements a custom spi_copy funtion to copy u-boot from SF to RAM. This is faster then iROM spi_copy funtion as this runs spi at 50Mhz and also in WORD mode of operation. Changed a printf in pimux.c to debug just to avoid the the compilation error in SPL. Removed the enum for boot mode from spl_boot.c as it was already define in spl.h Signed-off-by: Alim Akhtar alim.akh...@samsung.com Signed-off-by: Tom Wai-Hong Tam waih...@chromium.org Signed-off-by: Rajeshwari Shinde rajeshwar...@samsung.com Rebased on top of MMC series: Signed-off-by: Simon Glass s...@chromium.org --- Changes in v2: - Rebase on top of MMC series - Fix new checkpatch warnings Acked-by: Simon Glass s...@chromium.org For all your SPI patches (listed below): Tested-by: Simon Glass s...@chromium.org I have applied the MMC series and all of your SPI patches, and tested on snow. Everything works correctly I think. Original SPI speed is very slow: SMDK5250 # time sf read 4000 0 40 time: 1 minutes, 9.906 seconds, 69906 ticks With your SPI patches and my 50MHz device tree patch I get: SMDK5250 # time sf read 4000 0 40 time: 1.427 seconds, 1427 ticks which is a dramatic improvement. If I turn on the caches (remove CONFIG_SYS_DCACHE_OFF from include/configs/exynos5-dt.h) then it goes at full sped: SMDK5250 # time sf read 4000 0 40 time: 0.699 seconds, 699 ticks Note that we are working on some further SPI code improvements locally and may send patches for that, but it will be after the merge window. For references, here are the patches in my tree when I tested (listed in the reverse order they were applied). 660d9fd (HEAD, ws/snow, snow) exynos: dts: Use 50MHz SPI flash speed on snow 0295f39 EXYNOS: SPL: Add a custom spi copy function e712583 EXYNOS: SPI: Support word transfers aac5c2d EXYNOS: SPI: Minimise access to SPI FIFO level 99f2680 EXYNOS: SPI: Support a delay after deactivate cbe66cb EXYNOS: Export timer_get_us() to get microsecond timer 893d152 EXYNOS: SPI: Support SPI_PREAMBLE mode f4cce8b SPI: Add support for preamble bytes f91072a exynos: Enable mmc for snow aa1b39f COMMON: MMC: Command to support EMMC booting and to resize EMMC boot partition a6358a3 SMDK5250: Enable EMMC booting e37cc9f MMC: APIs to support resize of EMMC boot partition 919c7f6 SMDK5250: Initialise and Enable DWMMC, support FDT and non-FDT 2277040 EXYNOS5: DWMMC: Initialise the local variable to avoid unwanted results. 206af80 EXYNOS5: DWMMC: Added FDT support for DWMMC 0e8e41c DWMMC: Initialise dwmci and resolve EMMC read write issues 087075c EXYNOS5: FDT: Add DWMMC device node data 0f2db62 FDT: Add compatible string for DWMMC 6e4e37c EXYNOS5: I2C: Add FDT and non-FDT support for I2C arch/arm/cpu/armv7/exynos/pinmux.c | 2 +- arch/arm/include/asm/arch-exynos/spi.h | 2 + arch/arm/include/asm/arch-exynos/spl.h | 1 + board/samsung/smdk5250/spl_boot.c | 132 + include/configs/exynos5250-dt.h| 3 + spl/Makefile | 4 + 6 files changed, 129 insertions(+), 15 deletions(-) Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] EXYNOS: SPL: Add a custom spi copy function
Dear Simon Glass, In message 1368285471-11039-1-git-send-email-...@chromium.org you wrote: From: Rajeshwari Shinde rajeshwar...@samsung.com This CL implements a custom spi_copy funtion to copy u-boot from SF to What is a CL ? Changed a printf in pimux.c to debug just to avoid the the compilation s/the the/the/ ?? Removed the enum for boot mode from spl_boot.c as it was already define in spl.h s/define/defined/ Line length in commit messages should be not more than 72 characters. +/** + * Copy uboot from spi flash to RAM + * + * @parma uboot_size size of u-boot to copy + * @param uboot_addr address of u-boot to copy + */ Incorrect multiline comment style. +static void exynos_spi_copy(unsigned int uboot_size, unsigned int uboot_addr) +{ + int upto, todo; + int i; + struct exynos_spi *regs = (struct exynos_spi *)CONFIG_ENV_SPI_BASE; + + set_spi_clk(PERIPH_ID_SPI1, 5000); /* set spi clock to 50Mhz */ + /* set the spi1 GPIO */ + exynos_pinmux_config(PERIPH_ID_SPI1, PINMUX_FLAG_NONE); + + /* set pktcnt and enable it */ + writel(4 | SPI_PACKET_CNT_EN, regs-pkt_cnt); + /* set FB_CLK_SEL */ + writel(SPI_FB_DELAY_180, regs-fb_clk); + /* set CH_WIDTH and BUS_WIDTH as word */ + setbits_le32(regs-mode_cfg, SPI_MODE_CH_WIDTH_WORD | + SPI_MODE_BUS_WIDTH_WORD); + clrbits_le32(regs-ch_cfg, SPI_CH_CPOL_L); /* CPOL: active high */ + + /* clear rx and tx channel if set priveously */ + clrbits_le32(regs-ch_cfg, SPI_RX_CH_ON | SPI_TX_CH_ON); + + typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst); We do not allow typedefs or variable declarations etc. right in the middle of the code. I'm surprised that checkpatch does not complain here about not to add new typedefs ?? + /* waiting for TX done */ + while (!(readl(regs-spi_sts) SPI_ST_TX_DONE)) + ; Potentially infinite loop. This (and similar code) should always have a timeout and appropriate error handling. + /* let us our own function to copy u-boot from SF */ Please fix the wording of this comment. diff --git a/spl/Makefile b/spl/Makefile index b5a8de7..5e7816a 100644 --- a/spl/Makefile +++ b/spl/Makefile @@ -94,6 +94,10 @@ LIBS-y += arch/$(ARCH)/cpu/tegra-common/libcputegra-common.o LIBS-y += $(CPUDIR)/tegra-common/libtegra-common.o endif +ifneq ($(CONFIG_EXYNOS4)$(CONFIG_EXYNOS5),) +LIBS-y += $(CPUDIR)/s5p-common/libs5p-common.o +endif This should be done without the ifneq. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Love all, trust a few. - William Shakespeare ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot