[U-Boot] [PATCH v3 1/3] rockchip: rk3399: derive ethaddr from cpuid

[U-Boot] [PATCH v3 1/3] rockchip: rk3399: derive ethaddr from cpuid

[Rohan Garg]

Generate a MAC address based on the cpuid available in the efuse
block: Use the first 6 byte of the cpuid's SHA256 hash and set the
locally administered bits. Also ensure that the multicast bit is
cleared.

The MAC address is only generated and set if there is no ethaddr
present in the saved environment.

This is based off of Klaus Goger's work in 8adc9d

Signed-off-by: Rohan Garg <rohan.garg@collabora.com>

---

 arch/arm/include/asm/arch-rockchip/misc.h |  13 +++
 arch/arm/mach-rockchip/Makefile           |   4 +
 arch/arm/mach-rockchip/misc.c             | 112 ++++++++++++++++++++++
 arch/arm/mach-rockchip/rk3399-board.c     |  21 ++++
 4 files changed, 150 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-rockchip/misc.h
 create mode 100644 arch/arm/mach-rockchip/misc.c

diff --git a/arch/arm/include/asm/arch-rockchip/misc.h b/arch/arm/include/asm/arch-rockchip/misc.h
new file mode 100644
index 0000000000..b6b03c934e
--- /dev/null
+++ b/arch/arm/include/asm/arch-rockchip/misc.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * RK3399: Architecture common definitions
+ *
+ * Copyright (C) 2019 Collabora Inc - https://www.collabora.com/
+ *      Rohan Garg <rohan.garg@collabora.com>
+ */
+
+int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
+			      const u32 cpuid_length,
+			      u8 *cpuid);
+int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length);
+int rockchip_setup_macaddr(void);
diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index a12b8d4434..b32c4d6143 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -37,6 +37,10 @@ endif
 
 obj-$(CONFIG_$(SPL_TPL_)RAM) += sdram_common.o
 
+ifdef CONFIG_MISC_INIT_R
+obj-y += misc.o
+endif
+
 obj-$(CONFIG_ROCKCHIP_RK3036) += rk3036/
 obj-$(CONFIG_ROCKCHIP_RK3128) += rk3128/
 ifndef CONFIG_TPL_BUILD
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c
new file mode 100644
index 0000000000..7b6ec18551
--- /dev/null
+++ b/arch/arm/mach-rockchip/misc.c
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * RK3399: Architecture common definitions
+ *
+ * Copyright (C) 2019 Collabora Inc - https://www.collabora.com/
+ *      Rohan Garg <rohan.garg@collabora.com>
+ *
+ * Based on puma-rk3399.c:
+ *      (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
+ */
+
+#include <common.h>
+#include <u-boot/sha256.h>
+#include <dm/uclass-internal.h>
+#include <environment.h>
+
+#include <asm/arch-rockchip/misc.h>
+
+int rockchip_setup_macaddr(void)
+{
+#if CONFIG_IS_ENABLED(CMD_NET)
+	int ret;
+	const char *cpuid = env_get("cpuid#");
+	u8 hash[SHA256_SUM_LEN];
+	int size = sizeof(hash);
+	u8 mac_addr[6];
+
+	/* Only generate a MAC address, if none is set in the environment */
+	if (env_get("ethaddr"))
+		return -1;
+
+	if (!cpuid) {
+		debug("%s: could not retrieve 'cpuid#'\n", __func__);
+		return -1;
+	}
+
+	ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, &size);
+	if (ret) {
+		debug("%s: failed to calculate SHA256\n", __func__);
+		return -1;
+	}
+
+	/* Copy 6 bytes of the hash to base the MAC address on */
+	memcpy(mac_addr, hash, 6);
+
+	/* Make this a valid MAC address and set it */
+	mac_addr[0] &= 0xfe;  /* clear multicast bit */
+	mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
+	eth_env_set_enetaddr("ethaddr", mac_addr);
+#endif
+	return 0;
+}
+
+int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
+			      const u32 cpuid_length,
+			      u8 *cpuid)
+{
+#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE)
+	struct udevice *dev;
+	int ret;
+
+	/* retrieve the device */
+	ret = uclass_get_device_by_driver(UCLASS_MISC,
+					  DM_GET_DRIVER(rockchip_efuse), &dev);
+	if (ret) {
+		debug("%s: could not find efuse device\n", __func__);
+		return -1;
+	}
+
+	/* read the cpu_id range from the efuses */
+	ret = misc_read(dev, cpuid_offset, cpuid, sizeof(cpuid));
+	if (ret) {
+		debug("%s: reading cpuid from the efuses failed\n",
+		      __func__);
+		return -1;
+	}
+#endif
+	return 0;
+}
+
+int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length)
+{
+	u8 low[cpuid_length / 2], high[cpuid_length / 2];
+	char cpuid_str[cpuid_length * 2 + 1];
+	u64 serialno;
+	char serialno_str[17];
+	int i;
+
+	memset(cpuid_str, 0, sizeof(cpuid_str));
+	for (i = 0; i < 16; i++)
+		sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]);
+
+	debug("cpuid: %s\n", cpuid_str);
+
+	/*
+	 * Mix the cpuid bytes using the same rules as in
+	 *   ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c
+	 */
+	for (i = 0; i < 8; i++) {
+		low[i] = cpuid[1 + (i << 1)];
+		high[i] = cpuid[i << 1];
+	}
+
+	serialno = crc32_no_comp(0, low, 8);
+	serialno |= (u64)crc32_no_comp(serialno, high, 8) << 32;
+	snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno);
+
+	env_set("cpuid#", cpuid_str);
+	env_set("serial#", serialno_str);
+
+	return 0;
+}
diff --git a/arch/arm/mach-rockchip/rk3399-board.c b/arch/arm/mach-rockchip/rk3399-board.c
index 443c87cccc..3ffaaceab6 100644
--- a/arch/arm/mach-rockchip/rk3399-board.c
+++ b/arch/arm/mach-rockchip/rk3399-board.c
@@ -5,9 +5,30 @@
 
 #include <common.h>
 #include <asm/arch-rockchip/boot_mode.h>
