[PATCH 1/2] rockchip: misc: Only assign serial# variable if unset

The serial# environment variable is a read-only special variable, that can only be set once. As a result, if the environment was saved to a persistent storage location, attempting to set it again in rockchip_cpuid_set will fail and halt the boot with the following error:
Solve this by checking whether the variable is already set before.
Signed-off-by: Paul Kocialkowski paul.kocialkowski@bootlin.com --- arch/arm/mach-rockchip/misc.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c index bed4317f7ece..a0c6a1c0b266 100644 --- a/arch/arm/mach-rockchip/misc.c +++ b/arch/arm/mach-rockchip/misc.c @@ -108,12 +108,16 @@ int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length) 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); + + if (!env_get("serial#")) { + 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("serial#", serialno_str); + }
return 0; }

rockchip_setup_macaddr will return -1 if ethaddr is already set, which gets propagated to misc_init_r and eventually halts the boot process.
While checking that the variable is not already set before attempting to setit is legitimate (it's a set-once variable), this is no good reason to halt the boot process.
Return the success return code if the variable is already set instead.
Signed-off-by: Paul Kocialkowski paul.kocialkowski@bootlin.com --- arch/arm/mach-rockchip/misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c index a0c6a1c0b266..bce10bb04f8f 100644 --- a/arch/arm/mach-rockchip/misc.c +++ b/arch/arm/mach-rockchip/misc.c @@ -29,7 +29,7 @@ int rockchip_setup_macaddr(void)
/* Only generate a MAC address, if none is set in the environment */ if (env_get("ethaddr")) - return -1; + return 0;
if (!cpuid) { debug("%s: could not retrieve 'cpuid#'\n", __func__);

On 03.12.19 16:46, Paul Kocialkowski wrote:
rockchip_setup_macaddr will return -1 if ethaddr is already set, which gets propagated to misc_init_r and eventually halts the boot process.
While checking that the variable is not already set before attempting to setit is legitimate (it's a set-once variable), this is no good reason to halt the boot process.
Return the success return code if the variable is already set instead.
Signed-off-by: Paul Kocialkowski paul.kocialkowski@bootlin.com
Same as "rockchip: misc: don't fail if eth_addr already set" from november 29 ;-)
arch/arm/mach-rockchip/misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c index a0c6a1c0b266..bce10bb04f8f 100644 --- a/arch/arm/mach-rockchip/misc.c +++ b/arch/arm/mach-rockchip/misc.c @@ -29,7 +29,7 @@ int rockchip_setup_macaddr(void)
/* Only generate a MAC address, if none is set in the environment */ if (env_get("ethaddr"))
return -1;
return 0;
if (!cpuid) { debug("%s: could not retrieve 'cpuid#'\n", __func__);

Hi Paul,
On 03.12.19 16:46, Paul Kocialkowski wrote:
The serial# environment variable is a read-only special variable, that can only be set once. As a result, if the environment was saved to a persistent storage location, attempting to set it again in rockchip_cpuid_set will fail and halt the boot with the following error:
Solve this by checking whether the variable is already set before.
Signed-off-by: Paul Kocialkowski paul.kocialkowski@bootlin.com
it looks like we're fixing the same problems a lot. Yesterday I also got a patch from Miquel about the px30 iommu clocks ;-) .
Here it's the same as "rockchip: misc: protect serial# from getting overwritten" from november 29 ;-)
Heiko
arch/arm/mach-rockchip/misc.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c index bed4317f7ece..a0c6a1c0b266 100644 --- a/arch/arm/mach-rockchip/misc.c +++ b/arch/arm/mach-rockchip/misc.c @@ -108,12 +108,16 @@ int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length) 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);
if (!env_get("serial#")) {
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("serial#", serialno_str);
}
return 0; }

Hi Heiko,
On Tue 03 Dec 19, 20:50, Heiko Stuebner wrote:
Hi Paul,
On 03.12.19 16:46, Paul Kocialkowski wrote:
The serial# environment variable is a read-only special variable, that can only be set once. As a result, if the environment was saved to a persistent storage location, attempting to set it again in rockchip_cpuid_set will fail and halt the boot with the following error:
Solve this by checking whether the variable is already set before.
Signed-off-by: Paul Kocialkowski paul.kocialkowski@bootlin.com
it looks like we're fixing the same problems a lot. Yesterday I also got a patch from Miquel about the px30 iommu clocks ;-) .
Here it's the same as "rockchip: misc: protect serial# from getting overwritten" from november 29 ;-)
Hehe good catch! We'll try to keep better track of the list before sending fixes next time ;)
Cheers,
Paul
Heiko
arch/arm/mach-rockchip/misc.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c index bed4317f7ece..a0c6a1c0b266 100644 --- a/arch/arm/mach-rockchip/misc.c +++ b/arch/arm/mach-rockchip/misc.c @@ -108,12 +108,16 @@ int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length) 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);
- if (!env_get("serial#")) {
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("serial#", serialno_str);
- } return 0; }
participants (2)
-
Heiko Stuebner
-
Paul Kocialkowski