[PATCH] cmd: add temperature command

[Simon Glass]

Hi Robert,

On Fri, 12 Aug 2022 at 06:19, Robert Marko <robert.marko at sartura.hr> wrote:
>
> Currently, there is no way for users to check the readings from thermal
> sensors from U-boot console, only some boards print it during boot.
>
> So, lets add a simple "temperature" command that allows listing thermal
> uclass devices and getting their value.
>
> Note that the thermal devices are intenionally probed if list is used as
> almost always they will not get probed otherwise and there is no way for
> users to manually call probe on a certain device from console.
>
> Assumption is made that temperature is returned in degrees C and not
> milidegrees like in Linux as this is what most drivers seem to return.
>
> Signed-off-by: Robert Marko <robert.marko at sartura.hr>
> ---
>  cmd/Kconfig       |  6 ++++
>  cmd/Makefile      |  1 +
>  cmd/temperature.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+)
>  create mode 100644 cmd/temperature.c

Don't forget to add doc/usage/command/.. and a sandbox test in test/cmd/

https://u-boot.readthedocs.io/en/latest/develop/testing.html

>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 3625ff2a50..9bd639c740 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1435,6 +1435,12 @@ config DEFAULT_SPI_MODE
>         depends on CMD_SPI
>         default 0
>
> +config CMD_TEMPERATURE
> +       bool "temperature"

How about:

temperature - display the temperature from thermal sensors

> +       depends on DM_THERMAL
> +       help
> +         Provides a way to get the temperature reading from thermal sensors.

It also allows listing.

> +
>  config CMD_TSI148
>         bool "tsi148 - Command to access tsi148 device"
>         help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 5e43a1e022..8874462f1a 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -152,6 +152,7 @@ obj-$(CONFIG_CMD_STRINGS) += strings.o
>  obj-$(CONFIG_CMD_SMC) += smccc.o
>  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o
>  obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
> +obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o
>  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
>  obj-$(CONFIG_CMD_TIME) += time.o
>  obj-$(CONFIG_CMD_TIMER) += timer.o
> diff --git a/cmd/temperature.c b/cmd/temperature.c
> new file mode 100644
> index 0000000000..ccf839058e
> --- /dev/null
> +++ b/cmd/temperature.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/*
> + * Copyright (c) 2022 Sartura Ltd.
> + * Written by Robert Marko <robert.marko at sartura.hr>
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <dm.h>
> +#include <dm/uclass-internal.h>

I hope you can drop this

> +#include <thermal.h>
> +
> +#define LIMIT_DEVNAME  30
> +
> +static int do_get(struct cmd_tbl *cmdtp, int flag, int argc,
> +                 char *const argv[])
> +{
> +       struct udevice *dev;
> +       int ret, temp;
> +
> +       if (argc < 2) {
> +               printf("thermal device not selected\n");
> +               return CMD_RET_FAILURE;
> +       }
> +
> +       ret = uclass_find_device_by_name(UCLASS_THERMAL, argv[1], &dev);

You should use the get function normally, since it probes the device
and for most devices you should only call their methods when the
device is probed/activated.

> +       if (ret) {
> +               printf("thermal device not found\n");
> +               return CMD_RET_FAILURE;
> +       }
> +
> +       ret = thermal_get_temp(dev, &temp);
> +       if (ret)
> +               return CMD_RET_FAILURE;
> +
> +       printf("%s: %d°C\n", dev->name, temp);
> +
> +       return CMD_RET_SUCCESS;
> +}
> +
> +static int do_list(struct cmd_tbl *cmdtp, int flag, int argc,
> +                  char *const argv[])
> +{
> +       struct udevice *dev;
> +
> +       printf("| %-*.*s| %-*.*s| %s\n",
> +              LIMIT_DEVNAME, LIMIT_DEVNAME, "Device",
> +              LIMIT_DEVNAME, LIMIT_DEVNAME, "Driver",
> +              "Parent");
> +
> +       uclass_foreach_dev_probe(UCLASS_THERMAL, dev) {
> +               printf("| %-*.*s| %-*.*s| %s\n",
> +                      LIMIT_DEVNAME, LIMIT_DEVNAME, dev->name,
> +                      LIMIT_DEVNAME, LIMIT_DEVNAME, dev->driver->name,
> +                      dev->parent->name);
> +       }

BTW as I'm sure you know, this loop probes the device, which is fine.

> +
> +       return CMD_RET_SUCCESS;
> +}
> +
> +static struct cmd_tbl temperature_subcmd[] = {
> +       U_BOOT_CMD_MKENT(list, 1, 1, do_list, "", ""),
> +       U_BOOT_CMD_MKENT(get, 2, 1, do_get, "", ""),
> +};
> +
> +static int do_temperature(struct cmd_tbl *cmdtp, int flag, int argc,
> +                         char *const argv[])
> +{
> +       struct cmd_tbl *cmd;
> +
> +       argc--;
> +       argv++;
> +
> +       cmd = find_cmd_tbl(argv[0], temperature_subcmd, ARRAY_SIZE(temperature_subcmd));
> +       if (!cmd || argc > cmd->maxargs)
> +               return CMD_RET_USAGE;
> +
> +       return cmd->cmd(cmdtp, flag, argc, argv);
> +}
> +
> +U_BOOT_CMD(temperature, CONFIG_SYS_MAXARGS, 1, do_temperature,
> +          "thermal sensor temperature",
> +          "list\t\tshow list of temperature sensors\n"
> +          "get [thermal device name]\tprint temperature"

in degrees C ?

> +);
> --
> 2.37.1
>

Regards,
Simon