+#include <asm/arch-rockchip/misc.h>
 
 int board_late_init(void)
 {
 	setup_boot_mode();
 	return 0;
 }
+
+int misc_init_r(void)
+{
+	const u32 cpuid_offset = 0x7;
+	const u32 cpuid_length = 0x10;
+	u8 cpuid[cpuid_length];
+	int ret;
+
+	ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
+	if (ret)
+		return ret;
+
+	ret = rockchip_cpuid_set(cpuid, cpuid_length);
+	if (ret)
+		return ret;
+
+	ret = rockchip_setup_macaddr();
+
+	return ret;
+}
-- 
2.17.1

[U-Boot] [PATCH v3 2/3] rockchip: rk3399: Enable CONFIG_MISC_INIT_R for the Rock PI 4

[Rohan Garg]

This enables us to set a static MAC address

Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
---

 configs/rock-pi-4-rk3399_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/rock-pi-4-rk3399_defconfig b/configs/rock-pi-4-rk3399_defconfig
index 14ae39a561..602be5a928 100644
--- a/configs/rock-pi-4-rk3399_defconfig
+++ b/configs/rock-pi-4-rk3399_defconfig
@@ -56,3 +56,4 @@ CONFIG_USB_ETHER_SMSC95XX=y
 CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_TINY_MEMSET=y
 CONFIG_ERRNO_STR=y
+CONFIG_MISC_INIT_R=y
-- 
2.17.1

[U-Boot] [PATCH v3 3/3] board: puma: Use rockchip_* helpers to setup cpuid and macaddr

[Rohan Garg]

We should use the shared helpers to setup the necessary parts

Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
---

 .../puma_rk3399/puma-rk3399.c                 | 108 +++---------------
 1 file changed, 18 insertions(+), 90 deletions(-)

diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
index 251cd2d566..fb6c7c1f0a 100644
--- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
+++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
@@ -17,6 +17,7 @@
 #include <asm/arch-rockchip/clock.h>
 #include <asm/arch-rockchip/hardware.h>
 #include <asm/arch-rockchip/grf_rk3399.h>
+#include <asm/arch-rockchip/misc.h>
 #include <asm/arch-rockchip/periph.h>
 #include <power/regulator.h>
 #include <u-boot/sha256.h>
@@ -36,94 +37,6 @@ int board_init(void)
 	return 0;
 }
 
