[PATCH v2 1/2] arm: mach-snapdragon: misc: Initialize eMMC if necessary

[PATCH v2 1/2] arm: mach-snapdragon: misc: Initialize eMMC if necessary

[Stephan Gerhold]

At the moment U-Boot produces an empty MAC address (02:00:00:00:00:00)
if the eMMC is not used by anything in U-Boot (e.g. with
CONFIG_ENV_IS_NOWHERE=y instead of having the environment on eMMC).
This happens because then there is nothing that actually initializes
the eMMC and reads the "cid" that is later accessed.

To fix this, call mmc_init() to ensure the eMMC is initialized.
There is no functional difference if the eMMC is already initialized
since then mmc_init() will just return without doing anything.

Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---

(no changes since v1)

 arch/arm/mach-snapdragon/misc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-snapdragon/misc.c b/arch/arm/mach-snapdragon/misc.c
index 985625a548..fbd5f4d051 100644
--- a/arch/arm/mach-snapdragon/misc.c
+++ b/arch/arm/mach-snapdragon/misc.c
@@ -33,6 +33,9 @@ u32 msm_board_serial(void)
 	if (!mmc_dev)
 		return 0;
 
+	if (mmc_init(mmc_dev))
+		return 0;
+
 	return UNSTUFF_BITS(mmc_dev->cid, 16, 32);
 }
 
-- 
2.32.0

[PATCH v2 2/2] arm: mach-snapdrgon: misc: Simplify msm_generate_mac_addr()

[Stephan Gerhold]

The logic in msm_generate_mac_addr() was originally taken from the LK
bootloader where the serial number is a string and must be parsed first.
However, in U-Boot msm_board_serial() returns an u32 and
msm_generate_mac_addr() has quite complicated code that will first
print it as a hex string and then immediately parse it again.

What this function actually does at the end is to put the serial number
encoded as big endian (the order used for the hex string) into the u8 *mac.
Use put_unaligned_be32() to do that with bit shifts instead of going
through the string format.

This should be slightly more efficient and cleaner but does not result
in any functional difference.

Cc: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---

Changes in v2:
  - Rebase on u-boot/master to fix conflict with recent changes
  - Add comment to clarify what put_unaligned_be32() does here

 arch/arm/mach-snapdragon/misc.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-snapdragon/misc.c b/arch/arm/mach-snapdragon/misc.c
index fbd5f4d051..7d452f4529 100644
--- a/arch/arm/mach-snapdragon/misc.c
+++ b/arch/arm/mach-snapdragon/misc.c
@@ -9,6 +9,7 @@
 #include <common.h>
 #include <mmc.h>
 #include <asm/arch/misc.h>
+#include <asm/unaligned.h>
 
 /* UNSTUFF_BITS macro taken from Linux Kernel: drivers/mmc/core/sd.c */
 #define UNSTUFF_BITS(resp, start, size) \
@@ -41,16 +42,14 @@ u32 msm_board_serial(void)
 
 void msm_generate_mac_addr(u8 *mac)
 {
-	int i;
-	char sn[9];
-
-	snprintf(sn, 9, "%08x", msm_board_serial());
-
-	/* fill in the mac with serialno, use locally adminstrated pool */
+	/* use locally adminstrated pool */
 	mac[0] = 0x02;
-	mac[1] = 00;
-	for (i = 3; i >= 0; i--) {
-		mac[i + 2] = hextoul(&sn[2 * i], NULL);
-		sn[2 * i] = 0;
-	}
+	mac[1] = 0x00;
+
+	/*
+	 * Put the 32-bit serial number in the last 32-bit of the MAC address.
+	 * Use big endian order so it is consistent with the serial number
+	 * written as a hexadecimal string, e.g. 0x1234abcd -> 02:00:12:34:ab:cd
+	 */
+	put_unaligned_be32(msm_board_serial(), &mac[2]);
 }
-- 
2.32.0

Re: [PATCH v2 2/2] arm: mach-snapdrgon: misc: Simplify msm_generate_mac_addr()

[Ramon Fried]

