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

7 Jul 2021

Hi Ricardo,
On 07/07/21 4:39 am, Ricardo Salveti wrote:
...
Hi Aswath,
On Thu, Mar 25, 2021 at 4:19 AM Aswath Govindraju a-govindraju@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-govindraju@ti.com
Reviewed-by: Lokesh Vutla lokeshvutla@ti.com
Reviewed-by: Jaehoon Chung jh80.chung@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.
Yes, as you mentioned uclass_get_device_by_seq() returns 0 if OK and -ve
on error is correct. However, the intention of this check here, is to
only call uclass_get_device() when uclass_get_device_by_seq() returns an
error.
uclass_get_device_by_seq() returns the device only if it is able find a
active device with matching sequence number "num" or will try to find a
device that is requesting this number. As this function also returns the
device we need not call uclass_get_device() again. Only on failure for
find a device with matching sequence number do we call uclass_get_device().
So, I believe the above mentioned check is correct.
...
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)




In the above method, the device is being fetched twice and I believe
that this method is incorrect.
...
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.
If making this change got it work correctly again. I believe there might
have been a error in assigning aliases for mmc devices in the device
tree. As only after this patch are aliases being read for mmc class of
devices from the device tree.
Thanks,
Aswath
...
Can you verify on your target hardware if this is indeed working as intended?
Thanks,

    