diff mbox series

[v2] Documentation/process: add soc maintainer handbook

Message ID 20230530-multiple-whooping-ee5706fceb67@wendy (mailing list archive)
State Superseded
Headers show
Series [v2] Documentation/process: add soc maintainer handbook | expand

Commit Message

Conor Dooley May 30, 2023, 12:49 p.m. UTC
Arnd suggested that adding a maintainer handbook for the SoC "subsystem"
would be helpful in trying to bring on board maintainers for the various
new platforms cropping up in RISC-V land.

Add a document briefly describing the role of the SoC subsystem and some
basic advice for (new) platform maintainers.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
Changes in v2:
- add Krzysztof's suggested method for avoiding inter-branch
  dependencies
- explicitly mention that tags should be signed
- link to the devicetree abi document, rather than trying to explain it
  here & reword that whole section
- fix some typos, capitalisation & unify bullet style

The devicetree abi doc feels quite out of date at this point, and could
probably do with a spring clean - but it also feels like hallowed ground
on which one should tread lightly, so I won't go near that til Rob is
back.
---
 Documentation/devicetree/bindings/ABI.rst     |   2 +
 .../devicetree/bindings/writing-schema.rst    |   2 +
 .../process/maintainer-handbooks.rst          |   3 +-
 Documentation/process/maintainer-soc.rst      | 178 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 5 files changed, 185 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/process/maintainer-soc.rst

Comments

Jonathan Corbet May 30, 2023, 1:02 p.m. UTC | #1
Conor Dooley <conor.dooley@microchip.com> writes:

> Arnd suggested that adding a maintainer handbook for the SoC "subsystem"
> would be helpful in trying to bring on board maintainers for the various
> new platforms cropping up in RISC-V land.
>
> Add a document briefly describing the role of the SoC subsystem and some
> basic advice for (new) platform maintainers.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Changes in v2:
> - add Krzysztof's suggested method for avoiding inter-branch
>   dependencies
> - explicitly mention that tags should be signed
> - link to the devicetree abi document, rather than trying to explain it
>   here & reword that whole section
> - fix some typos, capitalisation & unify bullet style
>
> The devicetree abi doc feels quite out of date at this point, and could
> probably do with a spring clean - but it also feels like hallowed ground
> on which one should tread lightly, so I won't go near that til Rob is
> back.

So, this is a nit, but worth considering...

>  Documentation/devicetree/bindings/ABI.rst     |   2 +
>  .../devicetree/bindings/writing-schema.rst    |   2 +
>  .../process/maintainer-handbooks.rst          |   3 +-
>  Documentation/process/maintainer-soc.rst      | 178 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  5 files changed, 185 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/process/maintainer-soc.rst
>
> diff --git a/Documentation/devicetree/bindings/ABI.rst b/Documentation/devicetree/bindings/ABI.rst
> index a885713cf184..93ec82f78ae5 100644
> --- a/Documentation/devicetree/bindings/ABI.rst
> +++ b/Documentation/devicetree/bindings/ABI.rst
> @@ -1,5 +1,7 @@
>  .. SPDX-License-Identifier: GPL-2.0
>  
> +.. _devicetree-abi:

Somehow we've developed this habit of putting labels at the top of each
file; I really think that they just add clutter and are best left out.
Without the label, this reference:

> +Perhaps one of the most important things to highlight is that dt-bindings
> +document the ABI between the devicetree and the kernel. Please see
> +:ref:`devicetree-abi` more information on the ABI.

...can just be written as "Please see
Documentation/devicetree/bindings/ABI.rst".  The cross-reference link
will be generated as expected, and readers of the plain-text docs don't
have to go grepping to find the reference.

Thanks,

jon
Conor Dooley May 30, 2023, 1:12 p.m. UTC | #2
On Tue, May 30, 2023 at 07:02:26AM -0600, Jonathan Corbet wrote:
> Conor Dooley <conor.dooley@microchip.com> writes:

> > diff --git a/Documentation/devicetree/bindings/ABI.rst b/Documentation/devicetree/bindings/ABI.rst
> > index a885713cf184..93ec82f78ae5 100644
> > --- a/Documentation/devicetree/bindings/ABI.rst
> > +++ b/Documentation/devicetree/bindings/ABI.rst
> > @@ -1,5 +1,7 @@
> >  .. SPDX-License-Identifier: GPL-2.0
> >  
> > +.. _devicetree-abi:
> 
> Somehow we've developed this habit of putting labels at the top of each
> file; I really think that they just add clutter and are best left out.
> Without the label, this reference:
> 
> > +Perhaps one of the most important things to highlight is that dt-bindings
> > +document the ABI between the devicetree and the kernel. Please see
> > +:ref:`devicetree-abi` more information on the ABI.
> 
> ...can just be written as "Please see
> Documentation/devicetree/bindings/ABI.rst".  The cross-reference link
> will be generated as expected, and readers of the plain-text docs don't
> have to go grepping to find the reference.

