All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Migrate some wiki pages to sphinx
@ 2022-06-27 17:17 Tom Rini
  2022-06-27 17:17 ` [PATCH 1/4] doc: Migrate CodingStyle wiki page " Tom Rini
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Tom Rini @ 2022-06-27 17:17 UTC (permalink / raw)
  To: u-boot

Hey all,

As you might have seen, the DENX wiki has undergone some updates in the
last few months.  As of now, most pages have been restored and are up to
date.  But it's also true that the wiki long predates our sphinx
documentation and readthedocs site (which I am very happy the work has
been put in on making and improving).  So to that end, I'm trying to
move everything that's on the wiki over to sphinx so that it will be
easier to access and keep up to date.  To that end, this small series
includes 3 pages being moved.

I don't know what to do about ReleaseCycle and the statics pages long
term.  I have a few ideas, but they're tricker so not what I want to
tackle first.

I'm thinking the next page to tackle is
https://www.denx.de/wiki/U-Boot/Patches which might well end up being
more than one page once moved.

And there's still some outdated information in what I'm posting here,
I'm not sure if a follow-up updating it is needed now, or just when
someone has a chance.

-- 
Tom



^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/4] doc: Migrate CodingStyle wiki page to sphinx
  2022-06-27 17:17 [PATCH 0/4] Migrate some wiki pages to sphinx Tom Rini
@ 2022-06-27 17:17 ` Tom Rini
  2022-06-30 10:06   ` Simon Glass
  2022-07-09  6:12   ` Heinrich Schuchardt
  2022-06-27 17:17 ` [PATCH 2/4] doc: Migrate DesignPrinciples " Tom Rini
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 23+ messages in thread
From: Tom Rini @ 2022-06-27 17:17 UTC (permalink / raw)
  To: u-boot

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.

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>`_.
+  * 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.
+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::
+
+   <common.h>
+   <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.
+
+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::
+
+   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)
+
+Add a ``p`` suffix on return arguments::
+
+   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::
+
+   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::
+
+   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


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/4] doc: Migrate DesignPrinciples wiki page to sphinx
  2022-06-27 17:17 [PATCH 0/4] Migrate some wiki pages to sphinx Tom Rini
  2022-06-27 17:17 ` [PATCH 1/4] doc: Migrate CodingStyle wiki page " Tom Rini
@ 2022-06-27 17:17 ` Tom Rini
  2022-06-30 10:06   ` Simon Glass
                     ` (2 more replies)
  2022-06-27 17:17 ` [PATCH 3/4] doc: codingstyle: Remove comment about '//' style comments Tom Rini
  2022-06-27 17:17 ` [PATCH 4/4] doc: Migrate Process wiki page to sphinx Tom Rini
  3 siblings, 3 replies; 23+ messages in thread
From: Tom Rini @ 2022-06-27 17:17 UTC (permalink / raw)
  To: u-boot

Move the current DesignPrinciples wiki page to
doc/develop/designprinciples.rst.  The changes here are for formatting
or slight rewording so that it reads well when linking to other sphinx
documents.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 doc/develop/designprinciples.rst | 197 +++++++++++++++++++++++++++++++
 doc/develop/index.rst            |   1 +
 2 files changed, 198 insertions(+)
 create mode 100644 doc/develop/designprinciples.rst

diff --git a/doc/develop/designprinciples.rst b/doc/develop/designprinciples.rst
new file mode 100644
index 000000000000..79694db77604
--- /dev/null
+++ b/doc/develop/designprinciples.rst
@@ -0,0 +1,197 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+U-Boot Design Principles
+========================
+
+The 10 Golden Rules of U-Boot design
+------------------------------------
+
+Keep it Small
+^^^^^^^^^^^^^
+
+U-Boot is a Boot Loader, i.e. its primary purpose in the shipping
+system is to load some operating system.
+That means that U-Boot is
+necessary to perform a certain task, but it's nothing you want to
+throw any significant resources at. Typically U-Boot is stored in
+relatively small NOR flash memory, which is expensive
+compared to the much larger NAND devices often used to store the
+operating system and the application.
+
+At the moment, U-Boot supports boards with just 128 !KiB ROM or with
+256 !KiB NOR flash. We should not easily ignore such configurations -
+they may be the exception in among all the other supported boards,
+but if a design uses such a resource-constrained hardware setup it is
+usually because costs are critical, i. e. because the number of
+manufactured boards might be tens or hundreds of thousands or even
+millions...
+
+A usable and useful configuration of U-Boot, including a basic
+interactive command interpreter, support for download over Ethernet
+and the capability to program the flash shall fit in no more than 128 !KiB.
+
+Keep it Fast
+^^^^^^^^^^^^
+
+The end user is not interested in running U-Boot. In most embedded
+systems he is not even aware that U-Boot exists. The user wants to
+run some application code, and that as soon as possible after switching
+on his device.
+
+It is therefore essential that U-Boot is as fast as possible,
+especially that it loads and boots the operating system as fast as possible.
+
+To achieve this, the following design principles shall be followed:
+
+* Enable caches as soon and whenever possible
+* Initialize devices only when they are needed within U-Boot, i.e. don't
+  initialize the Ethernet interface(s) unless U-Boot performs a download over
+  Ethernet; don't  initialize any IDE or USB devices unless U-Boot actually
+  tries to load files from these, etc.  (and don't forget to shut down these
+  devices after using them  - otherwise nasty things may happen when you try to
+  boot your OS).
+
+Also, building of U-Boot shall be as fast as possible.
+This makes it easier to run a build for all supported configurations
+or at least for all configurations of a specific architecture,
+which is essential for quality assurance.
+If building is cumbersome and slow, most people will omit
+this important step.
+
+Keep it Simple
+^^^^^^^^^^^^^^
+
+U-Boot is a boot loader, but it is also a tool used for board
+bring-up, for production testing, and for other activities
+
+Keep it Portable
+^^^^^^^^^^^^^^^^
+
+U-Boot is a boot loader, but it is also a tool used for board
+bring-up, for production testing, and for other activities that are
+very closely related to hardware development. So far, it has been
+ported to several hundreds of different boards on about 30 different
+processor families - please make sure that any code you add can be
+used on as many different platforms as possible.
+
+Avoid assembly language whenever possible - only the reset code with
+basic CPU initialization, maybe a static DRAM initialization and the C
+stack setup should be in assembly.
+All further initializations should be done in C using assembly/C
+subroutines or inline macros. These functions represent some
+kind of HAL functionality and should be defined consistently on all
+architectures. E.g. Basic MMU and cache control, stack pointer manipulation.
+Non-existing functions should expand into empty macros or error codes.
+
+Don't make assumptions over the environment where U-Boot is running.
+It may be communicating with a human operator on directly attached
+serial console, but it may be through a GSM modem as well, or driven
+by some automatic test or control system. So don't output any fancy
+control character sequences or similar.
+
+Keep it Configurable
+^^^^^^^^^^^^^^^^^^^^
+
+Section "Keep it Small" already explains about the size restrictions
+for U-Boot on one side. On the other side, U-Boot is a powerful tool
+with many, many extremely useful features. The maintainer or user of
+each board will have to decide which features are important to him and
+what shall be included with his specific board configuration to meet
+his current requirements and restrictions.
+
+Please make sure that it is easy to add or remove features from a
+board configuration, so everybody can make the best use of U-Boot on
+his system.
+
+If a feature is not included, it should not have any residual code
+bloating the build.
+
+Keep it Debuggable
+^^^^^^^^^^^^^^^^^^
+
+Of course debuggable code is a big benefit for all of us contributing
+in one way or another to the development of the U-Boot project. But
+as already mentioned in section "Keep it Portable" above, U-Boot is
+not only a tool in itself, it is often also used for hardware
+bring-up, so debugging U-Boot often means that we don't know if we are
+tracking down a problem in the U-Boot software or in the hardware we
+are running on. Code that is clean and easy to understand and to
+debug is all the more important to many of us.
+
+* One important feature of U-Boot is to enable output to the (usually serial)
+  console as soon as possible in the boot process, even if this causes
+  tradeoffs in other areas like memory footprint.
+* All initialization steps shall print some "begin doing this" message before
+  they actually start, and some "done" message when they complete. For example,
+  RAM initialization and size detection may print a "RAM: " before they start,
+  and "256 MB\n" when done.  The purpose of this is that you can always see
+  which initialization step was running if there should be any problem.  This
+  is important not only during software development, but also for the service
+  people dealing with broken hardware in the field.
+* U-Boot should be debuggable with simple JTAG or BDM equipment.  It shall use
+  a simple, single-threaded execution model.  Avoid any magic, which could
+  prevent easy debugging even when only 1 or 2 hardware breakpoints are
+  available.
+
+Keep it Usable
+^^^^^^^^^^^^^^
+
+Please always keep in mind that there are at least three different
+groups of users for U-Boot, with completely different expectations
+and requirements:
+
+* The end user of an embedded device just wants to run some application; he
+  does not even want to know that U-Boot exists and only rarely interacts with
+  it (for example to perform a reset to factory default settings etc.)
+* System designers and engineers working on the development of the application
+  and/or the operating system want a powerful tool that can boot from any boot
+  device they can imagine, they want it fast and scriptable and whatever - in
+  short, they want as many features supported as possible. And some more.
+* The engineer who ports U-Boot to a new board and the board maintainer want
+  U-Boot to be as simple as possible so porting it to and maintaining it on
+  their hardware is easy for them.
+* Make it easy to test. Add debug code (but don't re-invent the wheel - use
+  existing macros like debug() or debugX()).
+
+Please always keep in mind that U-Boot tries to meet all these
+different requirements.
+
+Keep it Maintainable
+^^^^^^^^^^^^^^^^^^^^
+
+* Avoid ``#ifdefs`` where possible
+* Use "weak" functions
+* Always follow the :doc:`codingstyle` requirements.
+
+Keep it Beautiful
+^^^^^^^^^^^^^^^^^
+
+* Keep the source code clean: strictly follow the :doc:`codingstyle`,
+  keep lists (target names in the Makefiles, board names, etc.)
+  alphabetically sorted, etc.
+* Keep U-Boot console output clean: output only really necessary information,
+  be terse but precise, keep output vertically aligned, do not use control
+  character sequences (e.g. backspaces or \\r to do "spinning wheel" activity
+  indicators), etc.
+
+Keep it Open
+^^^^^^^^^^^^
+
+Contribute your work back to the whole community. Submit your changes
+and extensions as patches to the U-Boot mailing list.
+
+Lemmas from the golden rules
+----------------------------
+
+Generic Code is Good Code
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+New code shall be as generic as possible and added to the U-Boot
+abstraction hierarchy as high as possible. As few code as possible shall be
+added in board directories as people usually do not expect re-usable code
+there.  Thus peripheral drivers should be put below
+"drivers" even if they start out supporting only one specific
+configuration.  Note that it is not a requirement for such a first
+instance to be generic as genericity generally cannot be extrapolated
+from a single data point.
+
diff --git a/doc/develop/index.rst b/doc/develop/index.rst
index dde47994c71a..c0f4f0ba413a 100644
--- a/doc/develop/index.rst
+++ b/doc/develop/index.rst
@@ -10,6 +10,7 @@ General
    :maxdepth: 1
 
    codingstyle
