Hi Simon,
On 4/29/21 6:10 PM, Simon Glass wrote:
Hi Patrick,
On Tue, 20 Apr 2021 at 03:21, Patrice CHOTARD <patrice.chot...@st.com> wrote:
Hi Patrick
-----Original Message-----
From: Uboot-stm32 <uboot-stm32-boun...@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-st...@st-md-mailman.stormreply.com>; Simon Glass
<s...@chromium.org>; Patrick DELAUNAY <patrick.delau...@st.com>; Sean Anderson
<sean...@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.delau...@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.
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 <patrice.chot...@foss.st.com>
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
