diff mbox series

[U-Boot] spl: dm_mmc: Initialize only the required mmc device

Message ID 20190909091036.25352-1-lokeshvutla@ti.com
State Accepted
Commit 80f02019ee6901a7fb0979677030f33fb9b1fa69
Delegated to: Peng Fan
Headers show
Series [U-Boot] spl: dm_mmc: Initialize only the required mmc device | expand

Commit Message

Lokesh Vutla Sept. 9, 2019, 9:10 a.m. UTC
In SPL, all the available mmc devices gets initialized during boot.
This might not work in cases where clocks are not available for
certain mmc devices(other than boot device) and the support for
enabling device might not be ready.

Texas Instruments' K3 J721E device having a central system controller
(dmsc) is one such example falling in this category. Below is the
sequence for the failing scenario:
- ROM comes up in SD mode and loads SPL by just initialing SD card.
- SPL loads dmsc firmware from SD Card.
Since ROM has enabled SD, SPL need not enable the SD, just need
to re initialize the card. But SPL is trying to initialize other MMC
instances which are in disabled state. Since dmsc firmware is not yet
available, devices cannot be enabled. So in SPL, initialize only the
mmc device that is needed.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 common/spl/spl_mmc.c | 14 ++++----------
 drivers/mmc/mmc.c    | 24 ++++++++++++++++++++++++
 include/mmc.h        |  1 +
 3 files changed, 29 insertions(+), 10 deletions(-)

Comments

