Hi Patrick

-----Original Message----- From: Uboot-stm32 uboot-stm32-bounces@st-md-mailman.stormreply.com On Behalf Of Patrick DELAUNAY Sent: mercredi 28 octobre 2020 11:07 To: u-boot@lists.denx.de Cc: U-Boot STM32 uboot-stm32@st-md-mailman.stormreply.com; Simon Glass sjg@chromium.org; Patrick DELAUNAY patrick.delaunay@st.com; Sean Anderson seanga2@gmail.com Subject: [Uboot-stm32] [PATCH 2/2] cmd: pinmux: support pin name in status command

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) { 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; }

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)

Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com

Thanks Patrice

-- 2.17.1

_______________________________________________ Uboot-stm32 mailing list Uboot-stm32@st-md-mailman.stormreply.com https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32

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 ?

other pinmux tests in already python should be migrate also ?

...

-- 2.17.1

Regards, Simon _______________________________________________ Uboot-stm32 mailing list Uboot-stm32@st-md-mailman.stormreply.com https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32

Reagards

Patrick

Hi Patrick,

On Tue, 20 Apr 2021 at 03:21, Patrice CHOTARD patrice.chotard@st.com wrote:

Hi Patrick

-----Original Message----- From: Uboot-stm32 uboot-stm32-bounces@st-md-mailman.stormreply.com On Behalf Of Patrick DELAUNAY Sent: mercredi 28 octobre 2020 11:07 To: u-boot@lists.denx.de Cc: U-Boot STM32 uboot-stm32@st-md-mailman.stormreply.com; Simon Glass sjg@chromium.org; Patrick DELAUNAY patrick.delaunay@st.com; Sean Anderson seanga2@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@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@foss.st.com

Thanks Patrice

Regards, Simon