+   designprinciples
 
 Implementation
 --------------
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/4] doc: codingstyle: Remove comment about '//' style comments
  2022-06-27 17:17 [PATCH 0/4] Migrate some wiki pages to sphinx Tom Rini
  2022-06-27 17:17 ` [PATCH 1/4] doc: Migrate CodingStyle wiki page " Tom Rini
  2022-06-27 17:17 ` [PATCH 2/4] doc: Migrate DesignPrinciples " Tom Rini
@ 2022-06-27 17:17 ` Tom Rini
  2022-06-30 10:06   ` Simon Glass
  2022-07-09  6:40   ` Heinrich Schuchardt
  2022-06-27 17:17 ` [PATCH 4/4] doc: Migrate Process wiki page to sphinx Tom Rini
  3 siblings, 2 replies; 23+ messages in thread
From: Tom Rini @ 2022-06-27 17:17 UTC (permalink / raw)
  To: u-boot

For some time now we've allowed for '//' style comments, which mirrors
the Linux kernel.  So drop this point here.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 doc/develop/codingstyle.rst | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst
index a41aed2764fb..bdf1efc6019a 100644
--- a/doc/develop/codingstyle.rst
+++ b/doc/develop/codingstyle.rst
@@ -29,11 +29,6 @@ The following rules apply:
   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:
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/4] doc: Migrate Process wiki page to sphinx
  2022-06-27 17:17 [PATCH 0/4] Migrate some wiki pages to sphinx Tom Rini
                   ` (2 preceding siblings ...)
  2022-06-27 17:17 ` [PATCH 3/4] doc: codingstyle: Remove comment about '//' style comments Tom Rini
@ 2022-06-27 17:17 ` Tom Rini
  2022-06-30 10:06   ` Simon Glass
                     ` (2 more replies)
  3 siblings, 3 replies; 23+ messages in thread
From: Tom Rini @ 2022-06-27 17:17 UTC (permalink / raw)
  To: u-boot

Move the current Process wiki page to doc/develop/process.rst.  The
changes here are for formatting or slight rewording so that it reads
well when linking to other sphinx documents.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 doc/develop/index.rst   |   1 +
 doc/develop/process.rst | 182 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 183 insertions(+)
 create mode 100644 doc/develop/process.rst

diff --git a/doc/develop/index.rst b/doc/develop/index.rst
index c0f4f0ba413a..eab00a55382a 100644
--- a/doc/develop/index.rst
+++ b/doc/develop/index.rst
@@ -11,6 +11,7 @@ General
 
    codingstyle
    designprinciples
+   process
 
 Implementation
 --------------
diff --git a/doc/develop/process.rst b/doc/develop/process.rst
new file mode 100644
index 000000000000..dd279fb9eff1
--- /dev/null
+++ b/doc/develop/process.rst
@@ -0,0 +1,182 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+U-Boot Development Process
+==========================
+
+Management Summary
+------------------
+
+* Development happens in Release Cycles of 3 months.
+* The first 2 weeks are called Merge Window, which is followed by a
+  Stabilization Period.
+* Patches with new code get only accepted while the Merge Window is open.
+* A patch that is generally in good shape and that was submitted while the
+  Merge Window was open is eligible to go into the upcoming release, even if
+  changes and resubmits are needed.
+* During the Stabilization Period, only patches that contain bug fixes get
+  applied.
+
+Phases of the Development Process
+---------------------------------
+
+U-Boot development takes place in `Release Cycles
+<https://www.denx.de/wiki/U-Boot/ReleaseCycle>`_.  A Release Cycle lasts
+normally for three months.
+
+The first two weeks of each Release Cycle are called *Merge Window*.
+
+It is followed by a *Stabilization Period*.
+
+The end of a Release Cycle is marked by the release of a new U-Boot version.
+
+Merge Window
+------------
+
+The Merge Window is the period when new patches get submitted
+(and hopefully accepted) for inclusion into U-Boot mainline.
+
+This is the only time when new code (like support for new processors or new
+boards, or other new features or reorganization of code) is accepted.
+
+Twilight Time
+-------------
+
+Usually patches do not get accepted as they are - the peer review that takes
+place will usually require changes and resubmits of the patches before they
+are considered to be ripe for inclusion into mainline.
+
+Also, the review often happens not immediately after a patch was submitted,
+but only when somebody (usually the responsible custodian) finds time to do
+this.
+
+In the result, the final version of such patches gets submitted after the
+merge window has been closed.
+
+It is current practice in U-Boot that such patches are eligible to go into the
+upcoming release.
+
+In the result, the release of the ``"-rc1"`` version does not immediately follow
+the closing of the Merge Window.
+
+Stabilization Period
+--------------------
+
+During the Stabilization Period only patches containing bug fixes get
+applied.
+
+Corner Cases
+------------
+
+Sometimes it is not clear if a patch contains a bug fix or not.
+For example, changes that remove dead code, unused macros etc. or
+that contain Coding Style fixes are not strict bug fixes.
+
+In such situations it is up to the responsible custodian to decide if he
+applies such patches even when the Merge Window is closed.
+
+Exception: at the end of the Stabilization Period only strict bug
+fixes my be applied.
+
+Sometimes patches miss the the Merge Window slightly - say by few
+hours or even a day. Patch acceptance is not as critical as a
+financial transaction, or such. So if there is such a slight delay,
+the custodian is free to turn a blind eye and accept it anyway. The
+idea of the development process is to make it foreseeable,
+but not to slow down development.
+
+It makes more sense if an engineer spends another day on testing and
+cleanup and submits the patch a couple of hours late, instead of
+submitting a green patch which will waste efforts from several people
+during several rounds of review and reposts.
+
+Differences to Linux Development Process
+----------------------------------------
+
+* In Linux, top-level maintainers will collect patches in their trees and send
+  pull requests to Linus as soon as the merge window opens.
+  So far, most U-Boot custodians do not work like that; they send pull requests
+  only at (or even after) the end of the merge window.
+* In Linux, the closing of the merge window is marked by the release of the
+  next ``"-rc1"``
+  In U-Boot, ``"-rc1"`` will only be released after all (or at least most of
+  the) patches that were submitted during the merge window have been applied.
+
+Custodians
+----------
+
+The Custodians take responsibility for some area of the U-Boot code.  The
+in-tree ``MAINTAINERS`` files list who is reponsible for which areas.
+
+It is their responsibility to pick up patches from the mailing list
+that fall into their responsibility, and to process these.
+
+A very important responsibility of each custodian is to provide
+feedback to the submitter of a patch about what is going on: if the
+patch was accepted, or if it was rejected (which exact list of
+reasons), if it needs to be reworked (with respective review
+comments). Even a "I have no time now, will look into it later"
+message is better than nothing. Also, if there are remarks to a
+patch, these should leave no doubt if they were just comments and the
+patch will be accepted anyway, or if the patch should be
+reworked/resubmitted, or if it was rejected.
+
+Work flow of a Custodian
+------------------------
+
+The normal flow of work in the U-Boot development process will look
+like this:
+
+#. A developer submits a patch via e-mail to the u-boot-users mailing list.
+   U-Boot has adopted the `Linux kernel signoff policy <https://groups.google.com/g/fa.linux.kernel/c/TLJIJVA-I6o?pli=1>`_, so the submitter must
+   include a ``Signed-off-by:`` line.
+#. Everybody who can is invited to review and test the changes.  Reviews should
+   reply on the mailing list with ``Acked-by`` lines.
+#. The responsible custodian
+
+   #. inspects this patch, especially for:
+
+   #. :doc:`codingstyle`
+   #. Basic logic:
+
+      * The patch fixes a real problem.
+      * The patch does not introduce new problems, especially it does not break
+        other boards or architectures
+
+   #. U-Boot Philosophy
+   #. Applies cleanly to the source tree
+   #. passes a ``MAKEALL`` compile test without creating new warnings
+
+#. Notes:
+
+  #. In some cases more than one custodian may be affected or feel responsible.
+     To avoid duplicated efforts, the custodian who starts processing the
+     patch should send a short ACK to the mailing list.
+  #. We should create some tool to automatically do this.
+  #. This is well documented in :doc:`designprinciples`.
+  #. The custodian decides himself how recent the code must be.  It is
+     acceptable to request patches against the last officially released
+     version of U-Boot or newer.  Of course a custodian can also accept
+     patches against older code.
+  #. Commits should show original author in the ``author`` field and include all
+      sign off/ack lines.
+
+5. The custodian decides to accept or to reject the patch.
+#. If accepted, the custodian adds the patch to his public git repository and
+   notifies the mailing list. This note should include:
+
+   * a short description of the changes
+   * the list of the affected boards / architectures etc.
+   * suggested tests
+
+   Although the custodian is supposed to perform his own tests
+   it is a well-known and accepted fact that he needs help from
+   other developers who - for example - have access to the required
+   hardware or tool chains.
+   The custodian request help for tests and feedback from
+   specific maintainers and U-Boot users.
+#. Once tests are passed, some agreed time limit expires, the custodian
+   requests that the changes in his public git repository be merged into the
+   main tree. If necessary, the custodian may have to adapt his changes to
+   allow for a clean merge.
+   Todo: define a reasonable time limit. 3 weeks?
+
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/4] doc: Migrate CodingStyle wiki page to sphinx
  2022-06-27 17:17 ` [PATCH 1/4] doc: Migrate CodingStyle wiki page " Tom Rini
@ 2022-06-30 10:06   ` Simon Glass
  2022-07-09  6:15     ` Heinrich Schuchardt
  2022-07-09  6:12   ` Heinrich Schuchardt
  1 sibling, 1 reply; 23+ messages in thread
From: Simon Glass @ 2022-06-30 10:06 UTC (permalink / raw)
  To: Tom Rini; +Cc: U-Boot Mailing List

On Mon, 27 Jun 2022 at 11:17, Tom Rini <trini@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.
>
> 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
>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/4] doc: Migrate DesignPrinciples wiki page to sphinx
  2022-06-27 17:17 ` [PATCH 2/4] doc: Migrate DesignPrinciples " Tom Rini
@ 2022-06-30 10:06   ` Simon Glass
  2022-07-08  7:22   ` Claudius Heine
  2022-07-09  6:37   ` Heinrich Schuchardt
  2 siblings, 0 replies; 23+ messages in thread
From: Simon Glass @ 2022-06-30 10:06 UTC (permalink / raw)
  To: Tom Rini; +Cc: U-Boot Mailing List

