Re: [PATCH v2 1/2] mmc: Check for device with a seq number equal to num before checking against index

[Ricardo Salveti]

Hi Aswath,

On Thu, Mar 25, 2021 at 4:19 AM Aswath Govindraju <a-govindr...@ti.com> wrote:
>
> First check if there is an alias for the device tree node defined with the
> given num before checking against device index.
>
> Signed-off-by: Aswath Govindraju <a-govindr...@ti.com>
> Reviewed-by: Lokesh Vutla <lokeshvu...@ti.com>
> Reviewed-by: Jaehoon Chung <jh80.ch...@samsung.com>
> ---
>  drivers/mmc/mmc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index b4c8e7f293bd..1e83007286b2 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -3052,9 +3052,11 @@ int mmc_init_device(int num)
>         struct mmc *m;
>         int ret;
>
> -       ret = uclass_get_device(UCLASS_MMC, num, &dev);
> -       if (ret)
> -               return ret;
> +       if (uclass_get_device_by_seq(UCLASS_MMC, num, &dev)) {
> +               ret = uclass_get_device(UCLASS_MMC, num, &dev);
> +               if (ret)
> +                       return ret;
> +       }

uclass_get_device_by_seq returns 0 if OK and -ve on error, so I
believe this check is incorrect and we never end up calling
uclass_get_device on the working path.

While bisecting to try to debug why my zynqmp-based device is unable
to boot u-boot, I stopped at this patch (merged as part of
v2021.07-rc1).

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 1e83007286b..56556353f8e 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -3052,11 +3052,13 @@ int mmc_init_device(int num)
        struct mmc *m;
        int ret;

-       if (uclass_get_device_by_seq(UCLASS_MMC, num, &dev)) {
-               ret = uclass_get_device(UCLASS_MMC, num, &dev);
-               if (ret)
-                       return ret;
-       }
+       ret = uclass_get_device_by_seq(UCLASS_MMC, num, &dev);
+       if (ret)
+               return ret;
+
+       ret = uclass_get_device(UCLASS_MMC, num, &dev);
+       if (ret)
+               return ret;

        m = mmc_get_mmc_dev(dev);
        if (!m)

This is enough for it to work correctly again, but I just wanted to
reply back here first before submitting as I'm surprised nobody
reported this issue before.

Can you verify on your target hardware if this is indeed working as intended?

Thanks,
-- 
Ricardo Salveti