On Tue, Aug 3, 2021 at 1:12 PM Stephan Gerhold <stephan@gerhold.net> wrote:
>
> The logic in msm_generate_mac_addr() was originally taken from the LK
> bootloader where the serial number is a string and must be parsed first.
> However, in U-Boot msm_board_serial() returns an u32 and
> msm_generate_mac_addr() has quite complicated code that will first
> print it as a hex string and then immediately parse it again.
>
> What this function actually does at the end is to put the serial number
> encoded as big endian (the order used for the hex string) into the u8 *mac.
> Use put_unaligned_be32() to do that with bit shifts instead of going
> through the string format.
>
> This should be slightly more efficient and cleaner but does not result
> in any functional difference.
>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>
> Changes in v2:
>   - Rebase on u-boot/master to fix conflict with recent changes
>   - Add comment to clarify what put_unaligned_be32() does here
>
>  arch/arm/mach-snapdragon/misc.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-snapdragon/misc.c b/arch/arm/mach-snapdragon/misc.c
> index fbd5f4d051..7d452f4529 100644
> --- a/arch/arm/mach-snapdragon/misc.c
> +++ b/arch/arm/mach-snapdragon/misc.c
> @@ -9,6 +9,7 @@
>  #include <common.h>
>  #include <mmc.h>
>  #include <asm/arch/misc.h>
> +#include <asm/unaligned.h>
>
>  /* UNSTUFF_BITS macro taken from Linux Kernel: drivers/mmc/core/sd.c */
>  #define UNSTUFF_BITS(resp, start, size) \
> @@ -41,16 +42,14 @@ u32 msm_board_serial(void)
>
>  void msm_generate_mac_addr(u8 *mac)
>  {
> -       int i;
> -       char sn[9];
> -
> -       snprintf(sn, 9, "%08x", msm_board_serial());
> -
> -       /* fill in the mac with serialno, use locally adminstrated pool */
> +       /* use locally adminstrated pool */
>         mac[0] = 0x02;
> -       mac[1] = 00;
> -       for (i = 3; i >= 0; i--) {
> -               mac[i + 2] = hextoul(&sn[2 * i], NULL);
> -               sn[2 * i] = 0;
> -       }
> +       mac[1] = 0x00;
> +
> +       /*
> +        * Put the 32-bit serial number in the last 32-bit of the MAC address.
> +        * Use big endian order so it is consistent with the serial number
> +        * written as a hexadecimal string, e.g. 0x1234abcd -> 02:00:12:34:ab:cd
> +        */
> +       put_unaligned_be32(msm_board_serial(), &mac[2]);
>  }
> --
> 2.32.0
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH v2 2/2] arm: mach-snapdrgon: misc: Simplify msm_generate_mac_addr()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

On Tue, Aug 03, 2021 at 12:12:38PM +0200, Stephan Gerhold wrote:

> The logic in msm_generate_mac_addr() was originally taken from the LK
> bootloader where the serial number is a string and must be parsed first.
> However, in U-Boot msm_board_serial() returns an u32 and
> msm_generate_mac_addr() has quite complicated code that will first
> print it as a hex string and then immediately parse it again.
> 
> What this function actually does at the end is to put the serial number
> encoded as big endian (the order used for the hex string) into the u8 *mac.
> Use put_unaligned_be32() to do that with bit shifts instead of going
> through the string format.
> 
> This should be slightly more efficient and cleaner but does not result
> in any functional difference.
> 
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 1/2] arm: mach-snapdragon: misc: Initialize eMMC if necessary

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

On Tue, Aug 03, 2021 at 12:12:37PM +0200, Stephan Gerhold wrote:

> At the moment U-Boot produces an empty MAC address (02:00:00:00:00:00)
> if the eMMC is not used by anything in U-Boot (e.g. with
> CONFIG_ENV_IS_NOWHERE=y instead of having the environment on eMMC).
> This happens because then there is nothing that actually initializes
> the eMMC and reads the "cid" that is later accessed.
> 
> To fix this, call mmc_init() to ensure the eMMC is initialized.
> There is no functional difference if the eMMC is already initialized
> since then mmc_init() will just return without doing anything.
> 
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]