Sure. As someone who does read these things in their editor that sounds
preferable to me. I didn't know that I could do that, as the whole
"building the docs" thing is new to me ;)
I'll wait a bit before resubmitting for obvious reasons.

Cheers,
Conor.
Krzysztof Kozlowski May 30, 2023, 1:14 p.m. UTC | #3
On 30/05/2023 14:49, Conor Dooley wrote:
> Arnd suggested that adding a maintainer handbook for the SoC "subsystem"
> would be helpful in trying to bring on board maintainers for the various
> new platforms cropping up in RISC-V land.
> 
> Add a document briefly describing the role of the SoC subsystem and some
> basic advice for (new) platform maintainers.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Changes in v2:
> - add Krzysztof's suggested method for avoiding inter-branch
>   dependencies
> - explicitly mention that tags should be signed
> - link to the devicetree abi document, rather than trying to explain it
>   here & reword that whole section
> - fix some typos, capitalisation & unify bullet style
> 
> The devicetree abi doc feels quite out of date at this point, and could
> probably do with a spring clean - but it also feels like hallowed groun

Yep, but that's another topic.

> d
> on which one should tread lightly, so I won't go near that til Rob is
> back.

Thanks!

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Randy Dunlap May 30, 2023, 3:28 p.m. UTC | #4
On 5/30/23 05:49, Conor Dooley wrote:
> Arnd suggested that adding a maintainer handbook for the SoC "subsystem"
> would be helpful in trying to bring on board maintainers for the various
> new platforms cropping up in RISC-V land.
> 
> Add a document briefly describing the role of the SoC subsystem and some
> basic advice for (new) platform maintainers.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Changes in v2:
> - add Krzysztof's suggested method for avoiding inter-branch
>   dependencies
> - explicitly mention that tags should be signed
> - link to the devicetree abi document, rather than trying to explain it
>   here & reword that whole section
> - fix some typos, capitalisation & unify bullet style
> 
> The devicetree abi doc feels quite out of date at this point, and could
> probably do with a spring clean - but it also feels like hallowed ground
> on which one should tread lightly, so I won't go near that til Rob is
> back.
> ---
>  Documentation/devicetree/bindings/ABI.rst     |   2 +
>  .../devicetree/bindings/writing-schema.rst    |   2 +
>  .../process/maintainer-handbooks.rst          |   3 +-
>  Documentation/process/maintainer-soc.rst      | 178 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  5 files changed, 185 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/process/maintainer-soc.rst
> 
> diff --git a/Documentation/devicetree/bindings/ABI.rst b/Documentation/devicetree/bindings/ABI.rst
> index a885713cf184..93ec82f78ae5 100644
> --- a/Documentation/devicetree/bindings/ABI.rst
> +++ b/Documentation/devicetree/bindings/ABI.rst
> @@ -1,5 +1,7 @@
>  .. SPDX-License-Identifier: GPL-2.0
>  
> +.. _devicetree-abi:
> +
>  ===================
>  Devicetree (DT) ABI
>  ===================
> diff --git a/Documentation/devicetree/bindings/writing-schema.rst b/Documentation/devicetree/bindings/writing-schema.rst
> index 4a381d20f2b4..640d857dabf3 100644
> --- a/Documentation/devicetree/bindings/writing-schema.rst
> +++ b/Documentation/devicetree/bindings/writing-schema.rst
> @@ -136,6 +136,8 @@ installed. Ensure they are in your PATH (~/.local/bin by default).
>  
>  Recommended is also to install yamllint (used by dtschema when present).

I don't see anything in Documentation/ about where to find yamllint...
please.

>  
> +.. _running-checks:
> +
>  Running checks
>  ~~~~~~~~~~~~~~
>  

