Message-ID: <02725a0f-c061-e7f2-9a01-8975c62ab5a7@marvell.com>
Date:   Tue, 29 Nov 2016 18:33:48 +0800
From:   Ziji Hu <huziji@...vell.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
CC:     Gregory CLEMENT <gregory.clement@...e-electrons.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        Jason Cooper <jason@...edaemon.net>,
        Andrew Lunn <andrew@...n.ch>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Jimmy Xu <zmxu@...vell.com>,
        Jisheng Zhang <jszhang@...vell.com>,
        Nadav Haklai <nadavh@...vell.com>, Ryan Gao <ygao@...vell.com>,
        Doug Jones <dougj@...vell.com>, Victor Gu <xigu@...vell.com>,
        "Wei(SOCP) Liu" <liuw@...vell.com>,
        Wilson Ding <dingwei@...vell.com>,
        Romain Perier <romain.perier@...e-electrons.com>,
        Yehuda Yitschak <yehuday@...vell.com>,
        Marcin Wojtas <mw@...ihalf.com>,
        Hanna Hawa <hannah@...vell.com>,
        Kostya Porotchkin <kostap@...vell.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell
 Xenon SDHC

Hi Ulf,

On 2016/11/29 15:49, Ulf Hansson wrote:
> On 29 November 2016 at 03:53, Ziji Hu <huziji@...vell.com> wrote:
>> Hi Ulf,
>>
>> On 2016/11/28 23:16, Ulf Hansson wrote:
>>> On 28 November 2016 at 12:38, Ziji Hu <huziji@...vell.com> wrote:
>>>> Hi Ulf,
>>>>
>>>> On 2016/11/28 19:13, Ulf Hansson wrote:
>>>>>>
>>>>>>     As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), to
>>>>>>     send commands for testing current sampling point set in our host PHY.
>>>>>>
>>>>>>     According to my test result, it shows that mmc_send_tuning() can only support
>>>>>>     tuning command (CMD21/CMD19).
>>>>>>     As a result, we cannot use mmc_send_tuning() when card is in the speed modes
>>>>>>     which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
>>>>>>     SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in those
>>>>>>     speed modes.
>>>>>>
>>>>>>     Could you please provide suggestions for the speed mode in which tuning is
>>>>>>     not available?
>>>>>>
>>>>>
>>>>> Normally the mmc host driver shouldn't have to care about what the
>>>>> card supports, as that is the responsibility of the mmc core to
>>>>> manage.
>>>>>
>>>>> The host should only need to implement the ->execute_tuning() ops,
>>>>> which gets called when the card supports tuning (CMD19/21). Does it
>>>>> make sense?
>>>>>
>>>>    I think it is irrelevant to tuning procedure.
>>>>
>>>>    Our host requires to adjust PHY setting after each time ios setting
>>>>    (SDCLK/bus width/speed mode) is changed.
>>>>    The simplified sequence is:
>>>>    mmc change ios --> mmc_set_ios() --> ->set_ios() --> after sdhci_set_ios(),
>>>>    adjust PHY setting.
>>>>    During PHY setting adjustment, out host driver has to send commands to
>>>>    test current sampling point. Tuning is another independent step.
>>>
>>> For those speed modes (or other ios changes) that *don't* requires
>>> tuning, then what will you do when you send the command to confirm the
>>> change of PHY setting and it fails?
>>>
>>> My assumption is that you will fail anyway, by propagating the error
>>> to the mmc core. At least that what was my understanding from your
>>> earlier replies, right!?
>>>
>>> Then, I think there are no point having the host driver sending a
>>> command to confirm the PHY settings, as the mmc core will anyway
>>> discover if something goes wrong when the next command is sent.
>>>
>>> Please correct me if I am wrong!
>>>
>>
>>    Sorry that I didn't make myself clear.
>>
>>    Our host PHY delay line consists of hundreds of sampling points.
>>    Each sampling point represents a different phase shift.
>>
>>    In lower speed mode, our host driver will scan the delay line.
>>    It will select and test multiple sampling points, other than testing
>>    only single sampling point.
>>
>>    If a sampling point fails to transfer cmd/data, our host driver will
>>    move to test next sampling point, until we find out a group of successful
>>    sampling points which can transfer cmd/data. At last we will select
>>    a perfect one from them.
> 
> Ahh, I see. Unfortunate, this is going to be very hard to implement properly.
> 
> The main problem is that the host driver has *no* knowledge about the
> internal state of the card, as that is the responsibility of the mmc
> core to keep track of.
> 
> If the host driver would send a command during every update of the
> "ios" setting, from ->set_ios(), for sure it would lead to commands
> being sent that are "forbidden" in the current internal state of the
> card.
> This would lead to that the card initialization sequence fails,
> because the card may move to an unknown internal state and the mmc
> core would have no knowledge about what happened.
> 

   Yes. In theory, host layer should not initiate a command by itself.

   We assume that bus is idle and card is stable in Tran state, when core layer
   asks host to switch "ios".
   Besides, we only select the commands which is valid in the whole procedure,
   such as CMD8 for eMMC.
   Those test commands are actually like read operations to card registers.
   The card will return to Tran state even if transfer fails. It is also easy
   for host to recover.  

> Hmm..
> 
> Can you specify, *exactly*, under which "ios updates" you need to
> verify updated PHY setting changes by sending a cmd/data? Also, please
> specify if it's enough to only test the CMD line or also DATA lines.
> 

   When one of the three parameters in below changes, our host driver needs
   to adjust PHY in lower speed mode.
   1. Speed Mode (timing): like legacy mode --> HS DDR
   2. Bus Clock: like 400KHz --> 50MHz
   3. Bus Width: like 1-bit --> 4-bit/8-bit

   For eMMC, we use CMD8 to test sampling point.
   For SD, we use CMD13.
   For SDIO, currently CMD52 is used to read a register from CCCR.
   Those commands in above are all valid during the whole procedure to switch
   to high speed mode from legacy mode.

   It is the best case if the test command can transfer both on CMD and DAT lines.
   CMD8 for eMMC can test both CMD line and DAT lines. CMD13 and CMD52 only test
   CMD line. We might use ACMD51 for SD and CMD53 for SDIO later thus DAT lines
   are also under test.

> Kind regards
> Uffe
> 

Powered by blists - more mailing lists