On Mon, 27 Jun 2022 at 11:18, Tom Rini <trini@konsulko.com> wrote:
>
> Move the current DesignPrinciples wiki page to
> doc/develop/designprinciples.rst.  The changes here are for formatting
> or slight rewording so that it reads well when linking to other sphinx
> documents.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  doc/develop/designprinciples.rst | 197 +++++++++++++++++++++++++++++++
>  doc/develop/index.rst            |   1 +
>  2 files changed, 198 insertions(+)
>  create mode 100644 doc/develop/designprinciples.rst

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/4] doc: codingstyle: Remove comment about '//' style comments
  2022-06-27 17:17 ` [PATCH 3/4] doc: codingstyle: Remove comment about '//' style comments Tom Rini
@ 2022-06-30 10:06   ` Simon Glass
  2022-07-09  6:40   ` Heinrich Schuchardt
  1 sibling, 0 replies; 23+ messages in thread
From: Simon Glass @ 2022-06-30 10:06 UTC (permalink / raw)
  To: Tom Rini; +Cc: U-Boot Mailing List

On Mon, 27 Jun 2022 at 11:17, Tom Rini <trini@konsulko.com> wrote:
>
> For some time now we've allowed for '//' style comments, which mirrors
> the Linux kernel.  So drop this point here.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  doc/develop/codingstyle.rst | 5 -----
>  1 file changed, 5 deletions(-)

Oh I didn't know that.

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] doc: Migrate Process wiki page to sphinx
  2022-06-27 17:17 ` [PATCH 4/4] doc: Migrate Process wiki page to sphinx Tom Rini
@ 2022-06-30 10:06   ` Simon Glass
  2022-07-08  7:06   ` Claudius Heine
  2022-07-09  6:55   ` Heinrich Schuchardt
  2 siblings, 0 replies; 23+ messages in thread
From: Simon Glass @ 2022-06-30 10:06 UTC (permalink / raw)
  To: Tom Rini; +Cc: U-Boot Mailing List

On Mon, 27 Jun 2022 at 11:18, Tom Rini <trini@konsulko.com> wrote:
>
> Move the current Process wiki page to doc/develop/process.rst.  The
> changes here are for formatting or slight rewording so that it reads
> well when linking to other sphinx documents.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  doc/develop/index.rst   |   1 +
>  doc/develop/process.rst | 182 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 183 insertions(+)
>  create mode 100644 doc/develop/process.rst

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] doc: Migrate Process wiki page to sphinx
  2022-06-27 17:17 ` [PATCH 4/4] doc: Migrate Process wiki page to sphinx Tom Rini
  2022-06-30 10:06   ` Simon Glass
@ 2022-07-08  7:06   ` Claudius Heine
  2022-07-08  7:22     ` Martin Bonner
  2022-07-09  6:55   ` Heinrich Schuchardt
  2 siblings, 1 reply; 23+ messages in thread
From: Claudius Heine @ 2022-07-08  7:06 UTC (permalink / raw)
  To: Tom Rini, u-boot

Hi Tom,

On 2022-06-27 19:17, Tom Rini wrote:
> Move the current Process wiki page to doc/develop/process.rst.  The
> changes here are for formatting or slight rewording so that it reads
> well when linking to other sphinx documents.
> 
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>   doc/develop/index.rst   |   1 +
>   doc/develop/process.rst | 182 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 183 insertions(+)
>   create mode 100644 doc/develop/process.rst
> 
> diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> index c0f4f0ba413a..eab00a55382a 100644
> --- a/doc/develop/index.rst
> +++ b/doc/develop/index.rst
> @@ -11,6 +11,7 @@ General
>   
>      codingstyle
>      designprinciples
> +   process
>   
>   Implementation
>   --------------
> diff --git a/doc/develop/process.rst b/doc/develop/process.rst
> new file mode 100644
> index 000000000000..dd279fb9eff1
> --- /dev/null
> +++ b/doc/develop/process.rst
> @@ -0,0 +1,182 @@
> +.. SPDX-License-Identifier: GPL-2.0+:
> +
> +U-Boot Development Process
> +==========================
> +
> +Management Summary
> +------------------
> +
> +* Development happens in Release Cycles of 3 months.
> +* The first 2 weeks are called Merge Window, which is followed by a
> +  Stabilization Period.
> +* Patches with new code get only accepted while the Merge Window is open.
> +* A patch that is generally in good shape and that was submitted while the
> +  Merge Window was open is eligible to go into the upcoming release, even if
> +  changes and resubmits are needed.
> +* During the Stabilization Period, only patches that contain bug fixes get
> +  applied.
> +
> +Phases of the Development Process
> +---------------------------------
> +
> +U-Boot development takes place in `Release Cycles
> +<https://www.denx.de/wiki/U-Boot/ReleaseCycle>`_.  A Release Cycle lasts
> +normally for three months.
> +
> +The first two weeks of each Release Cycle are called *Merge Window*.
> +
> +It is followed by a *Stabilization Period*.
> +
> +The end of a Release Cycle is marked by the release of a new U-Boot version.
> +
> +Merge Window
> +------------
> +
> +The Merge Window is the period when new patches get submitted
> +(and hopefully accepted) for inclusion into U-Boot mainline.
> +
> +This is the only time when new code (like support for new processors or new
> +boards, or other new features or reorganization of code) is accepted.
> +
> +Twilight Time
> +-------------
> +
> +Usually patches do not get accepted as they are - the peer review that takes
> +place will usually require changes and resubmits of the patches before they
> +are considered to be ripe for inclusion into mainline.
> +
> +Also, the review often happens not immediately after a patch was submitted,
> +but only when somebody (usually the responsible custodian) finds time to do
> +this.
> +
> +In the result, the final version of such patches gets submitted after the
> +merge window has been closed.
> +
> +It is current practice in U-Boot that such patches are eligible to go into the
> +upcoming release.
> +
> +In the result, the release of the ``"-rc1"`` version does not immediately follow
> +the closing of the Merge Window.
> +
> +Stabilization Period
> +--------------------
> +
> +During the Stabilization Period only patches containing bug fixes get
> +applied.
> +
> +Corner Cases
> +------------
> +
> +Sometimes it is not clear if a patch contains a bug fix or not.
> +For example, changes that remove dead code, unused macros etc. or
> +that contain Coding Style fixes are not strict bug fixes.
> +
> +In such situations it is up to the responsible custodian to decide if he

he applies -> they apply

> +applies such patches even when the Merge Window is closed.
> +
> +Exception: at the end of the Stabilization Period only strict bug
> +fixes my be applied.
> +
> +Sometimes patches miss the the Merge Window slightly - say by few

the the -> the

> +hours or even a day. Patch acceptance is not as critical as a
> +financial transaction, or such. So if there is such a slight delay,
> +the custodian is free to turn a blind eye and accept it anyway. The
> +idea of the development process is to make it foreseeable,
> +but not to slow down development.
> +
> +It makes more sense if an engineer spends another day on testing and
> +cleanup and submits the patch a couple of hours late, instead of
> +submitting a green patch which will waste efforts from several people
> +during several rounds of review and reposts.
> +
> +Differences to Linux Development Process
> +----------------------------------------
> +
> +* In Linux, top-level maintainers will collect patches in their trees and send
> +  pull requests to Linus as soon as the merge window opens.
> +  So far, most U-Boot custodians do not work like that; they send pull requests
> +  only at (or even after) the end of the merge window.
> +* In Linux, the closing of the merge window is marked by the release of the
> +  next ``"-rc1"``
> +  In U-Boot, ``"-rc1"`` will only be released after all (or at least most of
> +  the) patches that were submitted during the merge window have been applied.
> +
> +Custodians
> +----------
> +
> +The Custodians take responsibility for some area of the U-Boot code.  The
> +in-tree ``MAINTAINERS`` files list who is reponsible for which areas.

reponsible -> responsible

> +
> +It is their responsibility to pick up patches from the mailing list
> +that fall into their responsibility, and to process these.
> +
> +A very important responsibility of each custodian is to provide
> +feedback to the submitter of a patch about what is going on: if the
> +patch was accepted, or if it was rejected (which exact list of
> +reasons), if it needs to be reworked (with respective review
> +comments). Even a "I have no time now, will look into it later"
> +message is better than nothing. Also, if there are remarks to a
> +patch, these should leave no doubt if they were just comments and the
> +patch will be accepted anyway, or if the patch should be
> +reworked/resubmitted, or if it was rejected.
> +
> +Work flow of a Custodian
> +------------------------
> +
> +The normal flow of work in the U-Boot development process will look
> +like this:
> +
> +#. A developer submits a patch via e-mail to the u-boot-users mailing list.
> +   U-Boot has adopted the `Linux kernel signoff policy <https://groups.google.com/g/fa.linux.kernel/c/TLJIJVA-I6o?pli=1>`_, so the submitter must
> +   include a ``Signed-off-by:`` line.
> +#. Everybody who can is invited to review and test the changes.  Reviews should
> +   reply on the mailing list with ``Acked-by`` lines.
> +#. The responsible custodian
> +
> +   #. inspects this patch, especially for:
> +
> +   #. :doc:`codingstyle`
> +   #. Basic logic:
> +
> +      * The patch fixes a real problem.
> +      * The patch does not introduce new problems, especially it does not break
> +        other boards or architectures
> +
> +   #. U-Boot Philosophy
> +   #. Applies cleanly to the source tree
> +   #. passes a ``MAKEALL`` compile test without creating new warnings
> +
> +#. Notes:
> +
> +  #. In some cases more than one custodian may be affected or feel responsible.
> +     To avoid duplicated efforts, the custodian who starts processing the
> +     patch should send a short ACK to the mailing list.
> +  #. We should create some tool to automatically do this.
> +  #. This is well documented in :doc:`designprinciples`.
> +  #. The custodian decides himself how recent the code must be.  It is
> +     acceptable to request patches against the last officially released
> +     version of U-Boot or newer.  Of course a custodian can also accept
> +     patches against older code.
> +  #. Commits should show original author in the ``author`` field and include all
> +      sign off/ack lines.
> +
> +5. The custodian decides to accept or to reject the patch.
> +#. If accepted, the custodian adds the patch to his public git repository and
> +   notifies the mailing list. This note should include:
> +
> +   * a short description of the changes
> +   * the list of the affected boards / architectures etc.
> +   * suggested tests
> +
> +   Although the custodian is supposed to perform his own tests

his -> their

> +   it is a well-known and accepted fact that he needs help from

he -> they

> +   other developers who - for example - have access to the required
> +   hardware or tool chains.
> +   The custodian request help for tests and feedback from
> +   specific maintainers and U-Boot users.
> +#. Once tests are passed, some agreed time limit expires, the custodian
> +   requests that the changes in his public git repository be merged into the

his -> their

> +   main tree. If necessary, the custodian may have to adapt his changes to

his -> their

> +   allow for a clean merge.
> +   Todo: define a reasonable time limit. 3 weeks?

regards,
Claudius

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/4] doc: Migrate DesignPrinciples wiki page to sphinx
  2022-06-27 17:17 ` [PATCH 2/4] doc: Migrate DesignPrinciples " Tom Rini
  2022-06-30 10:06   ` Simon Glass
@ 2022-07-08  7:22   ` Claudius Heine
  2022-07-09  6:37   ` Heinrich Schuchardt
  2 siblings, 0 replies; 23+ messages in thread
From: Claudius Heine @ 2022-07-08  7:22 UTC (permalink / raw)
  To: Tom Rini, u-boot

Hi Tom,