> diff --git a/Documentation/process/maintainer-soc.rst b/Documentation/process/maintainer-soc.rst
> new file mode 100644
> index 000000000000..9683c7d199b2
> --- /dev/null
> +++ b/Documentation/process/maintainer-soc.rst
> @@ -0,0 +1,178 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _maintainer-soc:
> +
> +=============
> +SoC Subsystem
> +=============
> +
> +Overview
> +--------
> +
> +The SoC subsystem is a place of aggregation for SoC-specific code.
> +The main components of the subsystem are:
> +
> +* devicetrees for 32- & 64-bit ARM and RISC-V
> +* 32-bit ARM board files (arch/arm/mach*)
> +* 32- & 64-bit ARM defconfigs
> +* SoC specific drivers across architectures, in particular for 32- & 64-bit

     SoC-specific

> +  ARM, RISC-V and Loongarch
> +
> +These "SoC specific drivers" do not include clock, GPIO etc drivers that have

          SoC-specific                                GPIO, etc. drivers that have

> +other top-level maintainers. The drivers/soc/ directory is generally meant
> +for kernel-internal drivers that are used by other drivers to provide SoC

                                                                         SoC-

> +specific functionality like identifying a SoC revision or interfacing with

                     I would write:        an SoC

> +power domains.
> +
> +The SoC subsystem also serves as an intermediate location for changes to
> +drivers/bus, drivers/firmware, drivers/reset and drivers/memory.  The addition
> +of new platforms, or the removal of existing ones, often go through the SoC
> +tree as a dedicated branch covering multiple subsystems.
> +
> +The main SoC tree is housed on git.kernel.org:
> +  https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/
> +
> +Clearly this is quite a wide range of topics, which no one person, or even
> +small group of people are capable of maintaining.  Instead, the SoC subsystem
> +is comprised of many submaintainers, each taking care of individual platforms
> +and driver sub-directories.

submaintainers and sub-directories ? hm.

> +In this regard, "platform" usually refers to a series of SoCs from a given
> +vendor, for example, Nvidia's series of Tegra SoCs.  Many submaintainers operate
> +on a vendor level, responsible for multiple product lines.  For several reasons,
> +including acquisitions/different business units in a company, things vary
> +significantly here.  The various submaintainers are documented in the
> +MAINTAINERS file.
> +
> +Most of these submaintainers have their own trees where they stage patches,
> +sending pull requests to the main SoC tree.  These trees are usually, but not
> +always, listed in MAINTAINERS.  The main SoC maintainers can be reached via the
> +alias soc@kernel.org if there is no platform-specific maintainer, or if they
> +are unresponsive.
> +
> +What the SoC tree is not, however, is a location for architecture specific code

                                                        architecture-specific

> +changes.  Each architecture has it's own maintainers that are responsible for

                                   its

> +architectural details, cpu errata and the like.

                          CPU

> +
> +Information for (new) Submaintainers
> +------------------------------------
> +
> +As new platforms spring up, they often bring with them new submaintainers,
> +many of whom work for the silicon vendor, and may not be familiar with the
> +process.
> +
> +Devicetree ABI Stability
> +~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Perhaps one of the most important things to highlight is that dt-bindings
> +document the ABI between the devicetree and the kernel. Please see
> +:ref:`devicetree-abi` more information on the ABI.
> +
> +If changes are being made to a devicetree that are incompatible with old
> +kernels, the devicetree patch should not be applied until the driver is, or an
> +appropriate time later.  Most importantly, any incompatible changes should be
> +clearly pointed out in the patch description and pull request, along with the
> +expected impact on existing users, such as bootloaders or other operating
> +systems.
> +
> +Driver Branch Dependencies
> +~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +A common problem is synchronizing changes between device drivers and devicetree
> +files, even if a change is compatible in both directions, this may require

   files. Even if

> +coordinating how the changes get merged through different maintainer trees.
> +
> +Usually the branch that includes a driver change will also include the
> +corresponding change to the devicetree binding description, to ensure they are
> +in fact compatible.  This means that the devicetree branch can end up causing

s/in fact/remain/
(suggestion)

> +warnings in the "make dtbs_check" step.  If a devicetree change depends on
> +missing additions to a header file in include/dt-bindings/, it will fail the
> +"make dtbs" step and not get merged.
> +
> +There are multiple ways to deal with this:
> +
> +* Avoid defining custom macros in include/dt-bindings/ for hardware constants
> +  that can be derived from a datasheet -- binding macros in header file should

                                                            in a header file | in header files

