On 08/29/2012 10:21 AM, Seungwon Jeon wrote:
> On Tuesday, August 28, 2012, Jaehoon Chung <jh80.ch...@samsung.com> wrote:
>> synopsys spec is mentioned. (7.1 Software/Hardware Restrictions)
>> "Before disabling clock, ensure that card is not busy due to any previous 
>> data command."
>>
>> At specific board, i found the problem that didn't inform ciu.
>> (It didn't produce always.)
>> At that time, i checked the status register whether card is busy or not.
>> (Start bit of CMD register is cleared, but card is busy.)
>> Current code didn't check the card status. Just check whether the start-bit 
>> cleared.
>> host regard that command is completed, will send the next command.
>> But next command didn't issue because Card is busy.
>> This patch can prevent this situation.
>>
>> Signed-off-by: Jaehoon Chung <jh80.ch...@samsung.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c |   27 +++++++++++++++++++++------
>>  drivers/mmc/host/dw_mmc.h |    1 +
>>  2 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 36f98c0..e30d3ed 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -623,6 +623,24 @@ static void mci_send_cmd(struct dw_mci_slot *slot, u32 
>> cmd, u32 arg)
>>              cmd, arg, cmd_status);
>>  }
>>
>> +static void dw_mci_inform_ciu(struct dw_mci_slot *slot)
>> +{
>> +    struct dw_mci *host = slot->host;
>> +    unsigned long timeout = jiffies + msecs_to_jiffies(10);
> 10ms is from test?
It's tested...it's not exactly value..but never spent the time. 
> 
>> +    u32 ctrl, status;
>> +
>> +    do {
>> +            status = mci_readl(host, STATUS);
>> +            if (!(status & SDMMC_DATA_BUSY))
>> +                    break;
>> +            ctrl = mci_readl(host, CTRL);
>> +            ctrl |= SDMMC_CTRL_RESET;
>> +            mci_writel(host, CTRL, ctrl);
>> +    } while (time_before(jiffies, timeout));
>> +
>> +    mci_send_cmd(slot, SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
>> +}
>> +
>>  static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>>  {
>>      struct dw_mci *host = slot->host;
>> @@ -650,15 +668,13 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
> According to your quotation of manual("Before disabling clock, ...),
> Place of dw_mci_inform_ciu is proper here?
manual is mentioned "before the disabling clock". but i think no problem before 
send the command.
> 
>       dw_mci_inform_ciu(slot);
>       
>           /* disable clock */
>          mci_writel(host, CLKENA, 0);
>          mci_writel(host, CLKSRC, 0);
> 
>>              mci_writel(host, CLKSRC, 0);
>>
>>              /* inform CIU */
>> -            mci_send_cmd(slot,
>> -                         SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
>> +            dw_mci_inform_ciu(slot);
>>
>>              /* set clock to desired speed */
>>              mci_writel(host, CLKDIV, div);
>>
>>              /* inform CIU */
>> -            mci_send_cmd(slot,
>> -                         SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
>> +            dw_mci_inform_ciu(slot);
> And then, since start to change clock sequence, there is no need to check 
> busy-status.
> Just need to update clock with origin code.
Right, didn't check the busy-status. but there is also no reason that didn't 
need to check busy-status.
>>
>>              /* enable clock; only low power if no SDIO */
>>              clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>> @@ -667,8 +683,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot)
>>              mci_writel(host, CLKENA, clk_en_a);
>>
>>              /* inform CIU */
>> -            mci_send_cmd(slot,
>> -                         SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT, 0);
>> +            dw_mci_inform_ciu(slot);
> Same here.
> 
> Thanks,
> Seungwon Jeon
>>
>>              host->current_speed = slot->clock;
>>      }
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 15c27e1..b6a1a78 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -127,6 +127,7 @@
>>  #define SDMMC_CMD_INDX(n)           ((n) & 0x1F)
>>  /* Status register defines */
>>  #define SDMMC_GET_FCNT(x)           (((x)>>17) & 0x1FFF)
>> +#define SDMMC_DATA_BUSY                     BIT(9)
>>  /* Internal DMAC interrupt defines */
>>  #define SDMMC_IDMAC_INT_AI          BIT(9)
>>  #define SDMMC_IDMAC_INT_NI          BIT(8)
>> --
>> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to