On 2022-06-27 19:17, Tom Rini wrote:
> Move the current DesignPrinciples wiki page to
> doc/develop/designprinciples.rst.  The changes here are for formatting
> or slight rewording so that it reads well when linking to other sphinx
> documents.
> 
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>   doc/develop/designprinciples.rst | 197 +++++++++++++++++++++++++++++++
>   doc/develop/index.rst            |   1 +
>   2 files changed, 198 insertions(+)
>   create mode 100644 doc/develop/designprinciples.rst
> 
> diff --git a/doc/develop/designprinciples.rst b/doc/develop/designprinciples.rst
> new file mode 100644
> index 000000000000..79694db77604
> --- /dev/null
> +++ b/doc/develop/designprinciples.rst
> @@ -0,0 +1,197 @@
> +.. SPDX-License-Identifier: GPL-2.0+:
> +
> +U-Boot Design Principles
> +========================
> +
> +The 10 Golden Rules of U-Boot design
> +------------------------------------
> +
> +Keep it Small
> +^^^^^^^^^^^^^
> +
> +U-Boot is a Boot Loader, i.e. its primary purpose in the shipping
> +system is to load some operating system.
> +That means that U-Boot is
> +necessary to perform a certain task, but it's nothing you want to
> +throw any significant resources at. Typically U-Boot is stored in
> +relatively small NOR flash memory, which is expensive
> +compared to the much larger NAND devices often used to store the
> +operating system and the application.
> +
> +At the moment, U-Boot supports boards with just 128 !KiB ROM or with
> +256 !KiB NOR flash. We should not easily ignore such configurations -
> +they may be the exception in among all the other supported boards,
> +but if a design uses such a resource-constrained hardware setup it is
> +usually because costs are critical, i. e. because the number of
> +manufactured boards might be tens or hundreds of thousands or even
> +millions...
> +
> +A usable and useful configuration of U-Boot, including a basic
> +interactive command interpreter, support for download over Ethernet
> +and the capability to program the flash shall fit in no more than 128 !KiB.
> +
> +Keep it Fast
> +^^^^^^^^^^^^
> +
> +The end user is not interested in running U-Boot. In most embedded
> +systems he is not even aware that U-Boot exists. The user wants to

he is -> they are

> +run some application code, and that as soon as possible after switching
> +on his device.

his -> their

> +
> +It is therefore essential that U-Boot is as fast as possible,
> +especially that it loads and boots the operating system as fast as possible.
> +
> +To achieve this, the following design principles shall be followed:
> +
> +* Enable caches as soon and whenever possible
> +* Initialize devices only when they are needed within U-Boot, i.e. don't
> +  initialize the Ethernet interface(s) unless U-Boot performs a download over
> +  Ethernet; don't  initialize any IDE or USB devices unless U-Boot actually
> +  tries to load files from these, etc.  (and don't forget to shut down these
> +  devices after using them  - otherwise nasty things may happen when you try to
> +  boot your OS).
> +
> +Also, building of U-Boot shall be as fast as possible.
> +This makes it easier to run a build for all supported configurations
> +or at least for all configurations of a specific architecture,
> +which is essential for quality assurance.
> +If building is cumbersome and slow, most people will omit
> +this important step.
> +
> +Keep it Simple
> +^^^^^^^^^^^^^^
> +
> +U-Boot is a boot loader, but it is also a tool used for board
> +bring-up, for production testing, and for other activities

. is missing at the end of the sentence

> +
> +Keep it Portable
> +^^^^^^^^^^^^^^^^
> +
> +U-Boot is a boot loader, but it is also a tool used for board
> +bring-up, for production testing, and for other activities that are
> +very closely related to hardware development. So far, it has been
> +ported to several hundreds of different boards on about 30 different
> +processor families - please make sure that any code you add can be
> +used on as many different platforms as possible.
> +
> +Avoid assembly language whenever possible - only the reset code with
> +basic CPU initialization, maybe a static DRAM initialization and the C
> +stack setup should be in assembly.
> +All further initializations should be done in C using assembly/C
> +subroutines or inline macros. These functions represent some
> +kind of HAL functionality and should be defined consistently on all
> +architectures. E.g. Basic MMU and cache control, stack pointer manipulation.
> +Non-existing functions should expand into empty macros or error codes.
> +
> +Don't make assumptions over the environment where U-Boot is running.
> +It may be communicating with a human operator on directly attached
> +serial console, but it may be through a GSM modem as well, or driven
> +by some automatic test or control system. So don't output any fancy
> +control character sequences or similar.
> +
> +Keep it Configurable
> +^^^^^^^^^^^^^^^^^^^^
> +
> +Section "Keep it Small" already explains about the size restrictions
> +for U-Boot on one side. On the other side, U-Boot is a powerful tool
> +with many, many extremely useful features. The maintainer or user of
> +each board will have to decide which features are important to him and

him -> them

> +what shall be included with his specific board configuration to meet

his -> their

> +his current requirements and restrictions.

here again

> +
> +Please make sure that it is easy to add or remove features from a
> +board configuration, so everybody can make the best use of U-Boot on
> +his system.

here as well

> +
> +If a feature is not included, it should not have any residual code
> +bloating the build.
> +
> +Keep it Debuggable
> +^^^^^^^^^^^^^^^^^^
> +
> +Of course debuggable code is a big benefit for all of us contributing
> +in one way or another to the development of the U-Boot project. But
> +as already mentioned in section "Keep it Portable" above, U-Boot is
> +not only a tool in itself, it is often also used for hardware
> +bring-up, so debugging U-Boot often means that we don't know if we are
> +tracking down a problem in the U-Boot software or in the hardware we
> +are running on. Code that is clean and easy to understand and to
> +debug is all the more important to many of us.
> +
> +* One important feature of U-Boot is to enable output to the (usually serial)
> +  console as soon as possible in the boot process, even if this causes
> +  tradeoffs in other areas like memory footprint.
> +* All initialization steps shall print some "begin doing this" message before
> +  they actually start, and some "done" message when they complete. For example,
> +  RAM initialization and size detection may print a "RAM: " before they start,
> +  and "256 MB\n" when done.  The purpose of this is that you can always see
> +  which initialization step was running if there should be any problem.  This
> +  is important not only during software development, but also for the service
> +  people dealing with broken hardware in the field.
> +* U-Boot should be debuggable with simple JTAG or BDM equipment.  It shall use
> +  a simple, single-threaded execution model.  Avoid any magic, which could
> +  prevent easy debugging even when only 1 or 2 hardware breakpoints are
> +  available.
> +
> +Keep it Usable
> +^^^^^^^^^^^^^^
> +
> +Please always keep in mind that there are at least three different
> +groups of users for U-Boot, with completely different expectations
> +and requirements:
> +
> +* The end user of an embedded device just wants to run some application; he
> +  does not even want to know that U-Boot exists and only rarely interacts with

he does -> they do

> +  it (for example to perform a reset to factory default settings etc.)
> +* System designers and engineers working on the development of the application
> +  and/or the operating system want a powerful tool that can boot from any boot
> +  device they can imagine, they want it fast and scriptable and whatever - in
> +  short, they want as many features supported as possible. And some more.
> +* The engineer who ports U-Boot to a new board and the board maintainer want
> +  U-Boot to be as simple as possible so porting it to and maintaining it on
> +  their hardware is easy for them.
> +* Make it easy to test. Add debug code (but don't re-invent the wheel - use
> +  existing macros like debug() or debugX()).
> +
> +Please always keep in mind that U-Boot tries to meet all these
> +different requirements.
> +
> +Keep it Maintainable
> +^^^^^^^^^^^^^^^^^^^^
> +
> +* Avoid ``#ifdefs`` where possible
> +* Use "weak" functions
> +* Always follow the :doc:`codingstyle` requirements.
> +
> +Keep it Beautiful
> +^^^^^^^^^^^^^^^^^
> +
> +* Keep the source code clean: strictly follow the :doc:`codingstyle`,
> +  keep lists (target names in the Makefiles, board names, etc.)
> +  alphabetically sorted, etc.
> +* Keep U-Boot console output clean: output only really necessary information,
> +  be terse but precise, keep output vertically aligned, do not use control
> +  character sequences (e.g. backspaces or \\r to do "spinning wheel" activity
> +  indicators), etc.
> +
> +Keep it Open
> +^^^^^^^^^^^^
> +
> +Contribute your work back to the whole community. Submit your changes
> +and extensions as patches to the U-Boot mailing list.
> +
> +Lemmas from the golden rules
> +----------------------------
> +
> +Generic Code is Good Code
> +^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +New code shall be as generic as possible and added to the U-Boot
> +abstraction hierarchy as high as possible. As few code as possible shall be
> +added in board directories as people usually do not expect re-usable code
> +there.  Thus peripheral drivers should be put below
> +"drivers" even if they start out supporting only one specific
> +configuration.  Note that it is not a requirement for such a first
> +instance to be generic as genericity generally cannot be extrapolated
> +from a single data point.
> +
> diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> index dde47994c71a..c0f4f0ba413a 100644
> --- a/doc/develop/index.rst
> +++ b/doc/develop/index.rst
> @@ -10,6 +10,7 @@ General
>      :maxdepth: 1
>   
>      codingstyle
> +   designprinciples
>   
>   Implementation
>   --------------

regards,
Claudius

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] doc: Migrate Process wiki page to sphinx
  2022-07-08  7:06   ` Claudius Heine
@ 2022-07-08  7:22     ` Martin Bonner
  2022-07-08 17:44       ` Tom Rini
  0 siblings, 1 reply; 23+ messages in thread
From: Martin Bonner @ 2022-07-08  7:22 UTC (permalink / raw)
  To: Claudius Heine; +Cc: Tom Rini, u-boot

Martin


On Fri, 8 Jul 2022 at 08:06, Claudius Heine <ch@denx.de> wrote:

> Hi Tom,
>
> On 2022-06-27 19:17, Tom Rini wrote:
> > Move the current Process wiki page to doc/develop/process.rst.  The
> > changes here are for formatting or slight rewording so that it reads
> > well when linking to other sphinx documents.
> >
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >   doc/develop/index.rst   |   1 +
> >   doc/develop/process.rst | 182 ++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 183 insertions(+)
> >   create mode 100644 doc/develop/process.rst
> >
> > diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> > index c0f4f0ba413a..eab00a55382a 100644
> > --- a/doc/develop/index.rst
> > +++ b/doc/develop/index.rst
> > @@ -11,6 +11,7 @@ General
> >
> >      codingstyle
> >      designprinciples
> > +   process
> >
> >   Implementation
> >   --------------
> > diff --git a/doc/develop/process.rst b/doc/develop/process.rst
> > new file mode 100644
> > index 000000000000..dd279fb9eff1
> > --- /dev/null
> > +++ b/doc/develop/process.rst
> > @@ -0,0 +1,182 @@
> > +.. SPDX-License-Identifier: GPL-2.0+:
> > +
> > +U-Boot Development Process
> > +==========================
> > +
> > +Management Summary
> > +------------------
> > +
> > +* Development happens in Release Cycles of 3 months.
> > +* The first 2 weeks are called Merge Window, which is followed by a
> > +  Stabilization Period.
> > +* Patches with new code get only accepted while the Merge Window is
> open.
> > +* A patch that is generally in good shape and that was submitted while
> the
> > +  Merge Window was open is eligible to go into the upcoming release,
> even if
> > +  changes and resubmits are needed.
> > +* During the Stabilization Period, only patches that contain bug fixes
> get
> > +  applied.
> > +
> > +Phases of the Development Process
> > +---------------------------------
> > +
> > +U-Boot development takes place in `Release Cycles
> > +<https://www.denx.de/wiki/U-Boot/ReleaseCycle>`_.  A Release Cycle
> lasts
> > +normally for three months.
> > +
> > +The first two weeks of each Release Cycle are called *Merge Window*.
> > +
> > +It is followed by a *Stabilization Period*.
> > +
> > +The end of a Release Cycle is marked by the release of a new U-Boot
> version.
> > +
> > +Merge Window
> > +------------
> > +
> > +The Merge Window is the period when new patches get submitted
> > +(and hopefully accepted) for inclusion into U-Boot mainline.
> > +
> > +This is the only time when new code (like support for new processors or
> new
> > +boards, or other new features or reorganization of code) is accepted.
> > +
> > +Twilight Time
> > +-------------
> > +
> > +Usually patches do not get accepted as they are - the peer review that
> takes
> > +place will usually require changes and resubmits of the patches before
> they
> > +are considered to be ripe for inclusion into mainline.
> > +
> > +Also, the review often happens not immediately after a patch was
> submitted,
> > +but only when somebody (usually the responsible custodian) finds time
> to do
> > +this.
> > +
> > +In the result, the final version of such patches gets submitted after
> the
>
Should be: "The result is that the final version..." or "This can mean the
final version..."


