summaryrefslogtreecommitdiff
path: root/doc/develop/codingstyle.rst
blob: 013bfebf7e4806565065f7d152c8233c69e27903 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
.. 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>`_.
  U-Boot includes a `.clang-format` configuration file that can be used to
  automatically format code according to these standards.
  * 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.

* Python code shall conform to `PEP8 (Style Guide for Python Code)
  <https://peps.python.org/pep-0008/>`_. Use `pylint
  <https://github.com/pylint-dev/pylint>`_ for checking the code.

* Use the `b4 <https://git.kernel.org/pub/scm/utils/b4/b4.git/>`_ tool to prepare and
  send your patches. b4 has become the preferred tool to sending patches for many
  Linux kernel contributors, and U-Boot ships with a ready-to-use ``.b4-config`` that
  targets ``u-boot@lists.denx.de`` and integrates with ``scripts/get_maintainer.pl`` for
  recipient discovery.

  Start a topical series with ``b4 prep`` and keep the commits organised with
  ``git rebase -i``. ``b4 prep --edit-cover`` opens an editor for the cover
  letter, while ``b4 prep --auto-to-cc`` collects reviewers and maintainers from
  both the configuration file and ``scripts/get_maintainer.pl``.

  .. code-block:: bash

     b4 prep -n mmc-fixes
     git rebase -i origin/master
     b4 prep --edit-cover
     b4 prep --auto-to-cc

  Run the style checks before sending. ``b4 prep --check`` wraps the existing
  tooling so you see the output from ``scripts/checkpatch.pl`` alongside b4's
  own validation. You can always invoke ``scripts/checkpatch.pl`` directly for
  additional runs.

  .. code-block:: bash

     b4 prep --check

  When the series is ready, use ``b4 send``. Begin with ``--dry-run`` to review
  the generated emails and ``--reflect`` to copy yourself for records before
  dispatching to ``u-boot@lists.denx.de``.

  .. code-block:: bash

     b4 send --dry-run
     b4 send --reflect
     b4 send

  After reviews arrive, collect Acked-by/Tested-by tags with ``b4 trailers -u``
  and fold them into your commits before resending the updated series.

  .. code-block:: bash

     b4 trailers -u
     git rebase -i origin/master
     b4 send

* Run ``scripts/checkpatch.pl`` directly or via ``b4 prep --check`` so that all
  issues are resolved *before* posting on the mailing list. For more information,
  read :doc:`checkpatch`.

Code Formatting with clang-format
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

U-Boot provides a `.clang-format` configuration file that was copied directly
from the Linux kernel without any modifications, ensuring complete compatibility
with kernel coding standards. Here are common ways to use clang-format:

**Basic usage for single files:**

.. code-block:: bash

   clang-format -style=file -i <file>

**Format multiple files:**

.. code-block:: bash

   find . -name '*.c' -o -name '*.h' | xargs clang-format -style=file -i

**Integration with git (format only staged changes):**

.. code-block:: bash

   git clang-format

**Editor integration examples:**

* **Vim/Neovim:** Install vim-clang-format plugin
* **Emacs:** Use clang-format.el
* **VSCode:** Install "Clang-Format" extension
* **Most IDEs:** Have built-in or plugin support for clang-format

The `.clang-format` file in the repository root ensures consistent formatting
across the entire codebase and aligns with Linux kernel coding standards.

**Disabling clang-format for specific code blocks:**

In some cases, you may want to disable automatic formatting for specific code
sections, such as carefully formatted tables, assembly code, or imported code
from other projects. Use the following comments to control formatting:

.. code-block:: c

   // clang-format off
   static const struct register_config regs[] = {
       { 0x1000, 0x12345678 },  // Base address register
       { 0x1004, 0xabcdef00 },  // Control register
       { 0x1008, 0x00000001 },  // Status register
   };
   // clang-format on

**Controversial aspects of coding style enforcement:**

Coding style enforcement can be controversial, and it's difficult to have one
configuration that satisfies everyone's personal preferences. The goal of using
clang-format is consistency across the codebase rather than accommodating
individual preferences. While some developers may disagree with specific
formatting choices, maintaining a uniform style throughout the project makes
code more readable and maintainable for the entire development community.

