[U-Boot] mmc-uclass: spl: upriv->mmc override by host descriptor address

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(+)

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