On 08/30/2012 10:19 AM, Seungwon Jeon wrote:
> On Tuesday, August 29, 2012, Jaehoon Chung <[email protected]> wrote:
>> On 08/29/2012 10:21 AM, Seungwon Jeon wrote:
>>> On Tuesday, August 28, 2012, Jaehoon Chung <[email protected]> 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 <[email protected]>
>>>> ---
>>>>  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.
> Ok, I mentioned no check busy-status because host doesn't send command to 
> card in the middle of host's clock update.
> If you have a opinion, please let me know.
OK, i will apply with your opinion.
Then resend the patch.

Best Regards,
Jaehoon Chung
> 
> Thanks,
> Seungwon Jeon
> 
>>>>
>>>>            /* 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 [email protected]
>>> 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 [email protected]
>> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to