On 09/21/2017 06:24 AM, Michal Simek wrote:
Hi Stephen,
On 20.9.2017 19:05, Stephen Warren wrote:
On 09/20/2017 01:55 AM, Michal Simek wrote:
From: Vipul Kumar <vipul.ku...@xilinx.com>
This patch tests the gpio commands using the
gpio data from boardenv file.
Also one test will show the default status of all the gpio's
supported by the processor.
Nit: Wrap the commit description to something like 74 characters; the
text above is far too narrow.
Nit: GPIO not gpio in any free-form text (here and in comments in the
source file)
Ah, I'd briefly though about GPIO tests before, but was wondering how to
integrate e.g. a USB GPIO device to the host PC and interface that with
the target. Loopback GPIOs make this a lot easier:-)
good.
diff --git a/test/py/tests/test_gpio.py b/test/py/tests/test_gpio.py
+"""
+Note: This test relies on boardenv_* containing configuration values
to define
+which the gpio available for testing. Without this, this test will
be automat-
Nit: Double space before automatically.
+For example:
+
+# A list of gpio's that are going to be tested in order to validate the
+# test.
+env__gpio_val = {
+ "gpio": [0,1,2],
+}
Why is this a dictionary with only one key? Better to just assign the
array directly to env__gpio_val, or put both the "gpio" and
"list_of_gpios" into a single dictionary.
+# A list of gpio's that are shorted on the hardware board and first
gpio of
+# each group is configured as input and the other as output. If the
pins are
+# not shorted properly, then the test will be fail.
+env__gpio_input_output = {
+ "list_of_gpios": [ [36, 37], [38, 39]],
+}
Let's make the examples for env__gpio_val and env__gpio_input_output use
the same GPIO numbers so that they match, and it's obvious how the two
relate to each-other. I assume they're meant to?
The reason for two values is that you do that loopback between certain
pins. It means you have to list pairs.
I have tested this on the board where I can choose whatever
configuration by external wires. Also this list is not the same with the
above one because you are choosing which one is loopback but not all can
be done as loopback.
Also I have tested that you can specify 36,37 as one pair and then 37,36
pair which simply saying which pin is used as output and which as input.
I don't think it is right to say that both of them can bidirectional
because at least in our chip with have some pins which are just output
only and input only.
Sorry, I still don't understand why there are two lists. Certainly I see
that there's different testing applied to each list, but I don't see any
benefit from doing that. If you test just loopback pairs, then you've
tested all the other APIs as part of that, so you don't need a separate
test for a separate list of GPIOs.
Also, if the code was to include logic to test pairs of loopback GPIOs
in both directions, the code would need to be careful to set both GPIOs
to input after each test step. Without that, when setting up the output
GPIO for the next round of testing, both GPIOs might be outputs for a
while which could damage HW. I don't recall whether the current code is
safe in this case or not.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
