Re: [PATCH] omap: hsmmc funtionality breaks when CONFIG_PM not define

[kishore kadiyala <kishorek.kadiyala@gmail.com>]

Hi Adrian,

Is there any use case or a valid scenario  whether to use
omap_hsmmc_ps_ops or omap_hsmmc_ops ?
I meant using power_saving option and without power_saving option

Ideally I feel having omap_hsmmc_ps_ops as default with or without
CONFIG_PM would be better as it internally has a state handling.

Regards,
Kishore

On Tue, Jul 20, 2010 at 2:43 PM, Adrian Hunter <adrian.hunter@nokia.com> wrote:
> Ghorai, Sukumar wrote:
>>
>>> -----Original Message-----
>>> From: Adrian Hunter [mailto:adrian.hunter@nokia.com]
>>> Sent: Tuesday, July 20, 2010 2:10 PM
>>> To: Ghorai, Sukumar
>>> Cc: linux-mmc@vger.kernel.org; linux-omap@vger.kernel.org; Shilimkar,
>>> Santosh; Chikkature Rajashekar, Madhusudhan; Andrew Morton
>>> Subject: Re: [PATCH] omap: hsmmc funtionality breaks when CONFIG_PM not
>>> define
>>>
>>> Ghorai, Sukumar wrote:
>>>>
>>>> Adrian,
>>>>
>>>>> -----Original Message-----
>>>>> From: Adrian Hunter [mailto:adrian.hunter@nokia.com]
>>>>> Sent: Tuesday, July 20, 2010 1:40 PM
>>>>> To: Ghorai, Sukumar
>>>>> Cc: linux-mmc@vger.kernel.org; linux-omap@vger.kernel.org; Shilimkar,
>>>>> Santosh; Chikkature Rajashekar, Madhusudhan; Andrew Morton
>>>>> Subject: Re: [PATCH] omap: hsmmc funtionality breaks when CONFIG_PM not
>>>>> define
>>>>>
>>>>> Ghorai, Sukumar wrote:
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Adrian Hunter [mailto:adrian.hunter@nokia.com]
>>>>>>> Sent: Tuesday, July 20, 2010 12:39 PM
>>>>>>> To: Ghorai, Sukumar
>>>>>>> Cc: linux-mmc@vger.kernel.org; linux-omap@vger.kernel.org; Shilimkar,
>>>>>>> Santosh; Chikkature Rajashekar, Madhusudhan; Andrew Morton
>>>>>>> Subject: Re: [PATCH] omap: hsmmc funtionality breaks when CONFIG_PM
>>>
>>> not
>>>>>>>
>>>>>>> define
>>>>>>>
>>>>>>> Sukumar Ghorai wrote:
>>>>>>>>
>>>>>>>>    Issue if power_saving option passed from board file and
>>>
>>> CONFIG_PM
>>>>>>>
>>>>>>> not define.
>>>>>>>>
>>>>>>>>    This is because hosts refer to wrong operation table and that
>>>
>>> try
>>>>>
>>>>> to
>>>>>>>
>>>>>>> power save.
>>>>>>>
>>>>>>> power_saving is not related to power management.  It should work with
>>>>>
>>>>> or
>>>>>>>
>>>>>>> without
>>>>>>> CONFIG_PM.  What problem are you seeing?
>>>>>>
>>>>>> [Ghorai] when CONFIG_PM is not defined but power_saving flag is TRUE,
>>>>>
>>>>> then host operation table point to table which handles in state machine
>>>>> as: ENABLE -> CARDSLEEP -> REGSLEEP -> DISABLED for power/clock cut.
>>>
>>> And
>>>>>
>>>>> do the reverse for the enable clock/power.
>>>>>>
>>>>>> And power saving is not required and wont work when CONFIG_PM is not
>>>>>
>>>>> enabled.
>>>>>
>>>>> How is that a problem?  It would be useful to know what you need?
>>>>
>>>> [Ghorai] do men when CONFIG_PM is not enabled still we should do power
>>>
>>> saving in mmc?
>>>
>>> Why not?
>>>
>>> Because issue is iclk/fclk is quite depended on PRCM framework when we
>>> are
>>> using omap_hsmmc_ps_ops operation table.
>>>
>>> iclk is left alone.
>>>
>>> fclk is manipulated with or without power_saving.
>>>
>>> I still do not understand your problem sorry :-(
>>
>> [Ghorai] thanks and need input again,  we have two tables -
>> 1. static const struct mmc_host_ops omap_hsmmc_ops = {
>> }
>> --> This is without CONFIG_PM, assumed.
>
> No it is independent of CONFIG_PM
>
> With CONFIG_PM
>        - the driver will restore registers if the host controller
>        has been powered off / on by PRCM
>        - suspend / resume can be used
>
>>
>> 2. static const struct mmc_host_ops omap_hsmmc_ps_ops = {
>> }
>> -> this with CONFIG_PM, assumed.
>
> No it is independent of CONFIG_PM
>
> Without CONFIG_PM
>        - card will be put to sleep after 1 second
>        - card will be powered off after another 8 seconds
>
>>
>> a. And you feel we should remove #1
>
> Nothing should need to be removed.
>
>> b. use omap_hsmmc_ps_ops default.
>
> With power_saving it is yes.
>
>> c. And it should work in all case?
>
> It should work in all cases.
>
>>
>>>>>> So if CONFIG_PM is not enable, then it should do simple clock
>>>>>
>>>>> disable/enable, and not via the power state machine.
>>>>>
>>>>> If that is what you want, simply change your board file:
>>>>>
>>>>> #if CONFIG_PM
>>>>>        .power_saving = true,
>>>>> #else
>>>>>        .power_saving = false,
>>>>> #end
>>>>
>>>> [Ghorai] the fix I send is to guard in MMC/SD host driver to avoid
>>>
>>> mistake in board file(s). And this file is used for multiple omap3, omap4
>>> boards.
>>>>>>>>
>>>>>>>> Signed-off-by: Sukumar Ghorai <s-ghorai@ti.com>
>>>>>>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>>>>>>> CC: Madhusudhan Chikkature <madhu.cr@ti.com>
>>>>>>>> CC: Andrew Morton <akpm@linux-foundation.org>
>>>>>>>> ---
>>>>>>>>  Tested on omap3, omap4430 ES2.0
>>>>>>>>
>>>>>>>>  drivers/mmc/host/omap_hsmmc.c |    2 ++
>>>>>>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mmc/host/omap_hsmmc.c
>>>>>>>
>>>>>>> b/drivers/mmc/host/omap_hsmmc.c
>>>>>>>>
>>>>>>>> index b032828..f84eed0 100644
>>>>>>>> --- a/drivers/mmc/host/omap_hsmmc.c
>>>>>>>> +++ b/drivers/mmc/host/omap_hsmmc.c
>>>>>>>> @@ -2015,9 +2015,11 @@ static int __init omap_hsmmc_probe(struct
>>>>>>>
>>>>>>> platform_device *pdev)
>>>>>>>>
>>>>>>>>        platform_set_drvdata(pdev, host);
>>>>>>>>        INIT_WORK(&host->mmc_carddetect_work, omap_hsmmc_detect);
>>>>>>>>
>>>>>>>> +#ifdef CONFIG_PM
>>>>>>>>        if (mmc_slot(host).power_saving)
>>>>>>>>                mmc->ops        = &omap_hsmmc_ps_ops;
>>>>>>>>        else
>>>>>>>> +#endif
>>>>>>>>                mmc->ops        = &omap_hsmmc_ops;
>>>>>>>>
>>>>>>>>        /*
>>>>>>>> --
>>>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
>>>>>
>>>>> in
>>>>>>>>
>>>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>>>>
>>>>>> Regards,
>>>>>> Ghorai
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
>>>
>>> in
>>>>>>
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> 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@vger.kernel.org
>> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>