[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d63c5272-9085-d54f-e827-ca6b9b324aab@intel.com>
Date: Tue, 18 Oct 2016 16:09:14 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Ziji Hu <huziji@...vell.com>,
Gregory CLEMENT <gregory.clement@...e-electrons.com>,
Ulf Hansson <ulf.hansson@...aro.org>, linux-mmc@...r.kernel.org
Cc: 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,
Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
linux-arm-kernel@...ts.infradead.org,
"Jack(SH) Zhu" <jmzhu@...vell.com>, 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>,
Shiwu Zhang <zhangshw@...vell.com>,
Victor Gu <xigu@...vell.com>,
"Wei(SOCP) Liu" <liuw@...vell.com>,
Wilson Ding <dingwei@...vell.com>,
Xueping Liu <xpliu@...vell.com>,
Hilbert Zhang <zzhang@...vell.com>,
Liuliu Zhao <zhaoliul@...vell.com>,
Peng Zhu <zhupeng@...vell.com>, Yu Cao <yucao@...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
Subject: Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell
Xenon SDHC
On 18/10/16 15:04, Ziji Hu wrote:
> Hi Adrian,
>
> On 2016/10/17 15:55, Adrian Hunter wrote:
>> On 12/10/16 15:17, Ziji Hu wrote:
>>> Hi Adrian,
>>>
>>> On 2016/10/11 20:39, Adrian Hunter wrote:
> <snip>
>>>>> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
>>>>> +{
>>>>> + int err;
>>>>> + u8 *ext_csd = NULL;
>>>>> +
>>>>> + err = mmc_get_ext_csd(card, &ext_csd);
>>>>> + kfree(ext_csd);
>>>>> +
>>>>> + return err;
>>>>> +}
>>>>> +
>>>>> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
>>>>> +{
>>>>> + struct mmc_command cmd = {0};
>>>>> + int err;
>>>>> +
>>>>> + cmd.opcode = SD_IO_RW_DIRECT;
>>>>> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
>>>>> +
>>>>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>>>>> + if (err)
>>>>> + return err;
>>>>> +
>>>>> + if (cmd.resp[0] & R5_ERROR)
>>>>> + return -EIO;
>>>>> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
>>>>> + return -EINVAL;
>>>>> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
>>>>> + return -ERANGE;
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
>>>>> +{
>>>>> + struct mmc_command cmd = {0};
>>>>> + int err;
>>>>> +
>>>>> + cmd.opcode = MMC_SEND_STATUS;
>>>>> + cmd.arg = card->rca << 16;
>>>>> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>>>>> +
>>>>> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
>>>>> + return err;
>>>>> +}
>>>>> +
>>>>> +static int xenon_delay_adj_test(struct mmc_card *card)
>>>>> +{
>>>>> + WARN_ON(!card);
>>>>> + WARN_ON(!card->host);
>>>>> +
>>>>> + if (mmc_card_mmc(card))
>>>>> + return __xenon_emmc_delay_adj_test(card);
>>>>> + else if (mmc_card_sd(card))
>>>>> + return __xenon_sd_delay_adj_test(card);
>>>>> + else if (mmc_card_sdio(card))
>>>>> + return __xenon_sdio_delay_adj_test(card);
>>>>> + else
>>>>> + return -EINVAL;
>>>>> +}
>>>>
>>>> So you are issuing commands from the ->set_ios() callback. I would want to
>>>> get Ulf's OK for that before going further.
>>>>
>>> Yes, you are correct.
>>> In some speed mode, Xenon SDHC has to send a series of transfers to search for a perfect sampling point in PHY delay line.
>>> It is like tuning process.
>>>
>>>> One thing: you will need to ensure you don't trigger get HS400 re-tuning
>>>> because it will call back into ->set_ios().
>>>>
>>> Could you please make the term "HS400 re-tuning" more detailed?
>>> In current MMC driver, "HS400 re-tuning" will go back to HS200, execute HS200 tuning and come back to HS400.
>>> I'm sure our Xenon SDHC will not execute it.
>>
>> Currently, re-tuning is automatically enabled whenever tuning is executed,
>> and then re-tuning will be done periodically or after CRC errors. The
>> function to disable re-tuning mmc_retune_disable() is not exported, however
>> if you have you are determining the sampling point your own way, you could
>> simply not implement ->execute_tuning() and then there would be no tuning
>> and no re-tuning.
>>
>
> It is a little complex in our Xenon SDHC.
> For the speed mode which requests tuning, such as SDR104 and HS200, our driver will execute standard tuning as spec requires.
> However, for those speed mode in which tuning is not requested in spec, our driver has to issues commands to search for the best sampling point.
>
> It seems that HS400 re-tuning/tuning is disabled by default. Is it correct?
No, it is enabled by default - there is currently no periodic re-tuning for
HS400 but CRC errors or runtime suspend / resume will cause re-tuning.
>
>>>
>>> However, in coming eMMC 5.2, there is a real HS400 re-tuning, in which tuning can be directly executed in HS400 mode.
>>> Our Xenon SDHC will neither trigger this HS400 re-tuning.
>>> But since so far there is no such feature in MMC driver, I cannot give you a 100% guarantee now.
>>>
>>>> And you have the problem that you need to get a reference to the card before
>>>> the card device has been added. As I wrote in response to the previous
>>>> patch, you should get Ulf's help with that too.
>>>>
>>> Sure.
>>> I will get card_candidate solved at first.
>>> Thank you again for your review and help.
>>>
>>> Thank you.
>>>
>>> Best regards,
>>> Hu Ziji
>>>>
>>>
>>
>
Powered by blists - more mailing lists