Hi Adrian,

On 2016/10/11 20:39, Adrian Hunter wrote:
> On 07/10/16 18:22, Gregory CLEMENT wrote:
>> From: Ziji Hu <huz...@marvell.com>
>>
>> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
>> Three types of PHYs are supported.
>>
>> Add support to multiple types of PHYs init and configuration.
>> Add register definitions of PHYs.
>>
>> Signed-off-by: Hu Ziji <huz...@marvell.com>
>> Reviewed-by: Gregory CLEMENT <gregory.clem...@free-electrons.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clem...@free-electrons.com>
>> ---
>>  MAINTAINERS                        |    1 +-
>>  drivers/mmc/host/Makefile          |    2 +-
>>  drivers/mmc/host/sdhci-xenon-phy.c | 1141 +++++++++++++++++++++++++++++-
>>  drivers/mmc/host/sdhci-xenon-phy.h |  157 ++++-
>>  drivers/mmc/host/sdhci-xenon.c     |    4 +-
>>  drivers/mmc/host/sdhci-xenon.h     |   17 +-
>>  6 files changed, 1321 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
>>  create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 859420e5dfd3..b5673c2ee5f2 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7583,6 +7583,7 @@ M:     Ziji Hu <huz...@marvell.com>
>>  L:  linux-...@vger.kernel.org
>>  S:  Supported
>>  F:  drivers/mmc/host/sdhci-xenon.*
>> +F:  drivers/mmc/host/sdhci-xenon-phy.*
>>  F:  Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>>  
>>  MATROX FRAMEBUFFER DRIVER
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index 75eaf743486c..4f2854556ff7 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -82,4 +82,4 @@ ifeq ($(CONFIG_CB710_DEBUG),y)
>>  endif
>>  
>>  obj-$(CONFIG_MMC_SDHCI_XENON)       += sdhci-xenon-driver.o
>> -sdhci-xenon-driver-y                += sdhci-xenon.o
>> +sdhci-xenon-driver-y                += sdhci-xenon.o sdhci-xenon-phy.o
>> diff --git a/drivers/mmc/host/sdhci-xenon-phy.c 
>> b/drivers/mmc/host/sdhci-xenon-phy.c
>> new file mode 100644
>> index 000000000000..4eb8fea1bec9
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-xenon-phy.c
> 
> <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.

        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
> 

Reply via email to