-static void setup_macaddr(void)
-{
-#if CONFIG_IS_ENABLED(CMD_NET)
-	int ret;
-	const char *cpuid = env_get("cpuid#");
-	u8 hash[SHA256_SUM_LEN];
-	int size = sizeof(hash);
-	u8 mac_addr[6];
-
-	/* Only generate a MAC address, if none is set in the environment */
-	if (env_get("ethaddr"))
-		return;
-
-	if (!cpuid) {
-		debug("%s: could not retrieve 'cpuid#'\n", __func__);
-		return;
-	}
-
-	ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, &size);
-	if (ret) {
-		debug("%s: failed to calculate SHA256\n", __func__);
-		return;
-	}
-
-	/* Copy 6 bytes of the hash to base the MAC address on */
-	memcpy(mac_addr, hash, 6);
-
-	/* Make this a valid MAC address and set it */
-	mac_addr[0] &= 0xfe;  /* clear multicast bit */
-	mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
-	eth_env_set_enetaddr("ethaddr", mac_addr);
-#endif
-}
-
-static void setup_serial(void)
-{
-#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE)
-	const u32 cpuid_offset = 0x7;
-	const u32 cpuid_length = 0x10;
-
-	struct udevice *dev;
-	int ret, i;
-	u8 cpuid[cpuid_length];
-	u8 low[cpuid_length/2], high[cpuid_length/2];
-	char cpuid_str[cpuid_length * 2 + 1];
-	u64 serialno;
-	char serialno_str[17];
-
-	/* retrieve the device */
-	ret = uclass_get_device_by_driver(UCLASS_MISC,
-					  DM_GET_DRIVER(rockchip_efuse), &dev);
-	if (ret) {
-		debug("%s: could not find efuse device\n", __func__);
-		return;
-	}
-
-	/* read the cpu_id range from the efuses */
-	ret = misc_read(dev, cpuid_offset, &cpuid, sizeof(cpuid));
-	if (ret) {
-		debug("%s: reading cpuid from the efuses failed\n",
-		      __func__);
-		return;
-	}
-
-	memset(cpuid_str, 0, sizeof(cpuid_str));
-	for (i = 0; i < 16; i++)
-		sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]);
-
-	debug("cpuid: %s\n", cpuid_str);
-
-	/*
-	 * Mix the cpuid bytes using the same rules as in
-	 *   ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c
-	 */
-	for (i = 0; i < 8; i++) {
-		low[i] = cpuid[1 + (i << 1)];
-		high[i] = cpuid[i << 1];
-	}
-
-	serialno = crc32_no_comp(0, low, 8);
-	serialno |= (u64)crc32_no_comp(serialno, high, 8) << 32;
-	snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno);
-
-	env_set("cpuid#", cpuid_str);
-	env_set("serial#", serialno_str);
-#endif
-}
-
 static void setup_iodomain(void)
 {
 	const u32 GRF_IO_VSEL_GPIO4CD_SHIFT = 3;
@@ -213,8 +126,23 @@ static int setup_boottargets(void)
 
 int misc_init_r(void)
 {
-	setup_serial();
-	setup_macaddr();
+	const u32 cpuid_offset = 0x7;
+	const u32 cpuid_length = 0x10;
+	u8 cpuid[cpuid_length];
+	int ret;
+
+	ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
+	if (ret)
+		return ret;
+
+	ret = rockchip_cpuid_set(cpuid, cpuid_length);
+	if (ret)
+		return ret;
+
+	ret = rockchip_setup_macaddr();
+	if (ret)
+		return ret;
+
 	setup_iodomain();
 	setup_boottargets();
 
-- 
2.17.1

[U-Boot] [PATCH v3 3/3] board: puma: Use rockchip_* helpers to setup cpuid and macaddr【请注意，邮件由u-boot-bounces@lists.denx.de代发】 setup cpuid and macaddr

[Kever Yang]

Hi Rohan,

On 2019/7/24 下午7:09, Rohan Garg wrote:
> We should use the shared helpers to setup the necessary parts
>
> Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
> ---
>
>   .../puma_rk3399/puma-rk3399.c                 | 108 +++---------------
>   1 file changed, 18 insertions(+), 90 deletions(-)
>
> diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> index 251cd2d566..fb6c7c1f0a 100644
> --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> @@ -17,6 +17,7 @@
>   #include <asm/arch-rockchip/clock.h>
>   #include <asm/arch-rockchip/hardware.h>
>   #include <asm/arch-rockchip/grf_rk3399.h>
> +#include <asm/arch-rockchip/misc.h>
>   #include <asm/arch-rockchip/periph.h>
>   #include <power/regulator.h>
>   #include <u-boot/sha256.h>
> @@ -36,94 +37,6 @@ int board_init(void)
>   	return 0;
>   }
>   
> -static void setup_macaddr(void)
> -{
> -#if CONFIG_IS_ENABLED(CMD_NET)
> -	int ret;
> -	const char *cpuid = env_get("cpuid#");
> -	u8 hash[SHA256_SUM_LEN];
> -	int size = sizeof(hash);
> -	u8 mac_addr[6];
> -
> -	/* Only generate a MAC address, if none is set in the environment */
> -	if (env_get("ethaddr"))
> -		return;
> -
> -	if (!cpuid) {
> -		debug("%s: could not retrieve 'cpuid#'\n", __func__);
> -		return;
> -	}
> -
> -	ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, &size);
> -	if (ret) {
> -		debug("%s: failed to calculate SHA256\n", __func__);
> -		return;
> -	}
> -
> -	/* Copy 6 bytes of the hash to base the MAC address on */
> -	memcpy(mac_addr, hash, 6);
> -
> -	/* Make this a valid MAC address and set it */
> -	mac_addr[0] &= 0xfe;  /* clear multicast bit */
> -	mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
> -	eth_env_set_enetaddr("ethaddr", mac_addr);
> -#endif
> -}
> -
> -static void setup_serial(void)
> -{
> -#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE)
> -	const u32 cpuid_offset = 0x7;
> -	const u32 cpuid_length = 0x10;
> -
> -	struct udevice *dev;
> -	int ret, i;
> -	u8 cpuid[cpuid_length];
> -	u8 low[cpuid_length/2], high[cpuid_length/2];
> -	char cpuid_str[cpuid_length * 2 + 1];
> -	u64 serialno;
> -	char serialno_str[17];
> -
> -	/* retrieve the device */
> -	ret = uclass_get_device_by_driver(UCLASS_MISC,
> -					  DM_GET_DRIVER(rockchip_efuse), &dev);
> -	if (ret) {
> -		debug("%s: could not find efuse device\n", __func__);
> -		return;
> -	}
> -
> -	/* read the cpu_id range from the efuses */
> -	ret = misc_read(dev, cpuid_offset, &cpuid, sizeof(cpuid));
> -	if (ret) {
> -		debug("%s: reading cpuid from the efuses failed\n",
> -		      __func__);
> -		return;
> -	}
> -
> -	memset(cpuid_str, 0, sizeof(cpuid_str));
> -	for (i = 0; i < 16; i++)
> -		sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]);
> -
> -	debug("cpuid: %s\n", cpuid_str);
> -
> -	/*
> -	 * Mix the cpuid bytes using the same rules as in
> -	 *   ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c
> -	 */
> -	for (i = 0; i < 8; i++) {
> -		low[i] = cpuid[1 + (i << 1)];
> -		high[i] = cpuid[i << 1];
> -	}
> -
> -	serialno = crc32_no_comp(0, low, 8);
> -	serialno |= (u64)crc32_no_comp(serialno, high, 8) << 32;
> -	snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno);
> -
> -	env_set("cpuid#", cpuid_str);
> -	env_set("serial#", serialno_str);
> -#endif
> -}
> -
>   static void setup_iodomain(void)
>   {
>   	const u32 GRF_IO_VSEL_GPIO4CD_SHIFT = 3;
> @@ -213,8 +126,23 @@ static int setup_boottargets(void)
>   
>   int misc_init_r(void)


After misc_init_r has add into rk3399_board, then this misc_init_r() 
here will be multi-defined?

In this case, maybe we need to remove the misc_init_r() from 
rk3399_board.c first? :(


Thanks,

- Kever

>   {
> -	setup_serial();
> -	setup_macaddr();
> +	const u32 cpuid_offset = 0x7;
> +	const u32 cpuid_length = 0x10;
> +	u8 cpuid[cpuid_length];
> +	int ret;
> +
> +	ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
> +	if (ret)
> +		return ret;
> +
> +	ret = rockchip_cpuid_set(cpuid, cpuid_length);
> +	if (ret)
> +		return ret;
> +
> +	ret = rockchip_setup_macaddr();
> +	if (ret)
> +		return ret;
> +
>   	setup_iodomain();
>   	setup_boottargets();
>   

[U-Boot] [PATCH v3 3/3] board: puma: Use rockchip_* helpers to setup cpuid and macaddr【请注意，邮件由u-boot-bounces@lists.denx.de代发】 setup cpuid and macaddr

[Rohan Garg]

Hey
On Wednesday, 24 July 2019 14:26:44 CEST Kever Yang wrote:
> Hi Rohan,
> 
> On 2019/7/24 下午7:09, Rohan Garg wrote:
> > We should use the shared helpers to setup the necessary parts
> > 
> > Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
> > ---
> > 
> >   .../puma_rk3399/puma-rk3399.c                 | 108 +++---------------
> >   1 file changed, 18 insertions(+), 90 deletions(-)
> > 
> > diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > b/board/theobroma-systems/puma_rk3399/puma-rk3399.c index
> > 251cd2d566..fb6c7c1f0a 100644
> > --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > @@ -17,6 +17,7 @@
> > 
> >   #include <asm/arch-rockchip/clock.h>
> >   #include <asm/arch-rockchip/hardware.h>
> >   #include <asm/arch-rockchip/grf_rk3399.h>
> > 
> > +#include <asm/arch-rockchip/misc.h>
> > 
> >   #include <asm/arch-rockchip/periph.h>
> >   #include <power/regulator.h>
> >   #include <u-boot/sha256.h>
> > 
> > @@ -36,94 +37,6 @@ int board_init(void)
> > 
> >   	return 0;
> >   
> >   }
> > 
> > -static void setup_macaddr(void)
> > -{
> > -#if CONFIG_IS_ENABLED(CMD_NET)
> > -	int ret;
> > -	const char *cpuid = env_get("cpuid#");
> > -	u8 hash[SHA256_SUM_LEN];
> > -	int size = sizeof(hash);
> > -	u8 mac_addr[6];
> > -
> > -	/* Only generate a MAC address, if none is set in the environment 
*/
> > -	if (env_get("ethaddr"))
> > -		return;
> > -
> > -	if (!cpuid) {
> > -		debug("%s: could not retrieve 'cpuid#'\n", __func__);
> > -		return;
> > -	}
> > -
> > -	ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, 
&size);
> > -	if (ret) {
> > -		debug("%s: failed to calculate SHA256\n", __func__);
> > -		return;
> > -	}
> > -
> > -	/* Copy 6 bytes of the hash to base the MAC address on */
> > -	memcpy(mac_addr, hash, 6);
> > -
> > -	/* Make this a valid MAC address and set it */
> > -	mac_addr[0] &= 0xfe;  /* clear multicast bit */
> > -	mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
> > -	eth_env_set_enetaddr("ethaddr", mac_addr);
> > -#endif
> > -}
> > -
> > -static void setup_serial(void)
> > -{
> > -#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE)
> > -	const u32 cpuid_offset = 0x7;
> > -	const u32 cpuid_length = 0x10;
> > -
> > -	struct udevice *dev;
> > -	int ret, i;
> > -	u8 cpuid[cpuid_length];
> > -	u8 low[cpuid_length/2], high[cpuid_length/2];
> > -	char cpuid_str[cpuid_length * 2 + 1];
> > -	u64 serialno;
> > -	char serialno_str[17];
> > -
> > -	/* retrieve the device */
> > -	ret = uclass_get_device_by_driver(UCLASS_MISC,
> > -					  
DM_GET_DRIVER(rockchip_efuse), &dev);
> > -	if (ret) {
> > -		debug("%s: could not find efuse device\n", __func__);
> > -		return;
> > -	}
> > -
> > -	/* read the cpu_id range from the efuses */
> > -	ret = misc_read(dev, cpuid_offset, &cpuid, sizeof(cpuid));
> > -	if (ret) {
> > -		debug("%s: reading cpuid from the efuses failed\n",
> > -		      __func__);
> > -		return;
> > -	}
> > -
> > -	memset(cpuid_str, 0, sizeof(cpuid_str));
> > -	for (i = 0; i < 16; i++)
> > -		sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]);
> > -
> > -	debug("cpuid: %s\n", cpuid_str);
> > -
> > -	/*
> > -	 * Mix the cpuid bytes using the same rules as in
> > -	 *   ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c
> > -	 */
> > -	for (i = 0; i < 8; i++) {
> > -		low[i] = cpuid[1 + (i << 1)];
> > -		high[i] = cpuid[i << 1];
> > -	}
> > -
> > -	serialno = crc32_no_comp(0, low, 8);
> > -	serialno |= (u64)crc32_no_comp(serialno, high, 8) << 32;
> > -	snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno);
> > -
> > -	env_set("cpuid#", cpuid_str);
> > -	env_set("serial#", serialno_str);
> > -#endif
> > -}
> > -
> > 
> >   static void setup_iodomain(void)
> >   {
> >   
> >   	const u32 GRF_IO_VSEL_GPIO4CD_SHIFT = 3;
> > 
> > @@ -213,8 +126,23 @@ static int setup_boottargets(void)
> > 
> >   int misc_init_r(void)
> 
> After misc_init_r has add into rk3399_board, then this misc_init_r()
> here will be multi-defined?
> 
> In this case, maybe we need to remove the misc_init_r() from
> rk3399_board.c first? :(
> 

I'm a bit confused. I thought the idea was to hoist this code up into rk3399-
board.c?

On a separate note, perhaps setup_iodomain should move into a more generic 
helper for the rk3399 boards.

I'm also not sure why setup_boottargets fiddles around with boot order, but 
perhaps we shouldn't be doing that? None of the other boards seem to do that ( 
or at least none that I could find anyway ).

Cheers
Rohan Garg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190725/35d48f31/attachment.sig>

[U-Boot] [PATCH v3 3/3] board: puma: Use rockchip_* helpers to setup cpuid and macaddr【请注意，邮件由u-boot-bounces@lists.denx.de代发】 setup cpuid and macaddr

[Philipp Tomsich]

> On 25.07.2019, at 18:39, Rohan Garg <rohan.garg@collabora.com> wrote:
> 
> Hey
> On Wednesday, 24 July 2019 14:26:44 CEST Kever Yang wrote:
>> Hi Rohan,
>> 
>> On 2019/7/24 下午7:09, Rohan Garg wrote:
>>> We should use the shared helpers to setup the necessary parts
>>> 
>>> Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
>>> ---
>>> 
>>>  .../puma_rk3399/puma-rk3399.c                 | 108 +++---------------
>>>  1 file changed, 18 insertions(+), 90 deletions(-)
>>> 
>>> diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>>> b/board/theobroma-systems/puma_rk3399/puma-rk3399.c index
>>> 251cd2d566..fb6c7c1f0a 100644
>>> --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>>> +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>>> @@ -17,6 +17,7 @@
>>> 
>>>  #include <asm/arch-rockchip/clock.h>
>>>  #include <asm/arch-rockchip/hardware.h>
>>>  #include <asm/arch-rockchip/grf_rk3399.h>
>>> 
>>> +#include <asm/arch-rockchip/misc.h>
>>> 
>>>  #include <asm/arch-rockchip/periph.h>
>>>  #include <power/regulator.h>
>>>  #include <u-boot/sha256.h>
>>> 
>>> @@ -36,94 +37,6 @@ int board_init(void)
>>> 
>>>  	return 0;
>>> 
>>>  }
>>> 
>>> -static void setup_macaddr(void)
>>> -{
>>> -#if CONFIG_IS_ENABLED(CMD_NET)
>>> -	int ret;
>>> -	const char *cpuid = env_get("cpuid#");
>>> -	u8 hash[SHA256_SUM_LEN];
>>> -	int size = sizeof(hash);
>>> -	u8 mac_addr[6];
>>> -
>>> -	/* Only generate a MAC address, if none is set in the environment 
> */
>>> -	if (env_get("ethaddr"))
>>> -		return;
>>> -
>>> -	if (!cpuid) {
>>> -		debug("%s: could not retrieve 'cpuid#'\n", __func__);
>>> -		return;
>>> -	}
>>> -
>>> -	ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, 
> &size);
>>> -	if (ret) {
>>> -		debug("%s: failed to calculate SHA256\n", __func__);
>>> -		return;
>>> -	}
>>> -
>>> -	/* Copy 6 bytes of the hash to base the MAC address on */
>>> -	memcpy(mac_addr, hash, 6);
>>> -
>>> -	/* Make this a valid MAC address and set it */
>>> -	mac_addr[0] &= 0xfe;  /* clear multicast bit */
>>> -	mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
>>> -	eth_env_set_enetaddr("ethaddr", mac_addr);
>>> -#endif
>>> -}
>>> -
>>> -static void setup_serial(void)
>>> -{
>>> -#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE)
>>> -	const u32 cpuid_offset = 0x7;
>>> -	const u32 cpuid_length = 0x10;
>>> -
>>> -	struct udevice *dev;
>>> -	int ret, i;
>>> -	u8 cpuid[cpuid_length];
>>> -	u8 low[cpuid_length/2], high[cpuid_length/2];
>>> -	char cpuid_str[cpuid_length * 2 + 1];
>>> -	u64 serialno;
>>> -	char serialno_str[17];
>>> -
>>> -	/* retrieve the device */
>>> -	ret = uclass_get_device_by_driver(UCLASS_MISC,
>>> -					  
> DM_GET_DRIVER(rockchip_efuse), &dev);
>>> -	if (ret) {
>>> -		debug("%s: could not find efuse device\n", __func__);
>>> -		return;
>>> -	}
>>> -
>>> -	/* read the cpu_id range from the efuses */
>>> -	ret = misc_read(dev, cpuid_offset, &cpuid, sizeof(cpuid));
>>> -	if (ret) {
>>> -		debug("%s: reading cpuid from the efuses failed\n",
>>> -		      __func__);
>>> -		return;
>>> -	}
>>> -
>>> -	memset(cpuid_str, 0, sizeof(cpuid_str));
>>> -	for (i = 0; i < 16; i++)
>>> -		sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]);
>>> -
>>> -	debug("cpuid: %s\n", cpuid_str);
>>> -
>>> -	/*
>>> -	 * Mix the cpuid bytes using the same rules as in
>>> -	 *   ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c
>>> -	 */
>>> -	for (i = 0; i < 8; i++) {
>>> -		low[i] = cpuid[1 + (i << 1)];
>>> -		high[i] = cpuid[i << 1];
>>> -	}
>>> -
>>> -	serialno = crc32_no_comp(0, low, 8);
>>> -	serialno |= (u64)crc32_no_comp(serialno, high, 8) << 32;
>>> -	snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno);
>>> -
>>> -	env_set("cpuid#", cpuid_str);
>>> -	env_set("serial#", serialno_str);
>>> -#endif
>>> -}
>>> -
>>> 
>>>  static void setup_iodomain(void)
>>>  {
>>> 
>>>  	const u32 GRF_IO_VSEL_GPIO4CD_SHIFT = 3;
>>> 
>>> @@ -213,8 +126,23 @@ static int setup_boottargets(void)
>>> 
>>>  int misc_init_r(void)
>> 
>> After misc_init_r has add into rk3399_board, then this misc_init_r()
>> here will be multi-defined?
>> 
>> In this case, maybe we need to remove the misc_init_r() from
>> rk3399_board.c first? :(
>> 
> 
> I'm a bit confused. I thought the idea was to hoist this code up into rk3399-
> board.c?
> 
> On a separate note, perhaps setup_iodomain should move into a more generic 
> helper for the rk3399 boards.
> 
> I'm also not sure why setup_boottargets fiddles around with boot order, but 
> perhaps we shouldn't be doing that? None of the other boards seem to do that ( 
> or at least none that I could find anyway ).

Please refer to the comment above setup_boottargets() in 
	board/theobroma-systems/puma_rk3399/puma-rk3399.c
for details on why the bootorder is modified for distroboot.

Please make sure that this is left in place, as it will otherwise break the expected
and documented behaviour (i.e. how the “BIOS_DISABLE” signal is expected to
work) for the RK3399-Q7.

Thanks,
Philipp.

[U-Boot] [PATCH v3 3/3] board: puma: Use rockchip_* helpers to setup cpuid and macaddr【请注意，邮件由u-boot-bounces@lists.denx.de代发】 setup cpuid and macaddr

[Ezequiel Garcia]

Hi Kever, Rohan,

On Wed, 2019-07-24 at 20:26 +0800, Kever Yang wrote:
> Hi Rohan,
> 
> On 2019/7/24 下午7:09, Rohan Garg wrote:
> > We should use the shared helpers to setup the necessary parts
> > 
> > Signed-off-by: Rohan Garg <rohan.garg@collabora.com>
> > ---
> > 
> >   .../puma_rk3399/puma-rk3399.c                 | 108 +++---------------
> >   1 file changed, 18 insertions(+), 90 deletions(-)
> > 
> > diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > index 251cd2d566..fb6c7c1f0a 100644
> > --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> > @@ -17,6 +17,7 @@
> >   #include <asm/arch-rockchip/clock.h>
> >   #include <asm/arch-rockchip/hardware.h>
> >   #include <asm/arch-rockchip/grf_rk3399.h>
> > +#include <asm/arch-rockchip/misc.h>
> >   #include <asm/arch-rockchip/periph.h>
> >   #include <power/regulator.h>
> >   #include <u-boot/sha256.h>
> > @@ -36,94 +37,6 @@ int board_init(void)
> >   	return 0;
> >   }
> >   
> > -static void setup_macaddr(void)
> > -{
> > -#if CONFIG_IS_ENABLED(CMD_NET)
> > -	int ret;
> > -	const char *cpuid = env_get("cpuid#");
> > -	u8 hash[SHA256_SUM_LEN];
> > -	int size = sizeof(hash);
> > -	u8 mac_addr[6];
> > -
> > -	/* Only generate a MAC address, if none is set in the environment */
> > -	if (env_get("ethaddr"))
> > -		return;
> > -
> > -	if (!cpuid) {
> > -		debug("%s: could not retrieve 'cpuid#'\n", __func__);
> > -		return;
> > -	}
> > -
> > -	ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, &size);
> > -	if (ret) {
> > -		debug("%s: failed to calculate SHA256\n", __func__);
> > -		return;
> > -	}
> > -
> > -	/* Copy 6 bytes of the hash to base the MAC address on */
> > -	memcpy(mac_addr, hash, 6);
> > -
> > -	/* Make this a valid MAC address and set it */
> > -	mac_addr[0] &= 0xfe;  /* clear multicast bit */
> > -	mac_addr[0] |= 0x02;  /* set local assignment bit (IEEE802) */
> > -	eth_env_set_enetaddr("ethaddr", mac_addr);
> > -#endif
> > -}
> > -
> > -static void setup_serial(void)
> > -{
> > -#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE)
> > -	const u32 cpuid_offset = 0x7;
> > -	const u32 cpuid_length = 0x10;
> > -
> > -	struct udevice *dev;
> > -	int ret, i;
> > -	u8 cpuid[cpuid_length];
> > -	u8 low[cpuid_length/2], high[cpuid_length/2];
> > -	char cpuid_str[cpuid_length * 2 + 1];
> > -	u64 serialno;
> > -	char serialno_str[17];
> > -
> > -	/* retrieve the device */
> > -	ret = uclass_get_device_by_driver(UCLASS_MISC,
> > -					  DM_GET_DRIVER(rockchip_efuse), &dev);
> > -	if (ret) {
> > -		debug("%s: could not find efuse device\n", __func__);
> > -		return;
> > -	}
> > -
> > -	/* read the cpu_id range from the efuses */
> > -	ret = misc_read(dev, cpuid_offset, &cpuid, sizeof(cpuid));
> > -	if (ret) {
> > -		debug("%s: reading cpuid from the efuses failed\n",
> > -		      __func__);
> > -		return;
> > -	}
> > -
> > -	memset(cpuid_str, 0, sizeof(cpuid_str));
> > -	for (i = 0; i < 16; i++)
> > -		sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]);
> > -
> > -	debug("cpuid: %s\n", cpuid_str);
> > -
> > -	/*
> > -	 * Mix the cpuid bytes using the same rules as in
> > -	 *   ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c
> > -	 */
> > -	for (i = 0; i < 8; i++) {
> > -		low[i] = cpuid[1 + (i << 1)];
> > -		high[i] = cpuid[i << 1];
> > -	}
> > -
> > -	serialno = crc32_no_comp(0, low, 8);
> > -	serialno |= (u64)crc32_no_comp(serialno, high, 8) << 32;
> > -	snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno);
> > -
> > -	env_set("cpuid#", cpuid_str);
> > -	env_set("serial#", serialno_str);
> > -#endif
> > -}
> > -
> >   static void setup_iodomain(void)
> >   {
> >   	const u32 GRF_IO_VSEL_GPIO4CD_SHIFT = 3;
> > @@ -213,8 +126,23 @@ static int setup_boottargets(void)
> >   
> >   int misc_init_r(void)
> 
> After misc_init_r has add into rk3399_board, then this misc_init_r() 
> here will be multi-defined?
> 
> In this case, maybe we need to remove the misc_init_r() from 
> rk3399_board.c first? :(
> 
> 

Good catch. Indeed, with the first patch applied the build would fail.

Does this mean we should let each board introduce
its own misc_init_r, or instead provide a common weak misc_init_r?

Thanks,
Ezequiel