On Thu, Feb 04, 2021 at 11:08:33AM -0500, Tom Rini wrote:
On Thu, Feb 04, 2021 at 08:26:52AM -0500, Tom Rini wrote:
On Thu, Feb 04, 2021 at 06:56:47AM -0300, Fabio Estevam wrote:
Hi Tom,
[Adding Bryan]
On Thu, Feb 4, 2021 at 12:49 AM Tom Rini trini@konsulko.com wrote:
CONFIG_SERIAL_TAG is only used to configure the ATAG serial record. It does nothing else. On i.MX8* this is already being discarded. On i.MX7 we see things like: warp7 : all -144 rodata +32 text -176 u-boot: add: 0/-2, grow: 0/-2 bytes: 0/-160 (-160) function old new delta params 4 - -4 get_board_serial 20 - -20 board_late_init 96 56 -40 boot_prep_linux 156 60 -96
But then it will remove the support of reading serial number via fuses which was added by:
commit 852cc548b3fdf6d5b46e2a96f876d14608ccdcf4 Author: Bryan O'Donoghue bryan.odonoghue@linaro.org Date: Mon Mar 26 15:27:34 2018 +0100
warp7: Set u-boot serial# based on OTP value u-boot has a standard "serial#" environment variable that is suitable for storing the iSerial number we will supply via the USB device descriptor. serial# is automatically picked up by the disk subsystem in u-boot - thus providing a handy unique identifier in /dev/disk/by-id as detailed below. Storing the hardware serial identifier in serial# means we can change the serial# if we want before USB enumeration - thus making iSerial automatic via OTP but overridable if necessary. This patch reads the defined OTP fuse and sets environment variable "serial#" to the value read. With this patch in place the USB mass storage device will appear in /dev/disk/by-id with a unique name based on the OTP value. For example /dev/disk/by-id/usb-Linux_UMS_disk_0_WaRP7-0xf42400d3000001d4-0:0 Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Cc: Fabio Estevam <fabio.estevam@nxp.com> Cc: Rui Miguel Silva <rui.silva@linaro.org> Cc: Ryan Harkin <ryan.harkin@linaro.org> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>I did miss that one. I'm not sure if that's abusing a symbol or clever use of the symbol. I will have to v2 this one then.
This should end up under CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG someway or another. I'm going to make an attempt at it, but the biggest hurdle is that most often "serial#" is just the fuse(s) value and not prepended by a board-specific string (OR if it is, it's just in the fdt property we read and set from).
So, I see 3 options here: 1. In arch/arm/mach-imx/mx7/soc.c and board/warp7/warp7.c switch from CONFIG_SERIAL_TAG to CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG (and migrate from warp7.h to defconfig to enable). This won't break existing users / documentation where the device showing up by serial number WITH "WaRP7" in the name is expected. The serial ATAG is defined regardless of CONFIG_SERIAL_TAG so it'll still be available, and we're not likely to remove ATAG support globally for some time yet. 2. Adjust arch/arm/mach-imx/mx7/soc.c to follow the common practice of arch_misc_init() calling the function to read the fuse(s) and set "serial#" to match that, without other identifiers. This would enable the feature for all imx7 boards but change the value on new/re-initialized WaRP7 boards and probably break documentation. 3. Do nothing. WaRP7 works as-is, using CONFIG_SERIAL_TAG is something I think was a bit too clever, but I can also see why it was used since after-all it's using the serial tag record, etc.
Option 3 is my least preferred choice, but, if the relevant custodians like that the most, that's fine with me.