On // comments: a) I thought I had read recently that they are now accepted. b) Can we stop calling them "C++ style"? They have been legal in standard C for all of this millennium! There are university graduates who were born after // comments became legal in standard C. c) If we don't currently accept them, can we change the style now (see previous point)?
A response along the lines of "this is just moving the wiki into .rst files - we can actually change things separately" would seem entirely reasonable. Martin On Mon, 11 Jul 2022 at 19:15, Tom Rini <tr...@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. > > Cc: Heinrich Schuchardt <xypron.g...@gmx.de> > Signed-off-by: Tom Rini <tr...@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 > + > + * 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. > + > +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; > + > +* 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: > + > +.. 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 > +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: > + > +.. code-block:: C > + > + struct udevice *dev; > + > +Use ``ret`` as the return value: > + > +.. 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_ret() or log_msg_ret() to return a value (see above). > + > +Add a ``p`` suffix on return arguments: > + > +.. 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: > + > +.. 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: > + > +.. 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:: > + > + make check > + make qcheck (skips some tests) > + > +Python tests are in test/py/tests - see the docs in test/py for info. > + > +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() > +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 > +root of the U-Boot tree. > 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 > -------------- > > -- > 2.25.1 > >