[PATCH v2 0/2] mmc: Add support for checking device number against alias index

[PATCH v2 0/2] mmc: Add support for checking device number against alias index

[Aswath Govindraju]

The following series of patches,
- add support to check for a device with a sequence number equal to device
  number before looking for matching index
- assign device number by reading aliases node's index by using dev_seq

Changes since v1:
- In patch 2 used dev_seq instead of using dev_read_alias_seq for reading
  alias index.

Aswath Govindraju (2):
  mmc: Check for device with a seq number equal to num  before checking
    against index
  mmc: mmc-uclass: Use dev_seq() to read aliases node's index

 drivers/mmc/mmc-uclass.c | 12 +++++-------
 drivers/mmc/mmc.c        |  8 +++++---
 2 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.17.1

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

[Aswath Govindraju]

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;
+	}
 
 	m = mmc_get_mmc_dev(dev);
 	if (!m)
-- 
2.17.1

[PATCH v2 2/2] mmc: mmc-uclass: Use dev_seq() to read aliases node's index

[Aswath Govindraju]

Use dev_seq() to read aliases node's index and pass it as device number
for creating bulk device.

Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
---
 drivers/mmc/mmc-uclass.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
index 53eabc9e612d..d36aae367e7c 100644
--- a/drivers/mmc/mmc-uclass.c
+++ b/drivers/mmc/mmc-uclass.c
@@ -383,18 +383,16 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
 {
 	struct blk_desc *bdesc;
 	struct udevice *bdev;
-	int ret, devnum = -1;
+	int ret;
 
 	if (!mmc_get_ops(dev))
 		return -ENOSYS;
-#ifndef CONFIG_SPL_BUILD
-	/* Use the fixed index with aliase node's index */
-	ret = dev_read_alias_seq(dev, &devnum);
-	debug("%s: alias ret=%d, devnum=%d\n", __func__, ret, devnum);
-#endif
+
+	/* Use the fixed index with aliases node's index */
+	debug("%s: alias devnum=%d\n", __func__, dev_seq(dev));
 
 	ret = blk_create_devicef(dev, "mmc_blk", "blk", IF_TYPE_MMC,
-			devnum, 512, 0, &bdev);
+			dev_seq(dev), 512, 0, &bdev);
 	if (ret) {
 		debug("Cannot create block device\n");
 		return ret;
-- 
2.17.1

[PATCH v2 2/2] mmc: mmc-uclass: Use dev_seq() to read aliases node's index

[Jaehoon Chung]

On 3/25/21 4:18 PM, Aswath Govindraju wrote:
> Use dev_seq() to read aliases node's index and pass it as device number
> for creating bulk device.
> 
> Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/mmc-uclass.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 53eabc9e612d..d36aae367e7c 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -383,18 +383,16 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
>  {
>  	struct blk_desc *bdesc;
>  	struct udevice *bdev;
> -	int ret, devnum = -1;
> +	int ret;
>  
>  	if (!mmc_get_ops(dev))
>  		return -ENOSYS;
> -#ifndef CONFIG_SPL_BUILD
> -	/* Use the fixed index with aliase node's index */
> -	ret = dev_read_alias_seq(dev, &devnum);
> -	debug("%s: alias ret=%d, devnum=%d\n", __func__, ret, devnum);
> -#endif
> +
> +	/* Use the fixed index with aliases node's index */
> +	debug("%s: alias devnum=%d\n", __func__, dev_seq(dev));
>  
>  	ret = blk_create_devicef(dev, "mmc_blk", "blk", IF_TYPE_MMC,
> -			devnum, 512, 0, &bdev);
> +			dev_seq(dev), 512, 0, &bdev);
>  	if (ret) {
>  		debug("Cannot create block device\n");
>  		return ret;
> 

[BUG] Re: [PATCH v2 2/2] mmc: mmc-uclass: Use dev_seq() to read aliases node's index

[Heinrich Schuchardt]

On 25.03.21 08:18, Aswath Govindraju wrote:
> Use dev_seq() to read aliases node's index and pass it as device number
> for creating bulk device.
>
> Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Since this patch merged as commit 2243d19e5618122 SD cards on
orangepi_pc_defconfig are not detected anymore.

=> mmc rescan
MMC Device 0 not found
no mmc device at slot 0
=>

Best regards

Heinrich

> ---
>  drivers/mmc/mmc-uclass.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> index 53eabc9e612d..d36aae367e7c 100644
> --- a/drivers/mmc/mmc-uclass.c
> +++ b/drivers/mmc/mmc-uclass.c
> @@ -383,18 +383,16 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
>  {
>  	struct blk_desc *bdesc;
>  	struct udevice *bdev;
> -	int ret, devnum = -1;
> +	int ret;
>
>  	if (!mmc_get_ops(dev))
>  		return -ENOSYS;
> -#ifndef CONFIG_SPL_BUILD
> -	/* Use the fixed index with aliase node's index */
> -	ret = dev_read_alias_seq(dev, &devnum);
> -	debug("%s: alias ret=%d, devnum=%d\n", __func__, ret, devnum);
> -#endif
> +
> +	/* Use the fixed index with aliases node's index */
> +	debug("%s: alias devnum=%d\n", __func__, dev_seq(dev));
>
>  	ret = blk_create_devicef(dev, "mmc_blk", "blk", IF_TYPE_MMC,
> -			devnum, 512, 0, &bdev);
> +			dev_seq(dev), 512, 0, &bdev);
>  	if (ret) {
>  		debug("Cannot create block device\n");
>  		return ret;
>

[BUG] Re: [PATCH v2 2/2] mmc: mmc-uclass: Use dev_seq() to read aliases node's index

[Heinrich Schuchardt]

On 20.05.21 18:32, Heinrich Schuchardt wrote:
> On 25.03.21 08:18, Aswath Govindraju wrote:
>> Use dev_seq() to read aliases node's index and pass it as device number
>> for creating bulk device.
>>
>> Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>
> Since this patch merged as commit 2243d19e5618122 SD cards on
> orangepi_pc_defconfig are not detected anymore.
>
> => mmc rescan
> MMC Device 0 not found
> no mmc device at slot 0
> =>
>
> Best regards
>
> Heinrich

Reverting the patch fixes the problem for the Orange Pi PC.
Unfortunately the commit message does not provide any motivation for the
change.

Was meant to fix anything?

Best regards

Heinrich

[BUG] Re: [PATCH v2 2/2] mmc: mmc-uclass: Use dev_seq() to read aliases node's index

[Andre Przywara]

On Thu, 20 May 2021 18:32:38 +0200
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

Hi Heinrich,

> On 25.03.21 08:18, Aswath Govindraju wrote:
> > Use dev_seq() to read aliases node's index and pass it as device number
> > for creating bulk device.
> >
> > Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
> > Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>  
> 
> Since this patch merged as commit 2243d19e5618122 SD cards on
> orangepi_pc_defconfig are not detected anymore.
> 
> => mmc rescan  
> MMC Device 0 not found
> no mmc device at slot 0
> =>  

Thanks for the report! This is a known issue, please join the
discussion here:
https://lists.denx.de/pipermail/u-boot/2021-April/447472.html

I am in the middle of some patch to make fastboot detect the
eMMC, which would render the need for this "mmc1 = &mmc2;" alias in
our sunxi-u-boot.dtsi obsolete. Removing this line should restore the
old probe order. However this is probably a bigger change (and affects
other platforms), so this is not 2021.07 material anymore.

I will push for this original patch of mine again (adding an mmc0
alias), once I have a proof of concept for the proper fastboot fix.

As you figured, this is a serious user-visible regression, so we need
to fix this.

Cheers,
Andre


> 
> Best regards
> 
> Heinrich
> 
> > ---
> >  drivers/mmc/mmc-uclass.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
> > index 53eabc9e612d..d36aae367e7c 100644
> > --- a/drivers/mmc/mmc-uclass.c
> > +++ b/drivers/mmc/mmc-uclass.c
> > @@ -383,18 +383,16 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
> >  {
> >  	struct blk_desc *bdesc;
> >  	struct udevice *bdev;
> > -	int ret, devnum = -1;
> > +	int ret;
> >
> >  	if (!mmc_get_ops(dev))
> >  		return -ENOSYS;
> > -#ifndef CONFIG_SPL_BUILD
> > -	/* Use the fixed index with aliase node's index */
> > -	ret = dev_read_alias_seq(dev, &devnum);
> > -	debug("%s: alias ret=%d, devnum=%d\n", __func__, ret, devnum);
> > -#endif
> > +
> > +	/* Use the fixed index with aliases node's index */
> > +	debug("%s: alias devnum=%d\n", __func__, dev_seq(dev));
> >
> >  	ret = blk_create_devicef(dev, "mmc_blk", "blk", IF_TYPE_MMC,
> > -			devnum, 512, 0, &bdev);
> > +			dev_seq(dev), 512, 0, &bdev);
> >  	if (ret) {
> >  		debug("Cannot create block device\n");
> >  		return ret;
> >  
> 

[BUG] Re: [PATCH v2 2/2] mmc: mmc-uclass: Use dev_seq() to read aliases node's index

[Heinrich Schuchardt]

On 20.05.21 18:55, Andre Przywara wrote:
> On Thu, 20 May 2021 18:32:38 +0200
> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Hi Heinrich,
>
>> On 25.03.21 08:18, Aswath Govindraju wrote:
>>> Use dev_seq() to read aliases node's index and pass it as device number
>>> for creating bulk device.
>>>
>>> Suggested-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> Signed-off-by: Aswath Govindraju <a-govindraju@ti.com>
>>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>>
>> Since this patch merged as commit 2243d19e5618122 SD cards on
>> orangepi_pc_defconfig are not detected anymore.
>>
>> => mmc rescan
>> MMC Device 0 not found
>> no mmc device at slot 0
>> =>
>
> Thanks for the report! This is a known issue, please join the
> discussion here:
> https://lists.denx.de/pipermail/u-boot/2021-April/447472.html
>
> I am in the middle of some patch to make fastboot detect the
> eMMC, which would render the need for this "mmc1 = &mmc2;" alias in
> our sunxi-u-boot.dtsi obsolete. Removing this line should restore the
> old probe order. However this is probably a bigger change (and affects
> other platforms), so this is not 2021.07 material anymore.
>
> I will push for this original patch of mine again (adding an mmc0
> alias), once I have a proof of concept for the proper fastboot fix.
>
> As you figured, this is a serious user-visible regression, so we need
> to fix this.
>
> Cheers,
> Andre

I applied the patch in
https://lists.denx.de/pipermail/u-boot/2021-April/447472.html
and Debian boots like a charm.

Either using my pre-existing boot option

/VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(2)/SD(0)/HD(1,MBR,0xb6b4facb,0x800,0x200000)/EFI\debian\grubarm.efi

Or using Debian's flash-kernel package.

As long as you don't have two mmc devices installed (SD-card + eMMC) I
would not expect any hickup due to the additional alias when using
distroboot.

Best regards

Heinrich

>
>
>>
>> Best regards
>>
>> Heinrich
>>
>>> ---
>>>  drivers/mmc/mmc-uclass.c | 12 +++++-------
>>>  1 file changed, 5 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c
>>> index 53eabc9e612d..d36aae367e7c 100644
>>> --- a/drivers/mmc/mmc-uclass.c
>>> +++ b/drivers/mmc/mmc-uclass.c
>>> @@ -383,18 +383,16 @@ int mmc_bind(struct udevice *dev, struct mmc *mmc, const struct mmc_config *cfg)
>>>  {
>>>  	struct blk_desc *bdesc;
>>>  	struct udevice *bdev;
>>> -	int ret, devnum = -1;
>>> +	int ret;
>>>
>>>  	if (!mmc_get_ops(dev))
>>>  		return -ENOSYS;
>>> -#ifndef CONFIG_SPL_BUILD
>>> -	/* Use the fixed index with aliase node's index */
>>> -	ret = dev_read_alias_seq(dev, &devnum);
>>> -	debug("%s: alias ret=%d, devnum=%d\n", __func__, ret, devnum);
>>> -#endif
>>> +
>>> +	/* Use the fixed index with aliases node's index */
>>> +	debug("%s: alias devnum=%d\n", __func__, dev_seq(dev));
>>>
>>>  	ret = blk_create_devicef(dev, "mmc_blk", "blk", IF_TYPE_MMC,
>>> -			devnum, 512, 0, &bdev);
>>> +			dev_seq(dev), 512, 0, &bdev);
>>>  	if (ret) {
>>>  		debug("Cannot create block device\n");
>>>  		return ret;
>>>
>>
>

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-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.

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

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

[Aswath Govindraju]

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,
> 

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

[Ricardo Salveti]

Hi Aswath,

Thanks for the quick response.

On Wed, Jul 7, 2021 at 1:55 AM Aswath Govindraju <a-govindraju@ti.com> wrote:
>
> 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.

You're right, I had to read more about how seq is related with aliases
to understand the difference here.

> > 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.

Was finally able to understand what happened, and it is indeed related
to how the aliases were used by this board.

At spl_mmc_find_device the function spl_mmc_get_device_index was
called with BOOT_DEVICE_MMC2 (sdcard on my board), following with
mmc_init_device (mmc_dev 1). This caused uclass_get_device_by_seq to
be called with num 1 (alias 1), which ended up loading the eMMC device
instead of sdcard, causing the boot failure.

Looking at my dts, this now makes sense as mmc0 was an alias to sdcard
while mmc1 to emmc. Inverting the alias node was enough to get my
board to boot again.

diff --git a/uz3eg-iocc/system-som.dtsi b/system-som.dtsi
index 192e343..ec2b8a2 100644
--- a/system-som.dtsi
+++ b/system-som.dtsi
@@ -21,8 +21,8 @@

        aliases {
                gpio0 = &gpio;
-               mmc0 = &sdhci1;
-               mmc1 = &sdhci0;
+               mmc1 = &sdhci1;
+               mmc0 = &sdhci0;
                rtc0 = &rtc;
                serial0 = &uart0;
                serial1 = &uart1;

Thanks,
-- 
Ricardo Salveti