[U-Boot] [PATCH v2 1/3] mmc: sdhci: fix the wrong operation when response type is R1b

[Jae hoon Chung]

Hi Lei.

I will try to  test with  your opinion.
Just wondering..If apply with my patch, is it something problem?
Anyway i will test and share the result. then resend the patch.

I want to use the generic sdhci code.

Best Regards,
Jaehoon Chung

2012/3/31 Lei Wen <adrian.wenl at gmail.com>:
> Hi Jaehoon,
>
> On Fri, Mar 30, 2012 at 2:23 PM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>> On 03/30/2012 02:24 PM, Lei Wen wrote:
>>
>>> Hi Jaehoon,
>>>
>>> On Fri, Mar 30, 2012 at 12:36 PM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>>>> Hi Lei.
>>>>
>>>> First, thanks for implemented the generic sdhci controller.
>>>
>>> It is my pleasure to share this common code, and I'm glad that it is
>>> used for other platforms now. :)
>>>
>>>>
>>>> On 03/30/2012 12:33 PM, Lei Wen wrote:
>>>>
>>>>> Hi Jaehoon,
>>>>>
>>>>> On Fri, Mar 30, 2012 at 10:39 AM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>>>>>> When response type is R1b, mask value is added the SDHCI_INT_DAT_END.
>>>>>> but in while(), didn't check that flag.
>>>>>> So sdhci controller didn't work fine.
>>>>>> CMD6 didn't always complete.
>>>>>
>>>>> Could you elaborate it more in details?
>>>>>         do {
>>>>>                 stat = sdhci_readl(host, SDHCI_INT_STATUS);
>>>>>                 if (stat & SDHCI_INT_ERROR)
>>>>>                         break;
>>>>>         } while ((stat & mask) != mask);
>>>>> Here in the while condition, if the status read out don't contain all mask,
>>>>> then the looping would continue.
>>>>> Do you mean you just need a retry max time set here?
>>>>
>>>> I found that didn't initialize the eMMC card.
>>>> Because when send CMD6, running infinite loop in there.
>>>> CMD6's mask is set to SDHCI_INT_RESPONSE and SDHCI_INT_DATA_END.
>>>> (Because response type is R1B).
>>>> Then mask value maybe is 0x3...
>>>> stat = sdhci_readl(host, SDHCI_INT_STATUS);
>>>> stat is 0x1.(cmd is done response).
>>>> but in while(), stat & mask is 0x1, and mask is 0x3.
>>>> ...doing while().
>>>> If just add the timeout, then always produced the timeout error.
>>>
>>> I see your problems. Your silicon only provide SDHCI_INT_RESPONSE for
>>> r1b, while the standard
>>> sdhci spec require both the flag is set when the r1b command is finished.
>>> So my point is you could add a quirk condition check in the previously
>>> endless loop checking.
>>> If the quirk is true, and timeout count is met, then you could jump
>>> out of original looping.
>>> And for later checking, this timeout could be regarded as no error,
>>> since the controller would never
>>> return the SDHCI_INT_DATA_END in your case.
>>
>>
>> Anyway it's need that use the timeout value. right?
> Yes, for the timeout patch here, I am ok with that.
>
>> And in our case, It's means that use the quirks?
> Using quirks, or set a timeout value large enough? Maybe latter is
> more generic. :)
>
>> I didn't find that the sdhci spec require the flag is set when the r1b command is finished.
>> where is specified?
> The spec I mention is
> https://www.sdcard.org/developers/overview/host_controller/simple_spec/Simplified_SD_Host_Controller_Spec.pdf
> You could look at page 64, the bit description for "Transfer Complete":
> (3) In the case of a command with busy
> This bit is set when busy is de-asserted. Refer to DAT Line Active
> and Command Inhibit (DAT) in the Present State register
>
>> I just understand that the transfer complete bit should be set to 1,
>> when send the command with busy flag or read/write with data.
>> Is this means?
>
> Yes, that is what I means, if you send a command with r1b flag, then
> you should expect both
> transfer complete and command complete bit is set, only command
> complete bit set doesn't mean
> your command is safely treated, and you could do the next step further.
>
>>
>> If you want to use the quirks, i will test with applied your opinion.
> I think you only need to add timeout into this while looping, and not
> touching other code.
>        do {
>                stat = sdhci_readl(host, SDHCI_INT_STATUS);
>                if (stat & SDHCI_INT_ERROR)
>                        break;
>        } while ((stat & mask) != mask);
>
>>
>> Best Regards,
>> Jaehoon Chung
>
> Thanks,
> Lei
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot