On 08/30/2012 10:19 AM, Seungwon Jeon wrote:
> On Tuesday, August 29, 2012, Jaehoon Chung <jh80.chung@xxxxxxxxxxx> wrote:
>> On 08/29/2012 10:21 AM, Seungwon Jeon wrote:
>>> On Tuesday, August 28, 2012, Jaehoon Chung <jh80.chung@xxxxxxxxxxx> 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.chung@xxxxxxxxxxx>
>>>> ---
>>>> 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 majordomo@xxxxxxxxxxxxxxx
>>> 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 majordomo@xxxxxxxxxxxxxxx
>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
