Message ID | 1509710746-32268-1-git-send-email-jagan@amarulasolutions.com |
---|---|
State | Deferred |
Delegated to: | Jaehoon Chung |
Headers | show |
Series | [U-Boot] mmc-uclass: spl: upriv->mmc override by host descriptor address | expand |
Hi Jagan, On 3 November 2017 at 06:05, Jagan Teki <jagannadh.teki@gmail.com> wrote: > This specific issue observed with SPL_DM_MMC in falcon mode on > rk3288 which used dw_mmc.c driver. > > Bug details: > ----------- > based on the falcon configuration, SPL trying to read the kernel from > specified sectors, while mmc sending multi-block command(CMD18) the > host descriptor address here next_addr(from dw_mmc.c, on below Bug log at blk_cnt = 938) > is trying to override upriv(which further corrupting upriv->mmc) since after > mmc pointing to wrong address which is causing next commands(CMD12) > is unable to get the host pointer since it's reading wrong address which > eventually block the booting at mmc stage. > > Bug log: > ------- > For blk_cnt = 946 > Before: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fc > dwmci_set_idma_desc: addr = 0x274dfc0, next_addr = 0x7e080 > After: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fc > For blk_cnt = 938 > Before: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fc > dwmci_set_idma_desc: addr = 0x274efc0, next_addr = 0x7e0c0 > After: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0x274efc0 > > Based on the above information, this patch is trying to allocate mmc_uclass_priv > using .priv_auto_alloc_size so if it is zero, the respective uclass driver is > responsible for allocating any data required. So in this scenario memory is not > override between upriv->mmc and host description. > > mmc_uclass_priv with ..priv_auto_alloc_size: > -------------------------------------------- > For blk_cnt = 938 > Before: mmc_get_mmc_dev: dev = 0xff71608c, upriv = 0x0, mmc = 0xff716100 > dwmci_set_idma_desc: addr = 0x274efc0, next_addr = 0x7e0c0 > After: mmc_get_mmc_dev: dev = 0xff71608c, upriv = 0x0, mmc = 0xff716100 > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Note: > - debug code: > https://paste.ubuntu.com/25879159/ > - Bug log: > https://paste.ubuntu.com/25879138/ > - Fix log: > https://paste.ubuntu.com/25879139/ > > drivers/mmc/mmc-uclass.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c > index 5dda20c..cdb0d28 100644 > --- a/drivers/mmc/mmc-uclass.c > +++ b/drivers/mmc/mmc-uclass.c > @@ -299,5 +299,9 @@ UCLASS_DRIVER(mmc) = { > .id = UCLASS_MMC, > .name = "mmc", > .flags = DM_UC_FLAG_SEQ_ALIAS, > +#ifdef CONFIG_SPL_BUILD > + .priv_auto_alloc_size = sizeof(struct mmc_uclass_priv), > +#else > .per_device_auto_alloc_size = sizeof(struct mmc_uclass_priv), > +#endif > }; > -- > 2.7.4 > This is so strange, I don't really understand it. But priv_auto_alloc_size should be used with dev_get_uclass_priv() which is what the code is using. priv_auto_alloc_size is used for the uclass' priv-> pointer. We don't need the uclass to store anything as far as I can tell. So I am not sure why your fix works, but it seems wrong to me. But as I say, it is strange, and perhaps there is something else wrong. Regards, Simon
diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index 5dda20c..cdb0d28 100644 --- a/drivers/mmc/mmc-uclass.c +++ b/drivers/mmc/mmc-uclass.c @@ -299,5 +299,9 @@ UCLASS_DRIVER(mmc) = { .id = UCLASS_MMC, .name = "mmc", .flags = DM_UC_FLAG_SEQ_ALIAS, +#ifdef CONFIG_SPL_BUILD + .priv_auto_alloc_size = sizeof(struct mmc_uclass_priv), +#else .per_device_auto_alloc_size = sizeof(struct mmc_uclass_priv), +#endif };
This specific issue observed with SPL_DM_MMC in falcon mode on rk3288 which used dw_mmc.c driver. Bug details: ----------- based on the falcon configuration, SPL trying to read the kernel from specified sectors, while mmc sending multi-block command(CMD18) the host descriptor address here next_addr(from dw_mmc.c, on below Bug log at blk_cnt = 938) is trying to override upriv(which further corrupting upriv->mmc) since after mmc pointing to wrong address which is causing next commands(CMD12) is unable to get the host pointer since it's reading wrong address which eventually block the booting at mmc stage. Bug log: ------- For blk_cnt = 946 Before: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fc dwmci_set_idma_desc: addr = 0x274dfc0, next_addr = 0x7e080 After: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fc For blk_cnt = 938 Before: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fc dwmci_set_idma_desc: addr = 0x274efc0, next_addr = 0x7e0c0 After: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0x274efc0 Based on the above information, this patch is trying to allocate mmc_uclass_priv using .priv_auto_alloc_size so if it is zero, the respective uclass driver is responsible for allocating any data required. So in this scenario memory is not override between upriv->mmc and host description. mmc_uclass_priv with ..priv_auto_alloc_size: -------------------------------------------- For blk_cnt = 938 Before: mmc_get_mmc_dev: dev = 0xff71608c, upriv = 0x0, mmc = 0xff716100 dwmci_set_idma_desc: addr = 0x274efc0, next_addr = 0x7e0c0 After: mmc_get_mmc_dev: dev = 0xff71608c, upriv = 0x0, mmc = 0xff716100 Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Note: - debug code: https://paste.ubuntu.com/25879159/ - Bug log: https://paste.ubuntu.com/25879138/ - Fix log: https://paste.ubuntu.com/25879139/ drivers/mmc/mmc-uclass.c | 4 ++++ 1 file changed, 4 insertions(+)