Re: [U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support

2013-03-04 Thread Wolfgang Denk
Dear Sonic Zhang,

In message cajxxz0o_ozk7w_cdfjkzpwdmfqwt1m-yn_bz6yzhrdfqt6w...@mail.gmail.com 
you wrote:
 
 Maybe I didn't describe it clearly. Yes, I return 0 at the end of this
 function. But, the same function may return UNUSABLE_ERR(-17) at the
 beginning if the data flags match MMC_DATA_WRITE. That's why the
 function can't return void. Is anything contradicting in my
 explanation?

I see.  Sorry, I missed that other return.

One additional question, though:

 /* kick off transfer */
 bfin_write_SDH_DATA_CTL(bfin_read_SDH_DATA_CTL() | DTX_DMA_E | DTX_E);
 
 -   return ret;
 +   return 0;

Are the bfin_read_SDH_DATA_CTL() and bfin_write_SDH_DATA_CTL()
supposed to always work, i. e. are we positively sure that these can
never fail, so there is no need to test the return code and handle
error conditions?

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
People have one thing in common: they are all different.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support

2013-03-04 Thread Sonic Zhang
Hi Wolfgang,

On Mon, Mar 4, 2013 at 7:21 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Sonic Zhang,

 In message 
 cajxxz0o_ozk7w_cdfjkzpwdmfqwt1m-yn_bz6yzhrdfqt6w...@mail.gmail.com you 
 wrote:

 Maybe I didn't describe it clearly. Yes, I return 0 at the end of this
 function. But, the same function may return UNUSABLE_ERR(-17) at the
 beginning if the data flags match MMC_DATA_WRITE. That's why the
 function can't return void. Is anything contradicting in my
 explanation?

 I see.  Sorry, I missed that other return.

 One additional question, though:

 /* kick off transfer */
 bfin_write_SDH_DATA_CTL(bfin_read_SDH_DATA_CTL() | DTX_DMA_E | 
 DTX_E);

 -   return ret;
 +   return 0;

 Are the bfin_read_SDH_DATA_CTL() and bfin_write_SDH_DATA_CTL()
 supposed to always work, i. e. are we positively sure that these can
 never fail, so there is no need to test the return code and handle
 error conditions?

Yes, bfin_write_XXX and bfin_read_XXX are simply defined as memory
access operations. They either succeed or cause hardware error
exception.

#define bfin_read_SDH_DATA_CTL()   bfin_read16(SDH_DATA_CTL)
#define _bfin_readX(addr, size, asm_size, asm_ext) ({ \
u32 __v; \
__asm__ __volatile__( \
%0 =  #asm_size [%1] #asm_ext ; \
: =d (__v) \
: a (addr) \
); \
__v; })


Regards,

Sonic
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support

2013-02-17 Thread Wolfgang Denk
Dear Sonic Zhang,

In message cajxxz0otwow6x2kx8yaujtrxc6qicvpmo2rp4mhwjqvqubh...@mail.gmail.com 
you wrote:

  @@ -113,16 +131,19 @@ static int sdh_setup_data(struct mmc *mmc, struct 
  mmc_data *data)
  ...
  - int ret = 0;
  ...
  - return ret;
  + return 0;
 
  If this function can always only return 0, then please make it void.
 
  Please fix globally.
 
 I am sorry, this function can't be changed to void, because it may
 return error. See bellow code.

You are contradicting yourself.  You _always_ return a 0 here.  Your
code CANNOT return an error code.

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
Build a system that even a fool can use and only a fool will want  to
use it.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support

2013-02-17 Thread Sonic Zhang
Hi Wolfgang,

On Mon, Feb 18, 2013 at 4:15 AM, Wolfgang Denk w...@denx.de wrote:
 Dear Sonic Zhang,

 In message 
 cajxxz0otwow6x2kx8yaujtrxc6qicvpmo2rp4mhwjqvqubh...@mail.gmail.com you 
 wrote:

  @@ -113,16 +131,19 @@ static int sdh_setup_data(struct mmc *mmc, struct 
  mmc_data *data)
  ...
  - int ret = 0;
  ...
  - return ret;
  + return 0;
 
  If this function can always only return 0, then please make it void.
 
  Please fix globally.

 I am sorry, this function can't be changed to void, because it may
 return error. See bellow code.

 You are contradicting yourself.  You _always_ return a 0 here.  Your
 code CANNOT return an error code.


Maybe I didn't describe it clearly. Yes, I return 0 at the end of this
function. But, the same function may return UNUSABLE_ERR(-17) at the
beginning if the data flags match MMC_DATA_WRITE. That's why the
function can't return void. Is anything contradicting in my
explanation?


@@ -113,16 +131,19 @@ static int sdh_setup_data(struct mmc *mmc,
struct mmc_data *data)
 {
u16 data_ctl = 0;
u16 dma_cfg = 0;
-   int ret = 0;
unsigned long data_size = data-blocksize * data-blocks;

/* Don't support write yet. */
if (data-flags  MMC_DATA_WRITE)
return UNUSABLE_ERR;

@@ -137,7 +158,7 @@ static int sdh_setup_data(struct mmc *mmc, struct
mmc_data *data)
/* kick off transfer */
bfin_write_SDH_DATA_CTL(bfin_read_SDH_DATA_CTL() | DTX_DMA_E | DTX_E);

-   return ret;
+   return 0;
 }



Regards,

Sonic Zhang
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support

2013-02-08 Thread Sonic Zhang
On Thu, Feb 7, 2013 at 6:17 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Sonic Zhang,

 In message 1360223258-6945-9-git-send-email-sonic@gmail.com you wrote:
 From: Sonic Zhang sonic.zh...@analog.com

 Add rsi/sdh support for bf60x.

 Checkpatch errors.  Please fix.

 @@ -113,16 +131,19 @@ static int sdh_setup_data(struct mmc *mmc, struct 
 mmc_data *data)
 ...
 - int ret = 0;
 ...
 - return ret;
 + return 0;

 If this function can always only return 0, then please make it void.

 Please fix globally.

I am sorry, this function can't be changed to void, because it may
return error. See bellow code.

