* [PATCH V6 RESEND 0/2] Poweroff Notify eMMC 4.5 @ 2011-09-16 13:39 Girish K S 2011-09-16 13:39 ` [PATCH V6 RESEND 1/2] mmc: core: Add power off notify feature(eMMC 4.5) Girish K S 2011-09-16 13:39 ` [PATCH V6 RESEND 2/2] mmc: core: Add Poweroff Notify handling eMMC 4.5 Girish K S 0 siblings, 2 replies; 6+ messages in thread From: Girish K S @ 2011-09-16 13:39 UTC (permalink / raw) To: linux-mmc; +Cc: cjb, kgene.kim, patches, linux-samsung-soc, Girish K S This patch version removes some checkpatch error. Earlier versions were checked with the older checkpatch.pl which didnt give any checkpatch errors. This patch uses the checkpatch.pl in the latest script folder. Girish K S (2): mmc: core: Add power off notify feature(eMMC 4.5) mmc: core: Add Poweroff Notify handling eMMC 4.5 drivers/mmc/core/core.c | 34 ++++++++++++++++++++++++++++++++++ drivers/mmc/core/mmc.c | 22 ++++++++++++++++++++-- drivers/mmc/host/sdhci.c | 10 ++++++++++ include/linux/mmc/card.h | 20 ++++++++++++++++++++ include/linux/mmc/host.h | 5 +++++ include/linux/mmc/mmc.h | 7 +++++++ 6 files changed, 96 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V6 RESEND 1/2] mmc: core: Add power off notify feature(eMMC 4.5) 2011-09-16 13:39 [PATCH V6 RESEND 0/2] Poweroff Notify eMMC 4.5 Girish K S @ 2011-09-16 13:39 ` Girish K S 2011-09-16 13:39 ` [PATCH V6 RESEND 2/2] mmc: core: Add Poweroff Notify handling eMMC 4.5 Girish K S 1 sibling, 0 replies; 6+ messages in thread From: Girish K S @ 2011-09-16 13:39 UTC (permalink / raw) To: linux-mmc; +Cc: cjb, kgene.kim, patches, linux-samsung-soc, Girish K S This patch adds the support for power off notify feature available in eMMC 4.5 devices. If the the host has support for this feature, then the mmc core will notify it to the device by setting the POWER_OFF_NOTIFICATION byte in the extended csd register with a value 1(POWER_ON). This patch should be applied after Seungwon Jeon's patch for cmd6 timeout. Signed-off-by: Girish K S <girish.shivananjappa@linaro.org> --- drivers/mmc/core/mmc.c | 19 +++++++++++++++++-- include/linux/mmc/card.h | 1 + include/linux/mmc/host.h | 1 + include/linux/mmc/mmc.h | 7 +++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 68eb368..2f06b37 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -410,10 +410,12 @@ static int mmc_read_ext_csd(struct mmc_card *card, u8 *ext_csd) else card->erased_byte = 0x0; - if (card->ext_csd.rev >= 6) + if (card->ext_csd.rev >= 6) { card->ext_csd.generic_cmd6_time = 10 * ext_csd[EXT_CSD_GENERIC_CMD6_TIME]; - + card->ext_csd.power_off_longtime = 10 * + ext_csd[EXT_CSD_POWER_OFF_LONG_TIME]; + } out: return err; } @@ -710,6 +712,19 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, } /* + * If the host supports the power_off_notify capability then + * set the notification byte in the ext_csd register of device + */ + if (host->caps & MMC_CAP_POWER_OFF_NOTIFY) { + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, + EXT_CSD_POWER_OFF_NOTIFICATION, + EXT_CSD_POWER_ON, + card->ext_csd.generic_cmd6_time); + if (err && err != -EBADMSG) + goto free_card; + } + + /* * Activate high speed (if supported) */ if ((card->ext_csd.hs_max_dtr != 0) && diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index e992fe3..2bf2843 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -53,6 +53,7 @@ struct mmc_ext_csd { unsigned int part_time; /* Units: ms */ unsigned int sa_timeout; /* Units: 100ns */ unsigned int generic_cmd6_time; /* Units: ms */ + unsigned int power_off_longtime; /* Units: ms */ unsigned int hs_max_dtr; unsigned int sectors; unsigned int card_type; diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index 1d09562..cf2dadc 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -229,6 +229,7 @@ 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 */ mmc_pm_flag_t pm_caps; /* supported pm features */ diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h index e869f00..a788e01 100644 --- a/include/linux/mmc/mmc.h +++ b/include/linux/mmc/mmc.h @@ -270,6 +270,7 @@ struct _mmc_csd { * EXT_CSD fields */ +#define EXT_CSD_POWER_OFF_NOTIFICATION 34 /* R/W */ #define EXT_CSD_PARTITION_ATTRIBUTE 156 /* R/W */ #define EXT_CSD_PARTITION_SUPPORT 160 /* RO */ #define EXT_CSD_WR_REL_PARAM 166 /* RO */ @@ -293,6 +294,7 @@ struct _mmc_csd { #define EXT_CSD_SEC_ERASE_MULT 230 /* RO */ #define EXT_CSD_SEC_FEATURE_SUPPORT 231 /* RO */ #define EXT_CSD_TRIM_MULT 232 /* RO */ +#define EXT_CSD_POWER_OFF_LONG_TIME 247 /*RO*/ #define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */ /* @@ -329,6 +331,11 @@ struct _mmc_csd { #define EXT_CSD_SEC_BD_BLK_EN BIT(2) #define EXT_CSD_SEC_GB_CL_EN BIT(4) +#define EXT_CSD_NO_POWER_NOTIFICATION 0 +#define EXT_CSD_POWER_ON 1 +#define EXT_CSD_POWER_OFF_SHORT 2 +#define EXT_CSD_POWER_OFF_LONG 3 + /* * MMC_SWITCH access modes */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V6 RESEND 2/2] mmc: core: Add Poweroff Notify handling eMMC 4.5 2011-09-16 13:39 [PATCH V6 RESEND 0/2] Poweroff Notify eMMC 4.5 Girish K S 2011-09-16 13:39 ` [PATCH V6 RESEND 1/2] mmc: core: Add power off notify feature(eMMC 4.5) Girish K S @ 2011-09-16 13:39 ` Girish K S 2011-10-10 23:09 ` Chris Ball 1 sibling, 1 reply; 6+ messages in thread From: Girish K S @ 2011-09-16 13:39 UTC (permalink / raw) To: linux-mmc; +Cc: cjb, kgene.kim, patches, linux-samsung-soc, Girish K S This patch adds the power off notification handling during suspend and system poweroff. For suspend mode short timeout is used, whereas for the normal poweroff long timeout is used. Signed-off-by: Girish K S <girish.shivananjappa@linaro.org> --- drivers/mmc/core/core.c | 34 ++++++++++++++++++++++++++++++++++ drivers/mmc/core/mmc.c | 5 ++++- drivers/mmc/host/sdhci.c | 10 ++++++++++ include/linux/mmc/card.h | 19 +++++++++++++++++++ include/linux/mmc/host.h | 4 ++++ 5 files changed, 71 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 91a0a74..157060f 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1130,9 +1130,41 @@ 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 (card != NULL && mmc_card_mmc(card) && + (mmc_card_powernotify_on(card))) { + + if (host->power_notify_type == MMC_HOST_PW_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); + + /*Set the card state to no notification after the poweroff*/ + 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. @@ -2022,6 +2054,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, spin_lock_irqsave(&host->lock, flags); host->rescan_disable = 1; + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; spin_unlock_irqrestore(&host->lock, flags); cancel_delayed_work_sync(&host->detect); @@ -2044,6 +2077,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, spin_lock_irqsave(&host->lock, flags); host->rescan_disable = 0; + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; spin_unlock_irqrestore(&host->lock, flags); mmc_detect_change(host, 0); diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 2f06b37..63cc77b 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -715,7 +715,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, * If the host supports the power_off_notify capability then * set the notification byte in the ext_csd register of device */ - if (host->caps & MMC_CAP_POWER_OFF_NOTIFY) { + if ((host->caps & MMC_CAP_POWER_OFF_NOTIFY) && + (mmc_card_powernotify_off(card))) { err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_POWER_OFF_NOTIFICATION, EXT_CSD_POWER_ON, @@ -724,6 +725,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, goto free_card; } + if (!err) + mmc_card_set_powernotify_on(card); /* * Activate high speed (if supported) */ diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 0e02cc1..92ea734 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2566,6 +2566,16 @@ int sdhci_add_host(struct sdhci_host *host) if (caps[1] & SDHCI_DRIVER_TYPE_D) mmc->caps |= MMC_CAP_DRIVER_TYPE_D; + /* + * If Notify capability is enabled and + * notify type is not initialised by host, set default to + * long power off notify timeout value + */ + if (mmc->caps & MMC_CAP_POWER_OFF_NOTIFY) + mmc->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; + else + mmc->power_notify_type = MMC_HOST_PW_NOTIFY_NONE; + /* 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..1d6a832 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -190,6 +190,11 @@ struct mmc_card { #define MMC_QUIRK_DISABLE_CD (1<<5) /* disconnect CD/DAT[3] resistor */ #define MMC_QUIRK_INAND_CMD38 (1<<6) /* iNAND devices have broken CMD38 */ #define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid CMD23 for regular multiblock */ + unsigned int poweroff_notify_state;/*eMMC4.5 notify feature */ +#define MMC_NO_POWER_NOTIFICATION 0 +#define MMC_POWERED_ON 1 +#define MMC_POWEROFF_SHORT 2 +#define MMC_POWEROFF_LONG 3 unsigned int erase_size; /* erase size in sectors */ unsigned int erase_shift; /* if erase unit is power 2 */ @@ -325,6 +330,20 @@ 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_off(c) \ + ((c)->poweroff_notify_state == MMC_NO_POWER_NOTIFICATION) + +#define mmc_card_set_powernotify_off(c) \ + ((c)->poweroff_notify_state = MMC_NO_POWER_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) + /* * Quirk add/remove for MMC products. */ diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index cf2dadc..4f24702 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -232,6 +232,10 @@ struct mmc_host { #define MMC_CAP_POWER_OFF_NOTIFY (1 << 31)/*Notify poweroff supported */ mmc_pm_flag_t pm_caps; /* supported pm features */ + unsigned int power_notify_type; +#define MMC_HOST_PW_NOTIFY_NONE 0 +#define MMC_HOST_PW_NOTIFY_SHORT 1 +#define MMC_HOST_PW_NOTIFY_LONG 2 #ifdef CONFIG_MMC_CLKGATE int clk_requests; /* internal reference counter */ -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V6 RESEND 2/2] mmc: core: Add Poweroff Notify handling eMMC 4.5 2011-09-16 13:39 ` [PATCH V6 RESEND 2/2] mmc: core: Add Poweroff Notify handling eMMC 4.5 Girish K S @ 2011-10-10 23:09 ` Chris Ball 2011-10-11 4:59 ` Girish K S 0 siblings, 1 reply; 6+ messages in thread From: Chris Ball @ 2011-10-10 23:09 UTC (permalink / raw) To: Girish K S; +Cc: linux-mmc, kgene.kim, patches, linux-samsung-soc Hi Girish, On Fri, Sep 16 2011, Girish K S wrote: > This patch adds the power off notification handling > during suspend and system poweroff. > For suspend mode short timeout is used, whereas for the > normal poweroff long timeout is used. > > Signed-off-by: Girish K S <girish.shivananjappa@linaro.org> > --- > drivers/mmc/core/core.c | 34 ++++++++++++++++++++++++++++++++++ > drivers/mmc/core/mmc.c | 5 ++++- > drivers/mmc/host/sdhci.c | 10 ++++++++++ > include/linux/mmc/card.h | 19 +++++++++++++++++++ > include/linux/mmc/host.h | 4 ++++ > 5 files changed, 71 insertions(+), 1 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 91a0a74..157060f 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -1130,9 +1130,41 @@ 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 (card != NULL && mmc_card_mmc(card) && > + (mmc_card_powernotify_on(card))) { > + > + if (host->power_notify_type == MMC_HOST_PW_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); You don't have any space between ".f" in the message. I suggest: pr_err("Device failed to respond within %d poweroff " "time. Forcefully powering down the device\n", timeout); > + > + /*Set the card state to no notification after the poweroff*/ A space between /* */. > + 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. > @@ -2022,6 +2054,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, > > spin_lock_irqsave(&host->lock, flags); > host->rescan_disable = 1; > + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; > spin_unlock_irqrestore(&host->lock, flags); > cancel_delayed_work_sync(&host->detect); > > @@ -2044,6 +2077,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, > > spin_lock_irqsave(&host->lock, flags); > host->rescan_disable = 0; > + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; > spin_unlock_irqrestore(&host->lock, flags); > mmc_detect_change(host, 0); > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 2f06b37..63cc77b 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -715,7 +715,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > * If the host supports the power_off_notify capability then > * set the notification byte in the ext_csd register of device > */ > - if (host->caps & MMC_CAP_POWER_OFF_NOTIFY) { > + if ((host->caps & MMC_CAP_POWER_OFF_NOTIFY) && The && is indented weirdly. Just use a space between ") &&". > + (mmc_card_powernotify_off(card))) { > err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > EXT_CSD_POWER_OFF_NOTIFICATION, > EXT_CSD_POWER_ON, > @@ -724,6 +725,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > goto free_card; > } > > + if (!err) > + mmc_card_set_powernotify_on(card); Extra newline needed here, as there was before you added these two lines. > /* > * Activate high speed (if supported) > */ > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 0e02cc1..92ea734 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2566,6 +2566,16 @@ int sdhci_add_host(struct sdhci_host *host) > if (caps[1] & SDHCI_DRIVER_TYPE_D) > mmc->caps |= MMC_CAP_DRIVER_TYPE_D; > > + /* > + * If Notify capability is enabled and > + * notify type is not initialised by host, set default to > + * long power off notify timeout value > + */ > + if (mmc->caps & MMC_CAP_POWER_OFF_NOTIFY) > + mmc->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; > + else > + mmc->power_notify_type = MMC_HOST_PW_NOTIFY_NONE; > + The comment says it sets the default to long power off if power_notify_type is unset, but the code sets power_notify_type to short power off regardless of whether power_notify_type was set. The comment and code should agree. > /* 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..1d6a832 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -190,6 +190,11 @@ struct mmc_card { > #define MMC_QUIRK_DISABLE_CD (1<<5) /* disconnect CD/DAT[3] resistor */ > #define MMC_QUIRK_INAND_CMD38 (1<<6) /* iNAND devices have broken CMD38 */ > #define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid CMD23 for regular multiblock */ > + unsigned int poweroff_notify_state;/*eMMC4.5 notify feature */ > +#define MMC_NO_POWER_NOTIFICATION 0 > +#define MMC_POWERED_ON 1 > +#define MMC_POWEROFF_SHORT 2 > +#define MMC_POWEROFF_LONG 3 > > unsigned int erase_size; /* erase size in sectors */ > unsigned int erase_shift; /* if erase unit is power 2 */ > @@ -325,6 +330,20 @@ 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_off(c) \ > + ((c)->poweroff_notify_state == MMC_NO_POWER_NOTIFICATION) > + > +#define mmc_card_set_powernotify_off(c) \ > + ((c)->poweroff_notify_state = MMC_NO_POWER_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) > + I don't think these macros need to exist. You can already do the same thing in one line of code, and it would be clearer to be modifying/testing poweroff_notify_state directly. The only case where it might save enough column space to allow fitting everything on one line is for MMC_NO_POWER_NOTIFICATION, but maybe that's just a sign that this #define should be made shorter somehow. Thanks! Please could you rebase against mmc-next, fix, and resend? - Chris. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V6 RESEND 2/2] mmc: core: Add Poweroff Notify handling eMMC 4.5 2011-10-10 23:09 ` Chris Ball @ 2011-10-11 4:59 ` Girish K S 2011-10-11 14:00 ` Chris Ball 0 siblings, 1 reply; 6+ messages in thread From: Girish K S @ 2011-10-11 4:59 UTC (permalink / raw) To: Chris Ball; +Cc: linux-mmc, kgene.kim, patches, linux-samsung-soc On 11 October 2011 04:39, Chris Ball <cjb@laptop.org> wrote: > Hi Girish, > > On Fri, Sep 16 2011, Girish K S wrote: >> This patch adds the power off notification handling >> during suspend and system poweroff. >> For suspend mode short timeout is used, whereas for the >> normal poweroff long timeout is used. >> >> Signed-off-by: Girish K S <girish.shivananjappa@linaro.org> >> --- >> drivers/mmc/core/core.c | 34 ++++++++++++++++++++++++++++++++++ >> drivers/mmc/core/mmc.c | 5 ++++- >> drivers/mmc/host/sdhci.c | 10 ++++++++++ >> include/linux/mmc/card.h | 19 +++++++++++++++++++ >> include/linux/mmc/host.h | 4 ++++ >> 5 files changed, 71 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 91a0a74..157060f 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -1130,9 +1130,41 @@ 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 (card != NULL && mmc_card_mmc(card) && >> + (mmc_card_powernotify_on(card))) { >> + >> + if (host->power_notify_type == MMC_HOST_PW_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); > > You don't have any space between ".f" in the message. I suggest: > > pr_err("Device failed to respond within %d poweroff " > "time. Forcefully powering down the device\n", > timeout); >> + >> + /*Set the card state to no notification after the poweroff*/ > > A space between /* */. > >> + 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. >> @@ -2022,6 +2054,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, >> >> spin_lock_irqsave(&host->lock, flags); >> host->rescan_disable = 1; >> + host->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; >> spin_unlock_irqrestore(&host->lock, flags); >> cancel_delayed_work_sync(&host->detect); >> >> @@ -2044,6 +2077,7 @@ int mmc_pm_notify(struct notifier_block *notify_block, >> >> spin_lock_irqsave(&host->lock, flags); >> host->rescan_disable = 0; >> + host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG; >> spin_unlock_irqrestore(&host->lock, flags); >> mmc_detect_change(host, 0); >> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index 2f06b37..63cc77b 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -715,7 +715,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, >> * If the host supports the power_off_notify capability then >> * set the notification byte in the ext_csd register of device >> */ >> - if (host->caps & MMC_CAP_POWER_OFF_NOTIFY) { >> + if ((host->caps & MMC_CAP_POWER_OFF_NOTIFY) && > > The && is indented weirdly. Just use a space between ") &&". > >> + (mmc_card_powernotify_off(card))) { >> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >> EXT_CSD_POWER_OFF_NOTIFICATION, >> EXT_CSD_POWER_ON, >> @@ -724,6 +725,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, >> goto free_card; >> } >> >> + if (!err) >> + mmc_card_set_powernotify_on(card); > > Extra newline needed here, as there was before you added these two lines. > >> /* >> * Activate high speed (if supported) >> */ >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index 0e02cc1..92ea734 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -2566,6 +2566,16 @@ int sdhci_add_host(struct sdhci_host *host) >> if (caps[1] & SDHCI_DRIVER_TYPE_D) >> mmc->caps |= MMC_CAP_DRIVER_TYPE_D; >> >> + /* >> + * If Notify capability is enabled and >> + * notify type is not initialised by host, set default to >> + * long power off notify timeout value >> + */ >> + if (mmc->caps & MMC_CAP_POWER_OFF_NOTIFY) >> + mmc->power_notify_type = MMC_HOST_PW_NOTIFY_SHORT; >> + else >> + mmc->power_notify_type = MMC_HOST_PW_NOTIFY_NONE; >> + > > The comment says it sets the default to long power off if power_notify_type > is unset, but the code sets power_notify_type to short power off regardless > of whether power_notify_type was set. The comment and code should agree. > >> /* 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..1d6a832 100644 >> --- a/include/linux/mmc/card.h >> +++ b/include/linux/mmc/card.h >> @@ -190,6 +190,11 @@ struct mmc_card { >> #define MMC_QUIRK_DISABLE_CD (1<<5) /* disconnect CD/DAT[3] resistor */ >> #define MMC_QUIRK_INAND_CMD38 (1<<6) /* iNAND devices have broken CMD38 */ >> #define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid CMD23 for regular multiblock */ >> + unsigned int poweroff_notify_state;/*eMMC4.5 notify feature */ >> +#define MMC_NO_POWER_NOTIFICATION 0 >> +#define MMC_POWERED_ON 1 >> +#define MMC_POWEROFF_SHORT 2 >> +#define MMC_POWEROFF_LONG 3 >> >> unsigned int erase_size; /* erase size in sectors */ >> unsigned int erase_shift; /* if erase unit is power 2 */ >> @@ -325,6 +330,20 @@ 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_off(c) \ >> + ((c)->poweroff_notify_state == MMC_NO_POWER_NOTIFICATION) >> + >> +#define mmc_card_set_powernotify_off(c) \ >> + ((c)->poweroff_notify_state = MMC_NO_POWER_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) >> + > > I don't think these macros need to exist. You can already do the same > thing in one line of code, and it would be clearer to be modifying/testing > poweroff_notify_state directly. The only case where it might save enough > column space to allow fitting everything on one line is for > MMC_NO_POWER_NOTIFICATION, but maybe that's just a sign that this > #define should be made shorter somehow. > > Thanks! Please could you rebase against mmc-next, fix, and resend? > > - Chris. Hi Chris, This is a very old version of the patch. My latest patch has been merged with Mr Chung's branch (the one with PULL REQUEST). These comments have been already taken care. Thanks Girish K S > -- > Chris Ball <cjb@laptop.org> <http://printf.net/> > One Laptop Per Child > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V6 RESEND 2/2] mmc: core: Add Poweroff Notify handling eMMC 4.5 2011-10-11 4:59 ` Girish K S @ 2011-10-11 14:00 ` Chris Ball 0 siblings, 0 replies; 6+ messages in thread From: Chris Ball @ 2011-10-11 14:00 UTC (permalink / raw) To: Girish K S; +Cc: linux-mmc, kgene.kim, patches, linux-samsung-soc Hi Girish, On Tue, Oct 11 2011, Girish K S wrote: > This is a very old version of the patch. My latest patch has been > merged with Mr Chung's branch (the one with PULL REQUEST). > These comments have been already taken care. No, that's not the case. Here is the version of the patch on the emmc4.5 branch that the pull request is for: http://git.infradead.org/users/kmpark/linux-2.6-samsung/commitdiff/dac139c9591545a40ea0058d9305514130762682 and it has the same problems I'm replying to here. Maybe the wrong version got merged? Thanks, - Chris. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-11 14:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-16 13:39 [PATCH V6 RESEND 0/2] Poweroff Notify eMMC 4.5 Girish K S 2011-09-16 13:39 ` [PATCH V6 RESEND 1/2] mmc: core: Add power off notify feature(eMMC 4.5) Girish K S 2011-09-16 13:39 ` [PATCH V6 RESEND 2/2] mmc: core: Add Poweroff Notify handling eMMC 4.5 Girish K S 2011-10-10 23:09 ` Chris Ball 2011-10-11 4:59 ` Girish K S 2011-10-11 14:00 ` Chris Ball
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox