| Message ID | 20240408223134.165877-3-j-humphreys@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | EFI: ti: Enable EFI capsule updates | expand |
On 4/9/24 00:31, Jonathan Humphreys wrote: > Define the firmwares updatable via EFI capsule update. Nits: %s/firmwares/firmware/ (firmware is uncountable). > > Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> > --- > board/ti/am64x/evm.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c > index b8de69da06c..876c5c25d42 100644 > --- a/board/ti/am64x/evm.c > +++ b/board/ti/am64x/evm.c > @@ -7,6 +7,7 @@ > * > */ > > +#include <efi_loader.h> > #include <asm/io.h> > #include <dm/uclass.h> > #include <k3-ddrss.h> > @@ -27,6 +28,38 @@ > > DECLARE_GLOBAL_DATA_PTR; > > +#if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) > +struct efi_fw_image fw_images[] = { > + { > + .image_type_id = K3_TIBOOT3_IMAGE_GUID, > + .fw_name = u"K3_TIBOOT3", > + .image_index = 1, > + }, > + { > + .image_type_id = K3_SPL_IMAGE_GUID, > + .fw_name = u"K3_SPL", > + .image_index = 2, > + }, > + { > + .image_type_id = K3_UBOOT_IMAGE_GUID, > + .fw_name = u"K3_UBOOT", > + .image_index = 3, > + } > +}; > + > +struct efi_capsule_update_info update_info = { > + .dfu_string = "sf 0:0=tiboot3.bin raw 0 100000;tispl.bin raw 100000 200000;u-boot.img raw 300000 400000", Is TI using other addresses than Phytec? doc/board/phytec/phycore-am64x.rst: fatload mmc 1 ${loadaddr} tiboot3.bin sf update $loadaddr 0x0 $filesize fatload mmc 1 ${loadaddr} tispl.bin sf update $loadaddr 0x80000 $filesize fatload mmc 1 ${loadaddr} u-boot.img sf update $loadaddr 0x280000 $filesize Please, add documentation in doc/board/ for all boards in the series specifying the location of the firmware in flash memory and how to update it(cf. section OSPI in doc/board/ti/am65x_evm.rst). > + .num_images = ARRAY_SIZE(fw_images), > + .images = fw_images, > +}; The series has been assigned to me in Patchwork. But changes should be tested and reviewed by the respective board maintainers (Vignesh, Tom for AM64x) as I have no access to TI boards. Best regards Heinrich > + > +void set_dfu_alt_info(char *interface, char *devstr) > +{ > + env_set("dfu_alt_info", update_info.dfu_string); > +} > + > +#endif /* EFI_HAVE_CAPSULE_SUPPORT */ > + > int board_init(void) > { > return 0;
Heinrich Schuchardt <xypron.glpk@gmx.de> writes: > On 4/9/24 00:31, Jonathan Humphreys wrote: >> Define the firmwares updatable via EFI capsule update. > > Nits: > > %s/firmwares/firmware/ (firmware is uncountable). > >> >> Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> >> --- >> board/ti/am64x/evm.c | 33 +++++++++++++++++++++++++++++++++ >> 1 file changed, 33 insertions(+) >> >> diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c >> index b8de69da06c..876c5c25d42 100644 >> --- a/board/ti/am64x/evm.c >> +++ b/board/ti/am64x/evm.c >> @@ -7,6 +7,7 @@ >> * >> */ >> >> +#include <efi_loader.h> >> #include <asm/io.h> >> #include <dm/uclass.h> >> #include <k3-ddrss.h> >> @@ -27,6 +28,38 @@ >> >> DECLARE_GLOBAL_DATA_PTR; >> >> +#if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) >> +struct efi_fw_image fw_images[] = { >> + { >> + .image_type_id = K3_TIBOOT3_IMAGE_GUID, >> + .fw_name = u"K3_TIBOOT3", >> + .image_index = 1, >> + }, >> + { >> + .image_type_id = K3_SPL_IMAGE_GUID, >> + .fw_name = u"K3_SPL", >> + .image_index = 2, >> + }, >> + { >> + .image_type_id = K3_UBOOT_IMAGE_GUID, >> + .fw_name = u"K3_UBOOT", >> + .image_index = 3, >> + } >> +}; >> + >> +struct efi_capsule_update_info update_info = { >> + .dfu_string = "sf 0:0=tiboot3.bin raw 0 100000;tispl.bin raw 100000 200000;u-boot.img raw 300000 400000", > > Is TI using other addresses than Phytec? > > doc/board/phytec/phycore-am64x.rst: > > fatload mmc 1 ${loadaddr} tiboot3.bin > sf update $loadaddr 0x0 $filesize > fatload mmc 1 ${loadaddr} tispl.bin > sf update $loadaddr 0x80000 $filesize > fatload mmc 1 ${loadaddr} u-boot.img > sf update $loadaddr 0x280000 $filesize > This patch has the correct offsets for am64x, so I guess phycore defined the offsets differently. > Please, add documentation in doc/board/ for all boards in the series > specifying the location of the firmware in flash memory and how to > update it(cf. section OSPI in doc/board/ti/am65x_evm.rst). > Yes, I will add patches to document OSPI layouts where missing. >> + .num_images = ARRAY_SIZE(fw_images), >> + .images = fw_images, >> +}; > > The series has been assigned to me in Patchwork. > > But changes should be tested and reviewed by the respective board > maintainers (Vignesh, Tom for AM64x) as I have no access to TI boards. > > Best regards > > Heinrich > >> + >> +void set_dfu_alt_info(char *interface, char *devstr) >> +{ >> + env_set("dfu_alt_info", update_info.dfu_string); >> +} >> + >> +#endif /* EFI_HAVE_CAPSULE_SUPPORT */ >> + >> int board_init(void) >> { >> return 0;
On Tue, Apr 09, 2024 at 06:31:02AM +0200, Heinrich Schuchardt wrote: > On 4/9/24 00:31, Jonathan Humphreys wrote: [snip] > The series has been assigned to me in Patchwork. > > But changes should be tested and reviewed by the respective board > maintainers (Vignesh, Tom for AM64x) as I have no access to TI boards. I'll pick it up once you're happy with the capsule side of things if you like, but in my mind it's more important to emphasize that this is what capsule support looks like as it rolls out to more platforms, is everyone happy with this?
Hi Tom From a capsule update POV the patch looks correct Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> On Wed, 10 Apr 2024 at 02:44, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Apr 09, 2024 at 06:31:02AM +0200, Heinrich Schuchardt wrote: > > On 4/9/24 00:31, Jonathan Humphreys wrote: > [snip] > > The series has been assigned to me in Patchwork. > > > > But changes should be tested and reviewed by the respective board > > maintainers (Vignesh, Tom for AM64x) as I have no access to TI boards. > > I'll pick it up once you're happy with the capsule side of things if you > like, but in my mind it's more important to emphasize that this is what > capsule support looks like as it rolls out to more platforms, is > everyone happy with this? > > -- > Tom
diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c index b8de69da06c..876c5c25d42 100644 --- a/board/ti/am64x/evm.c +++ b/board/ti/am64x/evm.c @@ -7,6 +7,7 @@ * */ +#include <efi_loader.h> #include <asm/io.h> #include <dm/uclass.h> #include <k3-ddrss.h> @@ -27,6 +28,38 @@ DECLARE_GLOBAL_DATA_PTR; +#if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) +struct efi_fw_image fw_images[] = { + { + .image_type_id = K3_TIBOOT3_IMAGE_GUID, + .fw_name = u"K3_TIBOOT3", + .image_index = 1, + }, + { + .image_type_id = K3_SPL_IMAGE_GUID, + .fw_name = u"K3_SPL", + .image_index = 2, + }, + { + .image_type_id = K3_UBOOT_IMAGE_GUID, + .fw_name = u"K3_UBOOT", + .image_index = 3, + } +}; + +struct efi_capsule_update_info update_info = { + .dfu_string = "sf 0:0=tiboot3.bin raw 0 100000;tispl.bin raw 100000 200000;u-boot.img raw 300000 400000", + .num_images = ARRAY_SIZE(fw_images), + .images = fw_images, +}; + +void set_dfu_alt_info(char *interface, char *devstr) +{ + env_set("dfu_alt_info", update_info.dfu_string); +} + +#endif /* EFI_HAVE_CAPSULE_SUPPORT */ + int board_init(void) { return 0;
Define the firmwares updatable via EFI capsule update. Signed-off-by: Jonathan Humphreys <j-humphreys@ti.com> --- board/ti/am64x/evm.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)