[U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated

[Jaehoon Chung]

On 07/02/2016 03:17 AM, Simon Glass wrote:
> Hi,
> 
> On 1 July 2016 at 04:37, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>> On 07/01/2016 04:28 AM, Simon Glass wrote:
>>> Hi Meteusz,
>>>
>>> On 30 June 2016 at 12:18, Mateusz Kulikowski
>>> <mateusz.kulikowski at gmail.com> wrote:
>>>> On 29.06.2016 05:27, Simon Glass wrote:
>>>>> Hi Matt,
>>>>>
>>>>> On 25 June 2016 at 14:57, Matt Corallo <linux at bluematt.me> wrote:
>>>>>> Signed-off-by: Matt Corallo <git at bluematt.me>
>>>>>> ---
>>>>>>  drivers/mmc/mmc.c | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>>>>>> index aabfc71..eba20f0 100644
>>>>>> --- a/drivers/mmc/mmc.c
>>>>>> +++ b/drivers/mmc/mmc.c
>>>>>> @@ -1734,7 +1734,8 @@ int mmc_init(struct mmc *mmc)
>>>>>>  #ifdef CONFIG_DM_MMC
>>>>>>         struct mmc_uclass_priv *upriv = dev_get_uclass_priv(mmc->dev);
>>>>>>
>>>>>> -       upriv->mmc = mmc;
>>>>>> +       if (upriv)
>>>>>> +               upriv->mmc = mmc;
>>>>>>  #endif
>>>>>>         if (mmc->has_init)
>>>>>>                 return 0;
>>>>>> --
>>>>>> 2.1.4
>>>>>
>>>>> Can you please add a commit message explaining why this is needed and
>>>>> what it fixes?  How can mmc_init() be called before the MMC device is
>>>>> there? Is this related to this patch?
>>>>>
>>>>> http://patchwork.ozlabs.org/patch/640735/
>>>>
>>>> It's related in a way that it fixed crash of dragonboard before I submitted my patch :)
>>>>
>>>> Not sure if it meant to fix dragonboard or some other board.
>>>>
>>>> Although - imho - it's nice to make check like that.. or at least some kind of assert.
>>
>> I have just one question..Maybe it's not related with this patch..
>> But i saw other driver did that upriv->mmc is assigned to mmc in each drivers.
>>
>> What's difference with "upriv->mmc = mmc" in mmc.c?
>>
>> ./drivers/mmc/fsl_esdhc.c:972:  upriv->mmc = priv->mmc;
>> ./drivers/mmc/mmc.c:1606:       upriv->mmc = mmc;
>> ./drivers/mmc/mmc-uclass.c:91:  return upriv->mmc;
>> ./drivers/mmc/msm_sdhci.c:147:  upriv->mmc = host->mmc;
>> ./drivers/mmc/omap_hsmmc.c:830: upriv->mmc = mmc;
>> ./drivers/mmc/rockchip_dw_mmc.c:114:    upriv->mmc = host->mmc;
>> ./drivers/mmc/socfpga_dw_mmc.c:110:     upriv->mmc = host->mmc;
>> ./drivers/mmc/uniphier-sd.c:727:        upriv->mmc = priv->mmc;
>> ./drivers/mmc/zynq_sdhci.c:52:  upriv->mmc = host->mmc;
>>
>> Is it duplicated?
>> (I didn't see in more detail.)
>>
>> Best Regards,
>> Jaehoon Chung
> 
> I am actually not happy with how this is at present. It should be
> handled by a function call. If you'd like to look at this please do,
> otherwise I will take a pass at it. It should be clear at this point
> what we need.

I will send the patch for cleaning.

Best Regards,
Jaehoon Chung

> 
>>
>>>
>>> An assert() would be fine with me.
>>>
>>> Regards,
>>> Simon
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>>>
>>>
>>
> 
> 
> Regards,
> Simon
> 
> 
>