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