[U-Boot] Problem with initialize in mmc_initialize in mmc.c

[Jaehoon Chung]

Hi,

On 11/27/2017 05:16 PM, Jaehoon Chung wrote:
> Hi Faiz,

Remove the some mail account, because of private problem.

> 
> On 11/02/2017 11:23 PM, Faiz Abbas wrote:
>>
>>
>> On Monday 30 October 2017 07:37 PM, Faiz Abbas wrote:
>>> Hi,
>>>
>>> The variable *initialized* in mmc_initialize() is declared as static and
>>> initialised to 0 in the following commit. This makes the compiler put it
>>> in the .bss section of the image.
>>>
>>> commit 1b26bab12e85e8b0d382d6775e40d14445249574
>>> Author: Daniel Kochmański <dkochmanski at turtle-solutions.eu>
>>> Date:   Fri May 29 16:55:43 2015 +0200
>>>
>>>     mmc: Protect `mmc_initialize` from initialising mmc multiple times
>>>
>>>     `mmc_initialize` might be called multiple times leading to the
>>> mmc-controllers being initialised twice, and initialising the
>>> `mmc_devices` list head twice which may lead to memory leaks.
>>>
>>>     Signed-off-by: Daniel Kochmański <dkochmanski at turtle-solutions.eu>
>>>     CC: Roy Spliet <r.spliet at ultimaker.com>
>>>     Cc: Ian Campbell <ijc at hellion.org.uk>
>>>     CC: Pantelis Antoniou <panto at antoniou-consulting.com>
>>>     Acked-by: Hans de Goede <hdegoede at redhat.com>
>>>     Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>>
>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>> index da47037..f12546a 100644
>>> --- a/drivers/mmc/mmc.c
>>> +++ b/drivers/mmc/mmc.c
>>> @@ -1762,6 +1762,11 @@ static void do_preinit(void)
>>>
>>>  int mmc_initialize(bd_t *bis)
>>>  {
>>> +       static int initialized = 0;
>>> +       if (initialized)        /* Avoid initializing mmc multiple times */
>>> +               return 0;
>>> +       initialized = 1;
>>> +
>>>         INIT_LIST_HEAD (&mmc_devices);
>>>         cur_dev_num = 0;
>>>
>>>
>>> .bss should not be accessed in u-boot before relocation because it
>>> overlaps with fdt and writing to variables in .bss can corrupt the fdt.
>>> MMC can be probed before relocation if it contains the u-boot
>>> environment. Therefore, I tried to move this variable to the .data
>>> section by
> 
> Will check with T32..but i didn't have seen about similar case..
> You means it should be the multiple initializing?
> 
> Best Regards,
> Jaehoon Chung
> 
>>>
>>> static int initialized __attribute__((section(".data")));
>>>
>>> When *initialized* was a part of .bss it was getting re-initilized to 0
>>> as a part of relocation. Therefore, mmc was getting probed again
>>> successfully after relocation with new addresses for mmc devices.
>>>
>>> Now when *initialized* is not a part of .bss, it holds its value across
>>> relocation and a second call to mmc_initialize() returns without doing
>>> anything. However, the *mmc_devices* list containing pointers to the
>>> older locations of mmc devices is not refreshed and thus mmc devices
>>> fail to enumerate.
>>>
>>> So *initialized* is a problem whether it is in .data or .bss.
>>> I am not sure how to fix this. Any help is appreciated.
>>
>> I have been told that all pointers are supposed to be updated during
>> relocation. Can anyone point to relevant code or example of the same?
>>
>> Thanks,
>> Faiz
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>>
>