Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1531181 > unrolled thread

Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC

Started byZiji Hu <huziji@marvell.com>
First post2016-11-28 11:20 +0100
Last post2016-11-29 13:10 +0100
Articles 9 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ziji Hu <huziji@marvell.com> - 2016-11-28 11:20 +0100
    Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ulf Hansson <ulf.hansson@linaro.org> - 2016-11-28 12:20 +0100
      Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ziji Hu <huziji@marvell.com> - 2016-11-28 12:40 +0100
        Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ulf Hansson <ulf.hansson@linaro.org> - 2016-11-28 16:20 +0100
          Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ziji Hu <huziji@marvell.com> - 2016-11-29 04:00 +0100
            Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ulf Hansson <ulf.hansson@linaro.org> - 2016-11-29 08:50 +0100
              Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ziji Hu <huziji@marvell.com> - 2016-11-29 11:40 +0100
                Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ulf Hansson <ulf.hansson@linaro.org> - 2016-11-29 12:20 +0100
                  Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell  Xenon SDHC Ziji Hu <huziji@marvell.com> - 2016-11-29 13:10 +0100

#1531181 — Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC

FromZiji Hu <huziji@marvell.com>
Date2016-11-28 11:20 +0100
SubjectRe: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
Message-ID<sIrct-P3-27@gated-at.bofh.it>
Hi Ulf,

On 2016/11/24 23:37, Ziji Hu wrote:
> Hi Ulf,
> 
> On 2016/11/24 22:33, Ulf Hansson wrote:
<snip>
>>>
>>>    As result, our SDHC driver has to implement the functionality to
>>>    send commands and check the results, in host layer.
>>>    If directly calling mmc_wait_for_cmd() is improper, could you please
>>>    give us some suggestions?
>>>
>>>    For eMMC, CMD8 is used to test current sampling point set in PHY.
>>
>> Try to use mmc_send_tuning().
>>
> 
>     Could you please tell me the requirement of "op_code" parameter in
>     mmc_send_tuning()?
>     According to mmc_send_tuning(),it seems that tuning command(CMD19/CMD21)
>     is required. Thus device will not response mmc_send_tuning() if current
>     speed mode doesn't support tuning command.
>     Please correct me if I am wrong.
>

    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?

    Thank you.

Best regards,
Hu Ziji

>>>
>>>>> +
>>>>> +       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;
>>>>
>>>> No thanks! MMC/SD/SDIO protocol code belongs in the core.
>>>>
>>>    For SDIO, SD_IO_RW_DIRECT command is sent to test current sampling point
>>>    in PHY.
>>>    Please help provide some suggestion to implement the command transfer.
>>
>> Again, I think mmc_send_tuning() should be possible for you to use.
>>
>> [...]
>>
>>>>> +       if (mmc->card)
>>>>> +               card = mmc->card;
>>>>> +       else
>>>>> +               /*
>>>>> +                * Only valid during initialization
>>>>> +                * before mmc->card is set
>>>>> +                */
>>>>> +               card = priv->card_candidate;
>>>>> +       if (unlikely(!card)) {
>>>>> +               dev_warn(mmc_dev(mmc), "card is not present\n");
>>>>> +               return -EINVAL;
>>>>> +       }
>>>>
>>>> That your host need to hold a copy of the card pointer, tells me that
>>>> something is not really correct.
>>>>
>>>> I might be wrong, if this turns out to be a special case, but I doubt
>>>> it. Although, if it *is* a special such case, we shall most likely try
>>>> to extend the the mmc core layer instead of adding all these hacks in
>>>> your host driver.
>>>>
>>>     This card pointer copies the temporary structure mmc_card
>>>     used in mmc_init_card(), mmc_sd_init_card() and mmc_sdio_init_card().
>>>     Since we call mmc_wait_for_cmd() to send test commands, we need a copy
>>>     of that temporary mmc_card here in our host driver.
>>
>> I see, thanks for clarifying.
>>
>>>
>>>     During PHY setting in card initialization, mmc_host->card is not updated
>>>     yet with that temporary mmc_card. Thus we are not able to directly use
>>>     mmc_host->card. Instead, this card pointer is introduced to enable
>>>     mmc_wait_for_cmd().
>>>
>>>     If we can improve our host driver to send test commands without mmc_card,
>>>     this card pointer can be removed.
>>>     Could you please share your opinion please?
>>
>> The mmc_send_tuning() API takes the mmc_host as parameter. If you
>> convert to that, perhaps you would be able to remove the need to hold
>> the card pointer.
>>
>> BTW, the reason why mmc_send_tuning() doesn't take the card as a
>> parameter, is exactly those you just described above.
>>
>    Got it.
>    Thanks a lot for the information.
> 
>    Thank you for the great help.
> 
> Best regards,
> Hu Ziji
> 
>> [...]
>>
>> Kind regards
>> Uffe
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [next] | [standalone]


#1531221

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-11-28 12:20 +0100
Message-ID<sIs8y-1nY-25@gated-at.bofh.it>
In reply to#1531181
>
>     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?

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]


#1531228

FromZiji Hu <huziji@marvell.com>
Date2016-11-28 12:40 +0100
Message-ID<sIsrT-1ue-1@gated-at.bofh.it>
In reply to#1531221
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.

   Thus our host needs a valid command in PHY setting adjustment. Tuning command
   can be borrowed to complete this task in SD SDR50. But for other speed mode,
   we have to find out a valid command.

   Any suggestion please?

   Thank you.

