[U-Boot] [PATCH v3] PXE: FDT: Add support for fdt in PXE

[Jason Hobbs]

Chander,

Comments inline.

On Thu, Sep 06, 2012 at 01:40:04AM -0400, Chander Kashyap wrote:
> Now DT support is becomming common for all new SoC's. Hence it is better to
> have option for getting specific FDT from the remote server.
> 
> This patch adds support for new lable i.e. fdt. If fdt_addr is specified
> then load fdt blob from the remote server to fdt_address.

If a fdt label is provided AND fdt_addr is specified, then load the blob
from the remote server to fdt_addr. fdt_addr alone still works on its
own if the fdt_blob has already been loaded some other way

> 
> Signed-off-by: Chander Kashyap <chander.kashyap at linaro.org>
> ---
> Changes in v2:
> 	- Removed the duplicate code.
> changes in v3:
> 	- Added documentation for "fdt" lable in doc/README.pxe 

s/lable/label - fix globally

> 
>  common/cmd_pxe.c |   23 +++++++++++++++++++++++
>  doc/README.pxe   |   10 ++++++++--
>  2 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
> index 6b31dea..0c81e08 100644
> --- a/common/cmd_pxe.c
> +++ b/common/cmd_pxe.c
> @@ -450,6 +450,7 @@ struct pxe_label {
>  	char *kernel;
>  	char *append;
>  	char *initrd;
> +	char *fdt;
>  	int attempted;
>  	int localboot;
>  	struct list_head list;
> @@ -517,6 +518,9 @@ static void label_destroy(struct pxe_label *label)
>  	if (label->initrd)
>  		free(label->initrd);
>  
> +	if (label->fdt)
> +		free(label->fdt);
> +
>  	free(label);
>  }
>  
> @@ -541,6 +545,9 @@ static void label_print(void *data)
>  
>  	if (label->initrd)
>  		printf("\t\tinitrd: %s\n", label->initrd);
> +
> +	if (label->fdt)
> +		printf("\tfdt: %s\n", label->fdt);
>  }
>  
>  /*
> @@ -633,6 +640,15 @@ static void label_boot(struct pxe_label *label)
>  	 */
>  	bootm_argv[3] = getenv("fdt_addr");
>  
> +	/* if fdt label is defined then get fdt from server */
> +	if (bootm_argv[3] && label->fdt) {
> +		if (get_relfile_envaddr(label->fdt, "fdt_addr") < 0) {
> +			printf("Skipping %s for failure retrieving fdt\n",
> +					label->name);
> +			return;
> +		}
> +	}
> +
>  	if (bootm_argv[3])
>  		bootm_argc = 4;
>  
> @@ -658,6 +674,7 @@ enum token_type {
>  	T_DEFAULT,
>  	T_PROMPT,
>  	T_INCLUDE,
> +	T_FDT,
>  	T_INVALID
>  };
>  
> @@ -685,6 +702,7 @@ static const struct token keywords[] = {
>  	{"append", T_APPEND},
>  	{"initrd", T_INITRD},
>  	{"include", T_INCLUDE},
> +	{"fdt", T_FDT},
>  	{NULL, T_INVALID}
>  };
>  
> @@ -1074,6 +1092,11 @@ static int parse_label(char **c, struct pxe_menu *cfg)
>  				err = parse_sliteral(c, &label->initrd);
>  			break;
>  
> +		case T_FDT:
> +			if (!label->fdt)
> +				err = parse_sliteral(c, &label->fdt);
> +			break;
> +
>  		case T_LOCALBOOT:
>  			err = parse_integer(c, &label->localboot);
>  			break;
> diff --git a/doc/README.pxe b/doc/README.pxe
> index 2bbf53d..835ca5a 100644
> --- a/doc/README.pxe
> +++ b/doc/README.pxe
> @@ -93,8 +93,9 @@ pxe boot
>       be passed to the bootm command to boot the kernel. These environment
>       variables are required to be set.
>  
> -     fdt_addr - the location of a fdt blob. If this is set, it will be passed
> -     to bootm when booting a kernel.
> +     fdt_addr - locations in RAM at which 'pxe boot' will store the fdt blob
> +     it retrieves from tftp, if "fdt" lable is defined in pxe file. If this is
> +     set, it will be passed to bootm when booting a kernel.

Thinking about this some more, I don't think you can use fdt_addr as the
place to store the blob. fdt_addr isn't garaunteed to be writeable RAM -
it could be a read only non volatile memory like NOR flash.

If you notice, all of the other tftp retrievals go to "_r" suffixed
variables, which are garaunteed (by convention) to be in writeable RAM.

You may need to take an approach where an addition fdt_addr_r variable
is used to point at the location to store the fdt blob.

Your description also makes it less clear that if fdt_addr is set, it
points to the blob, whether or not it was retrieved from tftp.

>  
>  pxe file format
>  ===============
> @@ -156,6 +157,11 @@ initrd <path>	    - if this label is chosen, use tftp to retrieve the initrd
>  		      the initrd_addr_r environment variable, and that address
>  		      will be passed to bootm.
>  
> +fdt <path>	    - if this label is chosen, use tftp to retrieve the fdt blob
> +		      at <path>. it will be stored at the address indicated in
> +		      the fdt_addr environment variable, and that address will
> +		      be passed to bootm.
> +

again, this should be changed to fdt_addr_r.


>  localboot <flag>    - Run the command defined by "localcmd" in the environment.
>  		      <flag> is ignored and is only here to match the syntax of
>  		      PXELINUX config files.
> -- 
> 1.7.9.5
>