diff mbox

[U-Boot,3/4] mmc: exynos dwmmc: check boot mode before init dwmmc

Message ID 1424178544-28632-4-git-send-email-p.marczak@samsung.com
State Superseded
Delegated to: Pantelis Antoniou
Headers show

Commit Message

Przemyslaw Marczak Feb. 17, 2015, 1:09 p.m. UTC
Before this commit, the mmc devices were always registered
in the same order. So dwmmc channel 0 was registered as mmc 0,
channel 1 as mmc 1, etc.
In case of possibility to boot from more then one device,
the CONFIG_SYS_MMC_ENV_DEV should always point to right mmc device.

This can be achieved by init boot device as first, so it will be
always registered as mmc 0. Thanks to this, the 'saveenv' command
will work fine for all mmc boot devices.

Exynos based boards usually uses mmc host channels configuration:
- 0, or 0+1 for 8 bit  - as a default boot device (usually eMMC)
- 2 for 4bit - as an optional boot device (usually SD card slot)

And usually the boot order is defined by OM pin configuration,
which can be changed in a few ways, eg.
- Odroid U3     - eMMC card insertion -> first boot from eMMC
- Odroid X2/XU3 - boot priority jumper

By this commit, Exynos dwmmc driver will check the OM pin configuration,
and then try to init the boot device and register it as mmc 0.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Minkyu Kang <mk7.kang@samsung.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Pantelis Antoniou <panto@intracom.gr>
Cc: Simon Glass <sjg@chromium.org>
Cc: Akshay Saraswat <akshay.s@samsung.com>
---
 drivers/mmc/exynos_dw_mmc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Simon Glass Feb. 18, 2015, 5:02 a.m. UTC | #1
Hi Przemyslaw,

On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> Before this commit, the mmc devices were always registered
> in the same order. So dwmmc channel 0 was registered as mmc 0,
> channel 1 as mmc 1, etc.
> In case of possibility to boot from more then one device,
> the CONFIG_SYS_MMC_ENV_DEV should always point to right mmc device.
>
> This can be achieved by init boot device as first, so it will be
> always registered as mmc 0. Thanks to this, the 'saveenv' command
> will work fine for all mmc boot devices.
>
> Exynos based boards usually uses mmc host channels configuration:
> - 0, or 0+1 for 8 bit  - as a default boot device (usually eMMC)
> - 2 for 4bit - as an optional boot device (usually SD card slot)
>
> And usually the boot order is defined by OM pin configuration,
> which can be changed in a few ways, eg.
> - Odroid U3     - eMMC card insertion -> first boot from eMMC
> - Odroid X2/XU3 - boot priority jumper
>
> By this commit, Exynos dwmmc driver will check the OM pin configuration,
> and then try to init the boot device and register it as mmc 0.

I think a better way to do this would be to make
CONFIG_SYS_MMC_ENV_DEV support an option where the device can be
selected at run-time.

However that would probably be better done when the drive rmodel
conversion is complete.

