[U-Boot] [PATCH] misc: fs_loader: Replace label with DT phandle

[Chee, Tien Fong]

On Fri, 2019-02-15 at 14:35 +0800, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee at intel.com>
> 
> In previously label which will be expanded to the node's full path
> was
> used, and now replacing label with most commonly used DT phandle. The
> codes were changed accordingly to the use of DT phandle and
> supporting
> multiple instances.
> 
> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
> ---
>  doc/driver-model/fs_firmware_loader.txt |   58
> +++++++++++++++++++++++++------
>  drivers/misc/fs_loader.c                |   36 ++++++++-----------
>  2 files changed, 62 insertions(+), 32 deletions(-)
> 
> diff --git a/doc/driver-model/fs_firmware_loader.txt b/doc/driver-
> model/fs_firmware_loader.txt
> index b9aee84..d9f966e 100644
> --- a/doc/driver-model/fs_firmware_loader.txt
> +++ b/doc/driver-model/fs_firmware_loader.txt
Any comment?

> @@ -1,4 +1,4 @@
> -# Copyright (C) 2018 Intel Corporation <www.intel.com>
> +# Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
>  #
>  # SPDX-License-Identifier:    GPL-2.0
>  
> @@ -46,15 +46,48 @@ Firmware storage device described in device tree
> source
>  		ubivol = "ubi0";
>  	};
>  
> -	Then, firmware_loader property would be set with the path of
> fs_loader
> -	node under /chosen node such as:
> +	Then, firmware-loader property can be added with any device
> node, which
> +	driver would use the firmware loader for loading.
> +
> +	The value of the firmware-loader property should be set with
> phandle
> +	of the fs-loader node.
> +	For example:
> +		firmware-loader = <&fs_loader0>;
> +
> +	If there are majority of devices using the same fs-loader
> node, then
> +	firmware-loader property can be added under /chosen node
> instead of
> +	adding to each of device node.
> +
> +	For example:
>  	/{
>  		chosen {
> -			firmware_loader = &fs_loader0;
> +			firmware-loader = <&fs_loader0>;
>  		};
>  	};
>  
> -	However, this driver is also designed to support U-boot
> environment
> +	In each respective driver of devices using firmware loader,
> the firmware
> +	loaded instance	should be created by DT phandle.
> +
> +	For example of getting DT phandle from /chosen and creating
> instance:
> +	chosen_node = ofnode_path("/chosen");
> +	if (!ofnode_valid(chosen_node)) {
> +		debug("/chosen node was not found.\n");
> +		return -ENOENT;
> +	}
> +
> +	phandle_p = ofnode_get_property(chosen_node, "firmware-
> loader", &size);
> +	if (!phandle_p) {
> +		debug("firmware-loader property was not found.\n");
> +		return -ENOENT;
> +	}
> +
> +	phandle = fdt32_to_cpu(*phandle_p);
> +	ret =
> uclass_get_device_by_phandle_id(UCLASS_FS_FIRMWARE_LOADER,
> +					     phandle, &dev);
> +	if (ret)
> +		return ret;
> +
> +	Firmware loader driver is also designed to support U-boot
> environment
>  	variables, so all these data from FDT can be overwritten
>  	through the U-boot environment variable during run time.
>  	For examples:
> @@ -104,9 +137,12 @@ return:
>  Description:
>  	The firmware is loaded directly into the buffer pointed to
> by buf
>  
> -Example of creating firmware loader instance and calling
> -request_firmware_into_buf API:
> -	if (uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0, &dev)) {
> -		request_firmware_into_buf(dev, filename,
> buffer_location,
> -					 buffer_size,
> offset_ofreading);
> -	}
> +Example of calling request_firmware_into_buf API after creating
> firmware loader
> +instance:
> +	ret =
> uclass_get_device_by_phandle_id(UCLASS_FS_FIRMWARE_LOADER,
> +					     phandle, &dev);
> +	if (ret)
> +		return ret;
> +
> +	request_firmware_into_buf(dev, filename, buffer_location,
> buffer_size,
> +				 offset_ofreading);
> diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
> index a2e3763..f42eeff 100644
> --- a/drivers/misc/fs_loader.c
> +++ b/drivers/misc/fs_loader.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>   /*
> - * Copyright (C) 2018 Intel Corporation <www.intel.com>
> + * Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
>   *
>   */
>  #include <common.h>
> @@ -219,32 +219,26 @@ int request_firmware_into_buf(struct udevice
> *dev,
>  
>  static int fs_loader_ofdata_to_platdata(struct udevice *dev)
>  {
> -	const char *fs_loader_path;
>  	u32 phandlepart[2];
>  
> -	fs_loader_path = ofnode_get_chosen_prop("firmware-loader");
> +	ofnode fs_loader_node = dev_ofnode(dev);
>  
> -	if (fs_loader_path) {
> -		ofnode fs_loader_node;
> +	if (ofnode_valid(fs_loader_node)) {
> +		struct device_platdata *plat;
>  
> -		fs_loader_node = ofnode_path(fs_loader_path);
> -		if (ofnode_valid(fs_loader_node)) {
> -			struct device_platdata *plat;
> -			plat = dev->platdata;
> -
> -			if (!ofnode_read_u32_array(fs_loader_node,
> -						  "phandlepart",
> -						  phandlepart, 2)) {
> -				plat->phandlepart.phandle =
> phandlepart[0];
> -				plat->phandlepart.partition =
> phandlepart[1];
> -			}
> +		plat = dev->platdata;
> +		if (!ofnode_read_u32_array(fs_loader_node,
> +					  "phandlepart",
> +					  phandlepart, 2)) {
> +			plat->phandlepart.phandle = phandlepart[0];
> +			plat->phandlepart.partition =
> phandlepart[1];
> +		}
>  
> -			plat->mtdpart = (char *)ofnode_read_string(
> -					 fs_loader_node, "mtdpart");
> +		plat->mtdpart = (char *)ofnode_read_string(
> +				 fs_loader_node, "mtdpart");
>  
> -			plat->ubivol = (char *)ofnode_read_string(
> -					 fs_loader_node, "ubivol");
> -		}
> +		plat->ubivol = (char *)ofnode_read_string(
> +				 fs_loader_node, "ubivol");
>  	}
>  
>  	return 0;

Thanks,
TF