doc: hardware: emulator: add gpio_emul documentation #67410
Conversation
cfriedt
force-pushed
the
add-documentation-for-gpio-emulator
branch
4 times, most recently
from
34de820 to
cefd244
Compare
Previously, the only documentation that existed for gpio_emul was in the form of source code. Create some additional documentation in the form of .rst to help clarify usage. Any additional instruction for how to use the GPIO Emulator driver can be added here. Signed-off-by: Christopher Friedt <[email protected]>
cfriedt
force-pushed
the
add-documentation-for-gpio-emulator
branch
from
cefd244 to
7acc2e6
Compare
|
Ready-ish I think - happy to make modifications, as needed. |
zephyrbot
requested review from
aaronemassey,
alevkoy,
asemjonovs,
jeremybettis,
tristan-google and
yperess
cfriedt
requested review from
henrikbrixandersen and
yperess
and removed request for
yperess,
aaronemassey,
alevkoy,
jeremybettis,
tristan-google and
asemjonovs
| driver does not follow the same behaviour required by Zephyr's GPIO Driver Model, and the new | ||
| driver requires improvement. | ||
|
|
||
| 2. The ``gpio_emul`` device allows us to perform fast functional testing to verify the expected |
There was a problem hiding this comment.
I am personally a fan of documentation with a conversational tone :), but I defer to other reviewers.
There was a problem hiding this comment.
Are you, or are you not? Just asking for clarification in case you forgot a word -- I have to admit that the tone certainly had me do a double take yesterday, even before you made this comment, and I would lean on the "not a fan" side myself as the page currently "reads" very differently than 99.9% of the rest of the documentation. That being said, I will also defer to others :) And in any case I will try to provide actionable feedback. Thanks for the work on this doc page, Chris!
There was a problem hiding this comment.
Thanks for the work on this doc page, Chris!
+1 to that, thank you, Chris!
Are you, or are you not?
Personally, I like reading conversational documentation. That being said, you brought up a good point that the rest of the Zephyr documentation does not follow a conversational theme. So you're probably right that it'd be better to rephrase more exactly.
There was a problem hiding this comment.
For some reason, I misread "conversational" as "controversial" initially 😅 I guess both could be true.
I will make some objective corrections first, and then will look at subjective corrections.
It might take a bit of time
|
|
||
| Use the emulated GPIO driver exactly the same as one would use any other GPIO driver. | ||
|
|
||
| Insert an instance into Devicetree, ensuring that the named compatible is ``zephyr,gpio-emul`` |
There was a problem hiding this comment.
You reminded me I need to go make a gh issue about needing support for tristate gpios.
There was a problem hiding this comment.
Thanks for documenting it,
But it does not seem ready yet.
Rendered version can be found here:
https://builds.zephyrproject.io/zephyr/pr/67410/docs/hardware/emulator/gpio_emulator.html
|
|
||
| Zephyr's GPIO (General Purpose Input-Output) emulator has several functions. | ||
|
|
||
| 1. It serves as a model for the basic GPIO Driver API; if a new GPIO driver is added for a new |
| #. Lightweight Vector Graphics Library (LVGL) sample with a "soft" button | ||
|
|
||
| .. zephyr-app-commands:: | ||
| :app: tests/drivers/eeprom/api |
| driver requires improvement. | ||
|
|
||
| 2. The ``gpio_emul`` device allows us to perform fast functional testing to verify the expected | ||
| behaviour of various libraries and applications. Why? In Zephyr, a GPIO is a GPIO is a GPIO |
There was a problem hiding this comment.
I would avoid funny phrases with meanings that are only known by speakers in some areas and times.. In 20 years nobody may know what this was supposed to mean, and already today people who are not of the same background as you will be left baffled by this sentence.
There was a problem hiding this comment.
+1, this 100% looked like a typo to me, tbh
There was a problem hiding this comment.
Lol, sure. I guess it's more of a native English thing / colloquialism
|
|
||
| 1. It serves as a model for the basic GPIO Driver API; if a new GPIO driver is added for a new | ||
| controller, and some aspect of the new driver fails Zephyr's GPIO Driver API Testsuite, while | ||
| the emulated GPIO driver (and others) pass, it may be an indicator that something in the new |
There was a problem hiding this comment.
This would be applicable to any Zephyr provided GPIO driver. But I guess nobody has agreed this particular driver is the reference implementation for the API(?). So this paragraph implies something I would not imply. Meaning, I would remove this paragraph all together.
There was a problem hiding this comment.
It was used that way when it was added and that's how I was instructed to design it by the previous gpio maintainer.
We also do not yet have a replacement GPIO maintainer.
In any case, I will remove it.
|
|
||
| 2. The ``gpio_emul`` device allows us to perform fast functional testing to verify the expected | ||
| behaviour of various libraries and applications. Why? In Zephyr, a GPIO is a GPIO is a GPIO | ||
| (from a functional perspective) because there is a common GPIO Driver API and model. How can |
There was a problem hiding this comment.
I'd say this format does not help with readability. In any case, this whole paragraph could have been 2 sentences.
Most readers prefer brief documentation.
| The ``zephyr,gpio-emul`` Devicetree bindings inherit from ``gpio-controller`` and ``base`` | ||
| Devicetree bindings. | ||
|
|
||
| It may be necessary to add the :kconfig:option:`CONFIG_GPIO_EMUL=y` option to your project |
There was a problem hiding this comment.
:kconfig:option:CONFIG_GPIO_EMUL is a link to the option, you cannot have the =y in the link itself
| }; | ||
|
|
||
| .. note:: | ||
| The ``zephyr,gpio-emul`` Devicetree bindings inherit from ``gpio-controller`` and ``base`` |
There was a problem hiding this comment.
Please replace zephyr,gpio-emul with a link to the binding doc page
(:dtcompatible:zephyr,gpio-emul)
| Usage | ||
| ===== | ||
|
|
||
| Use the emulated GPIO driver exactly the same as one would use any other GPIO driver. |
There was a problem hiding this comment.
Related to the above discussion of formality in documentation, "Use the emulated GPIO driver" is fairly informal, while "one would use" is fairly formal. Without expressing a strong opinion on which one I think you should choose, this sentence and the document should be internally consistent: "Use the emulated GPIO driver ... as you would use ..." or "One uses the emulated GPIO driver ... as one would use ..."
The same idea also applies to other constructions below like "may be specified" ("you may specify") and "it may be necessary" ("you may need").
| struct gpio_callback *cb, | ||
| gpio_port_pins_t pins) | ||
| { | ||
| /* |
There was a problem hiding this comment.
Maybe a snippet here that gets the interesting information out of port and pins and calls some made-up function with those objects as arguments?
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
Previously, the only documentation that existed for gpio_emul was in the form of source code.
Create some additional documentation in the form of .rst to help clarify usage. Any additional instruction for how to use the GPIO Emulator driver can be added here.