| Message ID | 1550212526-30510-1-git-send-email-tien.fong.chee@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] misc: fs_loader: Replace label with DT phandle | expand |
On Fri, 2019-02-15 at 14:35 +0800, tien.fong.chee@intel.com wrote: > From: Tien Fong Chee <tien.fong.chee@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@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
Hi Tien Fong, On Tue, 26 Feb 2019 at 05:37, Chee, Tien Fong <tien.fong.chee@intel.com> wrote: > > On Fri, 2019-02-15 at 14:35 +0800, tien.fong.chee@intel.com wrote: > > From: Tien Fong Chee <tien.fong.chee@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@intel.com> > > --- > > 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
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 <tien.fong.chee@intel.c > om> wrote: > > > > > > On Fri, 2019-02-15 at 14:35 +0800, tien.fong.chee@intel.com wrote: > > > > > > From: Tien Fong Chee <tien.fong.chee@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@intel.com> > > > --- > > > 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
Hi Tien Fong, On Mon, 11 Mar 2019 at 12:28, Chee, Tien Fong <tien.fong.chee@intel.com> 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 <tien.fong.chee@intel.c > > om> wrote: > > > > > > > > > On Fri, 2019-02-15 at 14:35 +0800, tien.fong.chee@intel.com wrote: > > > > > > > > From: Tien Fong Chee <tien.fong.chee@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@intel.com> > > > > --- > > > > 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
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 <tien.fong.chee@intel.c > 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 <tien.fong.chee@int > > > el.c > > > om> wrote: > > > > > > > > > > > > > > > > On Fri, 2019-02-15 at 14:35 +0800, tien.fong.chee@intel.com > > > > wrote: > > > > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@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@intel.com> > > > > > --- > > > > > 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
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 @@ -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;