> +  only be used as a last resort if there is no natural way to define a binding
> +
> +* Use literal values in the devicetree file in place of macros even when a
> +  header is required, and change them to the named representation in a
> +  following release
> +
> +* Defer the devicetree changes to a release after the binding and driver have
> +  already been merged
> +
> +* Change the bindings in a shared immutable branch that is used as the base for
> +  both the driver change and the devicetree changes
> +
> +* Add duplicate defines in the devicetree file guarded by an #ifndef section,
> +  removing them in a later release
> +
> +Devicetree Naming Convention
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The general naming scheme for devicetree files are as follows.  The aspects of a

                      scheme ...                  is

> +platform that are set at the SoC level, like cpu cores, are contained in a file

                                                CPU

> +named $soc.dtsi, for example, jh7100.dtsi.  Integration details, that will vary
> +from board to board, are described in $soc-$board.dtsi.  An example of this is
> +jh7100-beaglev-starlight.dts.  Often many boards are variations on a theme, and

                            ^^^ Why not dtsi, like the sentence before says?
or is the $soc-$board.dtsi wrong?

> +frequently there are intermediate files, such as jh7100-common.dtsi, which sit
> +between the $soc.dtsi and $soc-$board.dts files, containing the descriptions of
> +common hardware.
> +
> +Some platforms also have System on Modules, containing an SoC, which are then
> +integrated into several different boards. For these platforms, $soc-$som.dtsi
> +and $soc-$som-$board.dts are typical.
> +
> +Directories are usually named after the vendor of the SoC at the time of it's

                                                                            its

> +inclusion, leading to some historical directory names in the tree.
> +
> +Validating Devicetree Files
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +``make dtbs_check`` can be used to validate that devicetree files are compliant
> +with the dt-bindings that describe the ABI.  Please see :ref:`running-checks`
> +for more information on the validation of devicetrees.
> +
> +For new platforms, or additions to existing ones, ``make dtbs_check`` should not
> +add any new warnings.  For RISC-V, as it has the advantage of being a newer
> +architecture, ``make dtbs_check W=1`` is required to not add any new warnings.
> +If in any doubt about a devicetree change, reach out to the devicetree
> +maintainers.
> +
> +Branches and Pull Requests
> +~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Just as the main SoC tree has several branches, it is expected that
> +submaintainers will do the same. Driver, defconfig and devicetree changes should
> +all be split into separate branches and appear in separate pull requests to the
> +SoC maintainers.  Each branch should be usable by itself and avoid
> +regressions that originate from dependencies on other branches.
> +
> +Small sets of patches can also be sent as separate emails to soc@kernel.org,
> +grouped into the same categories.
> +
> +If changes do not fit into the normal patterns, there can be additional
> +top-level branches, e.g. for a treewide rework, or the addition of new SoC
> +platforms including dts files and drivers.
> +
> +Branches with a lot of changes can benefit from getting split up into separate
> +topics branches, even if they end up getting merged into the same branch of the
> +SoC tree.  An example here would be one branch for devicetree warning fixes, one
> +for a rework and one for newly added boards.
> +
> +Another common way to split up changes is to send an early pull request with the
> +majority of the changes at some point between rc1 and rc4, following up with one
> +or more smaller pull requests towards the end of the cycle that can add late
> +changes or address problems idenfied while testing the first set.

                               identified

> +
> +While there is no cut-off time for late pull requests, it helps to only send
> +small branches as time gets closer to the merge window.
> +
> +Pull requests for bugfixes for the current release can be sent at any time, but
> +again having multiple smaller branches is better than trying to combine too many
> +patches into one pull request.
> +
> +The subject line of a pull request should begin with "[GIT PULL]" and made using
> +a signed tag, rather than a branch.  This tag should contain a short description
> +summarising the changes in the pull request.  For more detail on sending pull
> +requests, please see :ref:`pullrequests`.

>
Conor Dooley May 30, 2023, 10:26 p.m. UTC | #5
Hey Randy,

On Tue, May 30, 2023 at 08:28:44AM -0700, Randy Dunlap wrote:
> On 5/30/23 05:49, Conor Dooley wrote:

> >  Recommended is also to install yamllint (used by dtschema when present).
> 
> I don't see anything in Documentation/ about where to find yamllint...
> please.

Sounds like an unrelated fix to me ;)

> submaintainers and sub-directories ? hm.

I had sub-maintainers before being asked to change! I think that
subdirectories is correct though & the hyphenated form is incorrect.