> > +merge window has been closed.
> > +
> > +It is current practice in U-Boot that such patches are eligible to go
> into the
> > +upcoming release.
> > +
> > +In the result, the release of the ``"-rc1"`` version does not
> immediately follow
>
Should be "The result is that the release..." or "Thus the release..."

The first of both my two choices is closer to the original wording, the
second feels to me (as a British English speaker) to be more idiomatic.
(Beware, idiomatic English is not necessarily good - not everyone here is a
native speaker, and it can be overly complex.)


> > +the closing of the Merge Window.
> > +
> > +Stabilization Period
> > +--------------------
> > +
> > +During the Stabilization Period only patches containing bug fixes get
> > +applied.
> > +
> > +Corner Cases
> > +------------
> > +
> > +Sometimes it is not clear if a patch contains a bug fix or not.
> > +For example, changes that remove dead code, unused macros etc. or
> > +that contain Coding Style fixes are not strict bug fixes.
> > +
> > +In such situations it is up to the responsible custodian to decide if he
>
> he applies -> they apply
>
> > +applies such patches even when the Merge Window is closed.
> > +
> > +Exception: at the end of the Stabilization Period only strict bug
> > +fixes my be applied.
> > +
> > +Sometimes patches miss the the Merge Window slightly - say by few
>
> the the -> the
>
> > +hours or even a day. Patch acceptance is not as critical as a
> > +financial transaction, or such. So if there is such a slight delay,
> > +the custodian is free to turn a blind eye and accept it anyway. The
> > +idea of the development process is to make it foreseeable,
> > +but not to slow down development.
> > +
> > +It makes more sense if an engineer spends another day on testing and
> > +cleanup and submits the patch a couple of hours late, instead of
> > +submitting a green patch which will waste efforts from several people
> > +during several rounds of review and reposts.
> > +
> > +Differences to Linux Development Process
> > +----------------------------------------
> > +
> > +* In Linux, top-level maintainers will collect patches in their trees
> and send
> > +  pull requests to Linus as soon as the merge window opens.
> > +  So far, most U-Boot custodians do not work like that; they send pull
> requests
> > +  only at (or even after) the end of the merge window.
> > +* In Linux, the closing of the merge window is marked by the release of
> the
> > +  next ``"-rc1"``
> > +  In U-Boot, ``"-rc1"`` will only be released after all (or at least
> most of
> > +  the) patches that were submitted during the merge window have been
> applied.
> > +
> > +Custodians
> > +----------
> > +
> > +The Custodians take responsibility for some area of the U-Boot code.
> The
> > +in-tree ``MAINTAINERS`` files list who is reponsible for which areas.
>
> reponsible -> responsible
>
> > +
> > +It is their responsibility to pick up patches from the mailing list
> > +that fall into their responsibility, and to process these.
> > +
> > +A very important responsibility of each custodian is to provide
> > +feedback to the submitter of a patch about what is going on: if the
> > +patch was accepted, or if it was rejected (which exact list of
> > +reasons), if it needs to be reworked (with respective review
> > +comments). Even a "I have no time now, will look into it later"
> > +message is better than nothing. Also, if there are remarks to a
> > +patch, these should leave no doubt if they were just comments and the
> > +patch will be accepted anyway, or if the patch should be
> > +reworked/resubmitted, or if it was rejected.
> > +
> > +Work flow of a Custodian
> > +------------------------
> > +
> > +The normal flow of work in the U-Boot development process will look
> > +like this:
> > +
> > +#. A developer submits a patch via e-mail to the u-boot-users mailing
> list.
> > +   U-Boot has adopted the `Linux kernel signoff policy <
> https://groups.google.com/g/fa.linux.kernel/c/TLJIJVA-I6o?pli=1>`_, so
> the submitter must
> > +   include a ``Signed-off-by:`` line.
> > +#. Everybody who can is invited to review and test the changes.
> Reviews should
> > +   reply on the mailing list with ``Acked-by`` lines.
> > +#. The responsible custodian
> > +
> > +   #. inspects this patch, especially for:
> > +
> > +   #. :doc:`codingstyle`
> > +   #. Basic logic:
> > +
> > +      * The patch fixes a real problem.
> > +      * The patch does not introduce new problems, especially it does
> not break
> > +        other boards or architectures
> > +
> > +   #. U-Boot Philosophy
> > +   #. Applies cleanly to the source tree
> > +   #. passes a ``MAKEALL`` compile test without creating new warnings
> > +
> > +#. Notes:
> > +
> > +  #. In some cases more than one custodian may be affected or feel
> responsible.
> > +     To avoid duplicated efforts, the custodian who starts processing
> the
> > +     patch should send a short ACK to the mailing list.
> > +  #. We should create some tool to automatically do this.
> > +  #. This is well documented in :doc:`designprinciples`.
> > +  #. The custodian decides himself how recent the code must be.  It is
> > +     acceptable to request patches against the last officially released
> > +     version of U-Boot or newer.  Of course a custodian can also accept
> > +     patches against older code.
> > +  #. Commits should show original author in the ``author`` field and
> include all
> > +      sign off/ack lines.
> > +
> > +5. The custodian decides to accept or to reject the patch.
> > +#. If accepted, the custodian adds the patch to his public git
> repository and
> > +   notifies the mailing list. This note should include:
> > +
> > +   * a short description of the changes
> > +   * the list of the affected boards / architectures etc.
> > +   * suggested tests
> > +
> > +   Although the custodian is supposed to perform his own tests
>
> his -> their
>
> > +   it is a well-known and accepted fact that he needs help from
>
> he -> they
>
> > +   other developers who - for example - have access to the required
> > +   hardware or tool chains.
> > +   The custodian request help for tests and feedback from
> > +   specific maintainers and U-Boot users.
> > +#. Once tests are passed, some agreed time limit expires, the custodian
> > +   requests that the changes in his public git repository be merged
> into the
>
> his -> their
>
> > +   main tree. If necessary, the custodian may have to adapt his changes
> to
>
> his -> their
>
> > +   allow for a clean merge.
> > +   Todo: define a reasonable time limit. 3 weeks?
>
> regards,
> Claudius
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] doc: Migrate Process wiki page to sphinx
  2022-07-08  7:22     ` Martin Bonner
@ 2022-07-08 17:44       ` Tom Rini
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-07-08 17:44 UTC (permalink / raw)
  To: Martin Bonner; +Cc: Claudius Heine, u-boot

