On 28 August 2014 09:11, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On 28 August 2014 04:24, Kuninori Morimoto
> <kuninori.morimoto.gx@xxxxxxxxx> wrote:
>> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
>>
>> This patch ensures that the clock has been stopped before
>> it calls tmio_mmc_set_clock().
>> The clock settings might be failed without this patch
>>
>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
>> ---
>> v3 -> v4
>>
>> - based on latest git://git.linaro.org/people/ulf.hansson/mmc.git#next
>>
>> drivers/mmc/host/tmio_mmc_pio.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
>> index ba45413..7289331 100644
>> --- a/drivers/mmc/host/tmio_mmc_pio.c
>> +++ b/drivers/mmc/host/tmio_mmc_pio.c
>> @@ -924,12 +924,14 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>> tmio_mmc_clk_stop(host);
>> break;
>> case MMC_POWER_UP:
>> + tmio_mmc_clk_stop(host);
>
> I don't think this is needed. Let me elaborate on why.
>
> There two scenarios for when you may come down this path.
>
> Either after ->probe() or after a MMC_POWER_OFF has been handled. In
> both scenarios, tmio_mmc_clk_stop(host) has already been invoked.
>
> Well, actually in the ->probe() we invoke tmio_mmc_clk_stop() and then
> tmio_mmc_reset(), should those maybe be done in reverse order to make
> sure the clock is stopped?
>
>> tmio_mmc_set_clock(host, ios->clock);
>> tmio_mmc_power_on(host, ios->vdd);
>> tmio_mmc_clk_start(host);
>> tmio_mmc_set_bus_width(host, ios->bus_width);
>> break;
>> case MMC_POWER_ON:
>> + tmio_mmc_clk_stop(host);
>> tmio_mmc_set_clock(host, ios->clock);
>> tmio_mmc_clk_start(host);
>> tmio_mmc_set_bus_width(host, ios->bus_width);
>> @@ -1199,6 +1201,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
>> tmio_mmc_clk_update(host);
>>
>> if (host->clk_cache) {
>> + tmio_mmc_clk_stop(host);
>
> This should also not be needed.
>
> To get runtime PM resumed, you must first be runtime PM suspended. In
> the runtime PM suspend callback we have already invoked
> tmio_mmc_set_clock().
/s /tmio_mmc_set_clock() / tmio_mmc_clk_stop()
>
> Well, again it may depend on if tmio_mmc_reset() should be followed by
> a tmio_mmc_clk_stop(), which isn't the case right now.
>
>> tmio_mmc_set_clock(host, host->clk_cache);
>> tmio_mmc_clk_start(host);
>> }
>> --
>> 1.7.9.5
>>
>
> Finally, another question - somewhat related to clocks but not so much
> to this patch.
>
> Shouldn't tmio_mmc_set_clock() when requested rate is 0, actually
> invoke tmio_mmc_clk_stop()? How will otherwise the clock be stopped?
>
> Kind regards
> Uffe
--
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
