Re: [PATCH v5 2/6] cmd: Add a sysinfo command

2023-10-16 Thread Simon Glass
Hi Detlev,

On Mon, 16 Oct 2023 at 08:21, Detlev Casanova
 wrote:
>
> On Monday, October 2, 2023 2:56:28 P.M. EDT Simon Glass wrote:
> > On Mon, 2 Oct 2023 at 09:21, Detlev Casanova
> >
> >  wrote:
> > > The command is able to show different information for the running
> > > system:
> > > * Model name
> > > * Board ID
> > > * Revision
> > >
> > > This command can be used by boot shell scripts to select configurations
> > > depending on the specific running system.
> > >
> > > Reviewed-by: Marek Vasut 
> > > Signed-off-by: Detlev Casanova 
> > > ---
> > >
> > >  cmd/Kconfig   |   6 +++
> > >  cmd/Makefile  |   1 +
> > >  cmd/sysinfo.c | 133 ++
> > >  3 files changed, 140 insertions(+)
> > >  create mode 100644 cmd/sysinfo.c
> >
> > Reviewed-by: Simon Glass 
> >
> > Would prefer more help text in the Kconfig as this is pretty vague
>
> Indeed, how would that sound ?
>
> Display information to identify the system. It supports showing a board id, a
> revision number as well as a human readable name.

Also it uses the sysinfo driver for your board to get the information.
You could link to the sysinfo docs or header file.

Regards,
Simon


Re: [PATCH v5 2/6] cmd: Add a sysinfo command

2023-10-16 Thread Detlev Casanova
On Monday, October 2, 2023 2:56:28 P.M. EDT Simon Glass wrote:
> On Mon, 2 Oct 2023 at 09:21, Detlev Casanova
> 
>  wrote:
> > The command is able to show different information for the running
> > system:
> > * Model name
> > * Board ID
> > * Revision
> > 
> > This command can be used by boot shell scripts to select configurations
> > depending on the specific running system.
> > 
> > Reviewed-by: Marek Vasut 
> > Signed-off-by: Detlev Casanova 
> > ---
> > 
> >  cmd/Kconfig   |   6 +++
> >  cmd/Makefile  |   1 +
> >  cmd/sysinfo.c | 133 ++
> >  3 files changed, 140 insertions(+)
> >  create mode 100644 cmd/sysinfo.c
> 
> Reviewed-by: Simon Glass 
> 
> Would prefer more help text in the Kconfig as this is pretty vague

Indeed, how would that sound ?

Display information to identify the system. It supports showing a board id, a 
revision number as well as a human readable name.

> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 43ca10f69cc..67d019aa795 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -218,6 +218,12 @@ config CMD_SBI
> > 
> > help
> > 
> >   Display information about the SBI implementation.
> > 
> > +config CMD_SYSINFO
> > +   bool "sysinfo"
> > +   depends on SYSINFO
> > +   help
> > + Display information about the system.
> > +
> > 
> >  endmenu
> >  
> >  menu "Boot commands"
> > 
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index 9bebf321c39..972f3a4720e 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -167,6 +167,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
> > 
> >  obj-$(CONFIG_CMD_STRINGS) += strings.o
> >  obj-$(CONFIG_CMD_SMC) += smccc.o
> >  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o
> > 
> > +obj-$(CONFIG_CMD_SYSINFO) += sysinfo.o
> > 
> >  obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
> >  obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o
> >  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
> > 
> > diff --git a/cmd/sysinfo.c b/cmd/sysinfo.c
> > new file mode 100644
> > index 000..46369ff9ac7
> > --- /dev/null
> > +++ b/cmd/sysinfo.c
> > @@ -0,0 +1,133 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2023
> > + * Detlev Casanova 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +static int get_sysinfo(struct udevice **devp)
> > +{
> > +   int ret = sysinfo_get(devp);
> > +
> > +   if (ret) {
> > +   printf("Cannot get sysinfo: %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   ret = sysinfo_detect(*devp);
> > +   if (ret) {
> > +   printf("Cannot detect sysinfo: %d\n", ret);
> > +   return ret;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +static int do_sysinfo_model(struct cmd_tbl *cmdtp, int flag, int argc,
> > +   char *const argv[])
> > +{
> > +   struct udevice *dev;
> > +   char model[64];
> > +   int ret = get_sysinfo();
> > +
> > +   if (ret)
> > +   return CMD_RET_FAILURE;
> > +
> > +   ret = sysinfo_get_str(dev,
> > + SYSINFO_ID_BOARD_MODEL,
> > + sizeof(model),
> > + model);
> > +   if (ret) {
> > +   printf("Cannot get sysinfo str: %d\n", ret);
> > +   return CMD_RET_FAILURE;
> > +   }
> > +
> > +   if (argc == 2)
> > +   ret = env_set(argv[1], model);
> > +   else
> > +   printf("%s\n", model);
> > +
> > +   if (ret)
> > +   return CMD_RET_FAILURE;
> > +   else
> > +   return CMD_RET_SUCCESS;
> > +}
> > +
> > +static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc,
> > +char *const argv[])
> > +{
> > +   struct udevice *dev;
> > +   u32 board_id;
> > +   int ret = get_sysinfo();
> > +
> > +   if (ret)
> > +   return CMD_RET_FAILURE;
> > +
> > +   ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_ID, _id);
> > +   if (ret) {
> > +   printf("Cannot get sysinfo int: %d\n", ret);
> > +   return CMD_RET_FAILURE;
> > +   }
> > +
> > +   if (argc == 2)
> > +   ret = env_set_hex(argv[1], board_id);
> > +   else
> > +   printf("0x%02x\n", board_id);
> > +
> > +   if (ret)
> > +   return CMD_RET_FAILURE;
> > +   else
> > +   return CMD_RET_SUCCESS;
> > +}
> > +
> > +static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc,
> > +  char *const argv[])
> > +{
> > +   struct udevice *dev;
> > +   int rev_major;
> > +   int rev_minor;
> > +   char rev[64];
> > +   int ret = get_sysinfo();
> > +
> > +   if (ret)
> > +   return CMD_RET_FAILURE;
> > +
> > +   ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MAJOR,
> > _major);
> > +   if (ret) {
> > +

Re: [PATCH v5 2/6] cmd: Add a sysinfo command

2023-10-02 Thread Simon Glass
On Mon, 2 Oct 2023 at 09:21, Detlev Casanova
 wrote:
>
> The command is able to show different information for the running
> system:
> * Model name
> * Board ID
> * Revision
>
> This command can be used by boot shell scripts to select configurations
> depending on the specific running system.
>
> Reviewed-by: Marek Vasut 
> Signed-off-by: Detlev Casanova 
> ---
>  cmd/Kconfig   |   6 +++
>  cmd/Makefile  |   1 +
>  cmd/sysinfo.c | 133 ++
>  3 files changed, 140 insertions(+)
>  create mode 100644 cmd/sysinfo.c

Reviewed-by: Simon Glass 

Would prefer more help text in the Kconfig as this is pretty vague


>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 43ca10f69cc..67d019aa795 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -218,6 +218,12 @@ config CMD_SBI
> help
>   Display information about the SBI implementation.
>
> +config CMD_SYSINFO
> +   bool "sysinfo"
> +   depends on SYSINFO
> +   help
> + Display information about the system.
> +
>  endmenu
>
>  menu "Boot commands"
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 9bebf321c39..972f3a4720e 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -167,6 +167,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
>  obj-$(CONFIG_CMD_STRINGS) += strings.o
>  obj-$(CONFIG_CMD_SMC) += smccc.o
>  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o
> +obj-$(CONFIG_CMD_SYSINFO) += sysinfo.o
>  obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
>  obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o
>  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
> diff --git a/cmd/sysinfo.c b/cmd/sysinfo.c
> new file mode 100644
> index 000..46369ff9ac7
> --- /dev/null
> +++ b/cmd/sysinfo.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2023
> + * Detlev Casanova 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static int get_sysinfo(struct udevice **devp)
> +{
> +   int ret = sysinfo_get(devp);
> +
> +   if (ret) {
> +   printf("Cannot get sysinfo: %d\n", ret);
> +   return ret;
> +   }
> +
> +   ret = sysinfo_detect(*devp);
> +   if (ret) {
> +   printf("Cannot detect sysinfo: %d\n", ret);
> +   return ret;
> +   }
> +
> +   return 0;
> +}
> +
> +static int do_sysinfo_model(struct cmd_tbl *cmdtp, int flag, int argc,
> +   char *const argv[])
> +{
> +   struct udevice *dev;
> +   char model[64];
> +   int ret = get_sysinfo();
> +
> +   if (ret)
> +   return CMD_RET_FAILURE;
> +
> +   ret = sysinfo_get_str(dev,
> + SYSINFO_ID_BOARD_MODEL,
> + sizeof(model),
> + model);
> +   if (ret) {
> +   printf("Cannot get sysinfo str: %d\n", ret);
> +   return CMD_RET_FAILURE;
> +   }
> +
> +   if (argc == 2)
> +   ret = env_set(argv[1], model);
> +   else
> +   printf("%s\n", model);
> +
> +   if (ret)
> +   return CMD_RET_FAILURE;
> +   else
> +   return CMD_RET_SUCCESS;
> +}
> +
> +static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc,
> +char *const argv[])
> +{
> +   struct udevice *dev;
> +   u32 board_id;
> +   int ret = get_sysinfo();
> +
> +   if (ret)
> +   return CMD_RET_FAILURE;
> +
> +   ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_ID, _id);
> +   if (ret) {
> +   printf("Cannot get sysinfo int: %d\n", ret);
> +   return CMD_RET_FAILURE;
> +   }
> +
> +   if (argc == 2)
> +   ret = env_set_hex(argv[1], board_id);
> +   else
> +   printf("0x%02x\n", board_id);
> +
> +   if (ret)
> +   return CMD_RET_FAILURE;
> +   else
> +   return CMD_RET_SUCCESS;
> +}
> +
> +static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc,
> +  char *const argv[])
> +{
> +   struct udevice *dev;
> +   int rev_major;
> +   int rev_minor;
> +   char rev[64];
> +   int ret = get_sysinfo();
> +
> +   if (ret)
> +   return CMD_RET_FAILURE;
> +
> +   ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MAJOR, _major);
> +   if (ret) {
> +   printf("Cannot get sysinfo int: %d\n", ret);
> +   return CMD_RET_FAILURE;
> +   }
> +
> +   ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MINOR, _minor);
> +   if (ret) {
> +   printf("Cannot get sysinfo int: %d\n", ret);
> +   return CMD_RET_FAILURE;
> +   }
> +
> +   snprintf(rev, sizeof(rev), "%d.%d", rev_major, rev_minor);
> +
> +   if (argc == 2)
> +   ret = env_set(argv[1], rev);
> +   else
> +   printf("%s\n", rev);
> +
> +   if (ret)
> +   return CMD_RET_FAILURE;
> +