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

2019-03-19 Thread Chee, Tien Fong
On Tue, 2019-03-19 at 09:22 +0800, Simon Glass wrote:
> Hi Tien Fong,
> 
> On Mon, 11 Mar 2019 at 12:28, Chee, Tien Fong  om> wrote:
> > 
> > 
> > On Sun, 2019-03-10 at 15:51 -0600, Simon Glass wrote:
> > > 
> > > Hi Tien Fong,
> > > 
> > > On Tue, 26 Feb 2019 at 05:37, Chee, Tien Fong  > > el.c
> > > om> wrote:
> > > > 
> > > > 
> > > > 
> > > > On Fri, 2019-02-15 at 14:35 +0800, tien.fong.c...@intel.com
> > > > wrote:
> > > > > 
> > > > > 
> > > > > From: Tien Fong Chee 
> > > > > 
> > > > > 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 
> > > > > ---
> > > > >  doc/driver-model/fs_firmware_loader.txt |   58
> > > > > +--
> > > > >  drivers/misc/fs_loader.c|   36 -
> > > > > 
> > > > > --
> > > > >  2 files changed, 62 insertions(+), 32 deletions(-)
> > > This seems OK to me, but I think this feature needs a test.
> > Yes, i have ran the test passed with FPGA driver.
> No I mean that you should add a test in test/py/tests
Okay, noted.
> 
> Regards,
> Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-03-18 Thread Simon Glass
Hi Tien Fong,

On Mon, 11 Mar 2019 at 12:28, Chee, Tien Fong  wrote:
>
> On Sun, 2019-03-10 at 15:51 -0600, Simon Glass wrote:
> > Hi Tien Fong,
> >
> > On Tue, 26 Feb 2019 at 05:37, Chee, Tien Fong  > om> wrote:
> > >
> > >
> > > On Fri, 2019-02-15 at 14:35 +0800, tien.fong.c...@intel.com wrote:
> > > >
> > > > From: Tien Fong Chee 
> > > >
> > > > 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 
> > > > ---
> > > >  doc/driver-model/fs_firmware_loader.txt |   58
> > > > +--
> > > >  drivers/misc/fs_loader.c|   36 -
> > > > --
> > > >  2 files changed, 62 insertions(+), 32 deletions(-)
> > This seems OK to me, but I think this feature needs a test.
> Yes, i have ran the test passed with FPGA driver.

No I mean that you should add a test in test/py/tests

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-03-10 Thread Chee, Tien Fong
On Sun, 2019-03-10 at 15:51 -0600, Simon Glass wrote:
> Hi Tien Fong,
> 
> On Tue, 26 Feb 2019 at 05:37, Chee, Tien Fong  om> wrote:
> > 
> > 
> > On Fri, 2019-02-15 at 14:35 +0800, tien.fong.c...@intel.com wrote:
> > > 
> > > From: Tien Fong Chee 
> > > 
> > > 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 
> > > ---
> > >  doc/driver-model/fs_firmware_loader.txt |   58
> > > +--
> > >  drivers/misc/fs_loader.c|   36 -
> > > --
> > >  2 files changed, 62 insertions(+), 32 deletions(-)
> This seems OK to me, but I think this feature needs a test.
Yes, i have ran the test passed with FPGA driver.
> 
> Regards,
> Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-03-10 Thread Simon Glass
Hi Tien Fong,

On Tue, 26 Feb 2019 at 05:37, Chee, Tien Fong  wrote:
>
> On Fri, 2019-02-15 at 14:35 +0800, tien.fong.c...@intel.com wrote:
> > From: Tien Fong Chee 
> >
> > 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 
> > ---
> >  doc/driver-model/fs_firmware_loader.txt |   58
> > +--
> >  drivers/misc/fs_loader.c|   36 ---
> >  2 files changed, 62 insertions(+), 32 deletions(-)

This seems OK to me, but I think this feature needs a test.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-02-26 Thread Chee, Tien Fong
On Fri, 2019-02-15 at 14:35 +0800, tien.fong.c...@intel.com wrote:
> From: Tien Fong Chee 
> 
> 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 
> ---
>  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 
> +# Copyright (C) 2018-2019 Intel Corporation 
>  #
>  # 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 = <_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 = _loader0;
> + firmware-loader = <_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", );
> + 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, );
> + 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, )) {
> - 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, );
> + 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 
> + * Copyright (C) 2018-2019 Intel Corporation 
>   *
>   */
>  #include 
> @@ -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