So this seems a reasonable patch given where we are.

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Cc: Minkyu Kang <mk7.kang@samsung.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Pantelis Antoniou <panto@intracom.gr>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Akshay Saraswat <akshay.s@samsung.com>
> ---
>  drivers/mmc/exynos_dw_mmc.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
> index dfa209b..91f0163 100644
> --- a/drivers/mmc/exynos_dw_mmc.c
> +++ b/drivers/mmc/exynos_dw_mmc.c
> @@ -13,6 +13,7 @@
>  #include <asm/arch/dwmmc.h>
>  #include <asm/arch/clk.h>
>  #include <asm/arch/pinmux.h>
> +#include <asm/arch/power.h>
>  #include <asm/gpio.h>
>  #include <asm-generic/errno.h>
>
> @@ -166,7 +167,6 @@ static int exynos_dwmci_get_config(const void *blob, int node,
>         if (host->dev_index == host->dev_id)
>                 host->dev_index = host->dev_id - PERIPH_ID_SDMMC0;
>
> -
>         /* Get the bus width from the device node */
>         host->buswidth = fdtdec_get_int(blob, node, "samsung,bus-width", 0);
>         if (host->buswidth <= 0) {
> @@ -229,12 +229,21 @@ int exynos_dwmmc_init(const void *blob)
>  {
>         int compat_id;
>         int node_list[DWMMC_MAX_CH_NUM];
> +       int boot_dev_node;
>         int err = 0, count;
>
>         compat_id = COMPAT_SAMSUNG_EXYNOS_DWMMC;
>
>         count = fdtdec_find_aliases_for_id(blob, "mmc",
>                                 compat_id, node_list, DWMMC_MAX_CH_NUM);
> +
> +       /* For DWMMC always set boot device as mmc 0 */
> +       if (count >= 3 && get_boot_mode() == BOOT_MODE_SD) {
> +               boot_dev_node = node_list[2];
> +               node_list[2] = node_list[0];
> +               node_list[0] = boot_dev_node;
> +       }
> +
>         err = exynos_dwmci_process_node(blob, node_list, count);
>
>         return err;
> --
> 1.9.1
>

Regards,
Simon
Przemyslaw Marczak Feb. 18, 2015, 10:50 a.m. UTC | #2
Hello Simon,

On 02/18/2015 06:02 AM, Simon Glass wrote:
> Hi Przemyslaw,
>
> On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>> Before this commit, the mmc devices were always registered
>> in the same order. So dwmmc channel 0 was registered as mmc 0,
>> channel 1 as mmc 1, etc.
>> In case of possibility to boot from more then one device,
>> the CONFIG_SYS_MMC_ENV_DEV should always point to right mmc device.
>>
>> This can be achieved by init boot device as first, so it will be
>> always registered as mmc 0. Thanks to this, the 'saveenv' command
>> will work fine for all mmc boot devices.
>>
>> Exynos based boards usually uses mmc host channels configuration:
>> - 0, or 0+1 for 8 bit  - as a default boot device (usually eMMC)
>> - 2 for 4bit - as an optional boot device (usually SD card slot)
>>
>> And usually the boot order is defined by OM pin configuration,
>> which can be changed in a few ways, eg.
>> - Odroid U3     - eMMC card insertion -> first boot from eMMC
>> - Odroid X2/XU3 - boot priority jumper
>>
>> By this commit, Exynos dwmmc driver will check the OM pin configuration,
>> and then try to init the boot device and register it as mmc 0.
>
> I think a better way to do this would be to make
> CONFIG_SYS_MMC_ENV_DEV support an option where the device can be
> selected at run-time.
>
> However that would probably be better done when the drive rmodel
> conversion is complete.
>
> So this seems a reasonable patch given where we are.
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>

This was just a quick solution to solve the issue on XU3, when the same 
binary can boot from sd or eMMC slots.


>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> Cc: Minkyu Kang <mk7.kang@samsung.com>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Pantelis Antoniou <panto@intracom.gr>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Akshay Saraswat <akshay.s@samsung.com>
>> ---
>>   drivers/mmc/exynos_dw_mmc.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
>> index dfa209b..91f0163 100644
>> --- a/drivers/mmc/exynos_dw_mmc.c
>> +++ b/drivers/mmc/exynos_dw_mmc.c
>> @@ -13,6 +13,7 @@
>>   #include <asm/arch/dwmmc.h>
>>   #include <asm/arch/clk.h>
>>   #include <asm/arch/pinmux.h>
>> +#include <asm/arch/power.h>
>>   #include <asm/gpio.h>
>>   #include <asm-generic/errno.h>
>>
>> @@ -166,7 +167,6 @@ static int exynos_dwmci_get_config(const void *blob, int node,
>>          if (host->dev_index == host->dev_id)
>>                  host->dev_index = host->dev_id - PERIPH_ID_SDMMC0;
>>
>> -
>>          /* Get the bus width from the device node */
>>          host->buswidth = fdtdec_get_int(blob, node, "samsung,bus-width", 0);
>>          if (host->buswidth <= 0) {
>> @@ -229,12 +229,21 @@ int exynos_dwmmc_init(const void *blob)
>>   {
>>          int compat_id;
>>          int node_list[DWMMC_MAX_CH_NUM];
>> +       int boot_dev_node;
>>          int err = 0, count;
>>
>>          compat_id = COMPAT_SAMSUNG_EXYNOS_DWMMC;
>>
>>          count = fdtdec_find_aliases_for_id(blob, "mmc",
>>                                  compat_id, node_list, DWMMC_MAX_CH_NUM);
>> +
>> +       /* For DWMMC always set boot device as mmc 0 */
>> +       if (count >= 3 && get_boot_mode() == BOOT_MODE_SD) {
>> +               boot_dev_node = node_list[2];
>> +               node_list[2] = node_list[0];
>> +               node_list[0] = boot_dev_node;
>> +       }
>> +
>>          err = exynos_dwmci_process_node(blob, node_list, count);
>>
>>          return err;
>> --
>> 1.9.1
>>
>
> Regards,
> Simon
>

Thank you for the review. I will send the second version soon.

Best regards,
Tom Rini Feb. 19, 2015, 2:01 p.m. UTC | #3
On Tue, Feb 17, 2015 at 10:02:03PM -0700, Simon Glass wrote:
> Hi Przemyslaw,
> 
> On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> > Before this commit, the mmc devices were always registered
> > in the same order. So dwmmc channel 0 was registered as mmc 0,
> > channel 1 as mmc 1, etc.
> > In case of possibility to boot from more then one device,
> > the CONFIG_SYS_MMC_ENV_DEV should always point to right mmc device.
> >
> > This can be achieved by init boot device as first, so it will be
> > always registered as mmc 0. Thanks to this, the 'saveenv' command
> > will work fine for all mmc boot devices.
> >
> > Exynos based boards usually uses mmc host channels configuration:
> > - 0, or 0+1 for 8 bit  - as a default boot device (usually eMMC)
> > - 2 for 4bit - as an optional boot device (usually SD card slot)
> >
> > And usually the boot order is defined by OM pin configuration,
> > which can be changed in a few ways, eg.
> > - Odroid U3     - eMMC card insertion -> first boot from eMMC
> > - Odroid X2/XU3 - boot priority jumper
> >
> > By this commit, Exynos dwmmc driver will check the OM pin configuration,
> > and then try to init the boot device and register it as mmc 0.
> 
> I think a better way to do this would be to make
> CONFIG_SYS_MMC_ENV_DEV support an option where the device can be
> selected at run-time.
> 
> However that would probably be better done when the drive rmodel
> conversion is complete.

Yes, lets hold off on this until driver model is in and we can do this
more cleanly rather than whack at this problem right now (I've seen
solutions to this kind of problem on am335x for example and it's still
going to be better to wait I think).
Tom Rini Feb. 19, 2015, 2:03 p.m. UTC | #4
On Wed, Feb 18, 2015 at 11:50:41AM +0100, Przemyslaw Marczak wrote:
> Hello Simon,
> 
> On 02/18/2015 06:02 AM, Simon Glass wrote:
> >Hi Przemyslaw,
> >
> >On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> >>Before this commit, the mmc devices were always registered
> >>in the same order. So dwmmc channel 0 was registered as mmc 0,
> >>channel 1 as mmc 1, etc.
> >>In case of possibility to boot from more then one device,
> >>the CONFIG_SYS_MMC_ENV_DEV should always point to right mmc device.
> >>
> >>This can be achieved by init boot device as first, so it will be
> >>always registered as mmc 0. Thanks to this, the 'saveenv' command
> >>will work fine for all mmc boot devices.
> >>
> >>Exynos based boards usually uses mmc host channels configuration:
> >>- 0, or 0+1 for 8 bit  - as a default boot device (usually eMMC)
> >>- 2 for 4bit - as an optional boot device (usually SD card slot)
> >>
> >>And usually the boot order is defined by OM pin configuration,
> >>which can be changed in a few ways, eg.
> >>- Odroid U3     - eMMC card insertion -> first boot from eMMC
> >>- Odroid X2/XU3 - boot priority jumper
> >>
> >>By this commit, Exynos dwmmc driver will check the OM pin configuration,
> >>and then try to init the boot device and register it as mmc 0.
> >
> >I think a better way to do this would be to make
> >CONFIG_SYS_MMC_ENV_DEV support an option where the device can be
> >selected at run-time.
> >
> >However that would probably be better done when the drive rmodel
> >conversion is complete.
> >
> >So this seems a reasonable patch given where we are.
> >
> >Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> 
> This was just a quick solution to solve the issue on XU3, when the
> same binary can boot from sd or eMMC slots.

XU3 isn't unique in this regard.  "am335x_evm" binaries runs on 4 very
different boards and we still just have to say that sometimes we default
to ENV in a place that isn't workable.
Przemyslaw Marczak Feb. 19, 2015, 2:36 p.m. UTC | #5
Hello Tom,

On 02/19/2015 03:03 PM, Tom Rini wrote:
> On Wed, Feb 18, 2015 at 11:50:41AM +0100, Przemyslaw Marczak wrote:
>> Hello Simon,
>>
>> On 02/18/2015 06:02 AM, Simon Glass wrote:
>>> Hi Przemyslaw,
>>>
>>> On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>>>> Before this commit, the mmc devices were always registered
>>>> in the same order. So dwmmc channel 0 was registered as mmc 0,
>>>> channel 1 as mmc 1, etc.
>>>> In case of possibility to boot from more then one device,
>>>> the CONFIG_SYS_MMC_ENV_DEV should always point to right mmc device.
>>>>
>>>> This can be achieved by init boot device as first, so it will be
>>>> always registered as mmc 0. Thanks to this, the 'saveenv' command
>>>> will work fine for all mmc boot devices.
>>>>
>>>> Exynos based boards usually uses mmc host channels configuration:
>>>> - 0, or 0+1 for 8 bit  - as a default boot device (usually eMMC)
>>>> - 2 for 4bit - as an optional boot device (usually SD card slot)
>>>>
>>>> And usually the boot order is defined by OM pin configuration,
>>>> which can be changed in a few ways, eg.
>>>> - Odroid U3     - eMMC card insertion -> first boot from eMMC
>>>> - Odroid X2/XU3 - boot priority jumper
>>>>
>>>> By this commit, Exynos dwmmc driver will check the OM pin configuration,
>>>> and then try to init the boot device and register it as mmc 0.
>>>
>>> I think a better way to do this would be to make
>>> CONFIG_SYS_MMC_ENV_DEV support an option where the device can be
>>> selected at run-time.
>>>
>>> However that would probably be better done when the drive rmodel
>>> conversion is complete.
>>>
>>> So this seems a reasonable patch given where we are.
>>>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>
>> This was just a quick solution to solve the issue on XU3, when the
>> same binary can boot from sd or eMMC slots.
>
> XU3 isn't unique in this regard.  "am335x_evm" binaries runs on 4 very
> different boards and we still just have to say that sometimes we default
> to ENV in a place that isn't workable.
>
>

Yes, I saw this issue on bb black. But it seems to be not a big problem 
to fix it.
When I finish the pmic and finally start work on adding it to the bb 
black, then maybe I will try to fix it somehow.

Regards,
Tom Rini Feb. 19, 2015, 4:45 p.m. UTC | #6
On Thu, Feb 19, 2015 at 03:36:57PM +0100, Przemyslaw Marczak wrote:
> Hello Tom,
> 
> On 02/19/2015 03:03 PM, Tom Rini wrote:
> >On Wed, Feb 18, 2015 at 11:50:41AM +0100, Przemyslaw Marczak wrote:
> >>Hello Simon,
> >>
> >>On 02/18/2015 06:02 AM, Simon Glass wrote:
> >>>Hi Przemyslaw,
> >>>
> >>>On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> >>>>Before this commit, the mmc devices were always registered
> >>>>in the same order. So dwmmc channel 0 was registered as mmc 0,
> >>>>channel 1 as mmc 1, etc.
> >>>>In case of possibility to boot from more then one device,
> >>>>the CONFIG_SYS_MMC_ENV_DEV should always point to right mmc device.
> >>>>
> >>>>This can be achieved by init boot device as first, so it will be
> >>>>always registered as mmc 0. Thanks to this, the 'saveenv' command
> >>>>will work fine for all mmc boot devices.
> >>>>
> >>>>Exynos based boards usually uses mmc host channels configuration:
> >>>>- 0, or 0+1 for 8 bit  - as a default boot device (usually eMMC)
> >>>>- 2 for 4bit - as an optional boot device (usually SD card slot)
> >>>>
> >>>>And usually the boot order is defined by OM pin configuration,
> >>>>which can be changed in a few ways, eg.
> >>>>- Odroid U3     - eMMC card insertion -> first boot from eMMC
> >>>>- Odroid X2/XU3 - boot priority jumper
> >>>>
> >>>>By this commit, Exynos dwmmc driver will check the OM pin configuration,
> >>>>and then try to init the boot device and register it as mmc 0.
> >>>
> >>>I think a better way to do this would be to make
> >>>CONFIG_SYS_MMC_ENV_DEV support an option where the device can be
> >>>selected at run-time.
> >>>
> >>>However that would probably be better done when the drive rmodel
> >>>conversion is complete.
> >>>
> >>>So this seems a reasonable patch given where we are.
> >>>
> >>>Reviewed-by: Simon Glass <sjg@chromium.org>
> >>>
> >>
> >>This was just a quick solution to solve the issue on XU3, when the
> >>same binary can boot from sd or eMMC slots.
> >
> >XU3 isn't unique in this regard.  "am335x_evm" binaries runs on 4 very
> >different boards and we still just have to say that sometimes we default
> >to ENV in a place that isn't workable.
> 
> Yes, I saw this issue on bb black. But it seems to be not a big
> problem to fix it.
> When I finish the pmic and finally start work on adding it to the bb
> black, then maybe I will try to fix it somehow.

It's not hard (extending some of the concepts we have already) but I
want to hold that off until we have driver model support instead as part
of a "carrot and stick" approach ;)
Przemyslaw Marczak Feb. 20, 2015, 9:36 a.m. UTC | #7
Hi,

On 02/19/2015 05:45 PM, Tom Rini wrote:
> On Thu, Feb 19, 2015 at 03:36:57PM +0100, Przemyslaw Marczak wrote:
>> Hello Tom,
>>
>> On 02/19/2015 03:03 PM, Tom Rini wrote:
>>> On Wed, Feb 18, 2015 at 11:50:41AM +0100, Przemyslaw Marczak wrote:
>>>> Hello Simon,
>>>>
>>>> On 02/18/2015 06:02 AM, Simon Glass wrote:
>>>>> Hi Przemyslaw,
>>>>>
>>>>> On 17 February 2015 at 06:09, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>>>>>> Before this commit, the mmc devices were always registered
>>>>>> in the same order. So dwmmc channel 0 was registered as mmc 0,
>>>>>> channel 1 as mmc 1, etc.
>>>>>> In case of possibility to boot from more then one device,
>>>>>> the CONFIG_SYS_MMC_ENV_DEV should always point to right mmc device.
>>>>>>
>>>>>> This can be achieved by init boot device as first, so it will be
>>>>>> always registered as mmc 0. Thanks to this, the 'saveenv' command
>>>>>> will work fine for all mmc boot devices.
>>>>>>
>>>>>> Exynos based boards usually uses mmc host channels configuration:
>>>>>> - 0, or 0+1 for 8 bit  - as a default boot device (usually eMMC)
>>>>>> - 2 for 4bit - as an optional boot device (usually SD card slot)
>>>>>>
>>>>>> And usually the boot order is defined by OM pin configuration,
>>>>>> which can be changed in a few ways, eg.
>>>>>> - Odroid U3     - eMMC card insertion -> first boot from eMMC
>>>>>> - Odroid X2/XU3 - boot priority jumper
>>>>>>
>>>>>> By this commit, Exynos dwmmc driver will check the OM pin configuration,
>>>>>> and then try to init the boot device and register it as mmc 0.
>>>>>
>>>>> I think a better way to do this would be to make
>>>>> CONFIG_SYS_MMC_ENV_DEV support an option where the device can be
>>>>> selected at run-time.
>>>>>
>>>>> However that would probably be better done when the drive rmodel
>>>>> conversion is complete.
>>>>>
>>>>> So this seems a reasonable patch given where we are.
>>>>>
>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>>
>>>>
>>>> This was just a quick solution to solve the issue on XU3, when the
>>>> same binary can boot from sd or eMMC slots.
>>>
>>> XU3 isn't unique in this regard.  "am335x_evm" binaries runs on 4 very
>>> different boards and we still just have to say that sometimes we default
>>> to ENV in a place that isn't workable.
>>
>> Yes, I saw this issue on bb black. But it seems to be not a big
>> problem to fix it.
>> When I finish the pmic and finally start work on adding it to the bb
>> black, then maybe I will try to fix it somehow.
>
> It's not hard (extending some of the concepts we have already) but I
> want to hold that off until we have driver model support instead as part
> of a "carrot and stick" approach ;)
>

Ok, that's fine.

Regards,
diff mbox

Patch

diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
index dfa209b..91f0163 100644
--- a/drivers/mmc/exynos_dw_mmc.c
+++ b/drivers/mmc/exynos_dw_mmc.c
@@ -13,6 +13,7 @@ 
 #include <asm/arch/dwmmc.h>
 #include <asm/arch/clk.h>
 #include <asm/arch/pinmux.h>
+#include <asm/arch/power.h>
 #include <asm/gpio.h>
 #include <asm-generic/errno.h>
 
@@ -166,7 +167,6 @@  static int exynos_dwmci_get_config(const void *blob, int node,
 	if (host->dev_index == host->dev_id)
 		host->dev_index = host->dev_id - PERIPH_ID_SDMMC0;
 
-
 	/* Get the bus width from the device node */
 	host->buswidth = fdtdec_get_int(blob, node, "samsung,bus-width", 0);
 	if (host->buswidth <= 0) {
@@ -229,12 +229,21 @@  int exynos_dwmmc_init(const void *blob)
 {
 	int compat_id;
 	int node_list[DWMMC_MAX_CH_NUM];
+	int boot_dev_node;
 	int err = 0, count;
 
 	compat_id = COMPAT_SAMSUNG_EXYNOS_DWMMC;
 
 	count = fdtdec_find_aliases_for_id(blob, "mmc",
 				compat_id, node_list, DWMMC_MAX_CH_NUM);
+
+	/* For DWMMC always set boot device as mmc 0 */
+	if (count >= 3 && get_boot_mode() == BOOT_MODE_SD) {
+		boot_dev_node = node_list[2];
+		node_list[2] = node_list[0];
+		node_list[0] = boot_dev_node;
+	}
+
 	err = exynos_dwmci_process_node(blob, node_list, count);
 
 	return err;