[U-Boot] [PATCH] sunxi: Serial number support, obtained from SID bits and passed through ATAG

[Hans de Goede]

Hi,

Thanks for the patch, I've 1 comment inline, please send
a version with that fixed then I'll queue it up for merging upstream.

On 18-03-15 20:46, Paul Kocialkowski wrote:
> Signed-off-by: Paul Kocialkowski <contact at paulk.fr>
> ---
>   board/sunxi/board.c            | 33 ++++++++++++++++++++++++++++++---
>   include/configs/sunxi-common.h |  1 +
>   2 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index becdc8b..0355de5 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -248,13 +248,34 @@ int g_dnl_board_usb_cable_connected(void)
>   }
>   #endif
>
> +#ifdef CONFIG_SERIAL_TAG
> +void get_board_serial(struct tag_serialnr *serialnr)
> +{
> +	char *serial_string = getenv("serial#");
> +	unsigned long long serial;
> +
> +	if (serial_string) {
> +		serial = simple_strtoull(serial_string, NULL, 16);
> +
> +		serialnr->high = (unsigned int) (serial >> 32);
> +		serialnr->low = (unsigned int) (serial & 0xffffffff);
> +	} else {
> +		serialnr->high = 0;
> +		serialnr->low = 0;
> +	}
> +}
> +#endif
> +
>   #ifdef CONFIG_MISC_INIT_R
>   int misc_init_r(void)
>   {
> -	unsigned int sid[4];
> +	char serial_string[17] = { 0 };
> +	unsigned int sid[4] = { 0 };
> +	int ret;
> +
> +	ret = sunxi_get_sid(sid);
>
> -	if (!getenv("ethaddr") && sunxi_get_sid(sid) == 0 &&
> -			sid[0] != 0 && sid[3] != 0) {
> +	if (!getenv("ethaddr") && ret == 0 && sid[0] != 0 && sid[3] != 0) {
>   		uint8_t mac_addr[6];
>
>   		mac_addr[0] = 0x02; /* Non OUI / registered MAC address */
> @@ -267,6 +288,12 @@ int misc_init_r(void)
>   		eth_setenv_enetaddr("ethaddr", mac_addr);
>   	}
>

You should check for get_sid succeeding before calling snprintf, also since
your buffer is always large enough, there is no need to use or error check
snprintf, and last you should check for serial# not already being set so that
the user can override it if he wants.

> +	ret = snprintf(serial_string, sizeof(serial_string), "%08x%08x",
> +		sid[0], sid[3]);
> +
> +	if (ret > 0)
> +		setenv("serial#", serial_string);
> +

IOW, you should replace the above with:

	if (!getenv("serial#") && ret == 0) {
		sprintf(serial_string, "%08x%08x", sid[0], sid[3]);
		setenv("serial#", serial_string);
	}

>   #if defined(CONFIG_MUSB_HOST) || defined(CONFIG_MUSB_GADGET)
>   	musb_register(&musb_plat, NULL, (void *)SUNXI_USB0_BASE);
>   #endif
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index ffd9f5c..61a45e1 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -98,6 +98,7 @@
>   #define CONFIG_SETUP_MEMORY_TAGS
>   #define CONFIG_CMDLINE_TAG
>   #define CONFIG_INITRD_TAG
> +#define CONFIG_SERIAL_TAG
>
>   /* mmc config */
>   #if !defined(CONFIG_UART0_PORT_F)
>

Otherwise this looks good.

Regards,

Hans


p.s.

A quick grep on serial# shows the following:
board/gateworks/gw_ventana/gw_ventana.c
1202: *   serial# env var
1207:   char *serial = getenv("serial#");
1432:           setenv("serial#", str);
1512:   fdt_setprop(blob, 0, "system-serial", getenv("serial#"),
1513:               strlen(getenv("serial#")) + 1);

Which may be used as an example for devicetree support, or not,
the first thing to do would be to write a

Documentation/devicetree/bindings/system-serial.txt

patch for the kernel and send that to the devicetree list for
review, once that is accepted we can actually start implementing
it.

Regards,

Hans