Hello Mr Jeon, i have modified the patch to handle the poweroff in suspend and normal condition. during suspend it is be short and during normal poweroff it is long. I will update the V2 patch with Mr Parks changes updated. regards Girish K S On 6 September 2011 12:31, Seungwon Jeon <tgih.jun@xxxxxxxxxxx> wrote: > Hi Girish K S, > > I think short type is proper rather than long type by default. > Long type seems to be unacceptable in suspend regarding long timeout. > One question, > Is this patch considered for system power off besides suspend? > > Beset regards, > Seungwon Jeon. > > Girish K S wrote: > >> Hi Mr Park, >> >> On 5 September 2011 18:03, Kyungmin Park <kmpark@xxxxxxxxxxxxx> wrote: >> > Hi Girish, >> > >> > I think it's still incomplete, does power off short function is called >> > at suspend properly? >> > there are some comments below. >> > >> > Thank you, >> > Kyungmin Park >> > >> > On Mon, Sep 5, 2011 at 8:49 PM, Girish K S >> > <girish.shivananjappa@xxxxxxxxxx> wrote: >> >> This patch adds the handling of the poweroff notify feature >> >> during powerdown phase. >> >> >> >> Signed-off-by: Girish K S <girish.shivananjappa@xxxxxxxxxx> >> >> --- >> >> drivers/mmc/core/core.c | 29 +++++++++++++++++++++++++++++ >> >> drivers/mmc/core/mmc.c | 3 +++ >> >> drivers/mmc/host/sdhci.c | 11 +++++++++++ >> >> include/linux/mmc/card.h | 29 +++++++++++++++++++++++++---- >> >> include/linux/mmc/host.h | 7 +++++-- >> >> 5 files changed, 73 insertions(+), 6 deletions(-) >> >> >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> >> index a65e1ca..37b09d7 100644 >> >> --- a/drivers/mmc/core/core.c >> >> +++ b/drivers/mmc/core/core.c >> >> @@ -1130,9 +1130,38 @@ static void mmc_power_up(struct mmc_host *host) >> >> >> >> static void mmc_power_off(struct mmc_host *host) >> >> { >> >> + struct mmc_card *card = host->card; >> >> + unsigned int notify_type; >> >> + unsigned int timeout; >> >> + int err; >> >> + >> >> host->ios.clock = 0; >> >> host->ios.vdd = 0; >> >> >> >> + if (mmc_card_mmc(card) && mmc_card_powernotify_on(card)) { >> >> + >> >> + if (host->power_notify_type == MMC_POWEROFF_NOTIFY_SHORT) >> { >> >> + notify_type = EXT_CSD_POWER_OFF_SHORT; >> >> + timeout = card->ext_csd.generic_cmd6_time; >> >> + mmc_card_set_powernotify_short(card); >> >> + } else { >> >> + notify_type = EXT_CSD_POWER_OFF_LONG; >> >> + timeout = card->ext_csd.power_off_longtime; >> >> + mmc_card_set_powernotify_long(card); >> >> + } >> >> + >> >> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> >> + EXT_CSD_POWER_OFF_NOTIFICATION, >> >> + notify_type, timeout); >> >> + >> >> + if (err && err != -EBADMSG) >> >> + printk(KERN_ERR "Device failed to respond " >> >> + "within %d poweroff time." >> >> + "forcefully powering down" >> >> + "the device\n", timeout); >> >> + >> >> + mmc_card_set_powernotify_off(card); >> >> + } >> >> /* >> >> * Reset ocr mask to be the highest possible voltage supported >> for >> >> * this mmc host. This value will be used at next power up. >> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> >> index 2f06b37..b369c6f 100644 >> >> --- a/drivers/mmc/core/mmc.c >> >> +++ b/drivers/mmc/core/mmc.c >> >> @@ -720,8 +720,11 @@ static int mmc_init_card(struct mmc_host *host, >> u32 ocr, >> >> EXT_CSD_POWER_OFF_NOTIFICATION, >> >> EXT_CSD_POWER_ON, >> >> card->ext_csd.generic_cmd6_time); >> >> + >> >> if (err && err != -EBADMSG) >> >> goto free_card; >> >> + >> >> + mmc_card_set_powernotify_on(card); >> >> } >> >> >> >> /* >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> >> index 0e02cc1..a24a31b 100644 >> >> --- a/drivers/mmc/host/sdhci.c >> >> +++ b/drivers/mmc/host/sdhci.c >> >> @@ -2566,6 +2566,17 @@ int sdhci_add_host(struct sdhci_host *host) >> >> if (caps[1] & SDHCI_DRIVER_TYPE_D) >> >> mmc->caps |= MMC_CAP_DRIVER_TYPE_D; >> >> >> >> + if (mmc->caps == MMC_CAP_POWER_OFF_NOTIFY) { >> > It should be if (mmc->caps & MMC_CAP_POWER_OFF_NOTIFY) { >> >> + /* >> >> + * If Notify capability is enabled and >> >> + * platform data is not initialised, set default to >> >> + * long power off notify timeout value >> >> + */ >> >> + if (mmc->power_notify_type == MMC_POWEROFF_NOTIFY_NONE) >> >> + mmc->power_notify_type = MMC_POWEROFF_NOTIFY_LONG; >> >> + } else { >> >> + mmc->power_notify_type = MMC_POWEROFF_NOTIFY_NONE; >> >> + } >> > What's the goal of these statements? >> the goal of the statement is to set the notification type to a default >> value. >> if host doesnt have the capability then notification type is none. >> If the host supports the capability and the notification type (whether >> the short or long is not set), then default is long type. >> >> /* Initial value for re-tuning timer count */ >> >> host->tuning_count = (caps[1] & SDHCI_RETUNING_TIMER_COUNT_MASK) >> >> >> >> SDHCI_RETUNING_TIMER_COUNT_SHIFT; >> >> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >> >> index 2bf2843..d40b7b3 100644 >> >> --- a/include/linux/mmc/card.h >> >> +++ b/include/linux/mmc/card.h >> >> @@ -179,7 +179,12 @@ struct mmc_card { >> >> #define MMC_STATE_HIGHSPEED_DDR (1<<4) /* card is in high speed >> mode */ >> >> #define MMC_STATE_ULTRAHIGHSPEED (1<<5) /* card is in >> ultra high speed mode */ >> >> #define MMC_CARD_SDXC (1<<6) /* card is SDXC */ >> >> - unsigned int quirks; /* card quirks */ >> >> + unsigned int poweroff_notify_state;/*eMMC4.5 notify >> feature */ >> >> +#define MMC_NO_POWER_NITOFICATION 0 >> >> +#define MMC_POWERED_ON 1 >> >> +#define MMC_POWEROFF_SHORT 2 >> >> +#define MMC_POWEROFF_LONG 3 >> >> + unsigned int quirks; /* card quirks */ >> > Don't modify the meaning-less change >> sorry my mistake. >> >> #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes >> outside of the VS CCCR range */ >> >> #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func- >> >cur_blksize */ >> >> /* for byte mode */ >> >> @@ -192,9 +197,9 @@ struct mmc_card { >> >> #define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid CMD23 for >> regular multiblock */ >> >> >> >> unsigned int erase_size; /* erase size in sectors */ >> >> - unsigned int erase_shift; /* if erase unit is power >> 2 */ >> >> - unsigned int pref_erase; /* in sectors */ >> >> - u8 erased_byte; /* value of erased bytes */ >> >> + unsigned int erase_shift; /* if erase unit is power >> 2 */ >> >> + unsigned int pref_erase; /* in sectors */ >> >> + u8 erased_byte; /* value of erased bytes */ >> > ditto >> >> >> >> u32 raw_cid[4]; /* raw card CID */ >> >> u32 raw_csd[4]; /* raw card CSD */ >> >> @@ -325,6 +330,22 @@ static inline void __maybe_unused >> remove_quirk(struct mmc_card *card, int data) >> >> #define mmc_sd_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED) >> >> #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC) >> >> >> >> +#define mmc_card_powernotify_on(c) \ >> >> + ((c)->poweroff_notify_state == MMC_POWERED_ON) >> >> +#define mmc_card_powernotify_short(c) \ >> >> + ((c)->poweroff_notify_state == MMC_POWEROFF_SHORT) >> >> +#define mmc_card_powernotify_long(c) \ >> >> + ((c)->poweroff_notify_state == MMC_POWEROFF_LONG) >> >> + >> >> +#define mmc_card_set_powernotify_off(c) \ >> >> + ((c)->poweroff_notify_state = MMC_NO_POWER_NITOFICATION) >> > Typo, NOTIFICATION >> >> +#define mmc_card_set_powernotify_on(c) \ >> >> + ((c)->poweroff_notify_state = MMC_POWERED_ON) >> >> +#define mmc_card_set_powernotify_short(c) \ >> >> + ((c)->poweroff_notify_state = MMC_POWEROFF_SHORT) >> >> +#define mmc_card_set_powernotify_long(c) \ >> >> + ((c)->poweroff_notify_state = MMC_POWEROFF_LONG) >> > What's the purpose the these defines? it's not used at patches. >> These macros have been used in the mmc_power_off function. Before >> calling the mmc_switch command >> in mmc_power_off its required to know whether the card has enabled the >> POWER_ON_NOTIFICATION byte >> in the extended csd register. mmc_switch has to be called only if >> POWER_ON_NOTIFICATION == POWER_ON >> so these macros have been defined to set the state variable in cards >> context. >> i will delete the remaining unused macros. >> >> + >> >> /* >> >> * Quirk add/remove for MMC products. >> >> */ >> >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> >> index 21c85e1..354752a 100644 >> >> --- a/include/linux/mmc/host.h >> >> +++ b/include/linux/mmc/host.h >> >> @@ -229,10 +229,13 @@ struct mmc_host { >> >> #define MMC_CAP_MAX_CURRENT_600 (1 << 28) /* Host max >> current limit is 600mA */ >> >> #define MMC_CAP_MAX_CURRENT_800 (1 << 29) /* Host max >> current limit is 800mA */ >> >> #define MMC_CAP_CMD23 (1 << 30) /* CMD23 supported. */ >> >> -#define MMC_CAP_POWER_OFF_NOTIFY (1 << 31)/*NOtify poweroff >> supported */ >> >> +#define MMC_CAP_POWER_OFF_NOTIFY (1 << 31)/*Notify poweroff >> supported */ >> > Please update the #1 patch first >> sure >> >> >> >> mmc_pm_flag_t pm_caps; /* supported pm features */ >> >> - >> >> +#define MMC_POWEROFF_NOTIFY_NONE 0 >> >> +#define MMC_POWEROFF_NOTIFY_SHORT 1 >> >> +#define MMC_POWEROFF_NOTIFY_LONG 2 >> > You already define the similar, please make one defines >> ok will make the necessary change >> >> +#define MMC_NO_POWER_NITOFICATION 0 >> >> +#define MMC_POWERED_ON 1 >> >> +#define MMC_POWEROFF_SHORT 2 >> >> +#define MMC_POWEROFF_LONG 3 >> >> + unsigned int power_notify_type; >> >> #ifdef CONFIG_MMC_CLKGATE >> >> int clk_requests; /* internal reference >> counter */ >> >> unsigned int clk_delay; /* number of MCI clk hold >> cycles */ >> >> -- >> >> 1.7.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-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html