[PATCH 1/7] doc: Migrate CodingStyle wiki page to Sphinx

Wed Jul 13 19:41:52 CEST 2022

On Wed, Jul 13, 2022 at 07:06:56PM +0200, Heinrich Schuchardt wrote:
> On 7/11/22 19:14, 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.
> > 
> > Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Signed-off-by: Tom Rini <trini at konsulko.com>
> > ---
> > Changes in v2:
> > - Assorted wiki -> Sphinx style corrections and a few typo fixes, per
> >    Heinrich
> > ---
> >   doc/develop/codingstyle.rst | 255 ++++++++++++++++++++++++++++++++++++
> >   doc/develop/index.rst       |   8 ++
> >   2 files changed, 263 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..bbeca42e656b
> > --- /dev/null
> > +++ b/doc/develop/codingstyle.rst
> > @@ -0,0 +1,255 @@
> > +.. 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 `Lindent script <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/Lindent>`_.
> > +  * The exception for net files to the `multi-line comment
> > +  <https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting>`_
> > +  applies only to Linux, not to U-Boot. Only large hunks which are copied
> > +  unchanged from Linux may retain that comment format.
> > +
> > +* 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.
> > +
> > +* 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!
> > +
> > +* 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.
> > +
> > +* 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
> > +
> > +  * Use TAB characters for indentation and vertical alignment, not spaces
> 
> This only holds true for C and assembler code.
> For Python we use 4 spaces per indent.
> 
> > +
> > +  * Make sure NOT to use DOS ``\r\n`` line feeds
> 
> %s/DOS/Windows/ - The documentation is not dedicated to veterans.
> %s/line feeds/line ends/ - a line feed is always 0x0a.
> 
> \r\n are escape codes in C. On a Windows machine these will be rendered
> as CR CR LF.
> 
> Please, use CR LF instead of \r\n.
> 
> > +
> > +  * 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)
> 
> No clue why 'The Right Thing' should be a trademark here.
> Please, remove this bogus.

The phrase "The Right Thing (tm)" is I guess an old joke, as it implies
it will probably work, but still might mess up from time to time.  I'd
rather keep it.

> > +
> > +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
> 
> Do you mean the extra asterisk?

Yes.

> > +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.
> > +
> > +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;
> > +
> > +* It avoids actually writing all offsets, which is (more) error- prone;
> 
> %s/error- prone/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,
> 
> %s/check_member()/the check_member() macro/

Since I'm in here anyhow, sure.

> > +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:
> > +
> > +.. code-block:: C
> > +
> > +   #include <common.h>
> > +   #include <bootstage.h>
> > +   #include <dm.h>
> > +   #include <others.h>
> > +   #include <asm/...>
> > +   #include <arm/arch/...>
> > +   #include <dm/device_compat/.h>
> > +   #include <linux/...>
> > +   #include "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.
> > +
> > +If your file uses driver model, include <dm.h> in the C file. Do not include
> 
> %s/uses/uses the/

We more consistently say "uses driver model" (9 times) not "uses the
driver model" (1 time).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220713/c7e1424b/attachment.sig>