> > +named $soc.dtsi, for example, jh7100.dtsi.  Integration details, that will vary
> > +from board to board, are described in $soc-$board.dtsi.  An example of this is
> > +jh7100-beaglev-starlight.dts.  Often many boards are variations on a theme, and
> 
>                             ^^^ Why not dtsi, like the sentence before says?
> or is the $soc-$board.dtsi wrong?

$soc-$board.dtsi is wrong.

> > +Another common way to split up changes is to send an early pull request with the
> > +majority of the changes at some point between rc1 and rc4, following up with one
> > +or more smaller pull requests towards the end of the cycle that can add late
> > +changes or address problems idenfied while testing the first set.
> 
>                                identified

Surprised nvim didn't whinge about that one.

Thanks for pointing out the wee mistakes all over the place,
Conor.
Bagas Sanjaya May 31, 2023, 6:30 a.m. UTC | #6
On Tue, May 30, 2023 at 01:49:36PM +0100, Conor Dooley wrote:
> diff --git a/Documentation/process/maintainer-soc.rst b/Documentation/process/maintainer-soc.rst
> new file mode 100644
> index 000000000000..9683c7d199b2
> --- /dev/null
> +++ b/Documentation/process/maintainer-soc.rst
> @@ -0,0 +1,178 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _maintainer-soc:
> +
> +=============
> +SoC Subsystem
> +=============
> +
> +Overview
> +--------
> +
> +The SoC subsystem is a place of aggregation for SoC-specific code.
> +The main components of the subsystem are:
> +
> +* devicetrees for 32- & 64-bit ARM and RISC-V
> +* 32-bit ARM board files (arch/arm/mach*)
> +* 32- & 64-bit ARM defconfigs
> +* SoC specific drivers across architectures, in particular for 32- & 64-bit
> +  ARM, RISC-V and Loongarch
> +
> +These "SoC specific drivers" do not include clock, GPIO etc drivers that have
> +other top-level maintainers. The drivers/soc/ directory is generally meant
> +for kernel-internal drivers that are used by other drivers to provide SoC
> +specific functionality like identifying a SoC revision or interfacing with
> +power domains.
> +
> +The SoC subsystem also serves as an intermediate location for changes to
> +drivers/bus, drivers/firmware, drivers/reset and drivers/memory.  The addition
> +of new platforms, or the removal of existing ones, often go through the SoC
> +tree as a dedicated branch covering multiple subsystems.
> +
> +The main SoC tree is housed on git.kernel.org:
> +  https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/
> +
> +Clearly this is quite a wide range of topics, which no one person, or even
> +small group of people are capable of maintaining.  Instead, the SoC subsystem
> +is comprised of many submaintainers, each taking care of individual platforms
> +and driver sub-directories.
> +In this regard, "platform" usually refers to a series of SoCs from a given
> +vendor, for example, Nvidia's series of Tegra SoCs.  Many submaintainers operate
> +on a vendor level, responsible for multiple product lines.  For several reasons,
> +including acquisitions/different business units in a company, things vary
> +significantly here.  The various submaintainers are documented in the
> +MAINTAINERS file.
> +
> +Most of these submaintainers have their own trees where they stage patches,
> +sending pull requests to the main SoC tree.  These trees are usually, but not
> +always, listed in MAINTAINERS.  The main SoC maintainers can be reached via the
> +alias soc@kernel.org if there is no platform-specific maintainer, or if they
> +are unresponsive.
> +
> +What the SoC tree is not, however, is a location for architecture specific code
> +changes.  Each architecture has it's own maintainers that are responsible for
> +architectural details, cpu errata and the like.
> +
> +Information for (new) Submaintainers
> +------------------------------------
> +
> +As new platforms spring up, they often bring with them new submaintainers,
> +many of whom work for the silicon vendor, and may not be familiar with the
> +process.
> +
> +Devicetree ABI Stability
> +~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Perhaps one of the most important things to highlight is that dt-bindings
> +document the ABI between the devicetree and the kernel. Please see
> +:ref:`devicetree-abi` more information on the ABI.
> +
> +If changes are being made to a devicetree that are incompatible with old
> +kernels, the devicetree patch should not be applied until the driver is, or an
Until the incompatible driver changes are merged?
> +appropriate time later.  Most importantly, any incompatible changes should be
> +clearly pointed out in the patch description and pull request, along with the
> +expected impact on existing users, such as bootloaders or other operating
> +systems.
> +
> +Driver Branch Dependencies
> +~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +A common problem is synchronizing changes between device drivers and devicetree
> +files, even if a change is compatible in both directions, this may require
> +coordinating how the changes get merged through different maintainer trees.
> +
> +Usually the branch that includes a driver change will also include the
> +corresponding change to the devicetree binding description, to ensure they are
> +in fact compatible.  This means that the devicetree branch can end up causing
> +warnings in the "make dtbs_check" step.  If a devicetree change depends on
> +missing additions to a header file in include/dt-bindings/, it will fail the
> +"make dtbs" step and not get merged.

