[Uboot-stm32] [PATCH 2/2] cmd: pinmux: support pin name in status command

Simon Glass sjg at chromium.org
Thu May 6 17:02:40 CEST 2021


Hi Patrick,

On Thu, 6 May 2021 at 02:38, Patrick DELAUNAY
<patrick.delaunay at foss.st.com> wrote:
>
> Hi,
>
> On 4/29/21 6:10 PM, Simon Glass wrote:
> > Hi Patrick,
> >
> > On Wed, 28 Oct 2020 at 03:06, Patrick Delaunay <patrick.delaunay at st.com> wrote:
> >> Allow pin name parameter for pimux staus command,
> >> as gpio command to get status of one pin.
> >>
> >> The possible usage of the command is:
> >>
> >>> pinmux dev pinctrl
> >>> pinmux status
> >>> pinmux status -a
> >>> pinmux status <pin-name>
> >> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> >> ---
> >>
> >>   cmd/pinmux.c                 | 41 +++++++++++++++++++++++++-----------
> >>   test/py/tests/test_pinmux.py | 29 +++++++++++++++++++++++++
> >>   2 files changed, 58 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/cmd/pinmux.c b/cmd/pinmux.c
> >> index af04c95a46..e096f16982 100644
> >> --- a/cmd/pinmux.c
> >> +++ b/cmd/pinmux.c
> >> @@ -41,19 +41,20 @@ static int do_dev(struct cmd_tbl *cmdtp, int flag, int argc,
> >>          return CMD_RET_SUCCESS;
> >>   }
> >>
> >> -static void show_pinmux(struct udevice *dev)
> >> +static bool show_pinmux(struct udevice *dev, char *name)
> > How about returning -ENOENT if there is no pin.
>
>
> OK
>
>
> >>   {
> >>          char pin_name[PINNAME_SIZE];
> >>          char pin_mux[PINMUX_SIZE];
> >>          int pins_count;
> >>          int i;
> >>          int ret;
> >> +       bool found = false;
> >>
> >>          pins_count = pinctrl_get_pins_count(dev);
> >>
> >>          if (pins_count < 0) {
> >>                  printf("Ops get_pins_count not supported by %s\n", dev->name);
> >> -               return;
> >> +               return found;
> > Here found will be false, so I think you are conflating different
> > errors. Better to return pins_count in this case.
> OK
> >>          }
> >>
> >>          for (i = 0; i < pins_count; i++) {
> >> @@ -61,43 +62,59 @@ static void show_pinmux(struct udevice *dev)
> >>                  if (ret) {
> >>                          printf("Ops get_pin_name error (%d) by %s\n",
> >>                                 ret, dev->name);
> >> -                       return;
> >> +                       return found;
> >>                  }
> >> -
> >> +               if (name && strcmp(name, pin_name))
> >> +                       continue;
> >> +               found = true;
> >>                  ret = pinctrl_get_pin_muxing(dev, i, pin_mux, PINMUX_SIZE);
> >>                  if (ret) {
> >>                          printf("Ops get_pin_muxing error (%d) by %s in %s\n",
> >>                                 ret, pin_name, dev->name);
> >> -                       return;
> >> +                       return found;
> >>                  }
> >>
> >>                  printf("%-*s: %-*s\n", PINNAME_SIZE, pin_name,
> >>                         PINMUX_SIZE, pin_mux);
> >>          }
> >> +
> >> +       return found;
> >>   }
> >>
> >>   static int do_status(struct cmd_tbl *cmdtp, int flag, int argc,
> >>                       char *const argv[])
> >>   {
> >>          struct udevice *dev;
> >> +       char *name;
> >> +       bool found = false;
> >>
> >>          if (argc < 2) {
> >>                  if (!currdev) {
> >>                          printf("pin-controller device not selected\n");
> >>                          return CMD_RET_FAILURE;
> >>                  }
> >> -               show_pinmux(currdev);
> >> +               show_pinmux(currdev, NULL);
> >>                  return CMD_RET_SUCCESS;
> >>          }
> >>
> >>          if (strcmp(argv[1], "-a"))
> >> -               return CMD_RET_USAGE;
> >> +               name = argv[1];
> >> +       else
> >> +               name = NULL;
> >>
> >>          uclass_foreach_dev_probe(UCLASS_PINCTRL, dev) {
> >> -               /* insert a separator between each pin-controller display */
> >> -               printf("--------------------------\n");
> >> -               printf("%s:\n", dev->name);
> >> -               show_pinmux(dev);
> >> +               if (!name) {
> >> +                       /* insert a separator between each pin-controller display */
> >> +                       printf("--------------------------\n");
> >> +                       printf("%s:\n", dev->name);
> >> +               }
> >> +               if (show_pinmux(dev, name))
> >> +                       found = true;
> >> +       }
> >> +
> >> +       if (name && !found) {
> >> +               printf("%s not found\n", name);
> >> +               return CMD_RET_FAILURE;
> >>          }
> >>
> >>          return CMD_RET_SUCCESS;
> >> @@ -148,5 +165,5 @@ U_BOOT_CMD(pinmux, CONFIG_SYS_MAXARGS, 1, do_pinmux,
> >>             "show pin-controller muxing",
> >>             "list                     - list UCLASS_PINCTRL devices\n"
> >>             "pinmux dev [pincontroller-name] - select pin-controller device\n"
> >> -          "pinmux status [-a]              - print pin-controller muxing [for all]\n"
> >> +          "pinmux status [-a | pin-name]   - print pin-controller muxing [for all | for pin-name]\n"
> >>   )
> >> diff --git a/test/py/tests/test_pinmux.py b/test/py/tests/test_pinmux.py
> >> index b3ae2ab024..fbde1d99b1 100644
> >> --- a/test/py/tests/test_pinmux.py
> >> +++ b/test/py/tests/test_pinmux.py
> >> @@ -82,3 +82,32 @@ def test_pinmux_status(u_boot_console):
> >>       assert ('P6        : GPIO1 drive-open-drain.' in output)
> >>       assert ('P7        : GPIO2 bias-pull-down input-enable.' in output)
> >>       assert ('P8        : GPIO3 bias-disable.' in output)
> >> +
> >> + at pytest.mark.buildconfigspec('cmd_pinmux')
> >> + at pytest.mark.boardspec('sandbox')
> >> +def test_pinmux_status_pinname(u_boot_console):
> >> +    """Test that 'pinmux status <pinname>' displays selected pin."""
> >> +
> >> +    output = u_boot_console.run_command('pinmux status a5')
> >> +    assert ('a5        : gpio output .' in output)
> >> +    assert (not 'pinctrl-gpio:' in output)
> >> +    assert (not 'pinctrl:' in output)
> >> +    assert (not 'a6' in output)
> >> +    assert (not 'P0' in output)
> >> +    assert (not 'P8' in output)
> >> +
> >> +    output = u_boot_console.run_command('pinmux status P7')
> >> +    assert (not 'pinctrl-gpio:' in output)
> >> +    assert (not 'pinctrl:' in output)
> >> +    assert (not 'a5' in output)
> >> +    assert (not 'P0' in output)
> >> +    assert (not 'P6' in output)
> >> +    assert ('P7        : GPIO2 bias-pull-down input-enable.' in output)
> >> +    assert (not 'P8' in output)
> >> +
> >> +    output = u_boot_console.run_command('pinmux status P9')
> >> +    assert (not 'pinctrl-gpio:' in output)
> >> +    assert (not 'pinctrl:' in output)
> >> +    assert (not 'a5' in output)
> >> +    assert (not 'P8' in output)
> >> +    assert ('P9 not found' in output)
> > Can we write this test in C? We can use run_command()...see acpi.c
>
>
> Any reason to prefer C test to python...
>
> I just complete the existing pinmux tests.
>
> For performance ?

I wrote this up here:

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

>
> other pinmux tests in already python should be migrate also ?

They may as well be, to the extent that they only run on sandbox.

Regards,
SImon


More information about the U-Boot mailing list