[U-Boot] [BUG] 0001-mmc-fix-the-wrong-disabling-clock.patch

[Heinrich Schuchardt]

On 01/29/2018 02:34 AM, Jaehoon Chung wrote:
> On 01/29/2018 09:33 AM, Heinrich Schuchardt wrote:
>> On 01/26/2018 07:58 AM, Jaehoon Chung wrote:
>>> Hi Heinrich,
>>>
>>> On 01/26/2018 12:28 PM, Jaehoon Chung wrote:
>>>> Hi,
>>>>
>>>> On 01/26/2018 04:37 AM, Heinrich Schuchardt wrote:
>>>>> With git master HEAD the Odroid C2 hangs after the command line countdown.
>>>>>
>>>>> Bisecting teaches that this is the patch causing the problem:
>>>>>
>>>>> 9546eb92cb648a8bba0aa9d5930ac751e6e5b9a4
>>>>> mmc: fix the wrong disabling clock
>>>>
>>>> When you search for commit with bisecting, this commit might be occurred a problem.
>>>> So i have sent the patches a below patches.
>>>>
>>>> I don't have odroid-c2 board. so it's difficult to find what happen to your board.
>>>> Could you share the log message?
>>>>
>>>> Almost all boards are working fine. and those patches make sense.
>>>>
>>>> If revert these patches, too many boards can't use MMC card.
>>>>
>>>>>
>>>>> Please, revert the patch.
>>>>
>>>> Well, my opinion is first step that we try to find what is main problem about odriod-c2 or others..
>>>> Not only revert the patch. Also i will try to solve your issue.
>>>
>>> I didn't have Odroid-C2 board, i can't test anything. Could you test with below code?
>>>
>>> diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
>>> index 3cd934634d..c17efd25c6 100644
>>> --- a/drivers/mmc/meson_gx_mmc.c
>>> +++ b/drivers/mmc/meson_gx_mmc.c
>>> @@ -35,6 +35,9 @@ static void meson_mmc_config_clock(struct mmc *mmc)
>>>         uint32_t meson_mmc_clk = 0;
>>>         unsigned int clk, clk_src, clk_div;
>>>  
>>> +       if (mmc->clock == 0)
>>> +               return 0;
>>
>> You should not return 0 from a void function.
>>
>> So, please, write
>>
>> 	if (!mmc->clock)
>> 		return;
> 
> Just for checking your problem.
> I didn't have odroid-c2, so i don't know whether it's working fine or not.
> Is it working fine?
> 
>>
>>> +
>>>         /* 1GHz / CLK_MAX_DIV = 15,9 MHz */
>>>         if (mmc->clock > 16000000) {
>>>                 clk = SD_EMMC_CLKSRC_DIV2;
>>>
>>
>> Current git HEAD plus this change allows reading from MMC.
>>
>> I added a print command and found that the functions is called twice
>> with a non-zero value:
>>
>> MMC:
>> meson_mmc_config_clock: mmc->clock: 380953
>> meson_mmc_config_clock: mmc->clock: 380953
>> mmc at 72000: 0, mmc at 74000: 1
>>
>> But quite a lot of changes have been merged since my first mail, so I am
>> not sure which change is relevant.
> 
> twice called?

There is are two mmcs:
mmc at 72000: 0, mmc at 74000: 1

Maybe that is why.

In my opinion, meson_gx_mmc is calling mmc_set_clock() in
meson_mmc_probe() at probing time.
> And after that, mmc_set_clock() is calling at initial time in mmc.c.
> So it should be called twice..this is just my thinking.
> 
> I want to know that odroid-c2 is working fine or not.?

With current git master + your patch it is running fine.
Looking at the debug output I doubt that checking for division by zero
is making a difference here.

But the git master has changed since my first mail in this thread.

Best regards

Heinrich


> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Best regards
>>
>> Heinrich
>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>>>
>>>> Best Regards,
>>>> Jaehoon Chung
>>>>
>>>>>
>>>>> I could not find the patch in
>>>>> https://lists.denx.de/pipermail/u-boot/2018-January/date.html
>>>>>
>>>>> http://patchwork.ozlabs.org/patch/864607/
>>>>> seems to be related but does not solve the issue.
>>>>>
>>>>> Best regards
>>>>>
>>>>> Heinrich
>>>>>
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot at lists.denx.de
>>>> https://lists.denx.de/listinfo/u-boot
>>>>
>>>
>>>
>>
>>
>>
>>
> 
>