static int sdh_setup_data(struct mmc *mmc, struct mmc_data *data)
{
u16 data_ctl = 0;
u16 dma_cfg = 0;
unsigned long data_size = data-blocksize * data-blocks;

/* Don't support write yet. */
if (data-flags  MMC_DATA_WRITE)
return UNUSABLE_ERR;
..

Regards,

Sonic
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support

2013-02-07 Thread Wolfgang Denk
Dear Sonic Zhang,

In message 1360223258-6945-9-git-send-email-sonic@gmail.com you wrote:
 From: Sonic Zhang sonic.zh...@analog.com
 
 Add rsi/sdh support for bf60x.

Checkpatch errors.  Please fix.

 @@ -113,16 +131,19 @@ static int sdh_setup_data(struct mmc *mmc, struct 
 mmc_data *data)
...
 - int ret = 0;
...
 - return ret;
 + return 0;

If this function can always only return 0, then please make it void.

Please fix globally.


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
It is dangerous to be right on a subject  on  which  the  established
authorities are wrong.-- Voltaire
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support

2013-02-07 Thread Sonic Zhang
On Thu, Feb 7, 2013 at 6:17 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Sonic Zhang,

 In message 1360223258-6945-9-git-send-email-sonic@gmail.com you wrote:
 From: Sonic Zhang sonic.zh...@analog.com

 Add rsi/sdh support for bf60x.

 Checkpatch errors.  Please fix.

OK


 @@ -113,16 +131,19 @@ static int sdh_setup_data(struct mmc *mmc, struct 
 mmc_data *data)
 ...
 - int ret = 0;
 ...
 - return ret;
 + return 0;

 If this function can always only return 0, then please make it void.

 Please fix globally.


Yes

Thanks,

Sonic
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support

2013-02-06 Thread Sonic Zhang
From: Sonic Zhang sonic.zh...@analog.com

Add rsi/sdh support for bf60x.

Signed-off-by: Sonic Zhang sonic.zh...@analog.com
Signed-off-by: Bob Liu lliu...@gmail.com
---
 arch/blackfin/include/asm/mach-common/bits/sdh.h |   38 +++-
 drivers/mmc/bfin_sdh.c   |   68 -
 2 files changed, 88 insertions(+), 18 deletions(-)

diff --git a/arch/blackfin/include/asm/mach-common/bits/sdh.h 
b/arch/blackfin/include/asm/mach-common/bits/sdh.h
index 8c5dd33..3495558 100644
--- a/arch/blackfin/include/asm/mach-common/bits/sdh.h
+++ b/arch/blackfin/include/asm/mach-common/bits/sdh.h
@@ -12,18 +12,35 @@
 #define CMD_INT_E  0x100  /* Command Interrupt */
 #defineCMD_PEND_E  0x200  /* Command Pending */
 #define CMD_E  0x400  /* Command Enable */
+#ifdef RSI_BLKSZ
+#define   CMD_CRC_CHECK_D  0x800  /* CRC Check is disabled */
+#defineCMD_DATA0_BUSY  0x1000 /* Check for Busy State on the 
DATA0 pin */
+#endif
 
 /* Bit masks for SDH_PWR_CTL */
+#ifndef RSI_BLKSZ
 #definePWR_ON  0x3/* Power On */
 #define SD_CMD_OD  0x40   /* Open Drain Output */
 #define   ROD_CTL  0x80   /* Rod Control */
+#endif
 
 /* Bit masks for SDH_CLK_CTL */
 #defineCLKDIV  0xff   /* MC_CLK Divisor */
 #define CLK_E  0x100  /* MC_CLK Bus Clock Enable */
 #define  PWR_SV_E  0x200  /* Power Save Enable */
 #define CLKDIV_BYPASS  0x400  /* Bypass Divisor */
-#define  WIDE_BUS  0x800  /* Wide Bus Mode Enable */
+#define BUS_MODE_MASK  0x1800 /* Bus Mode Mask */
+#define STD_BUS_1  0x000  /* Standard Bus 1 bit mode */
+#defineWIDE_BUS_4  0x800  /* Wide Bus 4 bit mode */
+#defineBYTE_BUS_8  0x1000 /* Byte Bus 8 bit mode */
+#ifdef RSI_BLKSZ
+#defineCARD_TYPE_MASK  0xe000 /* Card type mask */
+#define  CARD_TYPE_OFFSET  13 /* Card type offset */
+#defineCARD_TYPE_SDIO  0
+#defineCARD_TYPE_eMMC  1
+#define  CARD_TYPE_SD  2
+#define   CARD_TYPE_CEATA  3
+#endif
 
 /* Bit masks for SDH_RESP_CMD */
 #define  RESP_CMD  0x3f   /* Response Command */
@@ -33,7 +50,13 @@
 #define   DTX_DIR  0x2/* Data Transfer Direction */
 #define  DTX_MODE  0x4/* Data Transfer Mode */
 #define DTX_DMA_E  0x8/* Data Transfer DMA Enable */
+#ifndef RSI_BLKSZ
 #define  DTX_BLK_LGTH  0xf0   /* Data Transfer Block Length */
+#else
+
+/* Bit masks for SDH_BLK_SIZE */
+#define  DTX_BLK_LGTH  0x1fff /* Data Transfer Block Length */
+#endif
 
 /* Bit masks for SDH_STATUS */
 #define  CMD_CRC_FAIL  0x1/* CMD CRC Fail */
@@ -102,10 +125,13 @@
 /* Bit masks for SDH_E_STATUS */
 #define  SDIO_INT_DET  0x2/* SDIO Int Detected */
 #define   SD_CARD_DET  0x10   /* SD Card Detect */
+#define  SD_CARD_BUSYMODE  0x8000 /* Card is in Busy mode */
+#define   SD_CARD_SLPMODE  0x4000 /* Card in Sleep Mode */
+#define SD_CARD_READY  0x0002 /* Card Ready */
 
 /* Bit masks for SDH_E_MASK */
 #define  SDIO_MSK  0x2/* Mask SDIO Int Detected */
-#define   SCD_MSK  0x40   /* Mask Card Detect */
+#define   SCD_MSK  0x10   /* Mask Card Detect */
 
 /* Bit masks for SDH_CFG */
 #define   CLKS_EN  0x1/* Clocks Enable */
@@ -114,7 +140,15 @@
 #defineSD_RST  0x10   /* SDMMC Reset */
 #define PUP_SDDAT  0x20   /* Pull-up SD_DAT */
 #definePUP_SDDAT3  0x40   /* Pull-up SD_DAT3 */
+#ifndef RSI_BLKSZ
 #define PD_SDDAT3  0x80   /* Pull-down SD_DAT3 */
+#else
+#definePWR_ON  0x600  /* Power On */
+#define SD_CMD_OD  0x800  /* Open Drain Output */
+#define   BOOT_EN  0x1000 /* Boot Enable */
+#define BOOT_MODE  0x2000 /* Alternate Boot Mode */.
+#define   BOOT_ACK_EN  0x4000 /* Boot ACK is expected */
+#endif
 
 /* Bit masks for SDH_RD_WAIT_EN */
 #define   RWR  0x1/* Read Wait Request */
diff --git a/drivers/mmc/bfin_sdh.c b/drivers/mmc/bfin_sdh.c
index 8d59d46..f22429a 100644
--- a/drivers/mmc/bfin_sdh.c
+++ b/drivers/mmc/bfin_sdh.c
@@ -19,9 +19,7 @@
 #include asm/mach-common/bits/sdh.h
 #include asm/mach-common/bits/dma.h
 
-#if defined(__ADSPBF50x__) || defined(__ADSPBF51x__)
-# define bfin_read_SDH_PWR_CTL bfin_read_RSI_PWR_CONTROL
-# define bfin_write_SDH_PWR_CTLbfin_write_RSI_PWR_CONTROL
+#if defined(__ADSPBF50x__) ||