[-- Attachment #1: Type: text/plain, Size: 4518 bytes --]

On Fri, Jul 08, 2022 at 08:22:12AM +0100, Martin Bonner wrote:
> Martin
> 
> 
> On Fri, 8 Jul 2022 at 08:06, Claudius Heine <ch@denx.de> wrote:
> 
> > Hi Tom,
> >
> > On 2022-06-27 19:17, Tom Rini wrote:
> > > Move the current Process wiki page to doc/develop/process.rst.  The
> > > changes here are for formatting or slight rewording so that it reads
> > > well when linking to other sphinx documents.
> > >
> > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > ---
> > >   doc/develop/index.rst   |   1 +
> > >   doc/develop/process.rst | 182 ++++++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 183 insertions(+)
> > >   create mode 100644 doc/develop/process.rst
> > >
> > > diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> > > index c0f4f0ba413a..eab00a55382a 100644
> > > --- a/doc/develop/index.rst
> > > +++ b/doc/develop/index.rst
> > > @@ -11,6 +11,7 @@ General
> > >
> > >      codingstyle
> > >      designprinciples
> > > +   process
> > >
> > >   Implementation
> > >   --------------
> > > diff --git a/doc/develop/process.rst b/doc/develop/process.rst
> > > new file mode 100644
> > > index 000000000000..dd279fb9eff1
> > > --- /dev/null
> > > +++ b/doc/develop/process.rst
> > > @@ -0,0 +1,182 @@
> > > +.. SPDX-License-Identifier: GPL-2.0+:
> > > +
> > > +U-Boot Development Process
> > > +==========================
> > > +
> > > +Management Summary
> > > +------------------
> > > +
> > > +* Development happens in Release Cycles of 3 months.
> > > +* The first 2 weeks are called Merge Window, which is followed by a
> > > +  Stabilization Period.
> > > +* Patches with new code get only accepted while the Merge Window is
> > open.
> > > +* A patch that is generally in good shape and that was submitted while
> > the
> > > +  Merge Window was open is eligible to go into the upcoming release,
> > even if
> > > +  changes and resubmits are needed.
> > > +* During the Stabilization Period, only patches that contain bug fixes
> > get
> > > +  applied.
> > > +
> > > +Phases of the Development Process
> > > +---------------------------------
> > > +
> > > +U-Boot development takes place in `Release Cycles
> > > +<https://www.denx.de/wiki/U-Boot/ReleaseCycle>`_.  A Release Cycle
> > lasts
> > > +normally for three months.
> > > +
> > > +The first two weeks of each Release Cycle are called *Merge Window*.
> > > +
> > > +It is followed by a *Stabilization Period*.
> > > +
> > > +The end of a Release Cycle is marked by the release of a new U-Boot
> > version.
> > > +
> > > +Merge Window
> > > +------------
> > > +
> > > +The Merge Window is the period when new patches get submitted
> > > +(and hopefully accepted) for inclusion into U-Boot mainline.
> > > +
> > > +This is the only time when new code (like support for new processors or
> > new
> > > +boards, or other new features or reorganization of code) is accepted.
> > > +
> > > +Twilight Time
> > > +-------------
> > > +
> > > +Usually patches do not get accepted as they are - the peer review that
> > takes
> > > +place will usually require changes and resubmits of the patches before
> > they
> > > +are considered to be ripe for inclusion into mainline.
> > > +
> > > +Also, the review often happens not immediately after a patch was
> > submitted,
> > > +but only when somebody (usually the responsible custodian) finds time
> > to do
> > > +this.
> > > +
> > > +In the result, the final version of such patches gets submitted after
> > the
> >
> Should be: "The result is that the final version..." or "This can mean the
> final version..."
> 
> 
> > > +merge window has been closed.
> > > +
> > > +It is current practice in U-Boot that such patches are eligible to go
> > into the
> > > +upcoming release.
> > > +
> > > +In the result, the release of the ``"-rc1"`` version does not
> > immediately follow
> >
> Should be "The result is that the release..." or "Thus the release..."
> 
> The first of both my two choices is closer to the original wording, the
> second feels to me (as a British English speaker) to be more idiomatic.
> (Beware, idiomatic English is not necessarily good - not everyone here is a
> native speaker, and it can be overly complex.)

Thanks for the feedback.  I suspect that Wolfgang wrote these pages
initially.  I'll cc you both on patches to adjust the wording slightly.
I'm doing the read-aloud test on these sections now.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/4] doc: Migrate CodingStyle wiki page to sphinx
  2022-06-27 17:17 ` [PATCH 1/4] doc: Migrate CodingStyle wiki page " Tom Rini
  2022-06-30 10:06   ` Simon Glass
@ 2022-07-09  6:12   ` Heinrich Schuchardt
  2022-07-09 12:32     ` Tom Rini
  2022-07-09 12:36     ` Tom Rini
  1 sibling, 2 replies; 23+ messages in thread
From: Heinrich Schuchardt @ 2022-07-09  6:12 UTC (permalink / raw)
  To: Tom Rini; +Cc: Simon Glass, u-boot

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/4] doc: Migrate CodingStyle wiki page to sphinx
  2022-06-30 10:06   ` Simon Glass
@ 2022-07-09  6:15     ` Heinrich Schuchardt
  0 siblings, 0 replies; 23+ messages in thread
From: Heinrich Schuchardt @ 2022-07-09  6:15 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Tom Rini

On 6/30/22 12:06, Simon Glass wrote:
> On Mon, 27 Jun 2022 at 11:17, Tom Rini <trini@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.
>>
>> 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
>>
>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Please, don't add Reviewed-by if you did not have the time to thoroughly
review the patch. Acked-by is more adequate in this case.

Best regards

Heinrich




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/4] doc: Migrate DesignPrinciples wiki page to sphinx
  2022-06-27 17:17 ` [PATCH 2/4] doc: Migrate DesignPrinciples " Tom Rini
  2022-06-30 10:06   ` Simon Glass
  2022-07-08  7:22   ` Claudius Heine
@ 2022-07-09  6:37   ` Heinrich Schuchardt
  2022-07-09 12:39     ` Tom Rini
  2 siblings, 1 reply; 23+ messages in thread
From: Heinrich Schuchardt @ 2022-07-09  6:37 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Simon Glass

On 6/27/22 19:17, Tom Rini wrote:
> Move the current DesignPrinciples wiki page to
> doc/develop/designprinciples.rst.  The changes here are for formatting
> or slight rewording so that it reads well when linking to other sphinx
> documents.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>   doc/develop/designprinciples.rst | 197 +++++++++++++++++++++++++++++++
>   doc/develop/index.rst            |   1 +
>   2 files changed, 198 insertions(+)
>   create mode 100644 doc/develop/designprinciples.rst
>
> diff --git a/doc/develop/designprinciples.rst b/doc/develop/designprinciples.rst
> new file mode 100644
> index 000000000000..79694db77604
> --- /dev/null
> +++ b/doc/develop/designprinciples.rst
> @@ -0,0 +1,197 @@
> +.. SPDX-License-Identifier: GPL-2.0+:
> +
> +U-Boot Design Principles
> +========================
> +
> +The 10 Golden Rules of U-Boot design
> +------------------------------------
> +
> +Keep it Small
> +^^^^^^^^^^^^^
> +
> +U-Boot is a Boot Loader, i.e. its primary purpose in the shipping
> +system is to load some operating system.
> +That means that U-Boot is
> +necessary to perform a certain task, but it's nothing you want to
> +throw any significant resources at. Typically U-Boot is stored in
> +relatively small NOR flash memory, which is expensive
> +compared to the much larger NAND devices often used to store the
> +operating system and the application.
> +
> +At the moment, U-Boot supports boards with just 128 !KiB ROM or with
> +256 !KiB NOR flash. We should not easily ignore such configurations -

%s/!KiB/KiB/g

Are these numbers still true? U-Boot typically is larger than 512 KiB.

> +they may be the exception in among all the other supported boards,
> +but if a design uses such a resource-constrained hardware setup it is
> +usually because costs are critical, i. e. because the number of
> +manufactured boards might be tens or hundreds of thousands or even
> +millions...

%s/\.\.\././

> +
> +A usable and useful configuration of U-Boot, including a basic
> +interactive command interpreter, support for download over Ethernet
> +and the capability to program the flash shall fit in no more than 128 !KiB.

%s/!//

This number seems to be outdated. Typically U-Boot fits into 1 MiB.

> +
> +Keep it Fast
> +^^^^^^^^^^^^
> +
> +The end user is not interested in running U-Boot. In most embedded
> +systems he is not even aware that U-Boot exists. The user wants to
> +run some application code, and that as soon as possible after switching
> +on his device.
> +
> +It is therefore essential that U-Boot is as fast as possible,
> +especially that it loads and boots the operating system as fast as possible.
> +
> +To achieve this, the following design principles shall be followed:
> +
> +* Enable caches as soon and whenever possible

Separate bullet points by blank lines, please.

> +* Initialize devices only when they are needed within U-Boot, i.e. don't
> +  initialize the Ethernet interface(s) unless U-Boot performs a download over
> +  Ethernet; don't  initialize any IDE or USB devices unless U-Boot actually
> +  tries to load files from these, etc.  (and don't forget to shut down these

%s/  / /

> +  devices after using them  - otherwise nasty things may happen when you try to

%s/  / /

> +  boot your OS).
> +
> +Also, building of U-Boot shall be as fast as possible.
> +This makes it easier to run a build for all supported configurations
> +or at least for all configurations of a specific architecture,
> +which is essential for quality assurance.
> +If building is cumbersome and slow, most people will omit
> +this important step.
> +
> +Keep it Simple
> +^^^^^^^^^^^^^^
> +
> +U-Boot is a boot loader, but it is also a tool used for board
> +bring-up, for production testing, and for other activities

%/$/./

I cannot see how this sentence is related to the heading. The sentence
is repeated below.

> +
> +Keep it Portable
> +^^^^^^^^^^^^^^^^
> +
> +U-Boot is a boot loader, but it is also a tool used for board
> +bring-up, for production testing, and for other activities that are
> +very closely related to hardware development. So far, it has been
> +ported to several hundreds of different boards on about 30 different
> +processor families - please make sure that any code you add can be
> +used on as many different platforms as possible.
> +
> +Avoid assembly language whenever possible - only the reset code with
> +basic CPU initialization, maybe a static DRAM initialization and the C
> +stack setup should be in assembly.
> +All further initializations should be done in C using assembly/C

%s/initializations/initalization/

> +subroutines or inline macros. These functions represent some

Is extensive use of inline macros good coding style?

> +kind of HAL functionality and should be defined consistently on all
> +architectures. E.g. Basic MMU and cache control, stack pointer manipulation.
> +Non-existing functions should expand into empty macros or error codes.
> +
> +Don't make assumptions over the environment where U-Boot is running.

%s/over/about/

> +It may be communicating with a human operator on directly attached
> +serial console, but it may be through a GSM modem as well, or driven
> +by some automatic test or control system. So don't output any fancy
> +control character sequences or similar.
> +
> +Keep it Configurable
> +^^^^^^^^^^^^^^^^^^^^
> +
> +Section "Keep it Small" already explains about the size restrictions
> +for U-Boot on one side. On the other side, U-Boot is a powerful tool
> +with many, many extremely useful features. The maintainer or user of
> +each board will have to decide which features are important to him and
> +what shall be included with his specific board configuration to meet
> +his current requirements and restrictions.
> +
> +Please make sure that it is easy to add or remove features from a
> +board configuration, so everybody can make the best use of U-Boot on
> +his system.
> +
> +If a feature is not included, it should not have any residual code
> +bloating the build.
> +
> +Keep it Debuggable
> +^^^^^^^^^^^^^^^^^^
> +
> +Of course debuggable code is a big benefit for all of us contributing
> +in one way or another to the development of the U-Boot project. But
> +as already mentioned in section "Keep it Portable" above, U-Boot is
> +not only a tool in itself, it is often also used for hardware
> +bring-up, so debugging U-Boot often means that we don't know if we are
> +tracking down a problem in the U-Boot software or in the hardware we
> +are running on. Code that is clean and easy to understand and to
> +debug is all the more important to many of us.
> +
> +* One important feature of U-Boot is to enable output to the (usually serial)
> +  console as soon as possible in the boot process, even if this causes
> +  tradeoffs in other areas like memory footprint.

Please, add blank lines between bullet points.

> +* All initialization steps shall print some "begin doing this" message before
> +  they actually start, and some "done" message when they complete. For example,

Do we really want unnecessary output? This conflicts with improving boot
time.

Isn't using log_debug() preferred here?

> +  RAM initialization and size detection may print a "RAM: " before they start,
> +  and "256 MB\n" when done.  The purpose of this is that you can always see

%s/MB/MiB/

> +  which initialization step was running if there should be any problem.  This
> +  is important not only during software development, but also for the service
> +  people dealing with broken hardware in the field.
> +* U-Boot should be debuggable with simple JTAG or BDM equipment.  It shall use

%s/  / /

> +  a simple, single-threaded execution model.  Avoid any magic, which could

%s/  / /

> +  prevent easy debugging even when only 1 or 2 hardware breakpoints are
> +  available.
> +
> +Keep it Usable
> +^^^^^^^^^^^^^^
> +
> +Please always keep in mind that there are at least three different
> +groups of users for U-Boot, with completely different expectations
> +and requirements:
> +
> +* The end user of an embedded device just wants to run some application; he
> +  does not even want to know that U-Boot exists and only rarely interacts with
> +  it (for example to perform a reset to factory default settings etc.)

Missing empty lines.

> +* System designers and engineers working on the development of the application
> +  and/or the operating system want a powerful tool that can boot from any boot
> +  device they can imagine, they want it fast and scriptable and whatever - in

%s/and whatever//

> +  short, they want as many features supported as possible. And some more.

Avoid non-information:
%s/And some more.//

> +* The engineer who ports U-Boot to a new board and the board maintainer want
> +  U-Boot to be as simple as possible so porting it to and maintaining it on
> +  their hardware is easy for them.
> +* Make it easy to test. Add debug code (but don't re-invent the wheel - use
> +  existing macros like debug() or debugX()).
> +
> +Please always keep in mind that U-Boot tries to meet all these
> +different requirements.
> +
> +Keep it Maintainable
> +^^^^^^^^^^^^^^^^^^^^
> +
> +* Avoid ``#ifdefs`` where possible
> +* Use "weak" functions
> +* Always follow the :doc:`codingstyle` requirements.
> +
> +Keep it Beautiful
> +^^^^^^^^^^^^^^^^^
> +
> +* Keep the source code clean: strictly follow the :doc:`codingstyle`,
> +  keep lists (target names in the Makefiles, board names, etc.)
> +  alphabetically sorted, etc.

%s/$/\n/

Best regards

Heinrich

> +* Keep U-Boot console output clean: output only really necessary information,
> +  be terse but precise, keep output vertically aligned, do not use control
> +  character sequences (e.g. backspaces or \\r to do "spinning wheel" activity
> +  indicators), etc.
> +
> +Keep it Open
> +^^^^^^^^^^^^
> +
> +Contribute your work back to the whole community. Submit your changes
> +and extensions as patches to the U-Boot mailing list.
> +
> +Lemmas from the golden rules
> +----------------------------
> +
> +Generic Code is Good Code
> +^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +New code shall be as generic as possible and added to the U-Boot
> +abstraction hierarchy as high as possible. As few code as possible shall be
> +added in board directories as people usually do not expect re-usable code
> +there.  Thus peripheral drivers should be put below
> +"drivers" even if they start out supporting only one specific
> +configuration.  Note that it is not a requirement for such a first
> +instance to be generic as genericity generally cannot be extrapolated
> +from a single data point.
> +
> diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> index dde47994c71a..c0f4f0ba413a 100644
> --- a/doc/develop/index.rst
> +++ b/doc/develop/index.rst
> @@ -10,6 +10,7 @@ General
>      :maxdepth: 1
>
>      codingstyle
> +   designprinciples
>
>   Implementation
>   --------------


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/4] doc: codingstyle: Remove comment about '//' style comments
  2022-06-27 17:17 ` [PATCH 3/4] doc: codingstyle: Remove comment about '//' style comments Tom Rini
  2022-06-30 10:06   ` Simon Glass
@ 2022-07-09  6:40   ` Heinrich Schuchardt
  1 sibling, 0 replies; 23+ messages in thread
From: Heinrich Schuchardt @ 2022-07-09  6:40 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Simon Glass

On 6/27/22 19:17, Tom Rini wrote:
> For some time now we've allowed for '//' style comments, which mirrors
> the Linux kernel.  So drop this point here.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>   doc/develop/codingstyle.rst | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst
> index a41aed2764fb..bdf1efc6019a 100644
> --- a/doc/develop/codingstyle.rst
> +++ b/doc/develop/codingstyle.rst
> @@ -29,11 +29,6 @@ The following rules apply:
>     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:
>

Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] doc: Migrate Process wiki page to sphinx
  2022-06-27 17:17 ` [PATCH 4/4] doc: Migrate Process wiki page to sphinx Tom Rini
  2022-06-30 10:06   ` Simon Glass
  2022-07-08  7:06   ` Claudius Heine
@ 2022-07-09  6:55   ` Heinrich Schuchardt
  2022-07-09 12:41     ` Tom Rini
  2022-07-09 15:02     ` Tom Rini
  2 siblings, 2 replies; 23+ messages in thread
From: Heinrich Schuchardt @ 2022-07-09  6:55 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Simon Glass

On 6/27/22 19:17, Tom Rini wrote:
> Move the current Process wiki page to doc/develop/process.rst.  The
> changes here are for formatting or slight rewording so that it reads
> well when linking to other sphinx documents.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>   doc/develop/index.rst   |   1 +
>   doc/develop/process.rst | 182 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 183 insertions(+)
>   create mode 100644 doc/develop/process.rst
>
> diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> index c0f4f0ba413a..eab00a55382a 100644
> --- a/doc/develop/index.rst
> +++ b/doc/develop/index.rst
> @@ -11,6 +11,7 @@ General
>
>      codingstyle
>      designprinciples
> +   process
>
>   Implementation
>   --------------
> diff --git a/doc/develop/process.rst b/doc/develop/process.rst
> new file mode 100644
> index 000000000000..dd279fb9eff1
> --- /dev/null
> +++ b/doc/develop/process.rst
> @@ -0,0 +1,182 @@
> +.. SPDX-License-Identifier: GPL-2.0+:
> +
> +U-Boot Development Process
> +==========================
> +
> +Management Summary
> +------------------
> +
> +* Development happens in Release Cycles of 3 months.

Do we need all this capitalization?

> +* The first 2 weeks are called Merge Window, which is followed by a
> +  Stabilization Period.

Please, separate bullets by blank lines.

> +* Patches with new code get only accepted while the Merge Window is open.
> +* A patch that is generally in good shape and that was submitted while the
> +  Merge Window was open is eligible to go into the upcoming release, even if
> +  changes and resubmits are needed.
> +* During the Stabilization Period, only patches that contain bug fixes get
> +  applied.
> +
> +Phases of the Development Process
> +---------------------------------
> +
> +U-Boot development takes place in `Release Cycles
> +<https://www.denx.de/wiki/U-Boot/ReleaseCycle>`_.  A Release Cycle lasts
> +normally for three months.
> +
> +The first two weeks of each Release Cycle are called *Merge Window*.
> +
> +It is followed by a *Stabilization Period*.
> +
> +The end of a Release Cycle is marked by the release of a new U-Boot version.

Don't repeat yourself.

> +
> +Merge Window
> +------------
> +
> +The Merge Window is the period when new patches get submitted
> +(and hopefully accepted) for inclusion into U-Boot mainline.
> +
> +This is the only time when new code (like support for new processors or new
> +boards, or other new features or reorganization of code) is accepted.
> +
> +Twilight Time
> +-------------
> +
> +Usually patches do not get accepted as they are - the peer review that takes
> +place will usually require changes and resubmits of the patches before they
> +are considered to be ripe for inclusion into mainline.
> +
> +Also, the review often happens not immediately after a patch was submitted,
> +but only when somebody (usually the responsible custodian) finds time to do
> +this.
> +
> +In the result, the final version of such patches gets submitted after the
> +merge window has been closed.
> +
> +It is current practice in U-Boot that such patches are eligible to go into the
> +upcoming release.
> +
> +In the result, the release of the ``"-rc1"`` version does not immediately follow
> +the closing of the Merge Window.
> +
> +Stabilization Period
> +--------------------
> +
> +During the Stabilization Period only patches containing bug fixes get
> +applied.
> +
> +Corner Cases
> +------------
> +
> +Sometimes it is not clear if a patch contains a bug fix or not.
> +For example, changes that remove dead code, unused macros etc. or
> +that contain Coding Style fixes are not strict bug fixes.
> +
> +In such situations it is up to the responsible custodian to decide if he
> +applies such patches even when the Merge Window is closed.
> +
> +Exception: at the end of the Stabilization Period only strict bug
> +fixes my be applied.
> +
> +Sometimes patches miss the the Merge Window slightly - say by few
> +hours or even a day. Patch acceptance is not as critical as a
> +financial transaction, or such. So if there is such a slight delay,
> +the custodian is free to turn a blind eye and accept it anyway. The
> +idea of the development process is to make it foreseeable,
> +but not to slow down development.
> +
> +It makes more sense if an engineer spends another day on testing and
> +cleanup and submits the patch a couple of hours late, instead of
> +submitting a green patch which will waste efforts from several people
> +during several rounds of review and reposts.
> +
> +Differences to Linux Development Process
> +----------------------------------------

%s/to/to the/

> +
> +* In Linux, top-level maintainers will collect patches in their trees and send
> +  pull requests to Linus as soon as the merge window opens.
> +  So far, most U-Boot custodians do not work like that; they send pull requests
> +  only at (or even after) the end of the merge window.

%s/$/\n/

> +* In Linux, the closing of the merge window is marked by the release of the
> +  next ``"-rc1"``
> +  In U-Boot, ``"-rc1"`` will only be released after all (or at least most of
> +  the) patches that were submitted during the merge window have been applied.
> +
> +Custodians
> +----------
> +
> +The Custodians take responsibility for some area of the U-Boot code.  The
> +in-tree ``MAINTAINERS`` files list who is reponsible for which areas.

%/reponsible/responsible/

> +
> +It is their responsibility to pick up patches from the mailing list
> +that fall into their responsibility, and to process these.
> +
> +A very important responsibility of each custodian is to provide
> +feedback to the submitter of a patch about what is going on: if the
> +patch was accepted, or if it was rejected (which exact list of
> +reasons), if it needs to be reworked (with respective review
> +comments). Even a "I have no time now, will look into it later"
> +message is better than nothing. Also, if there are remarks to a
> +patch, these should leave no doubt if they were just comments and the
> +patch will be accepted anyway, or if the patch should be
> +reworked/resubmitted, or if it was rejected.
> +
> +Work flow of a Custodian
> +------------------------
> +
> +The normal flow of work in the U-Boot development process will look
> +like this:
> +
> +#. A developer submits a patch via e-mail to the u-boot-users mailing list.
> +   U-Boot has adopted the `Linux kernel signoff policy <https://groups.google.com/g/fa.linux.kernel/c/TLJIJVA-I6o?pli=1>`_, so the submitter must
> +   include a ``Signed-off-by:`` line.

Keep lines at 80 characters.

> +#. Everybody who can is invited to review and test the changes.  Reviews should
> +   reply on the mailing list with ``Acked-by`` lines.

%s/Acked-by/Reviewed-by/ ?

Please, refer to
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
for the usage of Reviewed-by and Acked-by.

> +#. The responsible custodian
> +
> +   #. inspects this patch, especially for:

This should not be a bullet but go into the line above.

> +
> +   #. :doc:`codingstyle`
> +   #. Basic logic:
> +
> +      * The patch fixes a real problem.
> +      * The patch does not introduce new problems, especially it does not break
> +        other boards or architectures
> +
> +   #. U-Boot Philosophy
> +   #. Applies cleanly to the source tree
> +   #. passes a ``MAKEALL`` compile test without creating new warnings
> +
> +#. Notes:
> +
> +  #. In some cases more than one custodian may be affected or feel responsible.
> +     To avoid duplicated efforts, the custodian who starts processing the
> +     patch should send a short ACK to the mailing list.
> +  #. We should create some tool to automatically do this.
> +  #. This is well documented in :doc:`designprinciples`.
> +  #. The custodian decides himself how recent the code must be.  It is
> +     acceptable to request patches against the last officially released
> +     version of U-Boot or newer.  Of course a custodian can also accept
> +     patches against older code.
> +  #. Commits should show original author in the ``author`` field and include all
> +      sign off/ack lines.
> +
> +5. The custodian decides to accept or to reject the patch.

Don't mix # bullets and numbers.

> +#. If accepted, the custodian adds the patch to his public git repository and
> +   notifies the mailing list. This note should include:
> +
> +   * a short description of the changes
> +   * the list of the affected boards / architectures etc.
> +   * suggested tests
> +
> +   Although the custodian is supposed to perform his own tests
> +   it is a well-known and accepted fact that he needs help from
> +   other developers who - for example - have access to the required
> +   hardware or tool chains.
> +   The custodian request help for tests and feedback from
> +   specific maintainers and U-Boot users.
> +#. Once tests are passed, some agreed time limit expires, the custodian

No clue what agreed time limit you are referring to.

The custodian will create a merge request when the merge window matching
the type of the patch (feature or bug) is open.

> +   requests that the changes in his public git repository be merged into the
> +   main tree. If necessary, the custodian may have to adapt his changes to
> +   allow for a clean merge.
> +   Todo: define a reasonable time limit. 3 weeks?