Best regards,
Hu Ziji

> Kind regards
> Uffe
> 

[toc] | [prev] | [next] | [standalone]


#1531388

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-11-28 16:20 +0100
Message-ID<sIvSO-3L8-17@gated-at.bofh.it>
In reply to#1531228
On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.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!

>
>    Thus our host needs a valid command in PHY setting adjustment. Tuning command
>    can be borrowed to complete this task in SD SDR50. But for other speed mode,
>    we have to find out a valid command.

I thought we agreed on this wasn't necessary? Please see my upper response.

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]


#1531874

FromZiji Hu <huziji@marvell.com>
Date2016-11-29 04:00 +0100
Message-ID<sIGOd-2j9-5@gated-at.bofh.it>
In reply to#1531388
Hi Ulf,

On 2016/11/28 23:16, Ulf Hansson wrote:
> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.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.

   Thank you.

Best regards,
Hu Ziji
 
>>
>>    Thus our host needs a valid command in PHY setting adjustment. Tuning command
>>    can be borrowed to complete this task in SD SDR50. But for other speed mode,
>>    we have to find out a valid command.
> 
> I thought we agreed on this wasn't necessary? Please see my upper response.
> 
> Kind regards
> Uffe
> 

[toc] | [prev] | [next] | [standalone]


#1531976

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-11-29 08:50 +0100
Message-ID<sILkS-5lO-23@gated-at.bofh.it>
In reply to#1531874
On 29 November 2016 at 03:53, Ziji Hu <huziji@marvell.com> wrote:
> Hi Ulf,
>
> On 2016/11/28 23:16, Ulf Hansson wrote:
>> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.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.

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.

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]


#1532112

FromZiji Hu <huziji@marvell.com>
Date2016-11-29 11:40 +0100
Message-ID<sINZn-7hY-1@gated-at.bofh.it>
In reply to#1531976
Hi Ulf,

On 2016/11/29 15:49, Ulf Hansson wrote:
> On 29 November 2016 at 03:53, Ziji Hu <huziji@marvell.com> wrote:
>> Hi Ulf,
>>
>> On 2016/11/28 23:16, Ulf Hansson wrote:
>>> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.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
> 

[toc] | [prev] | [next] | [standalone]


#1532143

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-11-29 12:20 +0100
Message-ID<sIOC6-7Kl-25@gated-at.bofh.it>
In reply to#1532112
[...]

>>>>
>>>
>>>    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".

Understand, but this is a wrong assumption. The card may very well in
another state than Tran state.

>    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.

For example, I would recommend you to investigate in detail the
sequence for when a CMD6 command is sent to the card.
The host must *not* start sending commands from ->set_ios() during a
CMD6 sequence. For example a CMD8 is not allowed.

Moreover, due to this, I wonder if it is even possible to get this HW
to work properly.

>
>> 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.

Thanks for sharing these details!

So, if possible, I would recommend you to discuss these issues with
some of the HW designers. Perhaps you can figure out an alternative
method of confirming/testing PHY setting changes? Sending commands to
the card just doesn't work well for all cases.

Kind regards
Uffe

[toc] | [prev] | [next] | [standalone]


#1532230

FromZiji Hu <huziji@marvell.com>
Date2016-11-29 13:10 +0100
Message-ID<sIPou-8ih-19@gated-at.bofh.it>
In reply to#1532143
Hi Ulf,

On 2016/11/29 19:11, Ulf Hansson wrote:
> [...]
> 
>>>>>
>>>>
>>>>    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".
> 
> Understand, but this is a wrong assumption. The card may very well in
> another state than Tran state.
> 

   Could you please provide an example that card might not be in Tran state?
   It seems that card should be in Tran state after CMD6 succeed.
   If CMD6 fails, mmc driver will not execute ios setting. Thus ->set_ios()
   will not be called.

>>    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.
> 
> For example, I would recommend you to investigate in detail the
> sequence for when a CMD6 command is sent to the card.
> The host must *not* start sending commands from ->set_ios() during a
> CMD6 sequence. For example a CMD8 is not allowed.
> 
> Moreover, due to this, I wonder if it is even possible to get this HW
> to work properly.
> 

   In my very own opinion, ->set_ios() is only executed after CMD6 sequence
   succeeds, based on current mmc.c/sd.c/sdio.c.
   I personally think that it should not interfere CMD6 sequence.

   I'm afraid that HW cannot help and SW driver has to take care of this.

>>
>>> 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.
> 
> Thanks for sharing these details!
> 
> So, if possible, I would recommend you to discuss these issues with
> some of the HW designers. Perhaps you can figure out an alternative
> method of confirming/testing PHY setting changes? Sending commands to
> the card just doesn't work well for all cases.
> 

   Thanks a lot for you patience.

   Actually, we, including HW engineers, have been working on this for
   a very long time. We also test a lot on many actual products. It is
   quiet stable in real use scenarios.

   I know it is still not good enough. It seems to be impossible to find
   another practical and reliable solution, based on our tests.
   Could you please provide some suggestions thus we can try our best to improve it
   to meet your requirement?

   Thank you.

Best regards,
Hu Ziji

> Kind regards
> Uffe
> 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web