[U-Boot] [PATCH] spl_mmc: always use find_mmc_device() to get mmc handler
Like cmd/mmc.c, the spl_mmc.c are using block driver interface like blk_dread() to access mmc devices, we need to get the mmc device handler by block driver interface so that we can always get correct device number, eg. the alias may change the device number which make the device number different in UCLASS_MMC list and UCLASS_BLK list.
Signed-off-by: Kever Yang kever.yang@rock-chips.com ---
common/spl/spl_mmc.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index b3619889f7..3a93e20f04 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -113,9 +113,6 @@ static int spl_mmc_get_device_index(u32 boot_device)
static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) { -#if CONFIG_IS_ENABLED(DM_MMC) - struct udevice *dev; -#endif int err, mmc_dev;
mmc_dev = spl_mmc_get_device_index(boot_device); @@ -130,14 +127,8 @@ static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) return err; }
-#if CONFIG_IS_ENABLED(DM_MMC) - err = uclass_get_device(UCLASS_MMC, mmc_dev, &dev); - if (!err) - *mmcp = mmc_get_mmc_dev(dev); -#else *mmcp = find_mmc_device(mmc_dev); err = *mmcp ? 0 : -ENODEV; -#endif if (err) { #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: could not find mmc device %d. error: %d\n",
+Peng
Hi Kever,
On 25/07/19 3:10 PM, Kever Yang wrote:
Like cmd/mmc.c, the spl_mmc.c are using block driver interface like blk_dread() to access mmc devices, we need to get the mmc device handler by block driver interface so that we can always get correct device number, eg. the alias may change the device number which make the device number different in UCLASS_MMC list and UCLASS_BLK list.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
common/spl/spl_mmc.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index b3619889f7..3a93e20f04 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -113,9 +113,6 @@ static int spl_mmc_get_device_index(u32 boot_device)
static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) { -#if CONFIG_IS_ENABLED(DM_MMC)
- struct udevice *dev;
-#endif int err, mmc_dev;
mmc_dev = spl_mmc_get_device_index(boot_device); @@ -130,14 +127,8 @@ static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) return err; }
-#if CONFIG_IS_ENABLED(DM_MMC)
- err = uclass_get_device(UCLASS_MMC, mmc_dev, &dev);
- if (!err)
*mmcp = mmc_get_mmc_dev(dev);-#else *mmcp = find_mmc_device(mmc_dev); err = *mmcp ? 0 : -ENODEV; -#endif
I am not against the change but trying to understand what is the problem you faced. mmc_initialize() would have initialized the devices based on the seq number that is provided in DT. So in your case shouldn't uclass_get_device give the right device or something else triggered this patch?
Also we are facing a different problem with mmc_initialize(). Very early in boot there is no access to any peripheral other than boot peripheral(Need system co processor to enable peripherals). There are 2 MMC controllers in our devices. So, SD boot is failing while loading system firmware as mmc_initialize is trying to probe both the controllers. In SPL, shouldn't we just probe the needed controller instead of calling mmc_initialize?
Thanks and regards, Lokesh
Hi,
We use the aliases to set the MMC device order. For example: mmc0 = &emmc; mmc1 = &sdmmc; These define the MMC order in block layer. But in the mmc layer, the MMC device order is defined in the dts MMC node. It may make mistake to get MMC device order. So if we use the block layer interface to access mmc devices, use find_mmc_device to get current devnum but not uclass_get_device.
Thanks and regards, Jason
jason.zhu@rock-chips.com
From: Lokesh Vutla Date: 2019-07-25 18:39 To: Kever Yang; u-boot@lists.denx.de CC: jason.zhu@rock-chips.com; Marek Vasut; Tien Fong Chee; Peng Fan Subject: Re: [U-Boot] [PATCH] spl_mmc: always use find_mmc_device() to get mmc handler +Peng
Hi Kever,
On 25/07/19 3:10 PM, Kever Yang wrote:
Like cmd/mmc.c, the spl_mmc.c are using block driver interface like blk_dread() to access mmc devices, we need to get the mmc device handler by block driver interface so that we can always get correct device number, eg. the alias may change the device number which make the device number different in UCLASS_MMC list and UCLASS_BLK list.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
common/spl/spl_mmc.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index b3619889f7..3a93e20f04 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -113,9 +113,6 @@ static int spl_mmc_get_device_index(u32 boot_device)
static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) { -#if CONFIG_IS_ENABLED(DM_MMC)
- struct udevice *dev;
-#endif int err, mmc_dev;
mmc_dev = spl_mmc_get_device_index(boot_device); @@ -130,14 +127,8 @@ static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) return err; }
-#if CONFIG_IS_ENABLED(DM_MMC)
- err = uclass_get_device(UCLASS_MMC, mmc_dev, &dev);
- if (!err)
- *mmcp = mmc_get_mmc_dev(dev);
-#else *mmcp = find_mmc_device(mmc_dev); err = *mmcp ? 0 : -ENODEV; -#endif
I am not against the change but trying to understand what is the problem you faced. mmc_initialize() would have initialized the devices based on the seq number that is provided in DT. So in your case shouldn't uclass_get_device give the right device or something else triggered this patch?
Also we are facing a different problem with mmc_initialize(). Very early in boot there is no access to any peripheral other than boot peripheral(Need system co processor to enable peripherals). There are 2 MMC controllers in our devices. So, SD boot is failing while loading system firmware as mmc_initialize is trying to probe both the controllers. In SPL, shouldn't we just probe the needed controller instead of calling mmc_initialize?
Thanks and regards, Lokesh
On Thu, Jul 25, 2019 at 11:32 AM jason.zhu@rock-chips.com jason.zhu@rock-chips.com wrote:
Hi,
We use the aliases to set the MMC device order. For example: mmc0 = &emmc; mmc1 = &sdmmc; These define the MMC order in block layer. But in the mmc layer, the MMC device order is defined in the dts MMC node. It may make mistake to get MMC device order. So if we use the block layer interface toaccess mmc devices, use find_mmc_device to get current devnum but not uclass_get_device.
I get a similar (same?) problem too (and will test this patch) - I've got no MMC1 on an AM335x, so the initialisation makes MMC2 the first device, which then leads me to this revolting hack in my board.c:
void board_boot_order(u32 *spl_boot_list) { u32 boot_device = spl_boot_device();
/* * This is a filthy hack... we don't have MMC1, but the SPL boot * code works based on device numbers, not sequence numbers, so * we have to ensure that when we're booted from the eMMC we pick * the right entry from the DT */ if (boot_device == BOOT_DEVICE_MMC2) spl_boot_list[0] = BOOT_DEVICE_MMC1; else spl_boot_list[0] = boot_device; }
Subject: Re: [U-Boot] [PATCH] spl_mmc: always use find_mmc_device() to get mmc handler
+Peng
Hi Kever,
On 25/07/19 3:10 PM, Kever Yang wrote:
Like cmd/mmc.c, the spl_mmc.c are using block driver interface like blk_dread() to access mmc devices, we need to get the mmc device handler by block driver interface so that we can always get correct device number, eg. the alias may change the device number which make the device number different in UCLASS_MMC list and UCLASS_BLK list.
It is unclear to me. spl_mmc_get_device_index will get the mmc_dev number, Then uclass_get_device will use it to get the mmc device.
You mean this not work on your platform?
And mmc device and blk device are connected with the uclass_platdata of the mmc device, so from mmc, you could always get the correct blk device.
Please share more info about your issue/fix.
Regards, Peng.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
common/spl/spl_mmc.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index b3619889f7..3a93e20f04 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -113,9 +113,6 @@ static int spl_mmc_get_device_index(u32 boot_device)
static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) { -#if CONFIG_IS_ENABLED(DM_MMC)
- struct udevice *dev;
-#endif int err, mmc_dev;
mmc_dev = spl_mmc_get_device_index(boot_device); @@ -130,14 +127,8 @@ static int spl_mmc_find_device(struct mmc
**mmcp, u32 boot_device)
return err;}
-#if CONFIG_IS_ENABLED(DM_MMC)
- err = uclass_get_device(UCLASS_MMC, mmc_dev, &dev);
- if (!err)
*mmcp = mmc_get_mmc_dev(dev);-#else *mmcp = find_mmc_device(mmc_dev); err = *mmcp ? 0 : -ENODEV; -#endif
I am not against the change but trying to understand what is the problem you faced. mmc_initialize() would have initialized the devices based on the seq number that is provided in DT. So in your case shouldn't uclass_get_device give the right device or something else triggered this patch?
Also we are facing a different problem with mmc_initialize(). Very early in boot there is no access to any peripheral other than boot peripheral(Need system co processor to enable peripherals). There are 2 MMC controllers in our devices. So, SD boot is failing while loading system firmware as mmc_initialize is trying to probe both the controllers. In SPL, shouldn't we just probe the needed controller instead of calling mmc_initialize?
Thanks and regards, Lokesh
Hi Peng,
On 2019/7/30 上午9:30, Peng Fan wrote:
Subject: Re: [U-Boot] [PATCH] spl_mmc: always use find_mmc_device() to get mmc handler
+Peng
Hi Kever,
On 25/07/19 3:10 PM, Kever Yang wrote:
Like cmd/mmc.c, the spl_mmc.c are using block driver interface like blk_dread() to access mmc devices, we need to get the mmc device handler by block driver interface so that we can always get correct device number, eg. the alias may change the device number which make the device number different in UCLASS_MMC list and UCLASS_BLK list.
It is unclear to me. spl_mmc_get_device_index will get the mmc_dev number, Then uclass_get_device will use it to get the mmc device.
You mean this not work on your platform?
The code as-is now works on my platform. It's true that spl_mmc_get_device_index will get the mmc_dev number , but the BOOT_DEVICE_MMC1 and BOOT_DEVICE_MMC2 does not have enough information about this is a SD card or a eMMC device. So we want to enable the feature of alias in dts to identify eMMC and SD card, which already used in U-Boot proper, and this is implemented in mmc_bind() and change the devnum for blk_create_devicef(), which may lead to the devnum in UCLASS_MMC list and in UCLASS_BLK list are different. In this case, if you get the devnum from UCLASS_MMC, you will get the wrong device, and the right one should be from UCLASS_BLK. I should send the patch for enable mmc alias in SPL at the same time, which may help you understand.
Thanks, - Kever
And mmc device and blk device are connected with the uclass_platdata of the mmc device, so from mmc, you could always get the correct blk device.
Please share more info about your issue/fix.
Regards, Peng.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
common/spl/spl_mmc.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index b3619889f7..3a93e20f04 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -113,9 +113,6 @@ static int spl_mmc_get_device_index(u32 boot_device)
static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device) { -#if CONFIG_IS_ENABLED(DM_MMC)
- struct udevice *dev;
-#endif int err, mmc_dev;
mmc_dev = spl_mmc_get_device_index(boot_device); @@ -130,14 +127,8 @@ static int spl_mmc_find_device(struct mmc
**mmcp, u32 boot_device)
return err;}
-#if CONFIG_IS_ENABLED(DM_MMC)
- err = uclass_get_device(UCLASS_MMC, mmc_dev, &dev);
- if (!err)
*mmcp = mmc_get_mmc_dev(dev);-#else *mmcp = find_mmc_device(mmc_dev); err = *mmcp ? 0 : -ENODEV; -#endif
I am not against the change but trying to understand what is the problem you faced. mmc_initialize() would have initialized the devices based on the seq number that is provided in DT. So in your case shouldn't uclass_get_device give the right device or something else triggered this patch?
Also we are facing a different problem with mmc_initialize(). Very early in boot there is no access to any peripheral other than boot peripheral(Need system co processor to enable peripherals). There are 2 MMC controllers in our devices. So, SD boot is failing while loading system firmware as mmc_initialize is trying to probe both the controllers. In SPL, shouldn't we just probe the needed controller instead of calling mmc_initialize?
Thanks and regards, Lokesh
participants (5)
-
Alex Kiernan -
jason.zhu＠rock-chips.com -
Kever Yang -
Lokesh Vutla -
Peng Fan