| Message ID | 20220825164245.1606958-4-quic_jaehyoo@quicinc.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | cmd/fru: move FRU handling support to common region | expand |
On 8/25/22 18:42, Jae Hyun Yoo wrote: > This command doesn't work with sandbox because direct memory access > causes a segfault error. Fix it up using map_sysmem(). > > Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > Changes from v3: > * None. > > Changes from v2: > * Added a 'Reviewed-by' tag. (Simon) > > Changes from v1: > * Newly added in v2. > > board/xilinx/common/board.c | 2 +- > cmd/fru.c | 19 ++++++++++++++++--- > include/fru.h | 4 ++-- > lib/fru_ops.c | 17 ++++++++--------- > 4 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c > index e58b11d7f757..3292083c5250 100644 > --- a/board/xilinx/common/board.c > +++ b/board/xilinx/common/board.c > @@ -241,7 +241,7 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name, > goto end; > } > > - fru_capture((unsigned long)fru_content); > + fru_capture(fru_content); > if (gd->flags & GD_FLG_RELOC || (_DEBUG && CONFIG_IS_ENABLED(DTB_RESELECT))) { > printf("Xilinx I2C FRU format at %s:\n", name); > ret = fru_display(0); > diff --git a/cmd/fru.c b/cmd/fru.c > index 2ec5012af5ac..b2cadbec9780 100644 > --- a/cmd/fru.c > +++ b/cmd/fru.c > @@ -9,12 +9,15 @@ > #include <fdtdec.h> > #include <fru.h> > #include <malloc.h> > +#include <mapmem.h> > > static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > { > unsigned long addr; > + const void *buf; > char *endp; > + int ret; > > if (argc < cmdtp->maxargs) > return CMD_RET_USAGE; > @@ -23,7 +26,11 @@ static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc, > if (*argv[1] == 0 || *endp != 0) > return -1; > > - return fru_capture(addr); > + buf = map_sysmem(addr, 0); > + ret = fru_capture(buf); > + unmap_sysmem(buf); > + > + return ret; > } > > static int do_fru_display(struct cmd_tbl *cmdtp, int flag, int argc, > @@ -37,13 +44,19 @@ static int do_fru_generate(struct cmd_tbl *cmdtp, int flag, int argc, > char *const argv[]) > { > unsigned long addr; > + const void *buf; > + int ret; > > if (argc < cmdtp->maxargs) > return CMD_RET_USAGE; > > - addr = hextoul(argv[2], NULL); > + addr = hextoul(argv[3], NULL); What's the reason for changing arguments here? Wasn't origin version correct? Thanks, Michal
Hello Michal, On 9/21/2022 6:07 AM, Michal Simek wrote: > > > On 8/25/22 18:42, Jae Hyun Yoo wrote: >> This command doesn't work with sandbox because direct memory access >> causes a segfault error. Fix it up using map_sysmem(). >> >> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> --- >> Changes from v3: >> * None. >> >> Changes from v2: >> * Added a 'Reviewed-by' tag. (Simon) >> >> Changes from v1: >> * Newly added in v2. >> >> board/xilinx/common/board.c | 2 +- >> cmd/fru.c | 19 ++++++++++++++++--- >> include/fru.h | 4 ++-- >> lib/fru_ops.c | 17 ++++++++--------- >> 4 files changed, 27 insertions(+), 15 deletions(-) >> >> diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c >> index e58b11d7f757..3292083c5250 100644 >> --- a/board/xilinx/common/board.c >> +++ b/board/xilinx/common/board.c >> @@ -241,7 +241,7 @@ static int xilinx_read_eeprom_fru(struct udevice >> *dev, char *name, >> goto end; >> } >> - fru_capture((unsigned long)fru_content); >> + fru_capture(fru_content); >> if (gd->flags & GD_FLG_RELOC || (_DEBUG && >> CONFIG_IS_ENABLED(DTB_RESELECT))) { >> printf("Xilinx I2C FRU format at %s:\n", name); >> ret = fru_display(0); >> diff --git a/cmd/fru.c b/cmd/fru.c >> index 2ec5012af5ac..b2cadbec9780 100644 >> --- a/cmd/fru.c >> +++ b/cmd/fru.c >> @@ -9,12 +9,15 @@ >> #include <fdtdec.h> >> #include <fru.h> >> #include <malloc.h> >> +#include <mapmem.h> >> static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc, >> char *const argv[]) >> { >> unsigned long addr; >> + const void *buf; >> char *endp; >> + int ret; >> if (argc < cmdtp->maxargs) >> return CMD_RET_USAGE; >> @@ -23,7 +26,11 @@ static int do_fru_capture(struct cmd_tbl *cmdtp, >> int flag, int argc, >> if (*argv[1] == 0 || *endp != 0) >> return -1; >> - return fru_capture(addr); >> + buf = map_sysmem(addr, 0); >> + ret = fru_capture(buf); >> + unmap_sysmem(buf); >> + >> + return ret; >> } >> static int do_fru_display(struct cmd_tbl *cmdtp, int flag, int argc, >> @@ -37,13 +44,19 @@ static int do_fru_generate(struct cmd_tbl *cmdtp, >> int flag, int argc, >> char *const argv[]) >> { >> unsigned long addr; >> + const void *buf; >> + int ret; >> if (argc < cmdtp->maxargs) >> return CMD_RET_USAGE; >> - addr = hextoul(argv[2], NULL); >> + addr = hextoul(argv[3], NULL); > > > What's the reason for changing arguments here? Wasn't origin version > correct? The 'board_gen' command is replaced with 'generate' command by this series and the 'generate' command has one more preceding argument to check '-b' and '-p' for board and product respectively. So actually this change is part of the 4th patch of this series. I'll move it to the patch in the next version. Thanks for your pointing it out. Jae
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c index e58b11d7f757..3292083c5250 100644 --- a/board/xilinx/common/board.c +++ b/board/xilinx/common/board.c @@ -241,7 +241,7 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name, goto end; } - fru_capture((unsigned long)fru_content); + fru_capture(fru_content); if (gd->flags & GD_FLG_RELOC || (_DEBUG && CONFIG_IS_ENABLED(DTB_RESELECT))) { printf("Xilinx I2C FRU format at %s:\n", name); ret = fru_display(0); diff --git a/cmd/fru.c b/cmd/fru.c index 2ec5012af5ac..b2cadbec9780 100644 --- a/cmd/fru.c +++ b/cmd/fru.c @@ -9,12 +9,15 @@ #include <fdtdec.h> #include <fru.h> #include <malloc.h> +#include <mapmem.h> static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { unsigned long addr; + const void *buf; char *endp; + int ret; if (argc < cmdtp->maxargs) return CMD_RET_USAGE; @@ -23,7 +26,11 @@ static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc, if (*argv[1] == 0 || *endp != 0) return -1; - return fru_capture(addr); + buf = map_sysmem(addr, 0); + ret = fru_capture(buf); + unmap_sysmem(buf); + + return ret; } static int do_fru_display(struct cmd_tbl *cmdtp, int flag, int argc, @@ -37,13 +44,19 @@ static int do_fru_generate(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { unsigned long addr; + const void *buf; + int ret; if (argc < cmdtp->maxargs) return CMD_RET_USAGE; - addr = hextoul(argv[2], NULL); + addr = hextoul(argv[3], NULL); + + buf = map_sysmem(addr, 0); + ret = fru_generate(buf, argc - 3, argv + 3); + unmap_sysmem(buf); - return fru_generate(addr, argc - 3, argv + 3); + return ret; } static struct cmd_tbl cmd_fru_sub[] = { diff --git a/include/fru.h b/include/fru.h index 41655864dbf5..b158b80b5866 100644 --- a/include/fru.h +++ b/include/fru.h @@ -107,8 +107,8 @@ struct fru_table { #define FRU_TYPELEN_TYPE_ASCII8 3 int fru_display(int verbose); -int fru_capture(unsigned long addr); -int fru_generate(unsigned long addr, int argc, char *const argv[]); +int fru_capture(const void *addr); +int fru_generate(const void *addr, int argc, char *const argv[]); u8 fru_checksum(u8 *addr, u8 len); int fru_check_type_len(u8 type_len, u8 language, u8 *type); const struct fru_table *fru_get_fru_data(void); diff --git a/lib/fru_ops.c b/lib/fru_ops.c index 3fe06fe430f2..a25ebccd110d 100644 --- a/lib/fru_ops.c +++ b/lib/fru_ops.c @@ -91,7 +91,7 @@ static u8 fru_gen_type_len(u8 *addr, char *name) return 1 + len; } -int fru_generate(unsigned long addr, int argc, char *const argv[]) +int fru_generate(const void *addr, int argc, char *const argv[]) { struct fru_common_hdr *header = (struct fru_common_hdr *)addr; struct fru_board_info_header *board_info; @@ -155,8 +155,8 @@ int fru_generate(unsigned long addr, int argc, char *const argv[]) debug("checksum %x(addr %x)\n", *member, len); - env_set_hex("fru_addr", addr); - env_set_hex("filesize", (unsigned long)member - addr + 1); + env_set_hex("fru_addr", (ulong)addr); + env_set_hex("filesize", (ulong)member - (ulong)addr + 1); return 0; } @@ -171,7 +171,7 @@ static void fru_delete_custom_fields(struct list_head *custom_fields) } } -static int fru_append_custom_info(unsigned long addr, +static int fru_append_custom_info(const void *addr, struct list_head *custom_fields) { struct fru_custom_info *info = (struct fru_custom_info *)addr; @@ -190,7 +190,7 @@ static int fru_append_custom_info(unsigned long addr, return 0; } -static int fru_parse_board(unsigned long addr) +static int fru_parse_board(const void *addr) { u8 i, type; int len; @@ -268,8 +268,7 @@ static void fru_delete_multirecs(struct list_head *multi_recs) } } -static int fru_append_multirec(unsigned long addr, - struct list_head *multi_recs) +static int fru_append_multirec(const void *addr, struct list_head *multi_recs) { struct fru_multirec_info *mr_src = (struct fru_multirec_info *)addr; struct fru_multirec_node *mr_new; @@ -286,7 +285,7 @@ static int fru_append_multirec(unsigned long addr, return 0; } -static int fru_parse_multirec(unsigned long addr) +static int fru_parse_multirec(const void *addr) { u8 hdr_len = sizeof(struct fru_multirec_hdr); struct fru_multirec_hdr *mr_hdr; @@ -308,7 +307,7 @@ static int fru_parse_multirec(unsigned long addr) return 0; } -int fru_capture(unsigned long addr) +int fru_capture(const void *addr) { struct fru_common_hdr *hdr; u8 checksum = 0;