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

From: Jaehoon Chung <jh80.chung@samsung.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
Date: Fri, 01 Jul 2016 20:37:49 +0900	[thread overview]
Message-ID: <5776560D.4050509@samsung.com> (raw)
In-Reply-To: <CAPnjgZ0QLm0rcgg+vJDbF06OgdDrLFF5ViKaD7+CdPV_3fPSbg@mail.gmail.com>

On 07/01/2016 04:28 AM, Simon Glass wrote:
> Hi Meteusz,
> 
> On 30 June 2016 at 12:18, Mateusz Kulikowski
> <mateusz.kulikowski@gmail.com> wrote:
>> On 29.06.2016 05:27, Simon Glass wrote:
>>> Hi Matt,
>>>
>>> On 25 June 2016 at 14:57, Matt Corallo <linux@bluematt.me> wrote:
>>>> Signed-off-by: Matt Corallo <git@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

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