MMC: Do not write to mmc_uclass_priv if it was not allocated
PATCH 1/1 MMC: Do not write to mmc_uclass_priv if it was not allocated
--- drivers/mmc/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/mmc/mmc.c +2 -1
@@ -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 ;
Signature · 1 line
2.1.4
PATCH 1/1 MMC: Do not write to mmc_uclass_priv if it was not allocated
Hi Matt,
Quoted 20 lines · Matt Corallo
Signed-off-by Matt Corallo <[email protected]>--- drivers/mmc/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mmc/mmc.c +2
-2@@ -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 ; --
Quoted 1 line
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
PATCH 1/1 MMC: Do not write to mmc_uclass_priv if it was not allocated
Quoted 29 lines · Simon Glass
Hi Matt,
Quoted 20 lines · Matt Corallo
Signed-off-by Matt Corallo <[email protected]>--- drivers/mmc/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mmc/mmc.c +2
-2@@ -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 ; --Quoted 1 line
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?
Quoted 1 line
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
PATCH 1/1 MMC: Do not write to mmc_uclass_priv if it was not allocated
Hi Meteusz,
Quoted 30 lines · Mateusz Kulikowski
Quoted 29 lines · Simon Glass
Hi Matt,
Quoted 20 lines · Matt Corallo
Signed-off-by Matt Corallo <[email protected]>--- drivers/mmc/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mmc/mmc.c +2
-2@@ -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 ; --Quoted 1 line
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?
Quoted 7 lines
Quoted 1 line
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
PATCH 1/1 MMC: Do not write to mmc_uclass_priv if it was not allocated
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
Quoted 47 lines · Simon Glass
Hi Meteusz,
Quoted 30 lines · Mateusz Kulikowski
Quoted 29 lines · Simon Glass
Hi Matt,
Quoted 20 lines · Matt Corallo
Signed-off-by Matt Corallo <[email protected]>--- drivers/mmc/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mmc/mmc.c +2
-2@@ -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 ; --Quoted 1 line
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?Quoted 7 lines
Quoted 1 line
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
PATCH 1/1 MMC: Do not write to mmc_uclass_priv if it was not allocated
In reply to Simon GlassQuoted 41 lines · Simon Glass
Hi Meteusz,
Quoted 37 lines · Mateusz Kulikowski
Quoted 29 lines · Simon Glass
Hi Matt,
Quoted 20 lines · Matt Corallo
Signed-off-by Matt Corallo <[email protected]>--- drivers/mmc/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mmc/mmc.c +2
-2@@ -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 ; --Quoted 1 line
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?Quoted 1 line
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
Quoted 12 lines
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
PATCH 1/1 MMC: Do not write to mmc_uclass_priv if it was not allocated
Hi,
Quoted 56 lines · Jaehoon Chung
Quoted 41 lines · Simon Glass
Hi Meteusz,
Quoted 37 lines · Mateusz Kulikowski
Quoted 29 lines · Simon Glass
Hi Matt,
Quoted 20 lines · Matt Corallo
Signed-off-by Matt Corallo <[email protected]>--- drivers/mmc/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mmc/mmc.c +2
-2@@ -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 ; --Quoted 1 line
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?Quoted 1 line
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->mmcis assigned to mmc in each drivers.What's difference with "
upriv->mmc= mmc" inmmc.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;
Quoted 7 lines
./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.
Quoted 4 lines
Quoted 4 lines
An
assert()would be fine with me.
Quoted 10 lines
Quoted 9 lines
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Regards,
Simon
PATCH 1/1 MMC: Do not write to mmc_uclass_priv if it was not allocated
Quoted 64 lines · Simon Glass
Hi,
Quoted 61 lines · Jaehoon Chung
Quoted 41 lines · Simon Glass
Hi Meteusz,
Quoted 37 lines · Mateusz Kulikowski
Quoted 29 lines · Simon Glass
Hi Matt,
Quoted 20 lines · Matt Corallo
Signed-off-by Matt Corallo <[email protected]>--- drivers/mmc/mmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/mmc/mmc.c +2
-2@@ -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 ; --Quoted 1 line
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?Quoted 1 line
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->mmcis assigned to mmc in each drivers.What's difference with "
upriv->mmc= mmc" inmmc.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.)
Quoted 7 lines
Quoted 2 lines
Best Regards,
Jaehoon ChungI 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
Quoted 22 lines
Quoted 5 lines
Quoted 5 lines
An
assert()would be fine with me.Quoted 10 lines
Quoted 9 lines
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-bootRegards,
Simon