Generated device driver docs #30104

mbolivar-nordic · 2020-11-18T05:21:15Z

We have thorough existing documentation on the abstract driver model.

We have some documentation on individual APIs (though the generated
content is not very useful and should probably be looked at; I suspect
99.9% of users are just reading the doxygen comments in the source
code instead of looking at these pages).

We do not, however, have documentation for actual device drivers, or
information about what devicetree compatibles and Kconfig options they
are associated with.

This PR is a proof of concept showing how an index of existing drivers
that cross-references DT and Kconfig as needed, with content
coming from the sources, can be made.

We recently added a generated devicetree bindings index. Link to it from the bindings page in the Guides section of the documentation so it's easier for beginners to find. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

Before we had a bindings index in the documentation, the generated header file was (somewhat unfortunately) often our best reference for what a particular binding or property within a binding ends up doing, so it made good sense to put the description in the generated file. Now that we have HTML documentation that's a bit more digestible than the generated file, though, we can just point users at that. Do that and remove the inline description from the generated file. This makes it possible to put C-style multiline comments in the descriptions themselves, which will be done in subsequent patches. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

This should be used from device driver C files to indicate the devicetree compatible or compatibles handled by the driver in a block comment at the top of the file that looks like this: /** * @file * @dtcompatible{vendor,device-name} * * \rst * * Detailed restructuredText description of the * driver goes here. * * \endrst */ Subsequent patches will pull information from driver sources into the documentation build system. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

TODO: this might not be the right way; Breathe seems to emit duplicate C declaration warnings from 'doxygenfile' even when we only request the detaileddescription in :sections:, which will prevent us from using it in this context unless there's a way to avoid that. Add some macros commonly encountered in driver source files to PREDEFINED, so doxygen doesn't choke on them. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

We're about to add yet another custom role, so let's document the existing ones while we're here to help people digging through these files figure things out. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

This allows RST documentation to do something like this: See :dtcompatible:`vnd,foo` for more information. This is transformed into a link to documentation for the "vnd,foo" compatible. Some devicetree compatibles may be handled by multiple bindings. This can happen when the binding file is bus-dependent. Therefore, to make this work, we need to change the way gen_devicetree_rest.py works to ensure we have a good source of information for this compatible regardless of how many bindings are associated with as follows: - When only a single binding is associated with a compatible, the :dtcompatible: link goes directly to the per-binding HTML page. - When multiple bindings are associated with a compatible, this goes to a new generated "disambiguation" page which links to all the per-binding pages. To avoid clashes, we stick the disambiguation pages into a compatibles/ subdirectory of the generated bindings index root directory. Also reorganize the generated bindings output directory into .../bindings/subdir/binding.rst files. For example, dts/bindings/arm/arm,dtcm.yaml now gets its generated content in .../bindings/arm/arm,dtcm.rst. This brings the 'category' of binding (like 'sensor', 'i2c', etc.) into the URL, which is a useful hint. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no> Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

This pulls various driver source files into the documentation build system's awareness, making their doxygen comments available to Breathe and other scripts. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

The purpose of this alias is to allow a C file to declare that it is, in fact, a Zephyr device driver. It is necessary because not every C file under drivers/ is a device driver. There are unfortunate exceptions, like drivers/can/can_shell.c. This can be combined with @dtcompatible in a file-level Doxygen string that allows a driver to name itself, give the devicetree compatible it uses for DT_DRV_COMPAT or otherwise uses to create struct devices, and give a detailed description of the driver. Subsequent patches will show how this works and can be pulled into the documentation. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

TODO: other compatibles besides I2C ones Add some details to help new users learn how to manage the nodes for compatibles corresponding to IP blocks on Nordic SoCs. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

TODO: - double check nothing is missing that has a dtcompatible - add docstrings for drivers without dtcompatibles - add details Use @zephyrdriver{} and @dtcompatible{} for all Nordic peripheral device drivers. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

