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-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
