diff mbox

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

Message ID 1360223258-6945-9-git-send-email-sonic.adi@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Sonic Zhang Feb. 7, 2013, 7:47 a.m. UTC 
From: Sonic Zhang <sonic.zhang@analog.com>

Add rsi/sdh support for bf60x.

Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Signed-off-by: Bob Liu <lliubbo@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(-)

Comments

Wolfgang Denk Feb. 7, 2013, 10:17 a.m. UTC | #1 
Dear Sonic Zhang,

In message <1360223258-6945-9-git-send-email-sonic.adi@gmail.com> you wrote:
> From: Sonic Zhang <sonic.zhang@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
Sonic Zhang Feb. 8, 2013, 4:33 a.m. UTC | #2 
On Thu, Feb 7, 2013 at 6:17 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Sonic Zhang,
>
> In message <1360223258-6945-9-git-send-email-sonic.adi@gmail.com> you wrote:
>> From: Sonic Zhang <sonic.zhang@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
Sonic Zhang Feb. 8, 2013, 8:35 a.m. UTC | #3 
On Thu, Feb 7, 2013 at 6:17 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Sonic Zhang,
>
> In message <1360223258-6945-9-git-send-email-sonic.adi@gmail.com> you wrote:
>> From: Sonic Zhang <sonic.zhang@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
Wolfgang Denk Feb. 17, 2013, 8:15 p.m. UTC | #4 
Dear Sonic Zhang,

In message <CAJxxZ0OTwow6X2KX8yAujtrxc6qicVPMO2rP4MHwJQVQUbhecw@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
Sonic Zhang Feb. 18, 2013, 7:38 a.m. UTC | #5 
Hi Wolfgang,

On Mon, Feb 18, 2013 at 4:15 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Sonic Zhang,
>
> In message <CAJxxZ0OTwow6X2KX8yAujtrxc6qicVPMO2rP4MHwJQVQUbhecw@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
Wolfgang Denk March 4, 2013, 11:21 a.m. UTC | #6 
Dear Sonic Zhang,

In message <CAJxxZ0O_OZk7w_CDfJkzPwDMFqwT1m-Yn_Bz6yzHRDFqT6wrBw@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
Sonic Zhang March 5, 2013, 2:22 a.m. UTC | #7 
Hi Wolfgang,

On Mon, Mar 4, 2013 at 7:21 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Sonic Zhang,
>
> In message <CAJxxZ0O_OZk7w_CDfJkzPwDMFqwT1m-Yn_Bz6yzHRDFqT6wrBw@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
diff mbox

Patch

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 */
 #define                CMD_PEND_E  0x200      /* Command Pending */
 #define                     CMD_E  0x400      /* Command Enable */
+#ifdef RSI_BLKSZ
+#define           CMD_CRC_CHECK_D  0x800      /* CRC Check is disabled */
+#define            CMD_DATA0_BUSY  0x1000     /* Check for Busy State on the DATA0 pin */
+#endif
 
 /* Bit masks for SDH_PWR_CTL */
+#ifndef RSI_BLKSZ
 #define                    PWR_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 */
 #define                    CLKDIV  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 */
+#define                WIDE_BUS_4  0x800      /* Wide Bus 4 bit mode */
+#define                BYTE_BUS_8  0x1000     /* Byte Bus 8 bit mode */
+#ifdef RSI_BLKSZ
+#define            CARD_TYPE_MASK  0xe000     /* Card type mask */
+#define          CARD_TYPE_OFFSET  13         /* Card type offset */
+#define            CARD_TYPE_SDIO  0
+#define            CARD_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  0x80000000 /* Card is in Busy mode */
+#define           SD_CARD_SLPMODE  0x40000000 /* Card in Sleep Mode */
+#define             SD_CARD_READY  0x00020000 /* 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 @@ 
 #define                    SD_RST  0x10       /* SDMMC Reset */
 #define                 PUP_SDDAT  0x20       /* Pull-up SD_DAT */
 #define                PUP_SDDAT3  0x40       /* Pull-up SD_DAT3 */
+#ifndef RSI_BLKSZ
 #define                 PD_SDDAT3  0x80       /* Pull-down SD_DAT3 */
+#else
+#define                    PWR_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_CTL		bfin_write_RSI_PWR_CONTROL
+#if defined(__ADSPBF50x__) || defined(__ADSPBF51x__) || defined(__ADSPBF60x__)
 # define bfin_read_SDH_CLK_CTL		bfin_read_RSI_CLK_CONTROL
 # define bfin_write_SDH_CLK_CTL		bfin_write_RSI_CLK_CONTROL
 # define bfin_write_SDH_ARGUMENT	bfin_write_RSI_ARGUMENT
@@ -38,10 +36,21 @@ 
 # define bfin_write_SDH_STATUS_CLR 	bfin_write_RSI_STATUSCL
 # define bfin_read_SDH_CFG		bfin_read_RSI_CONFIG
 # define bfin_write_SDH_CFG		bfin_write_RSI_CONFIG
+# if defined(__ADSPBF60x__)
+# define bfin_read_SDH_BLK_SIZE		bfin_read_RSI_BLKSZ
+# define bfin_write_SDH_BLK_SIZE	bfin_write_RSI_BLKSZ
+# define bfin_write_DMA_START_ADDR	bfin_write_DMA10_START_ADDR
+# define bfin_write_DMA_X_COUNT		bfin_write_DMA10_X_COUNT
+# define bfin_write_DMA_X_MODIFY	bfin_write_DMA10_X_MODIFY
+# define bfin_write_DMA_CONFIG		bfin_write_DMA10_CONFIG
+# else
+# define bfin_read_SDH_PWR_CTL		bfin_read_RSI_PWR_CONTROL
+# define bfin_write_SDH_PWR_CTL		bfin_write_RSI_PWR_CONTROL
 # define bfin_write_DMA_START_ADDR	bfin_write_DMA4_START_ADDR
 # define bfin_write_DMA_X_COUNT		bfin_write_DMA4_X_COUNT
 # define bfin_write_DMA_X_MODIFY	bfin_write_DMA4_X_MODIFY
 # define bfin_write_DMA_CONFIG		bfin_write_DMA4_CONFIG
