From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5FE4EC433EF for ; Sat, 9 Jul 2022 06:12:36 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1659D844F1; Sat, 9 Jul 2022 08:12:34 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=gmx.net header.i=@gmx.net header.b="OBQ2F6xR"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id ECD73844F5; Sat, 9 Jul 2022 08:12:32 +0200 (CEST) Received: from mout.gmx.net (mout.gmx.net [212.227.17.20]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id C13C7844EB for ; Sat, 9 Jul 2022 08:12:29 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xypron.glpk@gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1657347148; bh=iXE+nfz2gIE1JpqAYv2PJjrtmHmR/b9M5rcKk+kKPH0=; h=X-UI-Sender-Class:Date:Subject:To:References:From:Cc:In-Reply-To; b=OBQ2F6xRAJjy7am9d4nl976lbGAPrLNGjrMy6igEE95Lwt7uuO1RU80wtmuz5oG3z PiFg2mMzu9hpwzyOJWO6ZxUbW9wg4v7G9KUGW1EA25LIcK5a+X+0Qz7oNBaeowg19N dilt12yJvdjei8AblEGQtKcL+lM3OUhCEex3NCFY= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.94] ([62.143.94.109]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1N6KYb-1nUCdG42fZ-016itQ; Sat, 09 Jul 2022 08:12:28 +0200 Message-ID: Date: Sat, 9 Jul 2022 08:12:27 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.0.1 Subject: Re: [PATCH 1/4] doc: Migrate CodingStyle wiki page to sphinx To: Tom Rini References: <20220627171722.1153337-1-trini@konsulko.com> <20220627171722.1153337-2-trini@konsulko.com> Content-Language: en-US From: Heinrich Schuchardt Cc: Simon Glass , u-boot@lists.denx.de In-Reply-To: <20220627171722.1153337-2-trini@konsulko.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:6JOGGAE43VdUahKShiLgmTIBJ4w9DicVIVgofrF2jlVHIrygkhq 3v5xd3BWqsAPJhojIs72TI8gQObe2P5nbnEOzqYGaO5uRqT7wnmh0Llc00zewIvzlzxZnmn chq8IIjkhQB7pXGpMG71REPRp3afFsFy0AWpvpHC5ZArDAoyvYSYyhWVd420tiDo8xTD9Wl UlcVOVCv2khdqxSB6yOnA== X-UI-Out-Filterresults: notjunk:1;V03:K0:+qMWVjRrRyo=:wFWaqtfayLzeRkV6zkoufc 5TMsYTJ7gVjlqpLeDFOKpGAsmvZgQG0zEpCVMGU8gKE3tRQT4g3N21Zj0TEOgTEHNxiPrdEE6 g86yaMm3cvkVwR701HN7HdNmZ/lOYEHw2FGvvDwOjgleqLyQ9GTL7J9w26RDl/4+JBokZ50Yx vmy7mx6wWRE3LV0pMjf4awbO4rtnTW7ab/rSkK7RRE1Ioqaq9B0Tzyd6/rvSViO9O91XjDyBp 7SftmAlCa8kBf2+p5ZumnVFL6i90R+TZ+mlKFWWdhvAb9y214HpSi2VTHtfSzEvqQwla+5wrX 44vKR18bti2WQacmtcIeAE/CK1sYqHRR9sHhaVoFwE0+G8XnITz8RssFqXDBRzCNPc8kV/S14 SVNkkCg/M1pZpsdJB8hla+Y0or9tZZv7RurVxjNxqYeFIQmU/cjhJGTdPg4L20+bd/aTLzplP TuygNy1Cd8VBNgyV4yS07okR/3SkC9lzhF6CxfjY9L4w+EhiTmMz1mXlCu7h8FFoKhV6EIWdz 67B4qszstddmPH+GZUewJ8JPq1ux45mI8YDSd8PC5yIlSCqUfqJWGWUbpAVdFNxnpP/bEYfJA rCLO0bJWjXuvHK3Ue+zRbiUAqG8IJATZWrELa8MOQVOQLSGEBPMU5bvOYdZUc2f3BkmFmHKGi vAqCLs9fZHbIpayrBUrPm4+TlpYfuPvmZ5JBlNy5NUp49h6dNuZdMMohPKL3Fn9XmigOwluar dV0pqjtryhGjAd9fk1TSAD/v0XzvCsUVaQUkhKTRj5pPl38py3KrOSi3J/L9cCOE6haja7OL0 mJ1NscUPyH1CYvRblZaBmH7pHSpaf/qCUNA7Kmhg0S07aBegDgeUGKPrSzOp1w3kozTiRGtq1 sw4mY1lTzeh+k+brov4UcJlBIz/8cExpVxfmRlYgwZya2y5s4HOzr1Xn/pbYNjZ370bqzaTQF mnwcAsuDkEz0/iWYb1gvtlOaDQxGVLrvn+QppohveJv1r6WZkmZVUX1Ci30IVcam6ALTVG1sb 8JSIvRHWKCrMgMen8BXmZA0y7T7WugAU1y4TqB7Rorma54a1HRH088XyjDeIrPKsZ5yR3Ejve yR+TeW+SDWDe9D23AKlV45YBdHBDpZBlMnZ6vZjKXvMJ3Qn0bY5cHAHFQ== X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean 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 > --- > 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 > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > + > +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 w= ith no > +or only minimal changes. > + > +The following rules apply: > + > +* All contributions to U-Boot should conform to the `Linux kernel > + coding style `_ > + and the `Linent script `_. %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 > + `_ Incorrect indentation (add 2 spaces). > + applies only to Linux, not to U-Boot. Only large hunks which are copi= ed > + 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 p= atches > + 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 d= one > + *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 new= er > + 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 spac= es > + * 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 ``=3DM-x whitespace-global-mode=3D`` to get vi= sual > + feedback on the nasty details. ``=3DM-x whitespace-cleanup=3D`` doe= s The Right > + Thing (tm) > + > +Submissions of new code or patches that do not conform to these require= ments > +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 except= ion from > +multi-line comment rule of Coding Style. While not mandatory, adding > +documentation is strongly advised. The Linux kernel `kernel-doc `_ documentatio= n 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 acces= s, > + 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 burie= d 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 i= s going away at some point) should always be first, followed by other head= ers in order, then headers with directories, then local files:: Please, allow syntax highlighting: %s/::/:/ .. code-block:: C > + > + Please, add #include to each line. > + > + > + > + > + > + > + > + "local.h" > + > +Within that order, sort your includes. > + > +It is important to include common.h first since it provides basic featu= res 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 include= s 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 in the C file. Do not in= clude dm.h in a header file. Try to use forward declarations (e.g. ``struc= t 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 xx= x-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 d= o. If it is an exported function, put the comment in the header file so th= e 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 err= ors that are returned. If it is merely passing errors back from a function= it calls, then you can skip that. > + > +See `here `_ 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 =3D 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 **d= evp) > + { > + ... > + *devp =3D 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 =3D dev_get_priv(dev); > + > + struct pl01x_serial_platdata *plat =3D dev_get_platdata(dev); > + > +Other > +----- > + > +Some minor things: > + > +* Put a blank line before the last ``return`` in a function unless it i= s the only line:: > + %s/::/:/ .. code-block:: C > + struct udevice *pci_get_controller(struct udevice *dev) > + { > + while (device_is_on_pci_bus(dev)) > + dev =3D 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_com= mand() %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 ov= er a > +pipe to U-Boot. > + > +Tests run all supported CI systems (gitlab, travis, azure) using script= s 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 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > +General > +------- > + > +.. toctree:: > + :maxdepth: 1 > + > + codingstyle > + > Implementation > -------------- > a