Peng Fan Sept. 10, 2019, 2:20 a.m. UTC | #1
> Subject: [PATCH] spl: dm_mmc: Initialize only the required mmc device
> 
> In SPL, all the available mmc devices gets initialized during boot.
> This might not work in cases where clocks are not available for certain mmc
> devices(other than boot device) and the support for enabling device might not
> be ready.
> 
> Texas Instruments' K3 J721E device having a central system controller
> (dmsc) is one such example falling in this category. Below is the sequence for
> the failing scenario:
> - ROM comes up in SD mode and loads SPL by just initialing SD card.
> - SPL loads dmsc firmware from SD Card.
> Since ROM has enabled SD, SPL need not enable the SD, just need to re
> initialize the card. But SPL is trying to initialize other MMC instances which
> are in disabled state. Since dmsc firmware is not yet available, devices cannot
> be enabled. So in SPL, initialize only the mmc device that is needed.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  common/spl/spl_mmc.c | 14 ++++----------
>  drivers/mmc/mmc.c    | 24 ++++++++++++++++++++++++
>  include/mmc.h        |  1 +
>  3 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index
> b3619889f7..303f0f80bf 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -113,31 +113,25 @@ 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);
>  	if (mmc_dev < 0)
>  		return mmc_dev;
> 
> +#if CONFIG_IS_ENABLED(DM_MMC)
> +	err = mmc_init_device(mmc_dev);
> +#else
>  	err = mmc_initialize(NULL);
> +#endif /* DM_MMC */
>  	if (err) {
>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
>  		printf("spl: could not initialize mmc. error: %d\n", err);  #endif
>  		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", diff --git
> a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index eecc7d687e..ec8f92ce8f
> 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -2998,6 +2998,30 @@ int mmc_initialize(bd_t *bis)
>  	return 0;
>  }
> 
> +#if CONFIG_IS_ENABLED(DM_MMC)
> +int mmc_init_device(int num)
> +{
> +	struct udevice *dev;
> +	struct mmc *m;
> +	int ret;
> +
> +	ret = uclass_get_device(UCLASS_MMC, num, &dev);
> +	if (ret)
> +		return ret;
> +
> +	m = mmc_get_mmc_dev(dev);
> +	if (!m)
> +		return 0;
> +#ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
> +	mmc_set_preinit(m, 1);
> +#endif
> +	if (m->preinit)
> +		mmc_start_init(m);
> +
> +	return 0;
> +}
> +#endif
> +
>  #ifdef CONFIG_CMD_BKOPS_ENABLE
>  int mmc_set_bkops_enable(struct mmc *mmc)  { diff --git
> a/include/mmc.h b/include/mmc.h index 46422f41a4..878b4c9e57 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -698,6 +698,7 @@ void mmc_destroy(struct mmc *mmc);
>   */
>  int mmc_unbind(struct udevice *dev);
>  int mmc_initialize(bd_t *bis);
> +int mmc_init_device(int num);
>  int mmc_init(struct mmc *mmc);
>  int mmc_send_tuning(struct mmc *mmc, u32 opcode, int *cmd_error);

Reviewed-by: Peng Fan <peng.fan@nxp.com>

> 
> --
> 2.22.0
Lokesh Vutla Oct. 9, 2019, 4:47 a.m. UTC | #2
Hi Peng,

On 10/09/19 7:50 AM, Peng Fan wrote:
>> Subject: [PATCH] spl: dm_mmc: Initialize only the required mmc device
>>
>> In SPL, all the available mmc devices gets initialized during boot.
>> This might not work in cases where clocks are not available for certain mmc
>> devices(other than boot device) and the support for enabling device might not
>> be ready.
>>
>> Texas Instruments' K3 J721E device having a central system controller
>> (dmsc) is one such example falling in this category. Below is the sequence for
>> the failing scenario:
>> - ROM comes up in SD mode and loads SPL by just initialing SD card.
>> - SPL loads dmsc firmware from SD Card.
>> Since ROM has enabled SD, SPL need not enable the SD, just need to re
>> initialize the card. But SPL is trying to initialize other MMC instances which
>> are in disabled state. Since dmsc firmware is not yet available, devices cannot
>> be enabled. So in SPL, initialize only the mmc device that is needed.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>

Gentle Ping. Are you planning to take this patch for this merge window?

Thanks and regards,
Lokesh

>> ---
>>  common/spl/spl_mmc.c | 14 ++++----------
>>  drivers/mmc/mmc.c    | 24 ++++++++++++++++++++++++
>>  include/mmc.h        |  1 +
>>  3 files changed, 29 insertions(+), 10 deletions(-)
>>
>> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index
>> b3619889f7..303f0f80bf 100644
>> --- a/common/spl/spl_mmc.c
>> +++ b/common/spl/spl_mmc.c
>> @@ -113,31 +113,25 @@ 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);
>>  	if (mmc_dev < 0)
>>  		return mmc_dev;
>>
>> +#if CONFIG_IS_ENABLED(DM_MMC)
>> +	err = mmc_init_device(mmc_dev);
>> +#else
>>  	err = mmc_initialize(NULL);
>> +#endif /* DM_MMC */
>>  	if (err) {
>>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
>>  		printf("spl: could not initialize mmc. error: %d\n", err);  #endif
>>  		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", diff --git
>> a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index eecc7d687e..ec8f92ce8f
>> 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -2998,6 +2998,30 @@ int mmc_initialize(bd_t *bis)
>>  	return 0;
>>  }
>>
>> +#if CONFIG_IS_ENABLED(DM_MMC)
>> +int mmc_init_device(int num)
>> +{
>> +	struct udevice *dev;
>> +	struct mmc *m;
>> +	int ret;
>> +
>> +	ret = uclass_get_device(UCLASS_MMC, num, &dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	m = mmc_get_mmc_dev(dev);
>> +	if (!m)
>> +		return 0;
>> +#ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
>> +	mmc_set_preinit(m, 1);
>> +#endif
>> +	if (m->preinit)
>> +		mmc_start_init(m);
>> +
>> +	return 0;
>> +}
>> +#endif
>> +
>>  #ifdef CONFIG_CMD_BKOPS_ENABLE
>>  int mmc_set_bkops_enable(struct mmc *mmc)  { diff --git
>> a/include/mmc.h b/include/mmc.h index 46422f41a4..878b4c9e57 100644
>> --- a/include/mmc.h
>> +++ b/include/mmc.h
>> @@ -698,6 +698,7 @@ void mmc_destroy(struct mmc *mmc);
>>   */
>>  int mmc_unbind(struct udevice *dev);
>>  int mmc_initialize(bd_t *bis);
>> +int mmc_init_device(int num);
>>  int mmc_init(struct mmc *mmc);
>>  int mmc_send_tuning(struct mmc *mmc, u32 opcode, int *cmd_error);
> 
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> 
>>
>> --
>> 2.22.0
>
Peng Fan Oct. 9, 2019, 5:44 a.m. UTC | #3
> Subject: Re: [PATCH] spl: dm_mmc: Initialize only the required mmc device
> 
> Hi Peng,
> 
> On 10/09/19 7:50 AM, Peng Fan wrote:
> >> Subject: [PATCH] spl: dm_mmc: Initialize only the required mmc device
> >>
> >> In SPL, all the available mmc devices gets initialized during boot.
> >> This might not work in cases where clocks are not available for
> >> certain mmc devices(other than boot device) and the support for
> >> enabling device might not be ready.
> >>
> >> Texas Instruments' K3 J721E device having a central system controller
> >> (dmsc) is one such example falling in this category. Below is the
> >> sequence for the failing scenario:
> >> - ROM comes up in SD mode and loads SPL by just initialing SD card.
> >> - SPL loads dmsc firmware from SD Card.
> >> Since ROM has enabled SD, SPL need not enable the SD, just need to re
> >> initialize the card. But SPL is trying to initialize other MMC
> >> instances which are in disabled state. Since dmsc firmware is not yet
> >> available, devices cannot be enabled. So in SPL, initialize only the mmc
> device that is needed.
> >>
> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> 
> Gentle Ping. Are you planning to take this patch for this merge window?

Just back from holiday. Yes, I'll pick it up and prepare PR to Tom.

Thanks,
Peng.

> 
> Thanks and regards,
> Lokesh
> 
> >> ---
> >>  common/spl/spl_mmc.c | 14 ++++----------
> >>  drivers/mmc/mmc.c    | 24 ++++++++++++++++++++++++
> >>  include/mmc.h        |  1 +
> >>  3 files changed, 29 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index
> >> b3619889f7..303f0f80bf 100644
> >> --- a/common/spl/spl_mmc.c
> >> +++ b/common/spl/spl_mmc.c
> >> @@ -113,31 +113,25 @@ 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);
> >>  	if (mmc_dev < 0)
> >>  		return mmc_dev;
> >>
> >> +#if CONFIG_IS_ENABLED(DM_MMC)
> >> +	err = mmc_init_device(mmc_dev);
> >> +#else
> >>  	err = mmc_initialize(NULL);
> >> +#endif /* DM_MMC */
> >>  	if (err) {
> >>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> >>  		printf("spl: could not initialize mmc. error: %d\n", err);  #endif
> >>  		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", diff
> >> --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> >> eecc7d687e..ec8f92ce8f
> >> 100644
> >> --- a/drivers/mmc/mmc.c
> >> +++ b/drivers/mmc/mmc.c
> >> @@ -2998,6 +2998,30 @@ int mmc_initialize(bd_t *bis)
> >>  	return 0;
> >>  }
> >>
> >> +#if CONFIG_IS_ENABLED(DM_MMC)
> >> +int mmc_init_device(int num)
> >> +{
> >> +	struct udevice *dev;
> >> +	struct mmc *m;
> >> +	int ret;
> >> +
> >> +	ret = uclass_get_device(UCLASS_MMC, num, &dev);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	m = mmc_get_mmc_dev(dev);
> >> +	if (!m)
> >> +		return 0;
> >> +#ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
> >> +	mmc_set_preinit(m, 1);
> >> +#endif
> >> +	if (m->preinit)
> >> +		mmc_start_init(m);
> >> +
> >> +	return 0;
> >> +}
> >> +#endif
> >> +
> >>  #ifdef CONFIG_CMD_BKOPS_ENABLE
> >>  int mmc_set_bkops_enable(struct mmc *mmc)  { diff --git
> >> a/include/mmc.h b/include/mmc.h index 46422f41a4..878b4c9e57
> 100644
> >> --- a/include/mmc.h
> >> +++ b/include/mmc.h
> >> @@ -698,6 +698,7 @@ void mmc_destroy(struct mmc *mmc);
> >>   */
> >>  int mmc_unbind(struct udevice *dev);  int mmc_initialize(bd_t *bis);
> >> +int mmc_init_device(int num);
> >>  int mmc_init(struct mmc *mmc);
> >>  int mmc_send_tuning(struct mmc *mmc, u32 opcode, int *cmd_error);
> >
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> >
> >>
> >> --
> >> 2.22.0
> >
diff mbox series

Patch

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index b3619889f7..303f0f80bf 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -113,31 +113,25 @@  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);
 	if (mmc_dev < 0)
 		return mmc_dev;
 
+#if CONFIG_IS_ENABLED(DM_MMC)
+	err = mmc_init_device(mmc_dev);
+#else
 	err = mmc_initialize(NULL);
+#endif /* DM_MMC */
 	if (err) {
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		printf("spl: could not initialize mmc. error: %d\n", err);
 #endif
 		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",
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index eecc7d687e..ec8f92ce8f 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -2998,6 +2998,30 @@  int mmc_initialize(bd_t *bis)
 	return 0;
 }
 
+#if CONFIG_IS_ENABLED(DM_MMC)
+int mmc_init_device(int num)
+{
+	struct udevice *dev;
+	struct mmc *m;
+	int ret;
+
+	ret = uclass_get_device(UCLASS_MMC, num, &dev);
+	if (ret)
+		return ret;
+
+	m = mmc_get_mmc_dev(dev);
+	if (!m)
+		return 0;
+#ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
+	mmc_set_preinit(m, 1);
+#endif
+	if (m->preinit)
+		mmc_start_init(m);
+
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_CMD_BKOPS_ENABLE
 int mmc_set_bkops_enable(struct mmc *mmc)
 {
diff --git a/include/mmc.h b/include/mmc.h
index 46422f41a4..878b4c9e57 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -698,6 +698,7 @@  void mmc_destroy(struct mmc *mmc);
  */
 int mmc_unbind(struct udevice *dev);
 int mmc_initialize(bd_t *bis);
+int mmc_init_device(int num);
 int mmc_init(struct mmc *mmc);
 int mmc_send_tuning(struct mmc *mmc, u32 opcode, int *cmd_error);