On 27 March 2018 at 13:12, Simon Horman <ho...@verge.net.au> wrote:
> On Tue, Feb 27, 2018 at 02:02:23PM +0100, Wolfram Sang wrote:
>> > Perhaps someone need to explain in more detail what the HW controller
>> > needs to manage tuning for HS400? I don't like that we may end up
>> > getting it magically to work, then it's we better understand the
>> > details and if the current sequence provided by the mmc core can't
>> > fulfill the need for this case, we may instead consider adding an
>> > additional pre/post host ops.
>> Thanks. Exactly this thinking prevented me from saying "It works, let's
>> ship it" ;)
> Thanks, I've dug a little deeper into the history of this code and
> the associated hardware documentation and it seems that the desired
> sequence is as follows:
> 1. Reset HS400 mode (currently executed near beginning of tmio_mmc_set_ios())
> 2. Set clock for HS200 mode (tmio_mmc_set_ios())
> 3. Tune in HS200 mode
> 4. Set clock for HS200 mode (tmio_mmc_set_ios())
> 5. Set to HS400 mode (currently renesas_sdhi_prepare_hs400_tuning() which
>                       is executed near the end of tmio_mmc_set_ios())

This seems to also refer to how the eMMC spec describes the sequence,
and thus also implemented by the mmc core.

> X. It is also necessary to disable the SCC after setting HS400 mode
>    has occurred before setting to High Speed mode.
>    This is currently executed near the beginning of tmio_mmc_set_ios().
> The current implementation differs a little from the above and its
> hard to tell if that is because of hardware requirements or as
> an artefact of the implementation of steps 1 and 5 being hooked into
> tmio_mmc_set_ios().
> It seems to me that step 1 could use the MMC core prepare_hs400_tuning
> hook. Its less clear to me where steps 5 and X should hook in but perhaps
> tmio_mmc_set_ios() is the right place.

In ->set_ios() you are relying on checking the value of ->ios.timing
(MMC_TIMING_MMC_HS200 and MMC_TIMING_MMC_HS400) to understand what
actions to take.

For tuning, we found this was not sufficient when trying to cope with
all different MMC controller constraints, which is why we added the
->prepare_hs400_tuning() ops. I am guessing it should work for your
case as well.

However, if that isn't the case and you find reasons to add another
->[xyz]_hs400_tuning() callback, then I am open to discuss that.

Kind regards

Reply via email to