[PATCH 1/1] doc: add system reset to API documentation

Heinrich Schuchardt heinrich.schuchardt at canonical.com
Fri Sep 24 12:50:19 CEST 2021


On 9/24/21 7:51 AM, Sean Anderson wrote:
> On 9/24/21 1:27 AM, Alexandre Ghiti wrote:
>> On Thu, Sep 23, 2021 at 11:12 AM Heinrich Schuchardt
>> <heinrich.schuchardt at canonical.com> wrote:
>>>
>>> Complete the Sphinx documentation in include/sysreset.h
>>> Add the include to the generated HTML documentation of the U-Boot API.
>>>
>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>>> ---
>>>   doc/api/index.rst    |  1 +
>>>   doc/api/sysreset.rst |  7 ++++++
>>>   include/sysreset.h   | 53 ++++++++++++++++++++++++++++----------------
>>>   3 files changed, 42 insertions(+), 19 deletions(-)
>>>   create mode 100644 doc/api/sysreset.rst
>>>
>>> diff --git a/doc/api/index.rst b/doc/api/index.rst
>>> index ea02aa5715..281d1dca96 100644
>>> --- a/doc/api/index.rst
>>> +++ b/doc/api/index.rst
>>> @@ -15,5 +15,6 @@ U-Boot API documentation
>>>      rng
>>>      sandbox
>>>      serial
>>> +   sysreset
>>>      timer
>>>      unicode
>>> diff --git a/doc/api/sysreset.rst b/doc/api/sysreset.rst
>>> new file mode 100644
>>> index 0000000000..a51b06c387
>>> --- /dev/null
>>> +++ b/doc/api/sysreset.rst
>>> @@ -0,0 +1,7 @@
>>> +.. SPDX-License-Identifier: GPL-2.0+
>>> +
>>> +System reset
>>> +============
>>> +
>>> +.. kernel-doc:: include/sysreset.h
>>> +   :internal:
>>> diff --git a/include/sysreset.h b/include/sysreset.h
>>> index 701e4f5c86..e8411f9664 100644
>>> --- a/include/sysreset.h
>>> +++ b/include/sysreset.h
>>> @@ -9,43 +9,55 @@
>>>
>>>   struct udevice;
>>>
>>> +/**
>>> + * enum systreset_t - system reset types
>>> + */
>>>   enum sysreset_t {
>>> -       SYSRESET_WARM,  /* Reset CPU, keep GPIOs active */
>>> -       SYSRESET_COLD,  /* Reset CPU and GPIOs */
>>> -       SYSRESET_POWER, /* Reset PMIC (remove and restore power) */
>>> -       SYSRESET_POWER_OFF,     /* Turn off power */
>>> -
>>> +       /** @SYSRESET_WARM: reset CPU, keep GPIOs active */
>>> +       SYSRESET_WARM,
>>> +       /** @SYSRESET_COLD: reset CPU and GPIOs */
>>> +       SYSRESET_COLD,
>>> +       /** @SYSRESET_POWER: reset PMIC (remove and restore power) */
>>> +       SYSRESET_POWER,
>>> +       /** @SYSRESET_POWER_OFF: turn off power */
>>> +       SYSRESET_POWER_OFF,
>>> +       /** @SYSRESET_COUNT: number of available reset types */
>>>          SYSRESET_COUNT,
>>>   };
>>>
>>> +/**
>>> + * struct sysreset_ops - operations of system reset drivers
>>> + */
>>>   struct sysreset_ops {
>>>          /**
>>> -        * request() - request a sysreset of the given type
>>> +        * @request:    request a sysreset of the given type
>>>           *
>>>           * Note that this function may return before the reset takes 
>>> effect.
>>>           *
>>> +        * @dev:        Device to be used for system reset
>>>           * @type:       Reset type to request
>>> -        * @return -EINPROGRESS if the reset has been started and
>>> -        *              will complete soon, -EPROTONOSUPPORT if not 
>>> supported
>>> -        *              by this device, 0 if the reset has already 
>>> happened
>>> -        *              (in which case this method will not actually 
>>> return)
>>> +        * Return:
>>
>> Shouldn't this be @Return (and for all others below too)? Otherwise
>> the output of "Result" is not bold.
>> And I also noticed that all parameters and return descriptions appear
>> on the same line: include/dm/pinctrl.h adds newlines between each
>> parameter description and then every argument has its own line.
> 
> The issue here is that the recommended style [1] does not work well for
> struct members. To get things looking reasonably you have to mess with
> the formatting a bit. An alternative is to put a short description in
> the struct itself, and add a longer version in ifdef'd out "virtual"
> functions, such as in [2].

We should stick with [1]. If the formatting is not correct, it is a 
problem with the Sphinx C-domain.

We inherited doc/sphinx/parse-headers.pl from Linux. I guess this is the 
code which needs to be fixed.

Best regards

Heinrich

> 
> --Sean
> 
> [1] 
> https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#return-values 
> 
> [2] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/phylink.h#n81 
> 
> 
>>> +        * -EINPROGRESS if the reset has been started and
>>> +        * will complete soon, -EPROTONOSUPPORT if not supported
>>> +        * by this device, 0 if the reset has already happened
>>> +        * (in which case this method will not actually return)
>>>           */
>>>          int (*request)(struct udevice *dev, enum sysreset_t type);
>>>          /**
>>> -        * get_status() - get printable reset status information
>>> +        * @get_status: get printable reset status information
>>>           *
>>>           * @dev:        Device to check
>>>           * @buf:        Buffer to receive the textual reset information
>>>           * @size:       Size of the passed buffer
>>> -        * @return 0 if OK, -ve on error
>>> +        * Return:      0 if OK, -ve on error
>>>           */
>>>          int (*get_status)(struct udevice *dev, char *buf, int size);
>>>
>>>          /**
>>> -        * get_last() - get information on the last reset
>>> +        * @get_last:   get information on the last reset
>>>           *
>>>           * @dev:        Device to check
>>> -        * @return last reset state (enum sysreset_t) or -ve error
>>> +        * Return:      last reset state (enum :enum:`sysreset_t`) or 
>>> -ve error
>>>           */
>>>          int (*get_last)(struct udevice *dev);
>>>   };
>>> @@ -55,8 +67,9 @@ struct sysreset_ops {
>>>   /**
>>>    * sysreset_request() - request a sysreset
>>>    *
>>> + * @dev:       Device to be used for system reset
>>>    * @type:      Reset type to request
>>> - * @return 0 if OK, -EPROTONOSUPPORT if not supported by this device
>>> + * Return:     0 if OK, -EPROTONOSUPPORT if not supported by this 
>>> device
>>>    */
>>>   int sysreset_request(struct udevice *dev, enum sysreset_t type);
>>>
>>> @@ -66,7 +79,7 @@ int sysreset_request(struct udevice *dev, enum 
>>> sysreset_t type);
>>>    * @dev:       Device to check
>>>    * @buf:       Buffer to receive the textual reset information
>>>    * @size:      Size of the passed buffer
>>> - * @return 0 if OK, -ve on error
>>> + * Return:      0 if OK, -ve on error
>>>    */
>>>   int sysreset_get_status(struct udevice *dev, char *buf, int size);
>>>
>>> @@ -74,7 +87,7 @@ int sysreset_get_status(struct udevice *dev, char 
>>> *buf, int size);
>>>    * sysreset_get_last() - get information on the last reset
>>>    *
>>>    * @dev:       Device to check
>>> - * @return last reset state (enum sysreset_t) or -ve error
>>> + * Return:     last reset state (enum sysreset_t) or -ve error
>>>    */
>>>   int sysreset_get_last(struct udevice *dev);
>>>
>>> @@ -88,7 +101,7 @@ int sysreset_get_last(struct udevice *dev);
>>>    * If this function fails to reset, it will display a message and halt
>>>    *
>>>    * @type:      Reset type to request
>>> - * @return -EINPROGRESS if a reset is in progress, -ENOSYS if not 
>>> available
>>> + * Return:     -EINPROGRESS if a reset is in progress, -ENOSYS if 
>>> not available
>>>    */
>>>   int sysreset_walk(enum sysreset_t type);
>>>
>>> @@ -101,7 +114,7 @@ int sysreset_walk(enum sysreset_t type);
>>>    *
>>>    * If no device prives the information, this function returns -ENOENT
>>>    *
>>> - * @return last reset state (enum sysreset_t) or -ve error
>>> + * Return:     last reset state (enum sysreset_t) or -ve error
>>>    */
>>>   int sysreset_get_last_walk(void);
>>>
>>> @@ -110,6 +123,8 @@ int sysreset_get_last_walk(void);
>>>    *
>>>    * This calls sysreset_walk(). If it returns, indicating that reset 
>>> is not
>>>    * supported, it prints a message and halts.
>>> + *
>>> + * @type:      Reset type to request
>>>    */
>>>   void sysreset_walk_halt(enum sysreset_t type);
>>>
>>> -- 
>>> 2.32.0
>>>
> 



More information about the U-Boot mailing list