We have thorough existing documentation on the abstract driver model. We have some documentation on individual APIs (though the generated content is not very useful and should probably be looked at; I suspect 99.9% of users are just reading the doxygen comments in the source code instead of looking at these pages). We do not, however, have documentation for actual device drivers, or information about what devicetree compatibles and Kconfig options they are associated with. This patch is a proof of concept showing how the previous patches in the series can be stitched together into an index of existing drivers that cross-references DT and Kconfig as needed, with content coming from the sources. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

gmarull

I like the idea of having driver specific documentation, it can be really helpful. I think it would also be nice to have hand-written pages where one can add figures, diagrams, etc. and mix that with all the automatically extracted information. I understand that with these changes driver docs would only be able to reside in the sources.

mbolivar-nordic · 2020-11-23T19:26:29Z

. I understand that with these changes driver docs would only be able to reside in the sources.

No, that's not correct. The driver docs are allowed to be restructured text, so you can include any other files as you ordinarily would in .rst.

pabigot

Some initial thoughts before trying this out to see what it looks like. We do need something like this, I'm just nervous about how far it had to spread to meet the requirements.

pabigot · 2020-12-08T23:00:41Z

doc/CMakeLists.txt

@@ -148,6 +148,7 @@ file(GLOB_RECURSE  DOXY_SOURCES
  ${ZEPHYR_BASE}/lib/libc/*.[c,h,S]
  ${ZEPHYR_BASE}/subsys/testsuite/ztest/include/*.[h,c,S]
  ${ZEPHYR_BASE}/tests/*.[h,c,S]
+  ${ZEPHYR_BASE}/drivers/*.[c,h,S]


This worries me a little: until now headers that were private to a driver subsystem would never be accessed outside that subsystem, so there was little concern about ensuring public identifiers are globally unique.

Now MISRA apparently wants every identifier anywhere in the program to be unique, though that's not really feasible, and maybe this isn't going to bite us too bad, but it leaves me nervous.

pabigot · 2020-12-08T23:02:41Z

dts/bindings/i2c/nordic,nrf-twi-common.yaml

+        For pins P1.0 through P1.31, add 32 to the pin number. For
+        example, to use P1.2 for SDA, set:
+
+            sda-pin = <34>;  /* 32 + 2 */


It really would be nice to be able to use <&gpio0 16 0> and <&gpio1 2 0> like every other vendor, and NORDIC_DT_GPIO_ORDINAL(node_id, prop) to construct the value. I'm afraid it's too late, though.

It really would be nice to be able to use <&gpio0 16 0> and <&gpio1 2 0> like every other vendor, and NORDIC_DT_GPIO_ORDINAL(node_id, prop) to construct the value. I'm afraid it's too late, though.

I want to start by documenting what we have. Now that we have the ability to deprecate properties, we can introduce alternatives and gracefully deprecate these things over time.

pabigot · 2020-12-08T23:04:55Z

drivers/adc/adc_nrfx_adc.c

@@ -4,6 +4,12 @@
 * SPDX-License-Identifier: Apache-2.0
 */

+/**
+ * @file
+ * @zephyrdriver{Nordic Semiconductor nRF51x ADC}


Is there any reasonable chance the documentation build infrastructure could, instead of directly processing these implementation files with doxygen, just scan the tree for these first and put them all into a file that doxygen reads instead?

Do you mean doing a search for @zephyrdriver, getting a list of files, then adding those files to what doxygen processes?

I can look into that.

No, I meant constructing a single file that had all of these markers in them, collected from the entire tree, and adding it to what doxygen processes. IOW something that looks like:

/** * @file drivers/adc/adc_nrf_adc.c * @zephyrdriver{Nordic Semiconductor nRF51x ADC} * @dtcompatible{nordic,nrf-adc} */ /** * @file drivers/adc/adc_st_adc.c * @zephyrdriver{ST ADC} * @dtcompatible{st,adc} */ /** * @file drivers/adc/nxp_st_adc.c * @zephyrdriver{NXP ADC} * @dtcompatible{nxp,adc} */

If we still tried to run doxygen on the actual implementation file I think my concerns about exposing things formerly obscured wouldn't be addressed.

I don't like that: line numbers for error messages would be lost.

erwango · 2020-12-09T08:32:35Z

Looks good a first glance, but I need to look at the output.

erwango · 2020-12-09T10:48:22Z

Wanted to have a check on how this looks but I'm having trouble generating the doc :

  File "/local/mcu/zephyrproject/zephyr/doc/scripts/gen_driver_rest.py", line 142, in load_zephyrdriver_xml
    xml = doxygen_xml / f"{ref.attrib['refid']}.xml"
KeyError: 'refid'

I'm not facing this issue on master, so far it's still building..

  File "/local/home/frq07517/.local/lib/python3.6/site-packages/sphinx/builders/html/__init__.py", line 1234, in setup
    app.connect('config-inited', convert_html_css_files, priority=800)
TypeError: connect() got an unexpected keyword argument 'priority'

Is there some new dependency not documented ?

I'm running make htmldocs-fast

pabigot · 2020-12-09T12:15:53Z

A gap we run into is dealing with multiple compatibles. I don't see it here, but there's a lot going on....

To refresh one of my old examples: A SiLabs Si7021 is a sensor that is a clone of the Sensirion SHT21, but is more accurate and faster. A current PR (#30185) proposes to use the SI7006 driver for it. A devicetree node for the sensor might use:

compatible = "silabs,si7021", "sensirion,sht21", "silabs,si7006";

Note that somebody looking for an Si7021 driver and familiar with the SHT21 may not realize that the SI7006 driver is supposed to be compatible (I didn't, neither did the author of #29138). This suggests that restricting to one documented compatible per driver is not going to work.

A use case I have (#19904 and #20287) is to adapt a base driver like SI7006 to detect that a specific instance is an silabs,si7021 and to use that to change the behavior of the driver for that instance. So for a user it's important to know to list the specific compatible in the devicetree node.

That the SI7006 driver is capable of supporting silabs,si7021 is an characteristic of that driver. It is not specifically an attribute of the silabs,si7006 binding, even though it is that binding that must be located in order to use the driver. However, the binding is one location where we could meaningfully add the "compatible" compatible strings known to work with the driver.

Rather than have multiple @dtcompatible{} markings in the driver, could we require there to be only one (corresponding to its least specific compatible), and instead rework the bindings YAML to list the compatible compatibles? It could even be done in the compatible key, if we clarify that the true canonical identifier for the binding is derived from its file name, and that canonical name:

must be one of the ones in the compatible list, and
is the only one used to select the binding

The other compatibles would be used solely to link to and in documentation for the canonical binding.

mbolivar-nordic · 2020-12-10T01:53:23Z

Rather than have multiple @dtcompatible{} markings in the driver, could we require there to be only one (corresponding to its least specific compatible), and instead rework the bindings YAML to list the compatible compatibles?

I'm sympathetic to the idea, but at some point we need to just bite the bullet and move to dtschema. This is already a supported feature there, e.g.: https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/trivial-devices.yaml#L26

I'm not sure how much effort we want to continue to invest in our own bindings language, compared to teaching edtlib about dtschema. This sort of request starts to flip my bit in the direction of "wait for a dtschema move first", though.

mbolivar-nordic · 2020-12-10T02:09:02Z

@erwango I tried rebasing this to get you an up to date verison, but I ran into issues with the patch touching PREDEFINED. I think we're going to run into a game of whack-a-mole with all the macro magic we use, so I'm going to try to find another approach based on .rst files in drivers/ rather than doxygen.

erwango · 2020-12-10T08:30:35Z

Thanks for the feedback @mbolivar-nordic

mbolivar-nordic · 2021-01-26T19:25:22Z

I'm going to close this draft as the next version will take a totally separate approach. I hope that leaving this PR closed as-is will serve as a record for why this approach did not work.

mbolivar-nordic added 3 commits November 5, 2020 15:06

github-actions bot added area: ADC Analog-to-Digital Converter (ADC) area: Counter area: Devicetree area: Documentation area: I2C area: PWM Pulse Width Modulation labels Nov 18, 2020

mbolivar-nordic mentioned this pull request Nov 18, 2020

[Doc] How to link dts bindings with Kconfig flags #29779

Open

mbolivar-nordic added 8 commits November 18, 2020 11:36

doc: add docstring to link-roles.py

34a4713

We're about to add yet another custom role, so let's document the existing ones while we're here to help people digging through these files figure things out. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

doc: add drivers subdirectories to doxygen

799644a

This pulls various driver source files into the documentation build system's awareness, making their doxygen comments available to Breathe and other scripts. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

[wip] dts: bindings: improve documentation for Nordic IPs

c6d8ff4

TODO: other compatibles besides I2C ones Add some details to help new users learn how to manage the nodes for compatibles corresponding to IP blocks on Nordic SoCs. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>

mbolivar-nordic force-pushed the dt-driver-docs branch from 62512ca to 65bc4f4 Compare November 18, 2020 19:38

gmarull reviewed Nov 20, 2020

View reviewed changes

pabigot mentioned this pull request Nov 24, 2020

sensor: Add Silabs SI7021 support. #30185

Closed

nikolaushuber mentioned this pull request Dec 3, 2020

fixes for supply-gpios and updates for ccs811/efr32mg_sltb0004a #30401

Merged

pabigot reviewed Dec 8, 2020

View reviewed changes

avisconti mentioned this pull request Jan 21, 2021

dts: sensor: remove lis3dh binding #31450

Closed

mbolivar-nordic closed this Jan 26, 2021

mbolivar-nordic mentioned this pull request Mar 25, 2021

User Experience Study Recommendations #27073

Open

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generated device driver docs #30104

Generated device driver docs #30104

mbolivar-nordic commented Nov 18, 2020

gmarull left a comment

mbolivar-nordic commented Nov 23, 2020

pabigot left a comment

pabigot Dec 8, 2020

pabigot Dec 8, 2020

mbolivar-nordic Dec 8, 2020

pabigot Dec 8, 2020

mbolivar-nordic Dec 8, 2020

pabigot Dec 8, 2020

mbolivar-nordic Dec 8, 2020

erwango commented Dec 9, 2020

erwango commented Dec 9, 2020

pabigot commented Dec 9, 2020

mbolivar-nordic commented Dec 10, 2020

mbolivar-nordic commented Dec 10, 2020

erwango commented Dec 10, 2020

mbolivar-nordic commented Jan 26, 2021

Generated device driver docs #30104

Generated device driver docs #30104

Conversation

mbolivar-nordic commented Nov 18, 2020

gmarull left a comment

Choose a reason for hiding this comment

mbolivar-nordic commented Nov 23, 2020

pabigot left a comment

Choose a reason for hiding this comment

pabigot Dec 8, 2020

Choose a reason for hiding this comment

pabigot Dec 8, 2020

Choose a reason for hiding this comment

mbolivar-nordic Dec 8, 2020

Choose a reason for hiding this comment

pabigot Dec 8, 2020

Choose a reason for hiding this comment

mbolivar-nordic Dec 8, 2020

Choose a reason for hiding this comment

pabigot Dec 8, 2020

Choose a reason for hiding this comment

mbolivar-nordic Dec 8, 2020

Choose a reason for hiding this comment

erwango commented Dec 9, 2020

erwango commented Dec 9, 2020

pabigot commented Dec 9, 2020

mbolivar-nordic commented Dec 10, 2020

mbolivar-nordic commented Dec 10, 2020

erwango commented Dec 10, 2020

mbolivar-nordic commented Jan 26, 2021