Re: [Uboot-stm32] [PATCH 1/2] cmd: pinmux: update result of do_status
Hi Simon,
On 4/29/21 6:10 PM, Simon Glass wrote:
Hi Patrick,
On Tue, 20 Apr 2021 at 03:21, Patrice CHOTARD wrote:
Hi Patrick
-Original Message-
From: Uboot-stm32 On Behalf
Of Patrick DELAUNAY
Sent: mercredi 28 octobre 2020 11:07
To: u-boot@lists.denx.de
Cc: U-Boot STM32 ; Simon Glass
; Patrick DELAUNAY ; Sean Anderson
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
---
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.
ok even if this static is only a help of do_status I will continue to
return the result.
+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.
I think that any value < 0 was allowed ...
/**
* pinctrl_get_pins_count() - Display pin-controller pins number
* @dev:Pinctrl device to use
*
* This allows to know the number of pins owned by a given pin-controller
*
* Return: Number of pins if OK, or negative error code on failure
*/
intpinctrl_get_pins_count(structudevice*dev);
with
intpinctrl_get_pins_count(structudevice*dev)
{ struct pinctrl_ops *ops = pinctrl_get_ops(dev); if
(!ops->get_pins_count) return -ENOSYS;
returnops->get_pins_count(dev);
}
But after check you are right: the ops don't allow negative value for error
/**
* @get_pins_count:Get the number of selectable pins
*
* @dev:Pinctrl device to use
*
* This function is necessary to parse the "pins" property in DTS.
*
* @Return:
* number of selectable named pins available in this driver
*/
int(*get_pins_count)(structudevice*dev);
I will update the API doc and this check.
+ printf("Ops get_pins_count not supported by %s\n", dev->name);
+ return;
}
(...)
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
Thanks
Patrice
Regards,
Simon
___
Uboot-stm32 mailing list
uboot-st...@st-md-mailman.stormreply.com
https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32
Regards
Patrick
Re: [Uboot-stm32] [PATCH 1/2] cmd: pinmux: update result of do_status
Hi Patrick,
On Tue, 20 Apr 2021 at 03:21, Patrice CHOTARD wrote:
>
> Hi Patrick
>
> -Original Message-
> From: Uboot-stm32 On
> Behalf Of Patrick DELAUNAY
> Sent: mercredi 28 octobre 2020 11:07
> To: u-boot@lists.denx.de
> Cc: U-Boot STM32 ; Simon Glass
> ; Patrick DELAUNAY ; Sean
> Anderson
> 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
> ---
>
> 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/
RE: [Uboot-stm32] [PATCH 1/2] cmd: pinmux: update result of do_status
Hi Patrick
-Original Message-
From: Uboot-stm32 On Behalf
Of Patrick DELAUNAY
Sent: mercredi 28 octobre 2020 11:07
To: u-boot@lists.denx.de
Cc: U-Boot STM32 ; Simon Glass
; Patrick DELAUNAY ; Sean Anderson
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
---
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)
+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) {
+ 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
Thanks
Patrice
--
2.17.1
___
Uboot-stm32 mailing list
uboot-st...@st-md-mailman.stormreply.com
https://st-md-mailman.stormreply.com/mailman/listinfo/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
---
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)
+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) {
+ 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')
--
2.17.1