**Best practices for formatting:**

When using clang-format to format code, consider these best practices:

* **Format only changed blocks:** It's preferred to format only the blocks of
  code that have been modified rather than entire files. This keeps diffs
  focused on actual changes and makes code reviews easier.

* **Separate formatting commits:** If you need to format entire files, create
  a separate commit containing only formatting changes. This allows reviewers
  to easily distinguish between functional changes and pure formatting updates.

* **Use git clang-format:** The ``git clang-format`` command is particularly
  useful as it formats only the lines that have been modified in your current
  changes, avoiding unnecessary formatting of unchanged code.

* 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 also stick to the following formatting rules:

  * Remove any trailing white space

  * Use TAB characters for indentation and vertical alignment, not spaces

    * The exception here is Python which requires 4 spaces instead.

  * All source files need to be in "Unix" and not "DOS" or "Windows" format,
    with respect to line ends.

  * 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.

Our Python code documentation follows `PEP257 (Docstring Conventions)
<https://peps.python.org/pep-0257/>`_.

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 the check_member() macro 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. In all cases, they should be listed
in alphabetical order. First comes headers which are located directly in our
top-level include diretory. Second are headers within subdirectories, Finally
directory-local includes should be listed. See this example:

.. code-block:: C

   #include <bootstage.h>
   #include <dm.h>
   #include <others.h>
   #include <asm/...>
   #include <asm/arch/...>
   #include <dm/device_compat.h>
   #include <linux/...>
   #include "local.h"

For files that need to be compiled for the host (e.g. tools), you need to use
``#ifndef USE_HOSTCC`` to avoid including U-Boot specific include files. 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.

Conditional Compilation
-----------------------

Wherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c
files; doing so makes code harder to read and logic harder to follow.  Instead,
use such conditionals in a header file defining functions for use in those .c
files, providing no-op stub versions in the #else case, and then call those
functions unconditionally from .c files.  The compiler will avoid generating
any code for the stub calls, producing identical results, but the logic will
remain easy to follow.

Prefer to compile out entire functions, rather than portions of functions or
portions of expressions.  Rather than putting an ifdef in an expression, factor
out part or all of the expression into a separate helper function and apply the
conditional to that function.

If you have a function or variable which may potentially go unused in a
particular configuration, and the compiler would warn about its definition
going unused, mark the definition as __maybe_unused rather than wrapping it in
a preprocessor conditional.  (However, if a function or variable *always* goes
unused, delete it.)

Within code, where possible, use the IS_ENABLED macro to convert a Kconfig
symbol into a C boolean expression, and use it in a normal C conditional:

.. code-block:: c

	if (IS_ENABLED(CONFIG_SOMETHING)) {
		...
	}

The compiler will constant-fold the conditional away, and include or exclude
the block of code just as with an #ifdef, so this will not add any runtime
overhead.  However, this approach still allows the C compiler to see the code
inside the block, and check it for correctness (syntax, types, symbol
references, etc).  Thus, you still have to use an #ifdef if the code inside the
block references symbols that will not exist if the condition is not met.

When working with xPL (see :doc:`spl` for more information) we need to take
further care to use the right macro. In the case where a symbol may be
referenced with an xPL-specific Kconfig symbol, use the CONFIG_IS_ENABLED macro
instead, in a similar manner:

.. code-block:: c

	if (CONIG_IS_ENABLED(SOMETHING)) {
		...
	}

When dealing with a Kconfig symbol that has both a normal name and one or more
xPL-prefixed names, the Makefile needs special consideration as well. The
PHASE\_ macro helps us in this situation thusly:

.. code-block:: make

        obj-$(CONFIG_$(PHASE_)SOMETHING) += something.o

At the end of any non-trivial #if or #ifdef block (more than a few lines),
place a comment after the #endif on the same line, noting the conditional
expression used.  For instance:

.. code-block:: c

	#ifdef CONFIG_SOMETHING
	...
	#endif /* CONFIG_SOMETHING */

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, Azure) using scripts in the root of
the U-Boot tree.