[1/4] doc: Migrate CodingStyle wiki page to sphinx

Move the current CodingStyle wiki page to doc/develop/codingstyle.rst.
The changes here are for formatting or slight rewording so that it reads
well when linking to other sphinx documents.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 doc/develop/codingstyle.rst | 211 ++++++++++++++++++++++++++++++++++++
 doc/develop/index.rst       |   8 ++
 2 files changed, 219 insertions(+)
 create mode 100644 doc/develop/codingstyle.rst

On Mon, 27 Jun 2022 at 11:17, Tom Rini <trini@konsulko.com> wrote:
>
> Move the current CodingStyle wiki page to doc/develop/codingstyle.rst.
> The changes here are for formatting or slight rewording so that it reads
> well when linking to other sphinx documents.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  doc/develop/codingstyle.rst | 211 ++++++++++++++++++++++++++++++++++++
>  doc/develop/index.rst       |   8 ++
>  2 files changed, 219 insertions(+)
>  create mode 100644 doc/develop/codingstyle.rst
>

Reviewed-by: Simon Glass <sjg@chromium.org>

On 6/27/22 19:17, Tom Rini wrote:
> Move the current CodingStyle wiki page to doc/develop/codingstyle.rst.
> The changes here are for formatting or slight rewording so that it reads
> well when linking to other sphinx documents.

%s/sphinx/Sphinx/

>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>   doc/develop/codingstyle.rst | 211 ++++++++++++++++++++++++++++++++++++
>   doc/develop/index.rst       |   8 ++
>   2 files changed, 219 insertions(+)
>   create mode 100644 doc/develop/codingstyle.rst
>
> diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst
> new file mode 100644
> index 000000000000..a41aed2764fb
> --- /dev/null
> +++ b/doc/develop/codingstyle.rst
> @@ -0,0 +1,211 @@
> +.. SPDX-License-Identifier: GPL-2.0+:
> +
> +U-Boot Coding Style
> +===================
> +
> +The following Coding Style requirements shall be mandatory for all code contributed to
> +the U-Boot project.
> +
> +Exceptions are only allowed if code from other projects is integrated with no
> +or only minimal changes.
> +
> +The following rules apply:
> +
> +* All contributions to U-Boot should conform to the `Linux kernel
> +  coding style <https://www.kernel.org/doc/html/latest/process/coding-style.html>`_
> +  and the `Linent script <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/Lindent>`_.

%s/Linent/Lindent/g

Please, use blank lines between bullets.
Cf. https://sublime-and-sphinx-guide.readthedocs.io/en/latest/lists.html

Please, mention explicitly that all structures, enumerations, unions and
functions shall be described according to
https://docs.kernel.org/doc-guide/kernel-doc.html

> +  * The exception for net files to the `multi-line comment
> +  <https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting>`_

Incorrect indentation (add 2 spaces).

> +  applies only to Linux, not to U-Boot. Only large hunks which are copied
> +  unchanged from Linux may retain that comment format.

blank line missing

> +* Use patman to send your patches (``tools/patman/patman -H`` for full
> +  instructions). With a few tags in your commits this will check your patches
> +  and take care of emailing them.

ditto

> +* If you don't use patman, make sure to run ``scripts/checkpatch.pl``.  For
> +  more information, read :doc:`checkpatch`.  Note that this should be done
> +  *before* posting on the mailing list!

ditto

> +* Source files originating from different projects (for example the MTD
> +  subsystem or the hush shell code from the BusyBox project) may, after
> +  careful consideration, be exempted from these rules. For such files, the
> +  original coding style may be kept to ease subsequent migration to newer
> +  versions of those sources.

ditto

> +* Please note that U-Boot is implemented in C (and to some small parts in
> +  Assembler); no C++ is used, so please do not use C++ style comments (//) in
> +  your code.
> +
> +  * The sole exception here is for SPDX tags in some files (checkpatch.pl will warn you).
> +
> +* Please also stick to the following formatting rules:
> +
> +  * Remove any trailing white space

ditto

> +  * Use TAB characters for indentation and vertical alignment, not spaces
> +  * Make sure NOT to use DOS ``\r\n`` line feeds
> +  * Do not add more than 2 consecutive empty lines to source files
> +  * Do not add trailing empty lines to source files
> +  * Using the option ``git config --global color.diff auto`` will help to
> +    visually see whitespace problems in ``diff`` output from ``git``.
> +  * In Emacs one can use ``=M-x whitespace-global-mode=`` to get visual
> +    feedback on the nasty details. ``=M-x whitespace-cleanup=`` does The Right
> +    Thing (tm)
> +
> +Submissions of new code or patches that do not conform to these requirements
> +shall be rejected with a request to reformat the changes.
> +
> +U-Boot Code Documentation
> +-------------------------
> +
> +U-Boot adopted the kernel-doc annotation style, this is the only exception from
> +multi-line comment rule of Coding Style. While not mandatory, adding
> +documentation is strongly advised. The Linux kernel `kernel-doc <https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html>`_ documentation applies with no changes.

Developers tend to read the rst files with text editors. Please, keep
lines short (80 characters). `kernel-doc can go onto the next line.

> +applies with no changes.
> +
> +Use structures for I/O access
> +-----------------------------
> +
> +U-Boot typically uses a C structure to map out the registers in an I/O region, rather than offsets. The reasons for this are:
> +
> +* It dissociates the register location (offset) from the register type, which
> +  means the developer has to make sure the type is right for each access,
> +  whereas with the struct method, this is checked by the compiler;

Please, add blank lines between bullets.

> +* It avoids actually writing all offsets, which is (more) error- prone;
> +* It allows for better compile time sanity-checking of values we write to registers.
> +
> +Some reasons why you might not use C structures:
> +
> +* Where the registers appear at different offsets in different hardware
> +  revisions supported by the same driver
> +* Where the driver only uses a small subset of registers and it is not worth
> +  defining a struct to cover them all, with large empty regions
> +* Where the offset of a register might be hard to figure out when buried a long
> +  way down a structure, possibly with embedded sub-structures
> +* This may need to change to the kernel model if we allow for more run-time
> +  detection of what drivers are appropriate for what we're running on.
> +
> +Please use check_member() to verify that your structure is the expected size, or that particular members appear at the right offset.
> +
> +Include files
> +-------------
> +
> +You should follow this ordering in U-Boot. The common.h header (which is going away at some point) should always be first, followed by other headers in order, then headers with directories, then local files::

Please, allow syntax highlighting:

%s/::/:/

.. code-block:: C
> +
> +   <common.h>

Please, add #include to each line.

> +   <bootstage.h>
> +   <dm.h>
> +   <others.h>
> +   <asm/...>
> +   <arm/arch/...>
> +   <dm/device_compat/.h>
> +   <linux/...>
> +   "local.h"
> +
> +Within that order, sort your includes.
> +
> +It is important to include common.h first since it provides basic features used by most files, e.g. CONFIG options.
> +
> +For files that need to be compiled for the host (e.g. tools), you need to use ``#ifndef USE_HOSTCC`` to avoid including common.h since it includes a lot of internal U-Boot things. See common/image.c for an example.

Please, try to keep lines at 80 characters each.

> +
> +If your file uses driver model, include <dm.h> in the C file. Do not include dm.h in a header file. Try to use forward declarations (e.g. ``struct udevice``) instead.
> +
> +Filenames
> +---------
> +
> +For .c and .h files try to use underscore rather than hyphen unless you want the file to stand out (e.g. driver-model uclasses should be named xxx-uclass.h. Avoid upper case and keep the names fairly short.
> +
> +Function and struct comments
> +----------------------------
> +
> +Non-trivial functions should have a comment which describes what they do. If it is an exported function, put the comment in the header file so the API is in one place. If it is a static function, put it in the C file.
> +
> +If the function returns errors, mention that and list the different errors that are returned. If it is merely passing errors back from a function it calls, then you can skip that.
> +
> +See `here <https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation>`_ for style.
> +
> +Driver model
> +------------
> +
> +When declaring a device, try to use ``struct udevice *dev``, i.e. ``dev`` as the name::
> +
> +   struct udevice *dev;
> +
> +Use ``ret`` as the return value::

%s/::/:/

.. code-block:: C

> +
> +   struct udevice *dev;
> +   int ret;
> +
> +   ret = uclass_first_device_err(UCLASS_ACPI_PMC, &dev);
> +   if (ret)
> +           return log_msg_ret("pmc", dev);
> +
> +Consider using log_reg() or log_msg_ret() to return a value (see above)

%s/log_reg/log_ret/

> +
> +Add a ``p`` suffix on return arguments::
> +

%s/::/:/

.. code-block:: C

> +   int dm_pci_find_class(uint find_class, int index, struct udevice **devp)
> +   {
> +   ...
> +           *devp = dev;
> +
> +           return 0;
> +   }
> +
> +There are standard variable names that you should use in drivers:
> +
> +* ``struct xxx_priv`` and ``priv`` for dev_get_priv()
> +* ``struct xxx_plat`` and ``plat`` for dev_get_platdata()
> +
> +For example::
> +

%s/::/:/

.. code-block:: C

> +   struct simple_bus_plat {
> +      u32 base;
> +      u32 size;
> +      u32 target;
> +   };
> +
> +   /* Davinci MMC board definitions */
> +   struct davinci_mmc_priv {
> +      struct davinci_mmc_regs *reg_base;   /* Register base address */
> +      uint input_clk;      /* Input clock to MMC controller */
> +      struct gpio_desc cd_gpio;       /* Card Detect GPIO */
> +      struct gpio_desc wp_gpio;       /* Write Protect GPIO */
> +   };
> +
> +      struct rcar_gpio_priv *priv = dev_get_priv(dev);
> +
> +      struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
> +
> +Other
> +-----
> +
> +Some minor things:
> +
> +* Put a blank line before the last ``return`` in a function unless it is the only line::
> +

%s/::/:/

.. code-block:: C

> +   struct udevice *pci_get_controller(struct udevice *dev)
> +   {
> +      while (device_is_on_pci_bus(dev))
> +         dev = dev->parent;
> +
> +      return dev;
> +   }
> +
> +Tests
> +-----
> +
> +
> +Please add tests when you add code. Please change or expand tests when you change code.
> +
> +Run the tests with::
> +

%s/::/:/

.. code-block:: bash

> +   make check
> +   make qcheck   (skips some tests)
> +
> +Python tests are in test/py/tests - see the docs in test/py for info.

Please, add a reference to doc/develop/tests_writing.rst

> +
> +Try to write your tests in C if you can. For example, tests to check a command
> +will be much faster (10-100x or more) if they can directly call run_command()

%s/10-100x/10 - 100x/
Please, avoid duplicating information from tests_writing.rst.

> +and ut_check_console_line() instead of using Python to send commands over a
> +pipe to U-Boot.
> +
> +Tests run all supported CI systems (gitlab, travis, azure) using scripts in the

%s/gitlab/Gitlab/
%s/azure/Azure/

We don't use Travis CI anymore.

> +root of the U-Boot tree.
> +

Please, avoid empty line at end of file.

Best regards

Heinrich

> diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> index fe3564a9fbf4..dde47994c71a 100644
> --- a/doc/develop/index.rst
> +++ b/doc/develop/index.rst
> @@ -3,6 +3,14 @@
>   Develop U-Boot
>   ==============
>
> +General
> +-------
> +
> +.. toctree::
> +   :maxdepth: 1
> +
> +   codingstyle
> +
>   Implementation
>   --------------
>
a

On 6/30/22 12:06, Simon Glass wrote:
> On Mon, 27 Jun 2022 at 11:17, Tom Rini <trini@konsulko.com> wrote:
>>
>> Move the current CodingStyle wiki page to doc/develop/codingstyle.rst.
>> The changes here are for formatting or slight rewording so that it reads
>> well when linking to other sphinx documents.
>>
>> Signed-off-by: Tom Rini <trini@konsulko.com>
>> ---
>>   doc/develop/codingstyle.rst | 211 ++++++++++++++++++++++++++++++++++++
>>   doc/develop/index.rst       |   8 ++
>>   2 files changed, 219 insertions(+)
>>   create mode 100644 doc/develop/codingstyle.rst
>>
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Please, don't add Reviewed-by if you did not have the time to thoroughly
review the patch. Acked-by is more adequate in this case.

Best regards

Heinrich

On Sat, Jul 09, 2022 at 08:12:27AM +0200, Heinrich Schuchardt wrote:
> On 6/27/22 19:17, Tom Rini wrote:
> > Move the current CodingStyle wiki page to doc/develop/codingstyle.rst.
> > The changes here are for formatting or slight rewording so that it reads
> > well when linking to other sphinx documents.
> 
> %s/sphinx/Sphinx/

For the wiki->rST changes, OK, I'll do v2 (for the series).  For
everything else, I would prefer to do the import as-is as much as
possible and then start making real changes as follow-up patches.

On Sat, Jul 09, 2022 at 08:12:27AM +0200, Heinrich Schuchardt wrote:
> On 6/27/22 19:17, Tom Rini wrote:
> > Move the current CodingStyle wiki page to doc/develop/codingstyle.rst.
> > The changes here are for formatting or slight rewording so that it reads
> > well when linking to other sphinx documents.
[snip]
> > +U-Boot adopted the kernel-doc annotation style, this is the only exception from
> > +multi-line comment rule of Coding Style. While not mandatory, adding
> > +documentation is strongly advised. The Linux kernel `kernel-doc <https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html>`_ documentation applies with no changes.
> 
> Developers tend to read the rst files with text editors. Please, keep
> lines short (80 characters). `kernel-doc can go onto the next line.

Linux doesn't have a hard 80 character limit anymore and neither do we,
keep in mind.

> > +applies with no changes.
> > +
> > +Use structures for I/O access
> > +-----------------------------
> > +
> > +U-Boot typically uses a C structure to map out the registers in an I/O region, rather than offsets. The reasons for this are:
> > +
> > +* It dissociates the register location (offset) from the register type, which
> > +  means the developer has to make sure the type is right for each access,
> > +  whereas with the struct method, this is checked by the compiler;
> 
> Please, add blank lines between bullets.

Different than markdown, OK.

[snip]
> > +Tests
> > +-----
> > +
> > +
> > +Please add tests when you add code. Please change or expand tests when you change code.
> > +
> > +Run the tests with::
> > +
> 
> %s/::/:/
> 
> .. code-block:: bash
> 
> > +   make check
> > +   make qcheck   (skips some tests)
> > +
> > +Python tests are in test/py/tests - see the docs in test/py for info.
> 
> Please, add a reference to doc/develop/tests_writing.rst
> 
> > +
> > +Try to write your tests in C if you can. For example, tests to check a command
> > +will be much faster (10-100x or more) if they can directly call run_command()
> 
> %s/10-100x/10 - 100x/
> Please, avoid duplicating information from tests_writing.rst.
> 
> > +and ut_check_console_line() instead of using Python to send commands over a
> > +pipe to U-Boot.
> > +
> > +Tests run all supported CI systems (gitlab, travis, azure) using scripts in the
> 
> %s/gitlab/Gitlab/
> %s/azure/Azure/
> 
> We don't use Travis CI anymore.

This is more substantive than wiki->rST so I'll handle this in another
patch.