Sounds like passing `make dtbs` is a merging requirement.

> +Pull requests for bugfixes for the current release can be sent at any time, but
> +again having multiple smaller branches is better than trying to combine too many
> +patches into one pull request.
> +
> +The subject line of a pull request should begin with "[GIT PULL]" and made using
> +a signed tag, rather than a branch.  This tag should contain a short description
> +summarising the changes in the pull request.  For more detail on sending pull
> +requests, please see :ref:`pullrequests`.

As jon had said, I simply prefer to write the last cross-ref as:

```
... For more details on sending pull requests, see Documentation/maintainer/pull-requests.rst.
```

Thanks.
Krzysztof Kozlowski May 31, 2023, 6:51 a.m. UTC | #7
On 31/05/2023 08:30, Bagas Sanjaya wrote:
>> +appropriate time later.  Most importantly, any incompatible changes should be
>> +clearly pointed out in the patch description and pull request, along with the
>> +expected impact on existing users, such as bootloaders or other operating
>> +systems.
>> +
>> +Driver Branch Dependencies
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +A common problem is synchronizing changes between device drivers and devicetree
>> +files, even if a change is compatible in both directions, this may require
>> +coordinating how the changes get merged through different maintainer trees.
>> +
>> +Usually the branch that includes a driver change will also include the
>> +corresponding change to the devicetree binding description, to ensure they are
>> +in fact compatible.  This means that the devicetree branch can end up causing
>> +warnings in the "make dtbs_check" step.  If a devicetree change depends on
>> +missing additions to a header file in include/dt-bindings/, it will fail the
>> +"make dtbs" step and not get merged.
> 
> Sounds like passing `make dtbs` is a merging requirement.

And why shouldn't be?

Best regards,
Krzysztof
Conor Dooley May 31, 2023, 9:41 a.m. UTC | #8
On Wed, May 31, 2023 at 01:30:07PM +0700, Bagas Sanjaya wrote:
> On Tue, May 30, 2023 at 01:49:36PM +0100, Conor Dooley wrote:

> > +If changes are being made to a devicetree that are incompatible with old
> > +kernels, the devicetree patch should not be applied until the driver is, or an

> Until the incompatible driver changes are merged?

Yeah, could probably do with a bit of rework here.

> > +appropriate time later.  Most importantly, any incompatible changes should be
> > +clearly pointed out in the patch description and pull request, along with the
> > +expected impact on existing users, such as bootloaders or other operating
> > +systems.
> > +
> > +Driver Branch Dependencies
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +A common problem is synchronizing changes between device drivers and devicetree
> > +files, even if a change is compatible in both directions, this may require
> > +coordinating how the changes get merged through different maintainer trees.
> > +
> > +Usually the branch that includes a driver change will also include the
> > +corresponding change to the devicetree binding description, to ensure they are
> > +in fact compatible.  This means that the devicetree branch can end up causing
> > +warnings in the "make dtbs_check" step.  If a devicetree change depends on
> > +missing additions to a header file in include/dt-bindings/, it will fail the
> > +"make dtbs" step and not get merged.
> 
> Sounds like passing `make dtbs` is a merging requirement.

I think it goes without saying that not breaking the build is a merge
requirement!

Cheers,
Conor.
Bagas Sanjaya May 31, 2023, 12:22 p.m. UTC | #9
On 5/31/23 13:51, Krzysztof Kozlowski wrote:
>> Sounds like passing `make dtbs` is a merging requirement.
> 
> And why shouldn't be?
> 

I don't have anything more to say here...
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/ABI.rst b/Documentation/devicetree/bindings/ABI.rst
index a885713cf184..93ec82f78ae5 100644
--- a/Documentation/devicetree/bindings/ABI.rst
+++ b/Documentation/devicetree/bindings/ABI.rst
@@ -1,5 +1,7 @@ 
 .. SPDX-License-Identifier: GPL-2.0
 
+.. _devicetree-abi:
+
 ===================
 Devicetree (DT) ABI
 ===================
