[Uboot-stm32] [PATCH 1/2] cmd: pinmux: update result of do_status

[Patrick DELAUNAY]

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.chotard at st.com> wrote:
>> Hi Patrick
>>
>> -----Original Message-----
>> From: Uboot-stm32 <uboot-stm32-bounces at st-md-mailman.stormreply.com> On Behalf Of Patrick DELAUNAY
>> Sent: mercredi 28 octobre 2020 11:07
>> To: u-boot at lists.denx.de
>> Cc: U-Boot STM32 <uboot-stm32 at st-md-mailman.stormreply.com>; Simon Glass <sjg at chromium.org>; Patrick DELAUNAY <patrick.delaunay at st.com>; Sean Anderson <seanga2 at 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 at 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.chotard at foss.st.com>
>>
>> Thanks
>> Patrice
> Regards,
> Simon
> _______________________________________________
> Uboot-stm32 mailing list
> Uboot-stm32 at st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/uboot-stm32

Regards

Patrick