[U-Boot] [PATCH] mvebu_mmc: Driver addition

[Mario Schuknecht]

2014-08-25 11:55 GMT+02:00 Stefan Roese <sr at 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 at 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
>
>