diff --git a/Documentation/devicetree/bindings/writing-schema.rst b/Documentation/devicetree/bindings/writing-schema.rst
index 4a381d20f2b4..640d857dabf3 100644
--- a/Documentation/devicetree/bindings/writing-schema.rst
+++ b/Documentation/devicetree/bindings/writing-schema.rst
@@ -136,6 +136,8 @@  installed. Ensure they are in your PATH (~/.local/bin by default).
 
 Recommended is also to install yamllint (used by dtschema when present).
 
+.. _running-checks:
+
 Running checks
 ~~~~~~~~~~~~~~
 
diff --git a/Documentation/process/maintainer-handbooks.rst b/Documentation/process/maintainer-handbooks.rst
index d783060b4cc6..fe24cb665fb7 100644
--- a/Documentation/process/maintainer-handbooks.rst
+++ b/Documentation/process/maintainer-handbooks.rst
@@ -15,5 +15,6 @@  Contents:
    :numbered:
    :maxdepth: 2
 
-   maintainer-tip
    maintainer-netdev
+   maintainer-soc
+   maintainer-tip
diff --git a/Documentation/process/maintainer-soc.rst b/Documentation/process/maintainer-soc.rst
new file mode 100644
index 000000000000..9683c7d199b2
--- /dev/null
+++ b/Documentation/process/maintainer-soc.rst
@@ -0,0 +1,178 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _maintainer-soc:
+
+=============
+SoC Subsystem
+=============
+
+Overview
+--------
+
+The SoC subsystem is a place of aggregation for SoC-specific code.
+The main components of the subsystem are:
+
+* devicetrees for 32- & 64-bit ARM and RISC-V
+* 32-bit ARM board files (arch/arm/mach*)
+* 32- & 64-bit ARM defconfigs
+* SoC specific drivers across architectures, in particular for 32- & 64-bit
+  ARM, RISC-V and Loongarch
+
+These "SoC specific drivers" do not include clock, GPIO etc drivers that have
+other top-level maintainers. The drivers/soc/ directory is generally meant
+for kernel-internal drivers that are used by other drivers to provide SoC
+specific functionality like identifying a SoC revision or interfacing with
+power domains.
+
+The SoC subsystem also serves as an intermediate location for changes to
+drivers/bus, drivers/firmware, drivers/reset and drivers/memory.  The addition
+of new platforms, or the removal of existing ones, often go through the SoC
+tree as a dedicated branch covering multiple subsystems.
+
+The main SoC tree is housed on git.kernel.org:
+  https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/
+
+Clearly this is quite a wide range of topics, which no one person, or even
+small group of people are capable of maintaining.  Instead, the SoC subsystem
+is comprised of many submaintainers, each taking care of individual platforms
+and driver sub-directories.
+In this regard, "platform" usually refers to a series of SoCs from a given
+vendor, for example, Nvidia's series of Tegra SoCs.  Many submaintainers operate
+on a vendor level, responsible for multiple product lines.  For several reasons,
+including acquisitions/different business units in a company, things vary
+significantly here.  The various submaintainers are documented in the
+MAINTAINERS file.
+
+Most of these submaintainers have their own trees where they stage patches,
+sending pull requests to the main SoC tree.  These trees are usually, but not
+always, listed in MAINTAINERS.  The main SoC maintainers can be reached via the
+alias soc@kernel.org if there is no platform-specific maintainer, or if they
+are unresponsive.
+
+What the SoC tree is not, however, is a location for architecture specific code
+changes.  Each architecture has it's own maintainers that are responsible for
+architectural details, cpu errata and the like.
+
+Information for (new) Submaintainers
+------------------------------------
+
+As new platforms spring up, they often bring with them new submaintainers,
+many of whom work for the silicon vendor, and may not be familiar with the
+process.
+
+Devicetree ABI Stability
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+Perhaps one of the most important things to highlight is that dt-bindings
+document the ABI between the devicetree and the kernel. Please see
+:ref:`devicetree-abi` more information on the ABI.
+
+If changes are being made to a devicetree that are incompatible with old
+kernels, the devicetree patch should not be applied until the driver is, or an
+appropriate time later.  Most importantly, any incompatible changes should be
+clearly pointed out in the patch description and pull request, along with the
+expected impact on existing users, such as bootloaders or other operating
+systems.
+
+Driver Branch Dependencies
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+A common problem is synchronizing changes between device drivers and devicetree
+files, even if a change is compatible in both directions, this may require
+coordinating how the changes get merged through different maintainer trees.
+
+Usually the branch that includes a driver change will also include the
+corresponding change to the devicetree binding description, to ensure they are
+in fact compatible.  This means that the devicetree branch can end up causing
+warnings in the "make dtbs_check" step.  If a devicetree change depends on
+missing additions to a header file in include/dt-bindings/, it will fail the
+"make dtbs" step and not get merged.
+
+There are multiple ways to deal with this:
+
+* Avoid defining custom macros in include/dt-bindings/ for hardware constants
+  that can be derived from a datasheet -- binding macros in header file should
+  only be used as a last resort if there is no natural way to define a binding
+
+* Use literal values in the devicetree file in place of macros even when a
+  header is required, and change them to the named representation in a
+  following release
+
+* Defer the devicetree changes to a release after the binding and driver have
+  already been merged
+
+* Change the bindings in a shared immutable branch that is used as the base for
+  both the driver change and the devicetree changes
+
+* Add duplicate defines in the devicetree file guarded by an #ifndef section,
+  removing them in a later release
+
+Devicetree Naming Convention
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The general naming scheme for devicetree files are as follows.  The aspects of a
+platform that are set at the SoC level, like cpu cores, are contained in a file
+named $soc.dtsi, for example, jh7100.dtsi.  Integration details, that will vary
+from board to board, are described in $soc-$board.dtsi.  An example of this is
+jh7100-beaglev-starlight.dts.  Often many boards are variations on a theme, and
+frequently there are intermediate files, such as jh7100-common.dtsi, which sit
+between the $soc.dtsi and $soc-$board.dts files, containing the descriptions of
+common hardware.
+
+Some platforms also have System on Modules, containing an SoC, which are then
+integrated into several different boards. For these platforms, $soc-$som.dtsi
+and $soc-$som-$board.dts are typical.
+
+Directories are usually named after the vendor of the SoC at the time of it's
+inclusion, leading to some historical directory names in the tree.
+
+Validating Devicetree Files
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+``make dtbs_check`` can be used to validate that devicetree files are compliant
+with the dt-bindings that describe the ABI.  Please see :ref:`running-checks`
+for more information on the validation of devicetrees.
+
+For new platforms, or additions to existing ones, ``make dtbs_check`` should not
+add any new warnings.  For RISC-V, as it has the advantage of being a newer
+architecture, ``make dtbs_check W=1`` is required to not add any new warnings.
+If in any doubt about a devicetree change, reach out to the devicetree
+maintainers.
+
+Branches and Pull Requests
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Just as the main SoC tree has several branches, it is expected that
+submaintainers will do the same. Driver, defconfig and devicetree changes should
+all be split into separate branches and appear in separate pull requests to the
+SoC maintainers.  Each branch should be usable by itself and avoid
+regressions that originate from dependencies on other branches.
+
+Small sets of patches can also be sent as separate emails to soc@kernel.org,
+grouped into the same categories.
+
+If changes do not fit into the normal patterns, there can be additional
+top-level branches, e.g. for a treewide rework, or the addition of new SoC
+platforms including dts files and drivers.
+
+Branches with a lot of changes can benefit from getting split up into separate
+topics branches, even if they end up getting merged into the same branch of the
+SoC tree.  An example here would be one branch for devicetree warning fixes, one
+for a rework and one for newly added boards.
+
+Another common way to split up changes is to send an early pull request with the
+majority of the changes at some point between rc1 and rc4, following up with one
+or more smaller pull requests towards the end of the cycle that can add late
+changes or address problems idenfied while testing the first set.
+
+While there is no cut-off time for late pull requests, it helps to only send
+small branches as time gets closer to the merge window.
+
+Pull requests for bugfixes for the current release can be sent at any time, but
+again having multiple smaller branches is better than trying to combine too many
+patches into one pull request.
+
+The subject line of a pull request should begin with "[GIT PULL]" and made using
+a signed tag, rather than a branch.  This tag should contain a short description
+summarising the changes in the pull request.  For more detail on sending pull
+requests, please see :ref:`pullrequests`.
diff --git a/MAINTAINERS b/MAINTAINERS
index 7e0b87d5aa2e..29631c325857 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1815,6 +1815,7 @@  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 C:	irc://irc.libera.chat/armlinux
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
+F:	Documentation/process/maintainer-soc.rst
 F:	arch/arm/boot/dts/Makefile
 F:	arch/arm64/boot/dts/Makefile