This todo-line is not helpful. If the patch contains a new feature, the
custodian has to wait for the next merge window before issuing a pull
request for it.

> +

Please, avoid empty lines at the end of file.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/4] doc: Migrate CodingStyle wiki page to sphinx
  2022-07-09  6:12   ` Heinrich Schuchardt
@ 2022-07-09 12:32     ` Tom Rini
  2022-07-09 12:36     ` Tom Rini
  1 sibling, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-07-09 12:32 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Simon Glass, u-boot

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

On Sat, Jul 09, 2022 at 08:12:27AM +0200, Heinrich Schuchardt wrote:
> 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/

For the wiki->rST changes, OK, I'll do v2 (for the series).  For
everything else, I would prefer to do the import as-is as much as
possible and then start making real changes as follow-up patches.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/4] doc: Migrate CodingStyle wiki page to sphinx
  2022-07-09  6:12   ` Heinrich Schuchardt
  2022-07-09 12:32     ` Tom Rini
@ 2022-07-09 12:36     ` Tom Rini
  1 sibling, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-07-09 12:36 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: Simon Glass, u-boot

[-- Attachment #1: Type: text/plain, Size: 2563 bytes --]

On Sat, Jul 09, 2022 at 08:12:27AM +0200, Heinrich Schuchardt wrote:
> 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.
[snip]
> > +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.

Linux doesn't have a hard 80 character limit anymore and neither do we,
keep in mind.

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

Different than markdown, OK.

[snip]
> > +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.

This is more substantive than wiki->rST so I'll handle this in another
patch.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/4] doc: Migrate DesignPrinciples wiki page to sphinx
  2022-07-09  6:37   ` Heinrich Schuchardt
@ 2022-07-09 12:39     ` Tom Rini
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-07-09 12:39 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: u-boot, Simon Glass

[-- Attachment #1: Type: text/plain, Size: 2446 bytes --]

On Sat, Jul 09, 2022 at 08:37:12AM +0200, Heinrich Schuchardt wrote:
> On 6/27/22 19:17, Tom Rini wrote:
> > Move the current DesignPrinciples wiki page to
> > doc/develop/designprinciples.rst.  The changes here are for formatting
> > or slight rewording so that it reads well when linking to other sphinx
> > documents.
> > 
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >   doc/develop/designprinciples.rst | 197 +++++++++++++++++++++++++++++++
> >   doc/develop/index.rst            |   1 +
> >   2 files changed, 198 insertions(+)
> >   create mode 100644 doc/develop/designprinciples.rst
> > 
> > diff --git a/doc/develop/designprinciples.rst b/doc/develop/designprinciples.rst
> > new file mode 100644
> > index 000000000000..79694db77604
> > --- /dev/null
> > +++ b/doc/develop/designprinciples.rst
> > @@ -0,0 +1,197 @@
> > +.. SPDX-License-Identifier: GPL-2.0+:
> > +
> > +U-Boot Design Principles
> > +========================
> > +
> > +The 10 Golden Rules of U-Boot design
> > +------------------------------------
> > +
> > +Keep it Small
> > +^^^^^^^^^^^^^
> > +
> > +U-Boot is a Boot Loader, i.e. its primary purpose in the shipping
> > +system is to load some operating system.
> > +That means that U-Boot is
> > +necessary to perform a certain task, but it's nothing you want to
> > +throw any significant resources at. Typically U-Boot is stored in
> > +relatively small NOR flash memory, which is expensive
> > +compared to the much larger NAND devices often used to store the
> > +operating system and the application.
> > +
> > +At the moment, U-Boot supports boards with just 128 !KiB ROM or with
> > +256 !KiB NOR flash. We should not easily ignore such configurations -
> 
> %s/!KiB/KiB/g

I fixed that in a follow-up.

> Are these numbers still true? U-Boot typically is larger than 512 KiB.

More true than not still, yes.  And then very much so something to keep
in  mind for SPL.  It's more true that NOR isn't common than less than
512KiB isn't true.

[snip]
> > +Keep it Simple
> > +^^^^^^^^^^^^^^
> > +
> > +U-Boot is a boot loader, but it is also a tool used for board
> > +bring-up, for production testing, and for other activities
> 
> %/$/./
> 
> I cannot see how this sentence is related to the heading. The sentence
> is repeated below.

It's also verbatim what the original page says.  It's repetition for
emphasis.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] doc: Migrate Process wiki page to sphinx
  2022-07-09  6:55   ` Heinrich Schuchardt
@ 2022-07-09 12:41     ` Tom Rini
  2022-07-09 15:02     ` Tom Rini
  1 sibling, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-07-09 12:41 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: u-boot, Simon Glass

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]

On Sat, Jul 09, 2022 at 08:55:48AM +0200, Heinrich Schuchardt wrote:
> On 6/27/22 19:17, Tom Rini wrote:
> > Move the current Process wiki page to doc/develop/process.rst.  The
> > changes here are for formatting or slight rewording so that it reads
> > well when linking to other sphinx documents.
> > 
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >   doc/develop/index.rst   |   1 +
> >   doc/develop/process.rst | 182 ++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 183 insertions(+)
> >   create mode 100644 doc/develop/process.rst
> > 
> > diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> > index c0f4f0ba413a..eab00a55382a 100644
> > --- a/doc/develop/index.rst
> > +++ b/doc/develop/index.rst
> > @@ -11,6 +11,7 @@ General
> > 
> >      codingstyle
> >      designprinciples
> > +   process
> > 
> >   Implementation
> >   --------------
> > diff --git a/doc/develop/process.rst b/doc/develop/process.rst
> > new file mode 100644
> > index 000000000000..dd279fb9eff1
> > --- /dev/null
> > +++ b/doc/develop/process.rst
> > @@ -0,0 +1,182 @@
> > +.. SPDX-License-Identifier: GPL-2.0+:
> > +
> > +U-Boot Development Process
> > +==========================
> > +
> > +Management Summary
> > +------------------
> > +
> > +* Development happens in Release Cycles of 3 months.
> 
> Do we need all this capitalization?

It's for emphasis I believe.

> > +#. Notes:
> > +
> > +  #. In some cases more than one custodian may be affected or feel responsible.
> > +     To avoid duplicated efforts, the custodian who starts processing the
> > +     patch should send a short ACK to the mailing list.
> > +  #. We should create some tool to automatically do this.
> > +  #. This is well documented in :doc:`designprinciples`.
> > +  #. The custodian decides himself how recent the code must be.  It is
> > +     acceptable to request patches against the last officially released
> > +     version of U-Boot or newer.  Of course a custodian can also accept
> > +     patches against older code.
> > +  #. Commits should show original author in the ``author`` field and include all
> > +      sign off/ack lines.
> > +
> > +5. The custodian decides to accept or to reject the patch.
> 
> Don't mix # bullets and numbers.

I believe I needed to for them to render in order.  I guess I'll just
only do the number in v2.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/4] doc: Migrate Process wiki page to sphinx
  2022-07-09  6:55   ` Heinrich Schuchardt
  2022-07-09 12:41     ` Tom Rini
@ 2022-07-09 15:02     ` Tom Rini
  1 sibling, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-07-09 15:02 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: u-boot, Simon Glass

[-- Attachment #1: Type: text/plain, Size: 2629 bytes --]

On Sat, Jul 09, 2022 at 08:55:48AM +0200, Heinrich Schuchardt wrote:
> On 6/27/22 19:17, Tom Rini wrote:
[snip]
> > +Phases of the Development Process
> > +---------------------------------
> > +
> > +U-Boot development takes place in `Release Cycles
> > +<https://www.denx.de/wiki/U-Boot/ReleaseCycle>`_.  A Release Cycle lasts
> > +normally for three months.
> > +
> > +The first two weeks of each Release Cycle are called *Merge Window*.
> > +
> > +It is followed by a *Stabilization Period*.
> > +
> > +The end of a Release Cycle is marked by the release of a new U-Boot version.
> 
> Don't repeat yourself.

I'm not seeing repetition really here.

[snip]
> > +Work flow of a Custodian
> > +------------------------
> > +
> > +The normal flow of work in the U-Boot development process will look
> > +like this:
> > +
> > +#. A developer submits a patch via e-mail to the u-boot-users mailing list.
> > +   U-Boot has adopted the `Linux kernel signoff policy <https://groups.google.com/g/fa.linux.kernel/c/TLJIJVA-I6o?pli=1>`_, so the submitter must
> > +   include a ``Signed-off-by:`` line.
> 
> Keep lines at 80 characters.
> 
> > +#. Everybody who can is invited to review and test the changes.  Reviews should
> > +   reply on the mailing list with ``Acked-by`` lines.
> 
> %s/Acked-by/Reviewed-by/ ?
> 
> Please, refer to
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
> for the usage of Reviewed-by and Acked-by.

Yes, it would be good to reword this to reference the current kernel.org
documentation, I'll try something.

> > +#. The responsible custodian
> > +
> > +   #. inspects this patch, especially for:
> 
> This should not be a bullet but go into the line above.

That would read better, yes.

[snip]
> > +#. Once tests are passed, some agreed time limit expires, the custodian
> 
> No clue what agreed time limit you are referring to.
> 
> The custodian will create a merge request when the merge window matching
> the type of the patch (feature or bug) is open.

This has long been more informal rather than following a defined
process, yes.  I'll try and reword things a bit.

> 
> > +   requests that the changes in his public git repository be merged into the
> > +   main tree. If necessary, the custodian may have to adapt his changes to
> > +   allow for a clean merge.
> > +   Todo: define a reasonable time limit. 3 weeks?
> 
> This todo-line is not helpful. If the patch contains a new feature, the
> custodian has to wait for the next merge window before issuing a pull
> request for it.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2022-07-09 15:02 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27 17:17 [PATCH 0/4] Migrate some wiki pages to sphinx Tom Rini
2022-06-27 17:17 ` [PATCH 1/4] doc: Migrate CodingStyle wiki page " Tom Rini
2022-06-30 10:06   ` Simon Glass
2022-07-09  6:15     ` Heinrich Schuchardt
2022-07-09  6:12   ` Heinrich Schuchardt
2022-07-09 12:32     ` Tom Rini
2022-07-09 12:36     ` Tom Rini
2022-06-27 17:17 ` [PATCH 2/4] doc: Migrate DesignPrinciples " Tom Rini
2022-06-30 10:06   ` Simon Glass
2022-07-08  7:22   ` Claudius Heine
2022-07-09  6:37   ` Heinrich Schuchardt
2022-07-09 12:39     ` Tom Rini
2022-06-27 17:17 ` [PATCH 3/4] doc: codingstyle: Remove comment about '//' style comments Tom Rini
2022-06-30 10:06   ` Simon Glass
2022-07-09  6:40   ` Heinrich Schuchardt
2022-06-27 17:17 ` [PATCH 4/4] doc: Migrate Process wiki page to sphinx Tom Rini
2022-06-30 10:06   ` Simon Glass
2022-07-08  7:06   ` Claudius Heine
2022-07-08  7:22     ` Martin Bonner
2022-07-08 17:44       ` Tom Rini
2022-07-09  6:55   ` Heinrich Schuchardt
2022-07-09 12:41     ` Tom Rini
2022-07-09 15:02     ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.