Re: [U-Boot] [PATCH V2] exynos: spl: Add a custom spi copy function

2013-06-05 Thread Rajeshwari Birje
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

2013-06-02 Thread Jagan Teki
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

2013-05-28 Thread Rajeshwari Birje
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

2013-05-11 Thread Simon Glass
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

2013-05-11 Thread Wolfgang Denk
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