Re: [PATCH 3/3] mmc: tmio: Fix reset operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 07, 2018 at 01:22:52AM +0200, Niklas Söderlund wrote:
> From: Masaharu Hayakawa <masaharu.hayakawa.ry@xxxxxxxxxxx>
> 
> SD / MMC did not operate properly when suspend transition failed.
> Because the SCC was not reset at resume, issue of the command failed.
> Changed tmio_mmc_reset() to tmio_mmc_hw_reset() in order to add reset
> of SCC to tmio_mmc_host_runtime_resume().

So, my gut feeling is that this change, in general, seems OK. However,
the naming (tmio_mmc_reset and tmio_mmc_hw_reset) becomes confusing and
unneeded. tmio_mmc_reset now only gets called in tmio_mmc_hw_reset, so
we can merge the two functions into one to reduce the confusion.

Because we add stuff to the function which gets pointed to by
mmc_ops->hw_reset, it makes sense to me to double check when the MMC
core calls this pointer and if our additions still make sense there.
I'd think so, but better safe than sorry.

> On runtime power management resume, the host clock needs to be
> enabled before calling tmio_mmc_reset. If the mmc device has a power
> domain entry, the host clock is enabled via genpd_runtime_resume,
> running before tmio_mmc_host_runtime_resume. If the mmc device has
> no power domain entry, however, genpd_runtime_resume is not called.
> This patch changes tmio_mmc_host_runtime_resume to enable the host
> clock before calling tmio_mmc_reset.

I think this makes sense but should be a seperate patch. Or maybe even
something slightly different? Because we have a patch in the BSP [1] for
which I ever wondered in what case it was needed. But seeing the above,
it might make sense to add the clock enablement to the reset routine?
Niklas, do you have time to check that?

Thanks,

   Wolfram

[1] 940904fe4ea8 ("mmc: tmio: Add SDCLK enable after reset")

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux