[U-Boot] [PATCH] evb_rk3399: revert CONFIG_SYS_MMC_ENV_DEV to 0

[U-Boot] [PATCH] evb_rk3399: revert CONFIG_SYS_MMC_ENV_DEV to 0

[Max Kellermann]

This was changed to 1 in commit 0717dde057e, but a few months later,
commit 5f9411af37b swapped the order of eMMC and SD card by assigning
indexed aliases to `&sdhci` and `&sdmmc`.
---
 include/configs/evb_rk3399.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/evb_rk3399.h b/include/configs/evb_rk3399.h
index a99eeab484..b9c4d683f4 100644
--- a/include/configs/evb_rk3399.h
+++ b/include/configs/evb_rk3399.h
@@ -8,7 +8,7 @@
 
 #include <configs/rk3399_common.h>
 
-#define CONFIG_SYS_MMC_ENV_DEV 1
+#define CONFIG_SYS_MMC_ENV_DEV 0
 
 #define SDRAM_BANK_SIZE			(2UL << 30)
 
-- 
2.19.2

[U-Boot] [PATCH] evb_rk3399: revert CONFIG_SYS_MMC_ENV_DEV to 0

[Philipp Tomsich]

On 04.12.2018, at 12:00, Max Kellermann <max.kellermann@gmail.com> wrote:
> 
> This was changed to 1 in commit 0717dde057e, but a few months later,
> commit 5f9411af37b swapped the order of eMMC and SD card by assigning
> indexed aliases to `&sdhci` and `&sdmmc`.

If this is a straight revert, I’d appreciate it if the was marked as such by
creating the commit using a 'git revert’ and then adding additional background
to explain why the revert was done.

However, we have a bigger issue here: the original commit has been in the
tree since 2016 and I am reluctant to change such fundamental naming without
a rationale.  This is particularily in knowledge of the on-list discussion that we’ve
had with Kever recently regarding his request to change the search order for booting.

Could you explain why you think that this should be changed?

Thanks,
Philipp.

[U-Boot] [PATCH] evb_rk3399: revert CONFIG_SYS_MMC_ENV_DEV to 0

[Max Kellermann]

On 2018/12/13 22:59, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote:
> On 04.12.2018, at 12:00, Max Kellermann <max.kellermann@gmail.com> wrote:
> > 
> > This was changed to 1 in commit 0717dde057e, but a few months later,
> > commit 5f9411af37b swapped the order of eMMC and SD card by assigning
> > indexed aliases to `&sdhci` and `&sdmmc`.
> 
> If this is a straight revert, I’d appreciate it if the was marked as such by
> creating the commit using a 'git revert’ and then adding additional background
> to explain why the revert was done.

Technically, it is a straight revert, but I replaced git's boilerplate
text with a descriptive commit message, without omitting any
information from the boilerplate.  If you prefer to leave the full
boilerplate text in, I can re-add it and resend the patch.

However, semantically, it is not a revert, because it does not revert
the semantic effect of commit 0717dde057e; quite opposite, it restores
the effect of commit 0717dde057e after it was broken by commit
5f9411af37b.

> However, we have a bigger issue here: the original commit has been in the
> tree since 2016 and I am reluctant to change such fundamental naming without
> a rationale.  This is particularily in knowledge of the on-list discussion that we’ve
> had with Kever recently regarding his request to change the search order for booting.

The naming was (accidently?) changed in commit 5f9411af37b, which
effectively undid the effect of commit 0717dde057e, and my patch only
repairs that.

> Could you explain why you think that this should be changed?

The goal of commit 0717dde057e was to boot from eMMC by default, and
at the time, eMMC was "mmc1".  Later, commit 5f9411af37b swapped the
order of "mmc", making eMMC="mmc0".  This broke commit 0717dde057e.

This is how it looks like on my AIO-3399J:

 => mmc list
 dwmmc at fe320000: 1
 sdhci at fe330000: 0 (eMMC)

As you see, CONFIG_SYS_MMC_ENV_DEV=1 will boot from SD card, not from
eMMC.

I submitted the patch because it looked like the side effect of commit
5f9411af37b was an accident, so I attempted fixed it.

Max

[U-Boot] [PATCH] evb_rk3399: revert CONFIG_SYS_MMC_ENV_DEV to 0

[Philipp Tomsich]

+Kever

Max,

> On 14.12.2018, at 09:21, Max Kellermann <max@blarg.de> wrote:
> 
> On 2018/12/13 22:59, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote:
>> On 04.12.2018, at 12:00, Max Kellermann <max.kellermann@gmail.com> wrote:
>>> 
>>> This was changed to 1 in commit 0717dde057e, but a few months later,
>>> commit 5f9411af37b swapped the order of eMMC and SD card by assigning
>>> indexed aliases to `&sdhci` and `&sdmmc`.
>> 
>> If this is a straight revert, I’d appreciate it if the was marked as such by
>> creating the commit using a 'git revert’ and then adding additional background
>> to explain why the revert was done.
> 
> Technically, it is a straight revert, but I replaced git's boilerplate
> text with a descriptive commit message, without omitting any
> information from the boilerplate.  If you prefer to leave the full
> boilerplate text in, I can re-add it and resend the patch.
> 
> However, semantically, it is not a revert, because it does not revert
> the semantic effect of commit 0717dde057e; quite opposite, it restores
> the effect of commit 0717dde057e after it was broken by commit
> 5f9411af37b.
> 
>> However, we have a bigger issue here: the original commit has been in the
>> tree since 2016 and I am reluctant to change such fundamental naming without
>> a rationale.  This is particularily in knowledge of the on-list discussion that we’ve
>> had with Kever recently regarding his request to change the search order for booting.
> 
> The naming was (accidently?) changed in commit 5f9411af37b, which
> effectively undid the effect of commit 0717dde057e, and my patch only
> repairs that.
> 
>> Could you explain why you think that this should be changed?
> 
> The goal of commit 0717dde057e was to boot from eMMC by default, and
> at the time, eMMC was "mmc1".  Later, commit 5f9411af37b swapped the
> order of "mmc", making eMMC="mmc0".  This broke commit 0717dde057e.
> 
> This is how it looks like on my AIO-3399J:
> 
> => mmc list
> dwmmc at fe320000: 1
> sdhci at fe330000: 0 (eMMC)
> 
> As you see, CONFIG_SYS_MMC_ENV_DEV=1 will boot from SD card, not from
> eMMC.
> 
> I submitted the patch because it looked like the side effect of commit
> 5f9411af37b was an accident, so I attempted fixed it.

Unfortunately this seems less like a side-effect of that change than of the fact
that there’s a variety of RK3399 boards out there that don’t have dedicated 
board-support in U-Boot and the EVB-defaults may be wrong.
The RK3399-EVB board-suupport does not provide any support to either identify
what board it’s on or to allow overriding these device-order.

@Kever: Could you please confirm whether the CONFIG_SYS_MMC_ENV_DEV
is appropriate for the EVB itself or if it should be changed for the EVB’s config
globally?

Thanks,
Philipp.