[PATCH 1/1] doc: man-page for gpio command
Provide a man-page for the gpio command.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- doc/usage/cmd/gpio.rst | 90 ++++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 91 insertions(+) create mode 100644 doc/usage/cmd/gpio.rst
diff --git a/doc/usage/cmd/gpio.rst b/doc/usage/cmd/gpio.rst new file mode 100644 index 0000000000..f6a5668388 --- /dev/null +++ b/doc/usage/cmd/gpio.rst @@ -0,0 +1,90 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +gpio command +============ + +Synopsis +-------- + +:: + + gpio <input|set|clear|toggle> <pin> + gpio read <name> <pin> + gpio status [-a] [<bank>|<pin>] + +The gpio command is used to access General Purpose Inputs/Outputs. + +gpio input +---------- + +Switch the GPIO *pin* to input mode. + +gpio set +-------- + +Switch the GPIO *pin* to output mode and set the signal to 1. + +gpio clear +---------- + +Switch the GPIO *pin* to output mode and set the signal to 0. + +gpio toggle +----------- + +Switch the GPIO *pin* to output mode and reverse the signal state. + +gpio read +--------- + +Read the signal state of the GPIO *pin* and save it in environment variable +*name*. + +gpio status +----------- + +Display the status of one or multiple GPIOs. By default only claimed GPIOs +are displayed. + +-a + Display GPIOs irrespective of being claimed. + +bank + Name of a bank of GPIOs to be displayed. + +pin + Name of a single GPIO to be displayed or manipulated. + +Examples +-------- + +Switch the status of a GPIO:: + + => gpio set a5 + gpio: pin a5 (gpio 133) value is 1 + => gpio clear a5 + gpio: pin a5 (gpio 133) value is 0 + => gpio toggle a5 + gpio: pin a5 (gpio 133) value is 1 + => gpio read myvar a5 + gpio: pin a5 (gpio 133) value is 1 + => echo $myvar + 1 + => gpio toggle a5 + gpio: pin a5 (gpio 133) value is 0 + => gpio read myvar a5 + gpio: pin a5 (gpio 133) value is 0 + => echo $myvar + 0 + +Configuration +------------- + +The *gpio* command is only available if CONFIG_CMD_GPIO=y. +The *gpio read* command is only available if CONFIG_CMD_GPIO_READ=y. + +Return value +------------ + +If the command succeds the return value $? is set to 0. If an error occurs, the +return value $? is set to 1. diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 16a3db5c00..9ddc7b6cdd 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -45,6 +45,7 @@ Shell commands cmd/fatload cmd/fdt cmd/for + cmd/gpio cmd/load cmd/loadm cmd/loady
On Tue, 2 Aug 2022 at 06:28, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Provide a man-page for the gpio command.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
doc/usage/cmd/gpio.rst | 90 ++++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 91 insertions(+) create mode 100644 doc/usage/cmd/gpio.rst
Reviewed-by: Simon Glass sjg@chromium.org
Hi Heinrich,
On 8/2/22 14:28, Heinrich Schuchardt wrote:
Provide a man-page for the gpio command.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
doc/usage/cmd/gpio.rst | 90 ++++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 91 insertions(+) create mode 100644 doc/usage/cmd/gpio.rst
diff --git a/doc/usage/cmd/gpio.rst b/doc/usage/cmd/gpio.rst new file mode 100644 index 0000000000..f6a5668388 --- /dev/null +++ b/doc/usage/cmd/gpio.rst @@ -0,0 +1,90 @@ +.. SPDX-License-Identifier: GPL-2.0+:
+gpio command +============
+Synopsis +--------
+::
- gpio <input|set|clear|toggle> <pin>
- gpio read <name> <pin>
- gpio status [-a] [<bank>|<pin>]
+The gpio command is used to access General Purpose Inputs/Outputs.
+gpio input +----------
+Switch the GPIO *pin* to input mode.
+gpio set +--------
+Switch the GPIO *pin* to output mode and set the signal to 1.
I think this is supposed to follow the GPIOD_ACTIVE_LOW/GPIOD_ACTIVE_HIGH flag. So I think it's better to say "set the state to active"? Maybe add a few words on this active high/low thing to explicit that using gpio set does not necessarily mean that the GPIO output will have some voltage.
+gpio clear +----------
+Switch the GPIO *pin* to output mode and set the signal to 0.
Ditto.
+gpio toggle +-----------
+Switch the GPIO *pin* to output mode and reverse the signal state.
+gpio read +---------
+Read the signal state of the GPIO *pin* and save it in environment variable +*name*.
+gpio status +-----------
+Display the status of one or multiple GPIOs. By default only claimed GPIOs +are displayed.
+-a
- Display GPIOs irrespective of being claimed.
+bank
- Name of a bank of GPIOs to be displayed.
+pin
- Name of a single GPIO to be displayed or manipulated.
+Examples +--------
+Switch the status of a GPIO::
- => gpio set a5
- gpio: pin a5 (gpio 133) value is 1
and I guess we should maybe patch the gpio cmd to say here "high" instead of "1"?
- => gpio clear a5
- gpio: pin a5 (gpio 133) value is 0
ditto
- => gpio toggle a5
- gpio: pin a5 (gpio 133) value is 1
ditto
- => gpio read myvar a5
- gpio: pin a5 (gpio 133) value is 1
ditto
- => echo $myvar
- 1
- => gpio toggle a5
- gpio: pin a5 (gpio 133) value is 0
ditto
- => gpio read myvar a5
- gpio: pin a5 (gpio 133) value is 0
ditto
Cheers, Quentin
On 8/3/22 11:59, Quentin Schulz wrote:
Hi Heinrich,
On 8/2/22 14:28, Heinrich Schuchardt wrote:
Provide a man-page for the gpio command.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
doc/usage/cmd/gpio.rst | 90 ++++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 91 insertions(+) create mode 100644 doc/usage/cmd/gpio.rst
diff --git a/doc/usage/cmd/gpio.rst b/doc/usage/cmd/gpio.rst new file mode 100644 index 0000000000..f6a5668388 --- /dev/null +++ b/doc/usage/cmd/gpio.rst @@ -0,0 +1,90 @@ +.. SPDX-License-Identifier: GPL-2.0+:
+gpio command +============
+Synopsis +--------
+::
+ gpio <input|set|clear|toggle> <pin> + gpio read <name> <pin> + gpio status [-a] [<bank>|<pin>]
+The gpio command is used to access General Purpose Inputs/Outputs.
+gpio input +----------
+Switch the GPIO *pin* to input mode.
+gpio set +--------
+Switch the GPIO *pin* to output mode and set the signal to 1.
I think this is supposed to follow the GPIOD_ACTIVE_LOW/GPIOD_ACTIVE_HIGH flag. So I think it's better to say "set the state to active"? Maybe add a few words on this active high/low thing to explicit that using gpio set does not necessarily mean that the GPIO output will have some voltage.
There is no string GPIOD_ACTIVE_HIGH in U-Boot.
include/asm-generic/gpio.h:124: #define GPIOD_ACTIVE_LOW BIT(3) /* GPIO is active when value is low */
The gpio command lacks the capability to set active high or active low. It is not even displayed.
The function description of dm_gpio_get_value() documents the return value as 0 for inactive, 1 for active.
The function description of gpio_get_value() documents its return value as 0 for low or 1 for high.
The uclass implements gpio_get_value() by calling dm_gpio_get_value() without inverting the value.
One of both functions is incorrectly documented!
dm_gpio_get_value() calls _gpio_get_value() which calls the driver's get_value() function and inverts the value if GPIOD_ACTIVE_LOW is set.
The structure struct dm_gpio_ops lacks any documentation of the meaning of the return value.
To sum it up:
The function documentation is a mess and leaves it unclear what a value of 0 or 1 in the gpio command means.
The first thing to do is to get the documentation of struct dm_gpio_ops updated. Next ensure that all drivers implement it this way.
Once the yet to find maintainer of GPIO has done this we can revisit the documentation.
+gpio clear +----------
+Switch the GPIO *pin* to output mode and set the signal to 0.
Ditto.
+gpio toggle +-----------
+Switch the GPIO *pin* to output mode and reverse the signal state.
+gpio read +---------
+Read the signal state of the GPIO *pin* and save it in environment variable +*name*.
+gpio status +-----------
+Display the status of one or multiple GPIOs. By default only claimed GPIOs +are displayed.
+-a + Display GPIOs irrespective of being claimed.
+bank + Name of a bank of GPIOs to be displayed.
+pin + Name of a single GPIO to be displayed or manipulated.
+Examples +--------
+Switch the status of a GPIO::
+ => gpio set a5 + gpio: pin a5 (gpio 133) value is 1
and I guess we should maybe patch the gpio cmd to say here "high" instead of "1"?
For an active low GPIO value 1 might mean low.
It would be helpful to provide a way to display if a GPIO is active high or active low. But this patch is about documenting the existing functionality.
Best regards
Heinrich
+ => gpio clear a5 + gpio: pin a5 (gpio 133) value is 0
ditto
+ => gpio toggle a5 + gpio: pin a5 (gpio 133) value is 1
ditto
+ => gpio read myvar a5 + gpio: pin a5 (gpio 133) value is 1
ditto
+ => echo $myvar + 1 + => gpio toggle a5 + gpio: pin a5 (gpio 133) value is 0
ditto
+ => gpio read myvar a5 + gpio: pin a5 (gpio 133) value is 0
ditto
Cheers, Quentin
Hi Heinrich,
On 8/3/22 13:21, Heinrich Schuchardt wrote:
On 8/3/22 11:59, Quentin Schulz wrote:
Hi Heinrich,
On 8/2/22 14:28, Heinrich Schuchardt wrote:
Provide a man-page for the gpio command.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
doc/usage/cmd/gpio.rst | 90 ++++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 91 insertions(+) create mode 100644 doc/usage/cmd/gpio.rst
diff --git a/doc/usage/cmd/gpio.rst b/doc/usage/cmd/gpio.rst new file mode 100644 index 0000000000..f6a5668388 --- /dev/null +++ b/doc/usage/cmd/gpio.rst @@ -0,0 +1,90 @@ +.. SPDX-License-Identifier: GPL-2.0+:
+gpio command +============
+Synopsis +--------
+::
+ gpio <input|set|clear|toggle> <pin> + gpio read <name> <pin> + gpio status [-a] [<bank>|<pin>]
+The gpio command is used to access General Purpose Inputs/Outputs.
+gpio input +----------
+Switch the GPIO *pin* to input mode.
+gpio set +--------
+Switch the GPIO *pin* to output mode and set the signal to 1.
I think this is supposed to follow the GPIOD_ACTIVE_LOW/GPIOD_ACTIVE_HIGH flag. So I think it's better to say "set the state to active"? Maybe add a few words on this active high/low thing to explicit that using gpio set does not necessarily mean that the GPIO output will have some voltage.
There is no string GPIOD_ACTIVE_HIGH in U-Boot.
include/asm-generic/gpio.h:124: #define GPIOD_ACTIVE_LOW BIT(3) /* GPIO is active when value is low */
The gpio command lacks the capability to set active high or active low. It is not even displayed.
The function description of dm_gpio_get_value() documents the return value as 0 for inactive, 1 for active.
The function description of gpio_get_value() documents its return value as 0 for low or 1 for high.
The uclass implements gpio_get_value() by calling dm_gpio_get_value() without inverting the value.
One of both functions is incorrectly documented!
dm_gpio_get_value() calls _gpio_get_value() which calls the driver's get_value() function and inverts the value if GPIOD_ACTIVE_LOW is set.
The structure struct dm_gpio_ops lacks any documentation of the meaning of the return value.
Considering that _gpio_get_value does the inversion if the GPIOD_ACTIVE_LOW flag is set, the expected return value of dm_gpio_ops->get_value should be the raw value of the pin as returned by the SoC. Not documented, yes, but the meaning is clear to me.
To sum it up:
The function documentation is a mess and leaves it unclear what a value of 0 or 1 in the gpio command means.
It is clear for DM, not for non-DM.
The first thing to do is to get the documentation of struct dm_gpio_ops updated. Next ensure that all drivers implement it this way.
Agreed.
We still ave the issue with values returned by non-DM drivers, but should we care? The drivers should be moved to DM_GPIO?
Once the yet to find maintainer of GPIO has done this we can revisit the documentation.
Indeed.
I thought DM-only GPIO subsystem was on the map, like all subsystems, but I cannot find it on https://u-boot.readthedocs.io/en/latest/develop/driver-model/migration.html. Otherwise, the move to DM-only for GPIO should also help us in having a properly defined expectation of what the gpio command should return, since the DM version of gpio commands is handling active state. I'd have been inclined to say we shouldn't care too much about non-DM GPIO drivers since they'll disappear some day, are deprecated (as per documentation in header file) and if they get updated they should follow the yet-to-be-written in-file documentation, but I'm not too sure since DM_GPIO is not on the map of driver model migration at the moment?
+gpio clear +----------
+Switch the GPIO *pin* to output mode and set the signal to 0.
Ditto.
+gpio toggle +-----------
+Switch the GPIO *pin* to output mode and reverse the signal state.
+gpio read +---------
+Read the signal state of the GPIO *pin* and save it in environment variable +*name*.
+gpio status +-----------
+Display the status of one or multiple GPIOs. By default only claimed GPIOs +are displayed.
+-a + Display GPIOs irrespective of being claimed.
+bank + Name of a bank of GPIOs to be displayed.
+pin + Name of a single GPIO to be displayed or manipulated.
+Examples +--------
+Switch the status of a GPIO::
+ => gpio set a5 + gpio: pin a5 (gpio 133) value is 1
and I guess we should maybe patch the gpio cmd to say here "high" instead of "1"?
For an active low GPIO value 1 might mean low.
It would be helpful to provide a way to display if a GPIO is active high or active low. But this patch is about documenting the existing functionality.
Agreed, it was a suggestion on how to improve the gpio command output (in another patch obviously).
Though, this thread sadly highlighted that the gpio command returns something that cannot be trusted by the user without digging into the driver/datasheet (at least until we get DM-only GPIO subsystem) :/
BR, Quentin
On 8/3/22 13:56, Quentin Schulz wrote:
Hi Heinrich,
On 8/3/22 13:21, Heinrich Schuchardt wrote:
On 8/3/22 11:59, Quentin Schulz wrote:
Hi Heinrich,
On 8/2/22 14:28, Heinrich Schuchardt wrote:
Provide a man-page for the gpio command.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
doc/usage/cmd/gpio.rst | 90 ++++++++++++++++++++++++++++++++++++++++++ doc/usage/index.rst | 1 + 2 files changed, 91 insertions(+) create mode 100644 doc/usage/cmd/gpio.rst
diff --git a/doc/usage/cmd/gpio.rst b/doc/usage/cmd/gpio.rst new file mode 100644 index 0000000000..f6a5668388 --- /dev/null +++ b/doc/usage/cmd/gpio.rst @@ -0,0 +1,90 @@ +.. SPDX-License-Identifier: GPL-2.0+:
+gpio command +============
+Synopsis +--------
+::
+ gpio <input|set|clear|toggle> <pin> + gpio read <name> <pin> + gpio status [-a] [<bank>|<pin>]
+The gpio command is used to access General Purpose Inputs/Outputs.
+gpio input +----------
+Switch the GPIO *pin* to input mode.
+gpio set +--------
+Switch the GPIO *pin* to output mode and set the signal to 1.
I think this is supposed to follow the GPIOD_ACTIVE_LOW/GPIOD_ACTIVE_HIGH flag. So I think it's better to say "set the state to active"? Maybe add a few words on this active high/low thing to explicit that using gpio set does not necessarily mean that the GPIO output will have some voltage.
There is no string GPIOD_ACTIVE_HIGH in U-Boot.
include/asm-generic/gpio.h:124: #define GPIOD_ACTIVE_LOW BIT(3) /* GPIO is active when value is low */
The gpio command lacks the capability to set active high or active low. It is not even displayed.
The function description of dm_gpio_get_value() documents the return value as 0 for inactive, 1 for active.
The function description of gpio_get_value() documents its return value as 0 for low or 1 for high.
The uclass implements gpio_get_value() by calling dm_gpio_get_value() without inverting the value.
One of both functions is incorrectly documented!
dm_gpio_get_value() calls _gpio_get_value() which calls the driver's get_value() function and inverts the value if GPIOD_ACTIVE_LOW is set.
The structure struct dm_gpio_ops lacks any documentation of the meaning of the return value.
Considering that _gpio_get_value does the inversion if the GPIOD_ACTIVE_LOW flag is set, the expected return value of dm_gpio_ops->get_value should be the raw value of the pin as returned by the SoC. Not documented, yes, but the meaning is clear to me.
To sum it up:
The function documentation is a mess and leaves it unclear what a value of 0 or 1 in the gpio command means.
It is clear for DM, not for non-DM.
The first thing to do is to get the documentation of struct dm_gpio_ops updated. Next ensure that all drivers implement it this way.
Agreed.
We still ave the issue with values returned by non-DM drivers, but should we care? The drivers should be moved to DM_GPIO?
Once the yet to find maintainer of GPIO has done this we can revisit the documentation.
Indeed.
I thought DM-only GPIO subsystem was on the map, like all subsystems, but I cannot find it on https://u-boot.readthedocs.io/en/latest/develop/driver-model/migration.html. Otherwise, the move to DM-only for GPIO should also help us in having a properly defined expectation of what the gpio command should return, since the DM version of gpio commands is handling active state. I'd have been inclined to say we shouldn't care too much about non-DM GPIO drivers since they'll disappear some day, are deprecated (as per documentation in header file) and if they get updated they should follow the yet-to-be-written in-file documentation, but I'm not too sure since DM_GPIO is not on the map of driver model migration at the moment?
+gpio clear +----------
+Switch the GPIO *pin* to output mode and set the signal to 0.
Ditto.
+gpio toggle +-----------
+Switch the GPIO *pin* to output mode and reverse the signal state.
+gpio read +---------
+Read the signal state of the GPIO *pin* and save it in environment variable +*name*.
+gpio status +-----------
+Display the status of one or multiple GPIOs. By default only claimed GPIOs +are displayed.
+-a + Display GPIOs irrespective of being claimed.
+bank + Name of a bank of GPIOs to be displayed.
+pin + Name of a single GPIO to be displayed or manipulated.
+Examples +--------
+Switch the status of a GPIO::
+ => gpio set a5 + gpio: pin a5 (gpio 133) value is 1
and I guess we should maybe patch the gpio cmd to say here "high" instead of "1"?
For an active low GPIO value 1 might mean low.
It would be helpful to provide a way to display if a GPIO is active high or active low. But this patch is about documenting the existing functionality.
Agreed, it was a suggestion on how to improve the gpio command output (in another patch obviously).
Though, this thread sadly highlighted that the gpio command returns something that cannot be trusted by the user without digging into the driver/datasheet (at least until we get DM-only GPIO subsystem) :/
BR, Quentin
The following boards need to be converted to use DM_GPIO or otherwise should be dropped:
Simon Guinot * edminiv2_defconfig
Fabio Estevam * mx28evk_nand_defconfig * mx28evk_auart_console_defconfig * mx28evk_spi_defconfig
Otavio Salvador * warp_defconfig
Best regards
Heinrich
Hi Heinrich,
On Wed, Aug 3, 2022 at 11:31 AM Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Fabio Estevam
- mx28evk_nand_defconfig
- mx28evk_auart_console_defconfig
- mx28evk_spi_defconfig
I have sent a patch removing these defconfigs: https://lore.kernel.org/u-boot/20220803020404.4156197-1-festevam@gmail.com/T...
Thanks
participants (4)
-
Fabio Estevam -
Heinrich Schuchardt -
Quentin Schulz -
Simon Glass