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