From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ulf Hansson Subject: Re: [PATCH v3 1/2] MMC-4.5 Power OFF Notify rework Date: Mon, 14 May 2012 15:26:20 +0200 Message-ID: <4FB107FC.8010508@stericsson.com> References: <1336398084-23203-1-git-send-email-girish.shivananjappa@linaro.org> <4FACFB86.5030907@codeaurora.org> <4FB0E953.80507@stericsson.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from eu1sys200aog114.obsmtp.com ([207.126.144.137]:42868 "EHLO eu1sys200aog114.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756184Ab2ENN0f (ORCPT ); Mon, 14 May 2012 09:26:35 -0400 In-Reply-To: Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: Saugata Das Cc: Subhash Jadavani , Girish K S , "linux-mmc@vger.kernel.org" , "patches@linaro.org" On 05/14/2012 02:49 PM, Saugata Das wrote: > On 14 May 2012 16:45, Ulf Hansson wrote: >> Hi Girish, >> >> Please some comments below. >> >> >> On 05/11/2012 01:44 PM, Subhash Jadavani wrote: >>> >>> Hi Girish, >>> >>> On 5/7/2012 7:11 PM, Girish K S wrote: >>>> >>>> This is a rework of the existing POWER OFF NOTIFY patch. The current >>>> problem >>>> with the patch comes from the ambiguity on the usage of POWER OFF NOTIFY >>>> together with SLEEP and misunderstanding on the usage of MMC_POWER_OFF >>>> power_mode from mmc_set_ios in different host controller drivers. >>>> >>>> This new patch works around this problem by adding a new host CAP, >>>> MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND, which when set sends a >>>> POWER OFF NOTIFY from mmc_suspend instead of SLEEP. It is expected that >>>> host >>>> controller drivers will set this CAP, if they switch off both Vcc and >>>> Vccq >>>> from MMC_POWER_OFF condition within mmc_set_ios. However, note that there >>>> is no harm in sending MMC_POWER_NOTIFY even if Vccq is not switched off. >>>> >>>> This patch also sends POWER OFF NOTIFY from power management routines >>>> (e.g. >>>> mmc_power_save_host, mmc_pm_notify/PM_SUSPEND_PREPARE, mmc_stop_host), >>>> which >>>> does reinitialization of the eMMC on the return path of the power >>>> management >>>> routines (e.g. mmc_power_restore_host, mmc_pm_notify/PM_POST_RESTORE, >>>> mmc_start_host). >>>> >>>> This patch sets POWER_OFF_NOTIFICATION to POWER_OFF_SHORT if it is sent >>>> from >>>> the suspend sequence. If it is sent from shutdown sequence then it is set >>>> to >>>> POWER_OFF_LONG. >>>> >>>> Previuos implementation of PowerOff Notify as a core function is replaced >>>> as >>>> a device's bus operation. >>>> >>>> Signed-off-by: Saugata Das >>>> Signed-off-by: Girish K S >>>> >>>> changes in v3: >>>> This version addresses the review comments given by Subhash and Ulf >>>> changes in v2: >>>> This version addresses the changes suggested by Ulf >>>> --- >>>> drivers/mmc/core/core.c | 98 >>>> +++++++++++++++------------------------------- >>>> drivers/mmc/core/core.h | 1 + >>>> drivers/mmc/core/mmc.c | 73 ++++++++++++++++++++++++++++------ >>>> include/linux/mmc/card.h | 8 +++- >>>> include/linux/mmc/host.h | 9 ++-- >>>> 5 files changed, 105 insertions(+), 84 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >>>> index ba821fe..3db3b32 100644 >>>> --- a/drivers/mmc/core/core.c >>>> +++ b/drivers/mmc/core/core.c >>>> @@ -1100,48 +1100,6 @@ void mmc_set_driver_type(struct mmc_host *host, >>>> unsigned int drv_type) >>>> mmc_host_clk_release(host); >>>> } >>>> >>>> -static void mmc_poweroff_notify(struct mmc_host *host) >>>> -{ >>>> - struct mmc_card *card; >>>> - unsigned int timeout; >>>> - unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; >>>> - int err = 0; >>>> - >>>> - card = host->card; >>>> - mmc_claim_host(host); >>>> - >>>> - /* >>>> - * Send power notify command only if card >>>> - * is mmc and notify state is powered ON >>>> - */ >>>> - if (card&& mmc_card_mmc(card)&& >>>> - (card->poweroff_notify_state == MMC_POWERED_ON)) { >>>> - >>>> - if (host->power_notify_type == MMC_HOST_PW_NOTIFY_SHORT) { >>>> - notify_type = EXT_CSD_POWER_OFF_SHORT; >>>> - timeout = card->ext_csd.generic_cmd6_time; >>>> - card->poweroff_notify_state = MMC_POWEROFF_SHORT; >>>> - } else { >>>> - notify_type = EXT_CSD_POWER_OFF_LONG; >>>> - timeout = card->ext_csd.power_off_longtime; >>>> - card->poweroff_notify_state = MMC_POWEROFF_LONG; >>>> - } >>>> - >>>> - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>>> - EXT_CSD_POWER_OFF_NOTIFICATION, >>>> - notify_type, timeout); >>>> - >>>> - if (err&& err != -EBADMSG) >>>> - 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 >>>> */ >>>> - card->poweroff_notify_state = MMC_NO_POWER_NOTIFICATION; >>>> - } >>>> - mmc_release_host(host); >>>> -} >>>> - >>>> /* >>>> * Apply power to the MMC stack. This is a two-stage process. >>>> * First, we enable power to the card without the clock running. >>>> @@ -1198,30 +1156,12 @@ static void mmc_power_up(struct mmc_host *host) >>>> >>>> void mmc_power_off(struct mmc_host *host) >>>> { >>>> - int err = 0; >>>> mmc_host_clk_hold(host); >>>> >>>> host->ios.clock = 0; >>>> host->ios.vdd = 0; >>>> >>>> /* >>>> - * For eMMC 4.5 device send AWAKE command before >>>> - * POWER_OFF_NOTIFY command, because in sleep state >>>> - * eMMC 4.5 devices respond to only RESET and AWAKE cmd >>>> - */ >>>> - if (host->card&& mmc_card_is_sleep(host->card)&& >>>> - host->bus_ops->resume) { >>>> - err = host->bus_ops->resume(host); >>>> - >>>> - if (!err) >>>> - mmc_poweroff_notify(host); >>>> - else >>>> - pr_warning("%s: error %d during resume " >>>> - "(continue with poweroff sequence)\n", >>>> - mmc_hostname(host), err); >>>> - } >>>> - >>>> - /* >>>> * Reset ocr mask to be the highest possible voltage supported for >>>> * this mmc host. This value will be used at next power up. >>>> */ >>>> @@ -2081,9 +2021,16 @@ void mmc_stop_host(struct mmc_host *host) >>>> >>>> /* clear pm flags now and let card drivers set them as needed */ >>>> host->pm_flags = 0; >>>> - >>>> mmc_bus_get(host); >>>> if (host->bus_ops&& !host->bus_dead) { >>>> + >>>> + if (host->bus_ops->poweroff_notify) { >> >> >> Instead of checking EOPNOTSUP you can do: >> if (mmc_card_can_poweroff_notify ..) >> > > If I understood your suggestion well, you want to remove > mmc_card_can_poweroff_notify check from inside mmc_poweroff_notify and > move it outside here before calling poweroff_notify. It can be done. > OK, great! >> >> >>>> + int err = host->bus_ops->poweroff_notify(host); >>>> + if (err&& err != -EOPNOTSUPP) >>>> + pr_info("%s: error [%d] in poweroff >>>> notify\n", >>>> + mmc_hostname(host), err); >>>> + } >>>> + >>>> /* Calling bus_ops->remove() with a claimed host can >>>> deadlock */ >>>> if (host->bus_ops->remove) >>>> host->bus_ops->remove(host); >>>> @@ -2093,6 +2040,7 @@ void mmc_stop_host(struct mmc_host *host) >>>> mmc_power_off(host); >>>> mmc_release_host(host); >>>> mmc_bus_put(host); >>>> + >> >> >> white space >> > > Ok > >> >>>> return; >>>> } >>>> mmc_bus_put(host); >>>> @@ -2119,6 +2067,13 @@ int mmc_power_save_host(struct mmc_host *host) >>>> >>>> if (host->bus_ops->power_save) >>>> ret = host->bus_ops->power_save(host); >>>> + host->poweroff_notify_type = MMC_HOST_PW_OFF_NOTIFY_SHORT; >>> >>> Before sending this short power off notification, shouldn't we make sure >>> that host has defined MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND cap? >> >> >> Instead of checking EOPNOTSUP you can do: >> if (mmc_card_can_poweroff_notify ..) > > Ok (Same way as understood above) > >> >> Moreover MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND shall be checked during >> mmc_init_card, when setting poweroff_notify_state to MMC_POWERED_ON, that is >> not done today. >> > > MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND is only linked when putting > eMMC to SLEEP state. We need to handle scenario, where the platform > goes to power OFF before putting eMMC to SLEEP (e.g. mmc_stop_host). Yes, of course you are right! So no changes shall be done to "mmc_init_card" then. > So, MMC_POWERED_ON is linked to only MMC_CAP2_POWEROFF_NOTIFY and > right initial value of poweroff_notify_type as set from host drivers > (i.e. poweroff_notify_type should not be MMC_HOST_PW_NOTIFY_NONE). > In that context, that sound perfectly fine! > >> Thereafter you can only do this when checking if poweroff_notify shall be >> done: >> "if (mmc_card_can_poweroff_notify ..)" >> > > Ok > >> >>>> + if (host->bus_ops->poweroff_notify) { >>>> + int err = host->bus_ops->poweroff_notify(host); >>>> + if (err&& err != -EOPNOTSUPP) >>>> + pr_info("%s: error [%d] in poweroff notify\n", >>>> + mmc_hostname(host), err); >>>> + } >>>> >>>> mmc_bus_put(host); >>>> >>>> @@ -2142,7 +2097,6 @@ int mmc_power_restore_host(struct mmc_host *host) >>>> mmc_bus_put(host); >>>> return -EINVAL; >>>> } >>>> - more white space >>>> mmc_power_up(host); >>>> ret = host->bus_ops->power_restore(host); >>>> >>>> @@ -2161,8 +2115,11 @@ int mmc_card_awake(struct mmc_host *host) >>>> >>>> mmc_bus_get(host); >>>> >>>> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) >>>> + if (host->bus_ops&& !host->bus_dead&& host->bus_ops->awake) { >>>> err = host->bus_ops->awake(host); >>>> + if (!err) >>>> + mmc_card_clr_sleep(host->card); >>>> + } >>>> >>>> mmc_bus_put(host); >>>> >>>> @@ -2179,8 +2136,11 @@ int mmc_card_sleep(struct mmc_host *host) >>>> >>>> mmc_bus_get(host); >>>> >>>> - if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) >>>> + if (host->bus_ops&& !host->bus_dead&& host->bus_ops->sleep) { >>>> err = host->bus_ops->sleep(host); >>>> + if (!err) >>>> + mmc_card_set_sleep(host->card); >>>> + } >>>> >>>> mmc_bus_put(host); >>>> >>>> @@ -2373,12 +2333,18 @@ 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; >>>> + host->poweroff_notify_type = MMC_HOST_PW_OFF_NOTIFY_SHORT; >>> >>> Before sending this short power off notification, shouldn't we make sure >>> that host has defined MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND cap? >> >> Suggest to remove the "poweroff_notify_type" variable in the host. Use an >> in-parameter to the poweroff_notify bus_ops function instead. >> > > This is coming from the original version of the patch. Need to check > the reason for this and fix it, if needed. Realize that, but it would make scene to fix this in patch as well. If feasible of course. > >> >>>> spin_unlock_irqrestore(&host->lock, flags); >>>> cancel_delayed_work_sync(&host->detect); >>>> >>>> if (!host->bus_ops || host->bus_ops->suspend) >>>> break; >> >> >> You can do this instead: >> "if (mmc_card_can_poweroff_notify ..)" >> > > Ok > >>>> + if (host->bus_ops->poweroff_notify) { >>>> + int err = host->bus_ops->poweroff_notify(host); >>>> + if (err&& err != -EOPNOTSUPP) >>>> + pr_info("%s: error [%d] in poweroff >>>> notify\n", >>>> + mmc_hostname(host), err); >>>> + } >>>> >>>> /* Calling bus_ops->remove() with a claimed host can >>>> deadlock */ >>>> if (host->bus_ops->remove) >>>> @@ -2397,7 +2363,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; >>>> + host->poweroff_notify_type = MMC_HOST_PW_OFF_NOTIFY_LONG; >> >> >> Suggest to remove the "poweroff_notify_type" variable in the host. Use an >> in-parameter to the poweroff_notify bus_ops function instead. >> > > Same as above. Need to check the reason of having this in the original patch. > >> >>>> spin_unlock_irqrestore(&host->lock, flags); >>>> mmc_detect_change(host, 0); >>>> >>>> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h >>>> index 3bdafbc..351cbbe 100644 >>>> --- a/drivers/mmc/core/core.h >>>> +++ b/drivers/mmc/core/core.h >>>> @@ -25,6 +25,7 @@ struct mmc_bus_ops { >>>> int (*power_save)(struct mmc_host *); >>>> int (*power_restore)(struct mmc_host *); >>>> int (*alive)(struct mmc_host *); >>>> + int (*poweroff_notify)(struct mmc_host *); >>>> }; >>>> >>>> void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops >>>> *ops); >>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>>> index 54df5ad..a86e5f8 100644 >>>> --- a/drivers/mmc/core/mmc.c >>>> +++ b/drivers/mmc/core/mmc.c >>>> @@ -1282,6 +1282,47 @@ err: >>>> return err; >>>> } >>>> >>>> +static int mmc_poweroff_notify(struct mmc_host *host) >> >> >> Suggest to add a "poweroff_notify_type" as in-parameter instead of using the >> host struct for storing this information. >> > > Same as above. Need to check the reason of having this in the original patch. > >> >>>> +{ >>>> + struct mmc_card *card; >>>> + unsigned int timeout; >>>> + unsigned int notify_type = EXT_CSD_NO_POWER_NOTIFICATION; >>>> + int err = -EOPNOTSUPP; >>>> + >>>> + card = host->card; >>>> + >>>> + /* >>>> + * Send power notify command only if card >>>> + * is mmc and notify state is powered ON >>>> + */ >>>> + if (mmc_card_can_poweroff_notify(host->card)) { >> >> >> Suggest to move this check outside of this function. Please see how this is >> done for mmc_card_can_sleep and mmc_card_sleep. >> > > Ok > >> >>>> + if (host->poweroff_notify_type == >>>> + MMC_HOST_PW_OFF_NOTIFY_SHORT) { >>>> + notify_type = EXT_CSD_POWER_OFF_SHORT; >>>> + timeout = card->ext_csd.generic_cmd6_time; >>>> + } else { >>> >>> poweroff_notify_type can take 3 different values. >>> #define MMC_HOST_PW_OFF_NOTIFY_NONE 0 >>> #define MMC_HOST_PW_OFF_NOTIFY_SHORT 1 >>> #define MMC_HOST_PW_OFF_NOTIFY_LONG 2 >>> >>> So shouldn't we have explicit check for NOTIFY_LONG? >>> else if (notify_type == LONG) { ... } >>> else {error printing?} >>> >>>> + notify_type = EXT_CSD_POWER_OFF_LONG; >>>> + timeout = card->ext_csd.power_off_longtime; >>>> + } >>>> + >>>> + mmc_claim_host(host); >> >> >> Suggest to handle claim and release outside this function. Similar how >> mmc_card_sleep is implemented. >> > > Ok > >> >>>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>>> + EXT_CSD_POWER_OFF_NOTIFICATION, >>>> + notify_type, timeout); >>>> + mmc_release_host(host); >>>> + >>>> + if (err&& err != -EBADMSG) >>>> + pr_err("%s: Device failed to respond within %d " >>>> + "poweroff time. Forcefully powering down " >>>> + "the device\n", mmc_hostname(host), >>>> timeout); >>>> + else >>>> + card->poweroff_notify_state = >>>> + MMC_NO_POWER_NOTIFICATION; >>> >>> Just wondering, if we should really to set the notify_state to >>> NO_POWER_NOTIFICATION incase if err = -EBADMSG ? >>>> >>>> + } >>>> + >>>> + return err; >>>> +} >>>> + >>>> /* >>>> * Host is being removed. Free up the current card. >>>> */ >>>> @@ -1341,15 +1382,21 @@ static int mmc_suspend(struct mmc_host *host) >>>> BUG_ON(!host); >>>> BUG_ON(!host->card); >>>> >>>> - mmc_claim_host(host); >> >> >> Suggest to handle claim and release as before and not inside the >> mmc_poweroff_notify function. Similar how mmc_card_sleep is implemented. >> > > Ok > >> >>>> - if (mmc_card_can_sleep(host)) { >>>> - err = mmc_card_sleep(host); >>>> - if (!err) >>>> - mmc_card_set_sleep(host->card); >>>> - } else if (!mmc_host_is_spi(host)) >>>> - mmc_deselect_cards(host); >>>> - host->card->state&= ~(MMC_STATE_HIGHSPEED | >>>> MMC_STATE_HIGHSPEED_200); >>>> - mmc_release_host(host); >>>> + if (mmc_card_can_poweroff_notify(host->card)&& >>>> + (host->caps2& MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) { >> >> Sorry for not proposing this earlier; >> By checking the MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND could be done only >> from mmc_init_card function, it is enough to check >> "mmc_card_can_poweroff_notify". >> > > MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND is only linked for the special > feature of powering off eMMC during platform suspend (instead of > putting eMMC to SLEEP). It has no relation to the power OFF notify > feature which is done when platform is shutting down (e.g. > mmc_stop_host). So, mmc_init_card just checks > MMC_CAP2_POWEROFF_NOTIFY. Agree, you are right. MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND needs to be explicitly checked here, as done. > > >> >>>> + err = mmc_poweroff_notify(host); >>>> + } else { >>>> + mmc_claim_host(host); >>>> + if (mmc_card_can_sleep(host)) >>>> + err = mmc_card_sleep(host); >>>> + else if (!mmc_host_is_spi(host)) >>>> + mmc_deselect_cards(host); >>>> + mmc_release_host(host); >>>> + } >>>> + >>>> + if (!err) >>>> + host->card->state&= >>>> + ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); >>>> >>>> return err; >>>> } >>>> @@ -1368,11 +1415,11 @@ static int mmc_resume(struct mmc_host *host) >>>> BUG_ON(!host->card); >>>> >>>> mmc_claim_host(host); >>>> - if (mmc_card_is_sleep(host->card)) { >>>> + if (mmc_card_is_sleep(host->card)) >>>> err = mmc_card_awake(host); >>>> - mmc_card_clr_sleep(host->card); >>>> - } else >>>> + else >>>> err = mmc_init_card(host, host->ocr, host->card); >>>> + >>>> mmc_release_host(host); >>>> >>>> return err; >>>> @@ -1430,6 +1477,7 @@ static const struct mmc_bus_ops mmc_ops = { >>>> .resume = NULL, >>>> .power_restore = mmc_power_restore, >>>> .alive = mmc_alive, >>>> + .poweroff_notify = mmc_poweroff_notify, >>>> }; >>>> >>>> static const struct mmc_bus_ops mmc_ops_unsafe = { >>>> @@ -1441,6 +1489,7 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { >>>> .resume = mmc_resume, >>>> .power_restore = mmc_power_restore, >>>> .alive = mmc_alive, >>>> + .poweroff_notify = mmc_poweroff_notify, >>>> }; >>>> >>>> static void mmc_attach_bus_ops(struct mmc_host *host) >>>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >>>> index 629b823..a6cdc43 100644 >>>> --- a/include/linux/mmc/card.h >>>> +++ b/include/linux/mmc/card.h >>>> @@ -238,8 +238,6 @@ struct mmc_card { >>>> 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 >>>> */ >>>> @@ -465,6 +463,12 @@ static inline int mmc_card_long_read_time(const >>>> struct mmc_card *c) >>>> return c->quirks& MMC_QUIRK_LONG_READ_TIME; >>>> } >>>> >>>> +static inline int mmc_card_can_poweroff_notify(const struct mmc_card *c) >>>> +{ >>>> + return c&& mmc_card_mmc(c)&& >>>> + (c->poweroff_notify_state == >>>> MMC_POWERED_ON); >>>> +} >>>> + >>> >>> I guess you may want to move this function in drivers/mmc/core/core.c >>> along with other mmc_can_* function in same file. Look for >>> mmc_can_sanitize() or mmc_can_discard() functions as example. >>> >>>> #define mmc_card_name(c) ((c)->cid.prod_name) >>>> #define mmc_card_id(c) (dev_name(&(c)->dev)) >>>> >>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>>> index 0707d22..aad894e 100644 >>>> --- a/include/linux/mmc/host.h >>>> +++ b/include/linux/mmc/host.h >>>> @@ -238,12 +238,13 @@ struct mmc_host { >>>> #define MMC_CAP2_BROKEN_VOLTAGE (1<< 7) /* Use the broken >>>> voltage */ >>>> #define MMC_CAP2_DETECT_ON_ERR (1<< 8) /* On I/O err >>>> check card removal */ >>>> #define MMC_CAP2_HC_ERASE_SZ (1<< 9) /* High-capacity >>>> erase size */ >>>> +#define MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND (1<< 10) >>>> >>>> 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 >> >> >> Suggest to remove this entirely from the mmc host struct. >> > > As stated above, we need to check the reason of having this in the > original patch. Then, we will either remove this and add an argument > to mmc_poweroff_notify, or keep it as it is with some justification. > > >> >>>> + unsigned int poweroff_notify_type; >>> >>> Can you add additional tab to make poweroff_notify_state aligned to >>> other members in this same structure? >>>> >>>> +#define MMC_HOST_PW_OFF_NOTIFY_NONE 0 >>>> +#define MMC_HOST_PW_OFF_NOTIFY_SHORT 1 >>>> +#define MMC_HOST_PW_OFF_NOTIFY_LONG 2 >>>> >>>> #ifdef CONFIG_MMC_CLKGATE >>>> int clk_requests; /* internal reference >>>> counter */ >>> >>> >> >> Kind regards >> Ulf Hansson Thanks! Ulf Hansson