Hi Wolfgang Denk,
Thank you for review comments.
On Sun, May 12, 2013 at 2:01 AM, Wolfgang Denk wd@denx.de wrote:
Dear Simon Glass,
In message 1368285471-11039-1-git-send-email-sjg@chromium.org you wrote:
From: Rajeshwari Shinde rajeshwari.s@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, 50000000); /* 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, ®s->pkt_cnt);/* set FB_CLK_SEL */writel(SPI_FB_DELAY_180, ®s->fb_clk);/* set CH_WIDTH and BUS_WIDTH as word */setbits_le32(®s->mode_cfg, SPI_MODE_CH_WIDTH_WORD |SPI_MODE_BUS_WIDTH_WORD);clrbits_le32(®s->ch_cfg, SPI_CH_CPOL_L); /* CPOL: active high *//* clear rx and tx channel if set priveously */clrbits_le32(®s->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(®s->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: wd@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