+# endif
 # define PORTMUX_PINS \
 	{ P_RSI_DATA0, P_RSI_DATA1, P_RSI_DATA2, P_RSI_DATA3, P_RSI_CMD, P_RSI_CLK, 0 }
 #elif defined(__ADSPBF54x__)
@@ -70,6 +79,9 @@  sdh_send_cmd(struct mmc *mmc, struct mmc_cmd *mmc_cmd)
 		sdh_cmd |= CMD_RSP;
 	if (flags & MMC_RSP_136)
 		sdh_cmd |= CMD_L_RSP;
+#ifdef RSI_BLKSZ
+	sdh_cmd |= CMD_DATA0_BUSY;
+#endif
 
 	bfin_write_SDH_ARGUMENT(arg);
 	bfin_write_SDH_COMMAND(sdh_cmd);
@@ -104,6 +116,12 @@  sdh_send_cmd(struct mmc *mmc, struct mmc_cmd *mmc_cmd)
 
 	bfin_write_SDH_STATUS_CLR(CMD_SENT_STAT | CMD_RESP_END_STAT |
 				CMD_TIMEOUT_STAT | CMD_CRC_FAIL_STAT);
+#ifdef RSI_BLKSZ
+	/* wait till card ready */
+	while (!(bfin_read_RSI_ESTAT() & SD_CARD_READY))
+		;
+	bfin_write_RSI_ESTAT(SD_CARD_READY);
+#endif
 
 	return ret;
 }
@@ -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;
+#ifndef RSI_BLKSZ
 	data_ctl |= ((ffs(data_size) - 1) << 4);
+#else
+	bfin_write_SDH_BLK_SIZE(data_size);
+#endif
 	data_ctl |= DTX_DIR;
 	bfin_write_SDH_DATA_CTL(data_ctl);
-	dma_cfg = WDSIZE_32 | RESTART | WNR | DMAEN;
+	dma_cfg = WDSIZE_32 | PSIZE_32 | RESTART | WNR | DMAEN;
 
 	bfin_write_SDH_DATA_TIMER(-1);
 
@@ -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;
 }
 
 
@@ -147,13 +168,23 @@  static int bfin_sdh_request(struct mmc *mmc, struct mmc_cmd *cmd,
 	u32 status;
 	int ret = 0;
 
+	if (data) {
+		ret = sdh_setup_data(mmc, data);
+		if (ret)
+			return ret;
+	}
+
 	ret = sdh_send_cmd(mmc, cmd);
 	if (ret) {
+		bfin_write_SDH_COMMAND(0);
+		bfin_write_DMA_CONFIG(0);
+		bfin_write_SDH_DATA_CTL(0);
+		SSYNC();
 		printf("sending CMD%d failed\n", cmd->cmdidx);
 		return ret;
 	}
+
 	if (data) {
-		ret = sdh_setup_data(mmc, data);
 		do {
 			udelay(1);
 			status = bfin_read_SDH_STATUS();
@@ -208,10 +239,12 @@  static void bfin_sdh_set_ios(struct mmc *mmc)
 
 	if (mmc->bus_width == 4) {
 		cfg = bfin_read_SDH_CFG();
-		cfg &= ~0x80;
-		cfg |= 0x40;
+#ifndef RSI_BLKSZ
+		cfg &= ~PD_SDDAT3;
+#endif
+		cfg |= PUP_SDDAT3;
 		bfin_write_SDH_CFG(cfg);
-		clk_ctl |= WIDE_BUS;
+		clk_ctl |= WIDE_BUS_4;
 	}
 	bfin_write_SDH_CLK_CTL(clk_ctl);
 	sdh_set_clk(mmc->clock);
@@ -220,20 +253,23 @@  static void bfin_sdh_set_ios(struct mmc *mmc)
 static int bfin_sdh_init(struct mmc *mmc)
 {
 	const unsigned short pins[] = PORTMUX_PINS;
-	u16 pwr_ctl = 0;
+	int ret;
 
 	/* Initialize sdh controller */
-	peripheral_request_list(pins, "bfin_sdh");
+	ret = peripheral_request_list(pins, "bfin_sdh");
+	if (ret < 0)
+		return ret;
 #if defined(__ADSPBF54x__)
 	bfin_write_DMAC1_PERIMUX(bfin_read_DMAC1_PERIMUX() | 0x1);
 #endif
 	bfin_write_SDH_CFG(bfin_read_SDH_CFG() | CLKS_EN);
 	/* Disable card detect pin */
 	bfin_write_SDH_CFG((bfin_read_SDH_CFG() & 0x1F) | 0x60);
-
-	pwr_ctl |= ROD_CTL;
-	pwr_ctl |= PWR_ON;
-	bfin_write_SDH_PWR_CTL(pwr_ctl);
+#ifndef RSI_BLKSZ
+	bfin_write_SDH_PWR_CTL(PWR_ON | ROD_CTL);
+#else
+	bfin_write_SDH_CFG(bfin_read_SDH_CFG() | PWR_ON);
+#endif
 	return 0;
 }