| Message ID | 1408908676-25989-1-git-send-email-mario.schuknecht@dresearch-fe.de |
|---|---|
| State | Superseded |
| Delegated to: | Pantelis Antoniou |
| Headers | show |
On 24.08.2014 21:31, Mario Schuknecht wrote: > In function mvebu_mmc_write notice command timeout. It is possible that a > command is done, but a timeout occurred. > > Enable timeout in set bus function. > > Set window registers. Without that I could not use the driver on a Kirkwood > 88F6282 SoC. > > Set high capacity and 52MHz driver feature. Thanks. A few review comments below. > Signed-off-by: Mario Schuknecht <mario.schuknecht@dresearch-fe.de> > --- > drivers/mmc/mvebu_mmc.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 58 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/mvebu_mmc.c b/drivers/mmc/mvebu_mmc.c > index 9759198..53754aa 100644 > --- a/drivers/mmc/mvebu_mmc.c > +++ b/drivers/mmc/mvebu_mmc.c > @@ -17,8 +17,12 @@ > #include <asm/arch/kirkwood.h> > #include <mvebu_mmc.h> > > +DECLARE_GLOBAL_DATA_PTR; > + > #define DRIVER_NAME "MVEBU_MMC" > > +#define MVEBU_TARGET_DRAM 0 > + > static void mvebu_mmc_write(u32 offs, u32 val) > { > writel(val, CONFIG_SYS_MMC_BASE + (offs)); > @@ -164,6 +168,9 @@ static int mvebu_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, > return TIMEOUT; > } > } > + if (mvebu_mmc_read(SDIO_ERR_INTR_STATUS) & > + (SDIO_ERR_CMD_TIMEOUT | SDIO_ERR_DATA_TIMEOUT)) > + return TIMEOUT; > > /* Handling response */ > if (cmd->resp_type & MMC_RSP_136) { > @@ -271,6 +278,7 @@ static void mvebu_mmc_set_bus(unsigned int bus) > > /* default to maximum timeout */ > ctrl_reg |= SDIO_HOST_CTRL_TMOUT(SDIO_HOST_CTRL_TMOUT_MAX); > + ctrl_reg |= SDIO_HOST_CTRL_TMOUT_EN; > > ctrl_reg |= SDIO_HOST_CTRL_PUSH_PULL_EN; > > @@ -296,6 +304,53 @@ static void mvebu_mmc_set_ios(struct mmc *mmc) > mvebu_mmc_set_clk(mmc->clock); > } > > +/* > + * Set window register. > + */ > +static void mvebu_window_setup(void) > +{ > + int i; > + > + for (i = 0; i < 4; i++) { > + mvebu_mmc_write(WINDOW_CTRL(i), 0); > + mvebu_mmc_write(WINDOW_BASE(i), 0); > + } Spaces instead of tabs for indentation. Please fix globally (checkpatch clean) > + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { > + u32 size, base, attrib; > + > + /* Enable DRAM bank */ > + switch (i) { > + case 0: > + attrib = KWCPU_ATTR_DRAM_CS0; > + break; > + case 1: > + attrib = KWCPU_ATTR_DRAM_CS1; > + break; > + case 2: > + attrib = KWCPU_ATTR_DRAM_CS2; > + break; > + case 3: > + attrib = KWCPU_ATTR_DRAM_CS3; > + break; > + default: > + /* invalide bank, disable access */ > + attrib = 0; > + break; > + } > + > + size = gd->bd->bi_dram[i].size; > + base = gd->bd->bi_dram[i].start; > + if ((size) && (attrib)) size and attrib don't need parentheses here. > + mvebu_mmc_write(WINDOW_CTRL(i), > + MVCPU_WIN_CTRL_DATA(size, MVEBU_TARGET_DRAM, > + attrib, MVCPU_WIN_ENABLE)); > + else > + mvebu_mmc_write(WINDOW_CTRL(i), MVCPU_WIN_DISABLE); Please use correct coding style for multi-line statements. So this should look like this: if (size && attrib) { mvebu_mmc_write(WINDOW_CTRL(i), MVCPU_WIN_CTRL_DATA(size, MVEBU_TARGET_DRAM, attrib, MVCPU_WIN_ENABLE)); } else { mvebu_mmc_write(WINDOW_CTRL(i), MVCPU_WIN_DISABLE); } Thanks, Stefan
2014-08-25 11:55 GMT+02:00 Stefan Roese <sr@denx.de>: > On 24.08.2014 21:31, Mario Schuknecht wrote: > >> In function mvebu_mmc_write notice command timeout. It is possible that a >> command is done, but a timeout occurred. >> >> Enable timeout in set bus function. >> >> Set window registers. Without that I could not use the driver on a >> Kirkwood >> 88F6282 SoC. >> >> Set high capacity and 52MHz driver feature. >> > > Thanks. A few review comments below. > > > Signed-off-by: Mario Schuknecht <mario.schuknecht@dresearch-fe.de> >> --- >> drivers/mmc/mvebu_mmc.c | 59 ++++++++++++++++++++++++++++++ >> ++++++++++++++++++- >> 1 file changed, 58 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/mvebu_mmc.c b/drivers/mmc/mvebu_mmc.c >> index 9759198..53754aa 100644 >> --- a/drivers/mmc/mvebu_mmc.c >> +++ b/drivers/mmc/mvebu_mmc.c >> @@ -17,8 +17,12 @@ >> #include <asm/arch/kirkwood.h> >> #include <mvebu_mmc.h> >> >> +DECLARE_GLOBAL_DATA_PTR; >> + >> #define DRIVER_NAME "MVEBU_MMC" >> >> +#define MVEBU_TARGET_DRAM 0 >> + >> static void mvebu_mmc_write(u32 offs, u32 val) >> { >> writel(val, CONFIG_SYS_MMC_BASE + (offs)); >> @@ -164,6 +168,9 @@ static int mvebu_mmc_send_cmd(struct mmc *mmc, struct >> mmc_cmd *cmd, >> return TIMEOUT; >> } >> } >> + if (mvebu_mmc_read(SDIO_ERR_INTR_STATUS) & >> + (SDIO_ERR_CMD_TIMEOUT | SDIO_ERR_DATA_TIMEOUT)) >> + return TIMEOUT; >> >> /* Handling response */ >> if (cmd->resp_type & MMC_RSP_136) { >> @@ -271,6 +278,7 @@ static void mvebu_mmc_set_bus(unsigned int bus) >> >> /* default to maximum timeout */ >> ctrl_reg |= SDIO_HOST_CTRL_TMOUT(SDIO_HOST_CTRL_TMOUT_MAX); >> + ctrl_reg |= SDIO_HOST_CTRL_TMOUT_EN; >> >> ctrl_reg |= SDIO_HOST_CTRL_PUSH_PULL_EN; >> >> @@ -296,6 +304,53 @@ static void mvebu_mmc_set_ios(struct mmc *mmc) >> mvebu_mmc_set_clk(mmc->clock); >> } >> >> +/* >> + * Set window register. >> + */ >> +static void mvebu_window_setup(void) >> +{ >> + int i; >> + >> + for (i = 0; i < 4; i++) { >> + mvebu_mmc_write(WINDOW_CTRL(i), 0); >> + mvebu_mmc_write(WINDOW_BASE(i), 0); >> + } >> > > Spaces instead of tabs for indentation. Please fix globally (checkpatch > clean) > > A question. What is meant by "Spaces instead of tabs for indentation"? In the whole file (and other files) tabs are used for indentation. Should I create two patches. The first corrects all indentations (spaces instead of tabs) and the second contains the actual patch? > > + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { >> + u32 size, base, attrib; >> + >> + /* Enable DRAM bank */ >> + switch (i) { >> + case 0: >> + attrib = KWCPU_ATTR_DRAM_CS0; >> + break; >> + case 1: >> + attrib = KWCPU_ATTR_DRAM_CS1; >> + break; >> + case 2: >> + attrib = KWCPU_ATTR_DRAM_CS2; >> + break; >> + case 3: >> + attrib = KWCPU_ATTR_DRAM_CS3; >> + break; >> + default: >> + /* invalide bank, disable access */ >> + attrib = 0; >> + break; >> + } >> + >> + size = gd->bd->bi_dram[i].size; >> + base = gd->bd->bi_dram[i].start; >> + if ((size) && (attrib)) >> > > size and attrib don't need parentheses here. > You're right. > > + mvebu_mmc_write(WINDOW_CTRL(i), >> + MVCPU_WIN_CTRL_DATA(size, >> MVEBU_TARGET_DRAM, >> + attrib, MVCPU_WIN_ENABLE)); >> + else >> + mvebu_mmc_write(WINDOW_CTRL(i), >> MVCPU_WIN_DISABLE); >> > > Please use correct coding style for multi-line statements. So this > should look like this: > > if (size && attrib) { > mvebu_mmc_write(WINDOW_CTRL(i), > MVCPU_WIN_CTRL_DATA(size, MVEBU_TARGET_DRAM, > attrib, MVCPU_WIN_ENABLE)); > } else { > mvebu_mmc_write(WINDOW_CTRL(i), MVCPU_WIN_DISABLE); > } > > Ok, I'll change that. Regards, Mario > Thanks, > Stefan > >
On 25.08.2014 12:53, Mario Schuknecht wrote: > +/* > + * Set window register. > + */ > +static void mvebu_window_setup(void) > +{ > + int i; > + > + for (i = 0; i < 4; i++) { > + mvebu_mmc_write(WINDOW_CTRL(i)__, 0); > + mvebu_mmc_write(WINDOW_BASE(i)__, 0); > + } > > > Spaces instead of tabs for indentation. Please fix globally > (checkpatch clean) > > A question. What is meant by "Spaces instead of tabs for indentation"? > In the whole file (and other files) tabs are used for indentation. No. I meant, use only tabs for indentation. In the line above you have used spaces instead of tabs. Checkpatch should warn about those issues. Its recommended to run it before sending patches to the list. Thanks, Stefan
diff --git a/drivers/mmc/mvebu_mmc.c b/drivers/mmc/mvebu_mmc.c index 9759198..53754aa 100644 --- a/drivers/mmc/mvebu_mmc.c +++ b/drivers/mmc/mvebu_mmc.c @@ -17,8 +17,12 @@ #include <asm/arch/kirkwood.h> #include <mvebu_mmc.h> +DECLARE_GLOBAL_DATA_PTR; + #define DRIVER_NAME "MVEBU_MMC" +#define MVEBU_TARGET_DRAM 0 + static void mvebu_mmc_write(u32 offs, u32 val) { writel(val, CONFIG_SYS_MMC_BASE + (offs)); @@ -164,6 +168,9 @@ static int mvebu_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, return TIMEOUT; } } + if (mvebu_mmc_read(SDIO_ERR_INTR_STATUS) & + (SDIO_ERR_CMD_TIMEOUT | SDIO_ERR_DATA_TIMEOUT)) + return TIMEOUT; /* Handling response */ if (cmd->resp_type & MMC_RSP_136) { @@ -271,6 +278,7 @@ static void mvebu_mmc_set_bus(unsigned int bus) /* default to maximum timeout */ ctrl_reg |= SDIO_HOST_CTRL_TMOUT(SDIO_HOST_CTRL_TMOUT_MAX); + ctrl_reg |= SDIO_HOST_CTRL_TMOUT_EN; ctrl_reg |= SDIO_HOST_CTRL_PUSH_PULL_EN; @@ -296,6 +304,53 @@ static void mvebu_mmc_set_ios(struct mmc *mmc) mvebu_mmc_set_clk(mmc->clock); } +/* + * Set window register. + */ +static void mvebu_window_setup(void) +{ + int i; + + for (i = 0; i < 4; i++) { + mvebu_mmc_write(WINDOW_CTRL(i), 0); + mvebu_mmc_write(WINDOW_BASE(i), 0); + } + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) { + u32 size, base, attrib; + + /* Enable DRAM bank */ + switch (i) { + case 0: + attrib = KWCPU_ATTR_DRAM_CS0; + break; + case 1: + attrib = KWCPU_ATTR_DRAM_CS1; + break; + case 2: + attrib = KWCPU_ATTR_DRAM_CS2; + break; + case 3: + attrib = KWCPU_ATTR_DRAM_CS3; + break; + default: + /* invalide bank, disable access */ + attrib = 0; + break; + } + + size = gd->bd->bi_dram[i].size; + base = gd->bd->bi_dram[i].start; + if ((size) && (attrib)) + mvebu_mmc_write(WINDOW_CTRL(i), + MVCPU_WIN_CTRL_DATA(size, MVEBU_TARGET_DRAM, + attrib, MVCPU_WIN_ENABLE)); + else + mvebu_mmc_write(WINDOW_CTRL(i), MVCPU_WIN_DISABLE); + + mvebu_mmc_write(WINDOW_BASE(i), base); + } +} + static int mvebu_mmc_initialize(struct mmc *mmc) { debug("%s: mvebu_mmc_initialize", DRIVER_NAME); @@ -322,6 +377,8 @@ static int mvebu_mmc_initialize(struct mmc *mmc) mvebu_mmc_write(SDIO_NOR_INTR_EN, 0); mvebu_mmc_write(SDIO_ERR_INTR_EN, 0); + mvebu_window_setup(); + /* SW reset */ mvebu_mmc_write(SDIO_SW_RESET, SDIO_SW_RESET_NOW); @@ -342,7 +399,7 @@ static struct mmc_config mvebu_mmc_cfg = { .f_min = MVEBU_MMC_BASE_FAST_CLOCK / MVEBU_MMC_BASE_DIV_MAX, .f_max = MVEBU_MMC_CLOCKRATE_MAX, .voltages = MMC_VDD_32_33 | MMC_VDD_33_34, - .host_caps = MMC_MODE_4BIT | MMC_MODE_HS, + .host_caps = MMC_MODE_4BIT | MMC_MODE_HS | MMC_MODE_HC | MMC_MODE_HS_52MHz, .part_type = PART_TYPE_DOS, .b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT, };
In function mvebu_mmc_write notice command timeout. It is possible that a command is done, but a timeout occurred. Enable timeout in set bus function. Set window registers. Without that I could not use the driver on a Kirkwood 88F6282 SoC. Set high capacity and 52MHz driver feature. Signed-off-by: Mario Schuknecht <mario.schuknecht@dresearch-fe.de> --- drivers/mmc/mvebu_mmc.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)