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

Hi Patrick,

On Thu, 6 May 2021 at 02:38, Patrick DELAUNAY patrick.delaunay@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@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@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)

+@pytest.mark.buildconfigspec('cmd_pinmux') +@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