[Uboot-stm32] [PATCH 1/2] cmd: pinmux: update result of do_status
Simon Glass
sjg at chromium.org
Thu Apr 29 18:10:08 CEST 2021
Hi Patrick,
On Tue, 20 Apr 2021 at 03:21, Patrice CHOTARD <patrice.chotard at st.com> wrote:
>
> Hi Patrick
>
> -----Original Message-----
> From: Uboot-stm32 <uboot-stm32-bounces at st-md-mailman.stormreply.com> On Behalf Of Patrick DELAUNAY
> Sent: mercredi 28 octobre 2020 11:07
> To: u-boot at lists.denx.de
> Cc: U-Boot STM32 <uboot-stm32 at st-md-mailman.stormreply.com>; Simon Glass <sjg at chromium.org>; Patrick DELAUNAY <patrick.delaunay at st.com>; Sean Anderson <seanga2 at gmail.com>
> Subject: [Uboot-stm32] [PATCH 1/2] cmd: pinmux: update result of do_status
>
> Update the result of do_status and alway returns a CMD_RET_ value (-ENOSYS was a possible result of show_pinmux).
>
> This patch also adds pincontrol name in error messages (dev->name) and treats correctly the status sub command when pin-controller device is not selected.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> ---
>
> cmd/pinmux.c | 44 +++++++++++++++++++-----------------
> test/py/tests/test_pinmux.py | 4 ++--
> 2 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/cmd/pinmux.c b/cmd/pinmux.c index 9942b15419..af04c95a46 100644
> --- a/cmd/pinmux.c
> +++ b/cmd/pinmux.c
> @@ -41,7 +41,7 @@ static int do_dev(struct cmd_tbl *cmdtp, int flag, int argc,
> return CMD_RET_SUCCESS;
> }
>
> -static int show_pinmux(struct udevice *dev)
I think it is better to return the error code and let the caller
ignore it, If we later want to report the error code, we can.
> +static void show_pinmux(struct udevice *dev)
> {
> char pin_name[PINNAME_SIZE];
> char pin_mux[PINMUX_SIZE];
> @@ -51,54 +51,56 @@ static int show_pinmux(struct udevice *dev)
>
> pins_count = pinctrl_get_pins_count(dev);
>
> - if (pins_count == -ENOSYS) {
> - printf("Ops get_pins_count not supported\n");
> - return pins_count;
> + if (pins_count < 0) {
Why change this to < 0 ?
I believe that -ENOSYS is the only valid error. We should update the
get_pins_count() API function to indicate that.
> + printf("Ops get_pins_count not supported by %s\n", dev->name);
> + return;
> }
>
> for (i = 0; i < pins_count; i++) {
> ret = pinctrl_get_pin_name(dev, i, pin_name, PINNAME_SIZE);
> - if (ret == -ENOSYS) {
> - printf("Ops get_pin_name not supported\n");
> - return ret;
> + if (ret) {
> + printf("Ops get_pin_name error (%d) by %s\n",
> + ret, dev->name);
> + return;
> }
>
> ret = pinctrl_get_pin_muxing(dev, i, pin_mux, PINMUX_SIZE);
> if (ret) {
> - printf("Ops get_pin_muxing error (%d)\n", ret);
> - return ret;
> + printf("Ops get_pin_muxing error (%d) by %s in %s\n",
> + ret, pin_name, dev->name);
> + return;
> }
>
> printf("%-*s: %-*s\n", PINNAME_SIZE, pin_name,
> PINMUX_SIZE, pin_mux);
> }
> -
> - return 0;
> }
>
> static int do_status(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
> {
> struct udevice *dev;
> - int ret = CMD_RET_USAGE;
>
> - if (currdev && (argc < 2 || strcmp(argv[1], "-a")))
> - return show_pinmux(currdev);
> + if (argc < 2) {
> + if (!currdev) {
> + printf("pin-controller device not selected\n");
> + return CMD_RET_FAILURE;
> + }
> + show_pinmux(currdev);
> + return CMD_RET_SUCCESS;
> + }
>
> - if (argc < 2 || strcmp(argv[1], "-a"))
> - return ret;
> + if (strcmp(argv[1], "-a"))
> + return CMD_RET_USAGE;
>
> uclass_foreach_dev_probe(UCLASS_PINCTRL, dev) {
> /* insert a separator between each pin-controller display */
> printf("--------------------------\n");
> printf("%s:\n", dev->name);
> - ret = show_pinmux(dev);
> - if (ret < 0)
> - printf("Can't display pin muxing for %s\n",
> - dev->name);
> + show_pinmux(dev);
> }
>
> - return ret;
> + return CMD_RET_SUCCESS;
> }
>
> static int do_list(struct cmd_tbl *cmdtp, int flag, int argc, diff --git a/test/py/tests/test_pinmux.py b/test/py/tests/test_pinmux.py index 0cbbae000c..b3ae2ab024 100644
> --- a/test/py/tests/test_pinmux.py
> +++ b/test/py/tests/test_pinmux.py
> @@ -13,9 +13,9 @@ def test_pinmux_usage_1(u_boot_console):
> @pytest.mark.buildconfigspec('cmd_pinmux')
> def test_pinmux_usage_2(u_boot_console):
> """Test that 'pinmux status' executed without previous "pinmux dev"
> - command displays pinmux usage."""
> + command displays error message."""
> output = u_boot_console.run_command('pinmux status')
> - assert 'Usage:' in output
> + assert 'pin-controller device not selected' in output
>
> @pytest.mark.buildconfigspec('cmd_pinmux')
> @pytest.mark.boardspec('sandbox')
>
> Reviewed-by: Patrice Chotard <patrice.chotard at foss.st.com>
>
> Thanks
> Patrice
Regards,
Simon
More information about the U-Boot
mailing list