* [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
@ 2016-06-25 21:57 Matt Corallo
2016-06-29 3:27 ` Simon Glass
0 siblings, 1 reply; 8+ messages in thread
From: Matt Corallo @ 2016-06-25 21:57 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
2016-06-25 21:57 [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated Matt Corallo
@ 2016-06-29 3:27 ` Simon Glass
2016-06-30 19:18 ` Mateusz Kulikowski
0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2016-06-29 3:27 UTC (permalink / raw)
To: u-boot
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/
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
2016-06-29 3:27 ` Simon Glass
@ 2016-06-30 19:18 ` Mateusz Kulikowski
2016-06-30 19:28 ` Simon Glass
0 siblings, 1 reply; 8+ messages in thread
From: Mateusz Kulikowski @ 2016-06-30 19:18 UTC (permalink / raw)
To: u-boot
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.
Mateusz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
2016-06-30 19:18 ` Mateusz Kulikowski
@ 2016-06-30 19:28 ` Simon Glass
2016-06-30 22:35 ` Matt Corallo
2016-07-01 11:37 ` Jaehoon Chung
0 siblings, 2 replies; 8+ messages in thread
From: Simon Glass @ 2016-06-30 19:28 UTC (permalink / raw)
To: u-boot
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.
An assert() would be fine with me.
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
2016-06-30 19:28 ` Simon Glass
@ 2016-06-30 22:35 ` Matt Corallo
2016-07-01 11:37 ` Jaehoon Chung
1 sibling, 0 replies; 8+ messages in thread
From: Matt Corallo @ 2016-06-30 22:35 UTC (permalink / raw)
To: u-boot
Sorry, this was related to https://patchwork.ozlabs.org/patch/624614/,
not dragonboard, and yes, is the same issue that was addressed by the
patch you linked.
With this patch the MMC worked fine, but, indeed, as with dragonboard,
its better to assign mmc instead of removing this (this seems to be the
only place where mmc is actually used during boot for me, so maybe not?).
Matt
On 06/30/16 19:28, 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.
>
> An assert() would be fine with me.
>
> Regards,
> Simon
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
2016-06-30 19:28 ` Simon Glass
2016-06-30 22:35 ` Matt Corallo
@ 2016-07-01 11:37 ` Jaehoon Chung
2016-07-01 18:17 ` Simon Glass
1 sibling, 1 reply; 8+ messages in thread
From: Jaehoon Chung @ 2016-07-01 11:37 UTC (permalink / raw)
To: u-boot
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
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
2016-07-01 11:37 ` Jaehoon Chung
@ 2016-07-01 18:17 ` Simon Glass
2016-07-04 2:28 ` Jaehoon Chung
0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2016-07-01 18:17 UTC (permalink / raw)
To: u-boot
Hi,
On 1 July 2016 at 04:37, Jaehoon Chung <jh80.chung@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@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
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.
>
>>
>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated
2016-07-01 18:17 ` Simon Glass
@ 2016-07-04 2:28 ` Jaehoon Chung
0 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2016-07-04 2:28 UTC (permalink / raw)
To: u-boot
On 07/02/2016 03:17 AM, Simon Glass wrote:
> Hi,
>
> On 1 July 2016 at 04:37, Jaehoon Chung <jh80.chung@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@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
>
> 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
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-07-04 2:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-25 21:57 [U-Boot] [PATCH] MMC: Do not write to mmc_uclass_priv if it was not allocated Matt Corallo
2016-06-29 3:27 ` Simon Glass
2016-06-30 19:18 ` Mateusz Kulikowski
2016-06-30 19:28 ` Simon Glass
2016-06-30 22:35 ` Matt Corallo
2016-07-01 11:37 ` Jaehoon Chung
2016-07-01 18:17 ` Simon Glass
2016-07-04 2:28 ` Jaehoon Chung
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.