Re: [U-Boot] [PATCH 08/11] Blackfin: bf60x: add rsi/sdh support
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
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
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
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
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
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
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
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__) ||