mbox series

[v6,00/11] An effort to bring DT bindings compliance within U-Boot

Message ID 20240222093607.3085545-1-sumit.garg@linaro.org
Headers show
Series An effort to bring DT bindings compliance within U-Boot | expand

Message

Sumit Garg Feb. 22, 2024, 9:35 a.m. UTC
Changes in v6:
--------------
- v6_dt: https://github.com/b49020/u-boot/tree/v6_dt
- Patch #3: Incorporate fix for sandbox CI failure.
- Patch #6: Incorporate shell script comments from Marek.
- Patch #8: Incorporate documentation review comments from Paul.

Changes in v5:
--------------
- Rebased on tip of master (050a9b981d6a835133521b599be3ae189ce70f41)
- Created v5_dt branch for testing purposes:
  https://github.com/b49020/u-boot/tree/v5_dt
- Patch #6: Added support to cherry-pick fixes in subtree update script.
  Also, used https:// instead of git://.
- Patch #7: Fixed inappropriate documentation update.
- Patch #8: Document how to cherry-pick fixes from devicetree-rebasing
  tree.

Changes in v4:
--------------
- Switched subtree to be imported as dts/upstream sub-directory rather
  than devicetree-rebasing sub-directory to better suite U-Boot
  directory structure.
- Since we now have v6.7-dts tag available now, so switch subtree to
  that from its beginning.
- Patch #2: Incorporate build fix to adjust Bindings Makefile rules to
  old U-Boot Kbuild infrastructure.
- Patch #3: Incorporate fix to resolve rk3399 migration issue reported
  by Simon.
- Patch #4: New patch to reuse upstream DT includes by U-Boot as per
  Brian's use-case for TI K3 SoCs.
- Patch #5: Added a note to OF_UPSTREAM Kconfig option.
- Patch #6: New patch to add script dts/update-dts-subtree.sh as per
  Rob's comments.
- Patch #7: Separate patch to align documentation to use Kconfig symbols
  instead.
- Patch #8: Clarify subtree uprev schedule as a separate documentation
  section. Also, fixed documentation typos.
- Patch #9: Added commit description.

Changes in v3:
--------------
- Patch #4: Minor commit message update
- Patch #5: Replace CONFIG_* with Kconfig options
- Patch #7: Dropped Makefile portion and enabled OF_UPSTREAM for SoC
  instead.
- Patch #1, #3, #6 and #8: Picked up review tags

Changes in v2:
--------------
- Patch #1: excluded gitab CI config check and added commit description.
- Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/
- Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/
- Patch #5: s/U-boot/U-Boot/
- Patch #6 and #7: Picked up review tags

Prerequisite
------------

This patch series requires devicetree-rebasing git repo to be added as a
subtree to the main U-Boot repo via:

$ git subtree add --prefix dts/upstream \
      https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git \
      v6.7-dts --squash

Background
----------

This effort started while I was reviewing patch series corresponding to
Qcom platforms [1] which was about to import modified devicetree source
files from Linux kernel. I suppose keeping devicetree files sync with
Linux kernel without any DT bindings schema validation has been a pain
for U-Boot SoC/platform maintainers. There has been past discussions
about a single DT repo but that hasn't come up and Linux kernel remained
the place where DT source files as well as bindings are placed and
maintained.

However, Linux kernel DT maintainers proposed [2] for U-Boot to rather
use devicetree-rebasing repo [3] which is a forked copy from Linux
kernel for DT source files as well as bindings. It is tagged at every
Linux kernel major release or intermideate release candidates. So here I
have tried to reuse that to bring DT bingings compliance as well as a
standard way to maintain a regular sync of DT source files with Linux
kernel.

In order to maintain devicetree files sync, U-Boot will maintains a Git
subtree for devicetee-rebasing repo as `dts/upstream` sub-directory.
U-Boot will regularly sync `dts/upstream/` subtree whenever the next window
opens with the next available kernel major release.
`dts/update-dts-subtree.sh` script provides a wrapper around git subtree
pull command, usage from the top level U-Boot source tree, run:

$ ./dts/update-dts-subtree.sh pull <devicetree-rebasing-release-tag>

If required it is also possible to cherry-pick fixes from
devicetree-rebasing tree prior to next sync, usage:

$ ./dts/update-dts-subtree.sh pick <devicetree-rebasing-commit-id>

The RFC/prototype for this series has been discussed with Linux DT
maintainers as well as U-Boot maintainers here [4]. Now we would like to
reach out to wider U-Boot community to seek feedback.

[1] https://lore.kernel.org/all/CAFA6WYMLUD9cnkr=R0Uur+1UeTMkKjM2zDdMJtXb3nmrLk+pDg@mail.gmail.com/
[2] https://lore.kernel.org/all/CAL_JsqKEjv2tSGmT+0ZiO7_qbBfhTycbGnhJhYpKDFzfO9jzDg@mail.gmail.com/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
[4] https://github.com/u-boot/u-boot/pull/451

Changes
-------

Traditionally, U-Boot placed copies of devicetree source files from Linux
kernel into `arch/<arch>/dts/<name>.dts` which can be selected via setting
"<name>" when prompted for `DEFAULT_DEVICE_TREE` by Kconfig.

SoC/board maintainers are encouraged to migrate to use synced copies from
`dts/upstream/src/<arch>/<vendor>`. To do that enable `OF_UPSTREAM` for the
SoC being used via Kconfig and set up "<vendor>/<name>" when prompted for
`DEFAULT_DEVICE_TREE` by Kconfig.

An example have been shown for Amlogic meson-gxbb SoC and corresponding
derived boards via patch #10 and #11.

Devicetree bindings schema checks
---------------------------------

With devicetee-rebasing Git subtree, the devicetree bindings are also
regularly synced with Linux kernel as `dts/upstream/Bindings/`
sub-directory. This allows U-Boot to run devicetree bindings schema checks
which will bring compliance to U-Boot core/drivers regarding usage of
devicetree.

Dependencies
------------

The DT schema project must be installed in order to validate the DT schema
binding documents and validate DTS files using the DT schema. The DT schema
project can be installed with pip:

$ pip3 install dtschema

Note that 'dtschema' installation requires 'swig' and Python development
files installed first. On Debian/Ubuntu systems:

$ apt install swig python3-dev

Several executables (dt-doc-validate, dt-mk-schema, dt-validate) will be
installed. Ensure they are in your PATH (~/.local/bin by default).

Recommended is also to install yamllint (used by dtschema when present).

$ apt install yamllint

Running checks
--------------

In order to perform validation of DTB files, use the ``dtbs_check`` target:

$ make dtbs_check

It is also possible to run checks with a subset of matching schema files by
setting the ``DT_SCHEMA_FILES`` variable to 1 or more specific schema files
or patterns (partial match of a fixed string). Each file or pattern should
be separated by ':'.

$ make dtbs_check DT_SCHEMA_FILES=trivial-devices.yaml:rtc.yaml
$ make dtbs_check DT_SCHEMA_FILES=/gpio/
$ make dtbs_check DT_SCHEMA_FILES=trivial-devices.yaml

Sumit Garg (11):
  CI: Exclude devicetree-rebasing subtree for CONFIG checks
  Makefile: Add support for DT bindings schema checks
  scripts/Makefile.lib: Statically define *-u-boot.dtsi files location
  Makefile: Allow upstream DT subtree to provide DT includes
  dts: Add alternative location for upstream DTB builds
  dts: Add script to uprev dts/upstream subtree
  doc: devicetree: Align documentation to use Kconfig options
  doc: devicetree: Updates for devicetree-rebasing subtree
  MAINTAINERS: Add myself as devicetree-rebasing maintainer
  dts: meson-gxbb: Switch to using upstream DT
  dts: meson-gxbb: Drop redundant devicetree files

 .azure-pipelines.yml                    |   3 +-
 .gitlab-ci.yml                          |   3 +-
 MAINTAINERS                             |   5 +
 Makefile                                |  23 +-
 arch/arm/dts/Makefile                   |   8 -
 arch/arm/dts/meson-gxbb-kii-pro.dts     | 140 ----
 arch/arm/dts/meson-gxbb-nanopi-k2.dts   | 426 ------------
 arch/arm/dts/meson-gxbb-odroidc2.dts    | 414 -----------
 arch/arm/dts/meson-gxbb-p200.dts        | 100 ---
 arch/arm/dts/meson-gxbb-p201.dts        |  26 -
 arch/arm/dts/meson-gxbb-p20x.dtsi       | 250 -------
 arch/arm/dts/meson-gxbb-wetek-hub.dts   |  58 --
 arch/arm/dts/meson-gxbb-wetek-play2.dts | 119 ----
 arch/arm/dts/meson-gxbb-wetek.dtsi      | 292 --------
 arch/arm/dts/meson-gxbb.dtsi            | 870 ------------------------
 arch/arm/mach-meson/Kconfig             |   1 +
 configs/nanopi-k2_defconfig             |   2 +-
 configs/odroid-c2_defconfig             |   2 +-
 configs/p200_defconfig                  |   2 +-
 configs/p201_defconfig                  |   2 +-
 configs/videostrong-kii-pro_defconfig   |   2 +-
 configs/wetek-hub_defconfig             |   2 +-
 configs/wetek-play2_defconfig           |   2 +-
 doc/develop/devicetree/control.rst      | 159 +++--
 dts/Kconfig                             |  16 +
 dts/Makefile                            |  17 +-
 dts/update-dts-subtree.sh               |  48 ++
 dts/upstream/Bindings/Makefile          |   6 +-
 dts/upstream/src/arm64/Makefile         |  14 +
 scripts/Makefile.lib                    |  50 +-
 30 files changed, 283 insertions(+), 2779 deletions(-)
 delete mode 100644 arch/arm/dts/meson-gxbb-kii-pro.dts
 delete mode 100644 arch/arm/dts/meson-gxbb-nanopi-k2.dts
 delete mode 100644 arch/arm/dts/meson-gxbb-odroidc2.dts
 delete mode 100644 arch/arm/dts/meson-gxbb-p200.dts
 delete mode 100644 arch/arm/dts/meson-gxbb-p201.dts
 delete mode 100644 arch/arm/dts/meson-gxbb-p20x.dtsi
 delete mode 100644 arch/arm/dts/meson-gxbb-wetek-hub.dts
 delete mode 100644 arch/arm/dts/meson-gxbb-wetek-play2.dts
 delete mode 100644 arch/arm/dts/meson-gxbb-wetek.dtsi
 delete mode 100644 arch/arm/dts/meson-gxbb.dtsi
 create mode 100755 dts/update-dts-subtree.sh
 create mode 100644 dts/upstream/src/arm64/Makefile

Comments

Sumit Garg March 1, 2024, 6:17 a.m. UTC | #1
Hi Tom,

On Thu, 22 Feb 2024 at 15:06, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Changes in v6:
> --------------
> - v6_dt: https://github.com/b49020/u-boot/tree/v6_dt
> - Patch #3: Incorporate fix for sandbox CI failure.
> - Patch #6: Incorporate shell script comments from Marek.
> - Patch #8: Incorporate documentation review comments from Paul.
>

I haven't heard any major feedback/comments for v6. So can you pick up
this series for the U-Boot -next branch? As you may already be aware
that it is a dependency for [1] [2] [3]. Also, we would like to move
all new Qcom platforms to use upstream DT too.

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=396399
[2] https://patchwork.ozlabs.org/project/uboot/list/?series=396742
[3] https://patchwork.ozlabs.org/project/uboot/list/?series=396992&state=*

-Sumit

> Changes in v5:
> --------------
> - Rebased on tip of master (050a9b981d6a835133521b599be3ae189ce70f41)
> - Created v5_dt branch for testing purposes:
>   https://github.com/b49020/u-boot/tree/v5_dt
> - Patch #6: Added support to cherry-pick fixes in subtree update script.
>   Also, used https:// instead of git://.
> - Patch #7: Fixed inappropriate documentation update.
> - Patch #8: Document how to cherry-pick fixes from devicetree-rebasing
>   tree.
>
> Changes in v4:
> --------------
> - Switched subtree to be imported as dts/upstream sub-directory rather
>   than devicetree-rebasing sub-directory to better suite U-Boot
>   directory structure.
> - Since we now have v6.7-dts tag available now, so switch subtree to
>   that from its beginning.
> - Patch #2: Incorporate build fix to adjust Bindings Makefile rules to
>   old U-Boot Kbuild infrastructure.
> - Patch #3: Incorporate fix to resolve rk3399 migration issue reported
>   by Simon.
> - Patch #4: New patch to reuse upstream DT includes by U-Boot as per
>   Brian's use-case for TI K3 SoCs.
> - Patch #5: Added a note to OF_UPSTREAM Kconfig option.
> - Patch #6: New patch to add script dts/update-dts-subtree.sh as per
>   Rob's comments.
> - Patch #7: Separate patch to align documentation to use Kconfig symbols
>   instead.
> - Patch #8: Clarify subtree uprev schedule as a separate documentation
>   section. Also, fixed documentation typos.
> - Patch #9: Added commit description.
>
> Changes in v3:
> --------------
> - Patch #4: Minor commit message update
> - Patch #5: Replace CONFIG_* with Kconfig options
> - Patch #7: Dropped Makefile portion and enabled OF_UPSTREAM for SoC
>   instead.
> - Patch #1, #3, #6 and #8: Picked up review tags
>
> Changes in v2:
> --------------
> - Patch #1: excluded gitab CI config check and added commit description.
> - Patch #3: s/UBOOT_DTSI_LOC/u_boot_dtsi_loc/
> - Patch #4: s/DEVICE_TREE_LOC/dt_dir/ and s/U-boot/U-Boot/
> - Patch #5: s/U-boot/U-Boot/
> - Patch #6 and #7: Picked up review tags
>
> Prerequisite
> ------------
>
> This patch series requires devicetree-rebasing git repo to be added as a
> subtree to the main U-Boot repo via:
>
> $ git subtree add --prefix dts/upstream \
>       https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git \
>       v6.7-dts --squash
>
> Background
> ----------
>
> This effort started while I was reviewing patch series corresponding to
> Qcom platforms [1] which was about to import modified devicetree source
> files from Linux kernel. I suppose keeping devicetree files sync with
> Linux kernel without any DT bindings schema validation has been a pain
> for U-Boot SoC/platform maintainers. There has been past discussions
> about a single DT repo but that hasn't come up and Linux kernel remained
> the place where DT source files as well as bindings are placed and
> maintained.
>
> However, Linux kernel DT maintainers proposed [2] for U-Boot to rather
> use devicetree-rebasing repo [3] which is a forked copy from Linux
> kernel for DT source files as well as bindings. It is tagged at every
> Linux kernel major release or intermideate release candidates. So here I
> have tried to reuse that to bring DT bingings compliance as well as a
> standard way to maintain a regular sync of DT source files with Linux
> kernel.
>
> In order to maintain devicetree files sync, U-Boot will maintains a Git
> subtree for devicetee-rebasing repo as `dts/upstream` sub-directory.
> U-Boot will regularly sync `dts/upstream/` subtree whenever the next window
> opens with the next available kernel major release.
> `dts/update-dts-subtree.sh` script provides a wrapper around git subtree
> pull command, usage from the top level U-Boot source tree, run:
>
> $ ./dts/update-dts-subtree.sh pull <devicetree-rebasing-release-tag>
>
> If required it is also possible to cherry-pick fixes from
> devicetree-rebasing tree prior to next sync, usage:
>
> $ ./dts/update-dts-subtree.sh pick <devicetree-rebasing-commit-id>
>
> The RFC/prototype for this series has been discussed with Linux DT
> maintainers as well as U-Boot maintainers here [4]. Now we would like to
> reach out to wider U-Boot community to seek feedback.
>
> [1] https://lore.kernel.org/all/CAFA6WYMLUD9cnkr=R0Uur+1UeTMkKjM2zDdMJtXb3nmrLk+pDg@mail.gmail.com/
> [2] https://lore.kernel.org/all/CAL_JsqKEjv2tSGmT+0ZiO7_qbBfhTycbGnhJhYpKDFzfO9jzDg@mail.gmail.com/
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
> [4] https://github.com/u-boot/u-boot/pull/451
>
> Changes
> -------
>
> Traditionally, U-Boot placed copies of devicetree source files from Linux
> kernel into `arch/<arch>/dts/<name>.dts` which can be selected via setting
> "<name>" when prompted for `DEFAULT_DEVICE_TREE` by Kconfig.
>
> SoC/board maintainers are encouraged to migrate to use synced copies from
> `dts/upstream/src/<arch>/<vendor>`. To do that enable `OF_UPSTREAM` for the
> SoC being used via Kconfig and set up "<vendor>/<name>" when prompted for
> `DEFAULT_DEVICE_TREE` by Kconfig.
>
> An example have been shown for Amlogic meson-gxbb SoC and corresponding
> derived boards via patch #10 and #11.
>
> Devicetree bindings schema checks
> ---------------------------------
>
> With devicetee-rebasing Git subtree, the devicetree bindings are also
> regularly synced with Linux kernel as `dts/upstream/Bindings/`
> sub-directory. This allows U-Boot to run devicetree bindings schema checks
> which will bring compliance to U-Boot core/drivers regarding usage of
> devicetree.
>
> Dependencies
> ------------
>
> The DT schema project must be installed in order to validate the DT schema
> binding documents and validate DTS files using the DT schema. The DT schema
> project can be installed with pip:
>
> $ pip3 install dtschema
>
> Note that 'dtschema' installation requires 'swig' and Python development
> files installed first. On Debian/Ubuntu systems:
>
> $ apt install swig python3-dev
>
> Several executables (dt-doc-validate, dt-mk-schema, dt-validate) will be
> installed. Ensure they are in your PATH (~/.local/bin by default).
>
> Recommended is also to install yamllint (used by dtschema when present).
>
> $ apt install yamllint
>
> Running checks
> --------------
>
> In order to perform validation of DTB files, use the ``dtbs_check`` target:
>
> $ make dtbs_check
>
> It is also possible to run checks with a subset of matching schema files by
> setting the ``DT_SCHEMA_FILES`` variable to 1 or more specific schema files
> or patterns (partial match of a fixed string). Each file or pattern should
> be separated by ':'.
>
> $ make dtbs_check DT_SCHEMA_FILES=trivial-devices.yaml:rtc.yaml
> $ make dtbs_check DT_SCHEMA_FILES=/gpio/
> $ make dtbs_check DT_SCHEMA_FILES=trivial-devices.yaml
>
> Sumit Garg (11):
>   CI: Exclude devicetree-rebasing subtree for CONFIG checks
>   Makefile: Add support for DT bindings schema checks
>   scripts/Makefile.lib: Statically define *-u-boot.dtsi files location
>   Makefile: Allow upstream DT subtree to provide DT includes
>   dts: Add alternative location for upstream DTB builds
>   dts: Add script to uprev dts/upstream subtree
>   doc: devicetree: Align documentation to use Kconfig options
>   doc: devicetree: Updates for devicetree-rebasing subtree
>   MAINTAINERS: Add myself as devicetree-rebasing maintainer
>   dts: meson-gxbb: Switch to using upstream DT
>   dts: meson-gxbb: Drop redundant devicetree files
>
>  .azure-pipelines.yml                    |   3 +-
>  .gitlab-ci.yml                          |   3 +-
>  MAINTAINERS                             |   5 +
>  Makefile                                |  23 +-
>  arch/arm/dts/Makefile                   |   8 -
>  arch/arm/dts/meson-gxbb-kii-pro.dts     | 140 ----
>  arch/arm/dts/meson-gxbb-nanopi-k2.dts   | 426 ------------
>  arch/arm/dts/meson-gxbb-odroidc2.dts    | 414 -----------
>  arch/arm/dts/meson-gxbb-p200.dts        | 100 ---
>  arch/arm/dts/meson-gxbb-p201.dts        |  26 -
>  arch/arm/dts/meson-gxbb-p20x.dtsi       | 250 -------
>  arch/arm/dts/meson-gxbb-wetek-hub.dts   |  58 --
>  arch/arm/dts/meson-gxbb-wetek-play2.dts | 119 ----
>  arch/arm/dts/meson-gxbb-wetek.dtsi      | 292 --------
>  arch/arm/dts/meson-gxbb.dtsi            | 870 ------------------------
>  arch/arm/mach-meson/Kconfig             |   1 +
>  configs/nanopi-k2_defconfig             |   2 +-
>  configs/odroid-c2_defconfig             |   2 +-
>  configs/p200_defconfig                  |   2 +-
>  configs/p201_defconfig                  |   2 +-
>  configs/videostrong-kii-pro_defconfig   |   2 +-
>  configs/wetek-hub_defconfig             |   2 +-
>  configs/wetek-play2_defconfig           |   2 +-
>  doc/develop/devicetree/control.rst      | 159 +++--
>  dts/Kconfig                             |  16 +
>  dts/Makefile                            |  17 +-
>  dts/update-dts-subtree.sh               |  48 ++
>  dts/upstream/Bindings/Makefile          |   6 +-
>  dts/upstream/src/arm64/Makefile         |  14 +
>  scripts/Makefile.lib                    |  50 +-
>  30 files changed, 283 insertions(+), 2779 deletions(-)
>  delete mode 100644 arch/arm/dts/meson-gxbb-kii-pro.dts
>  delete mode 100644 arch/arm/dts/meson-gxbb-nanopi-k2.dts
>  delete mode 100644 arch/arm/dts/meson-gxbb-odroidc2.dts
>  delete mode 100644 arch/arm/dts/meson-gxbb-p200.dts
>  delete mode 100644 arch/arm/dts/meson-gxbb-p201.dts
>  delete mode 100644 arch/arm/dts/meson-gxbb-p20x.dtsi
>  delete mode 100644 arch/arm/dts/meson-gxbb-wetek-hub.dts
>  delete mode 100644 arch/arm/dts/meson-gxbb-wetek-play2.dts
>  delete mode 100644 arch/arm/dts/meson-gxbb-wetek.dtsi
>  delete mode 100644 arch/arm/dts/meson-gxbb.dtsi
>  create mode 100755 dts/update-dts-subtree.sh
>  create mode 100644 dts/upstream/src/arm64/Makefile
>
> --
> 2.34.1
>
Tom Rini March 1, 2024, 12:57 p.m. UTC | #2
On Thu, 22 Feb 2024 15:05:56 +0530, Sumit Garg wrote:

> Changes in v6:
> --------------
> - v6_dt: https://github.com/b49020/u-boot/tree/v6_dt
> - Patch #3: Incorporate fix for sandbox CI failure.
> - Patch #6: Incorporate shell script comments from Marek.
> - Patch #8: Incorporate documentation review comments from Paul.
> 
> [...]

Applied to u-boot/next, thanks!
Sumit Garg March 1, 2024, 1:30 p.m. UTC | #3
On Fri, 1 Mar 2024 at 18:27, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, 22 Feb 2024 15:05:56 +0530, Sumit Garg wrote:
>
> > Changes in v6:
> > --------------
> > - v6_dt: https://github.com/b49020/u-boot/tree/v6_dt
> > - Patch #3: Incorporate fix for sandbox CI failure.
> > - Patch #6: Incorporate shell script comments from Marek.
> > - Patch #8: Incorporate documentation review comments from Paul.
> >
> > [...]
>
> Applied to u-boot/next, thanks!
>

Thanks Tom and everyone involved with the reviews. I hope we can carry
forward from this effort towards a healthy devicetree story for
U-Boot.

-Sumit

> --
> Tom
>
>
Neil Armstrong March 1, 2024, 1:48 p.m. UTC | #4
On 01/03/2024 14:30, Sumit Garg wrote:
> On Fri, 1 Mar 2024 at 18:27, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Thu, 22 Feb 2024 15:05:56 +0530, Sumit Garg wrote:
>>
>>> Changes in v6:
>>> --------------
>>> - v6_dt: https://github.com/b49020/u-boot/tree/v6_dt
>>> - Patch #3: Incorporate fix for sandbox CI failure.
>>> - Patch #6: Incorporate shell script comments from Marek.
>>> - Patch #8: Incorporate documentation review comments from Paul.
>>>
>>> [...]
>>
>> Applied to u-boot/next, thanks!
>>
> 
> Thanks Tom and everyone involved with the reviews. I hope we can carry
> forward from this effort towards a healthy devicetree story for
> U-Boot.

Thanks Sumit!

I'll try to submit patches for Amlogic GXL & G12A to switch to this!

Neil

> 
> -Sumit
> 
>> --
>> Tom
>>
>>
Sumit Garg March 1, 2024, 2:53 p.m. UTC | #5
On Fri, 1 Mar 2024 at 19:18, <neil.armstrong@linaro.org> wrote:
>
> On 01/03/2024 14:30, Sumit Garg wrote:
> > On Fri, 1 Mar 2024 at 18:27, Tom Rini <trini@konsulko.com> wrote:
> >>
> >> On Thu, 22 Feb 2024 15:05:56 +0530, Sumit Garg wrote:
> >>
> >>> Changes in v6:
> >>> --------------
> >>> - v6_dt: https://github.com/b49020/u-boot/tree/v6_dt
> >>> - Patch #3: Incorporate fix for sandbox CI failure.
> >>> - Patch #6: Incorporate shell script comments from Marek.
> >>> - Patch #8: Incorporate documentation review comments from Paul.
> >>>
> >>> [...]
> >>
> >> Applied to u-boot/next, thanks!
> >>
> >
> > Thanks Tom and everyone involved with the reviews. I hope we can carry
> > forward from this effort towards a healthy devicetree story for
> > U-Boot.
>
> Thanks Sumit!
>
> I'll try to submit patches for Amlogic GXL & G12A to switch to this!
>

Thanks Neil for your support to be the first volunteer for this
OF_UPSTREAM switch.

-Sumit

> Neil
>
> >
> > -Sumit
> >
> >> --
> >> Tom
> >>
> >>
>
Fabio Estevam March 1, 2024, 4:49 p.m. UTC | #6
Hi Sumit,

On Fri, Mar 1, 2024 at 9:57 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, 22 Feb 2024 15:05:56 +0530, Sumit Garg wrote:
>
> > Changes in v6:
> > --------------
> > - v6_dt: https://github.com/b49020/u-boot/tree/v6_dt
> > - Patch #3: Incorporate fix for sandbox CI failure.
> > - Patch #6: Incorporate shell script comments from Marek.
> > - Patch #8: Incorporate documentation review comments from Paul.
> >
> > [...]
>
> Applied to u-boot/next, thanks!

I am trying to use OF_UPSTREAM with imx8mp_evk_defconfig:

diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig
index 328c3e388051..29d0c5ce6485 100644
--- a/arch/arm/mach-imx/imx8m/Kconfig
+++ b/arch/arm/mach-imx/imx8m/Kconfig
@@ -8,6 +8,7 @@ config IMX8M
        select LTO
        select ROM_UNIFIED_SECTIONS
        select ARMV8_CRYPTO
+       imply OF_UPSTREAM

 config IMX8MQ
        bool
diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig
index 2350d2f409b7..cdf840c683c0 100644
--- a/configs/imx8mp_evk_defconfig
+++ b/configs/imx8mp_evk_defconfig
@@ -8,7 +8,7 @@ CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_ENV_SIZE=0x1000
 CONFIG_ENV_OFFSET=0x400000
 CONFIG_DM_GPIO=y
-CONFIG_DEFAULT_DEVICE_TREE="imx8mp-evk"
+CONFIG_DEFAULT_DEVICE_TREE="freescale/imx8mp-evk"
 CONFIG_SPL_TEXT_BASE=0x920000
 CONFIG_TARGET_IMX8MP_EVK=y
 CONFIG_SYS_MONITOR_LEN=524288

I ran "./dts/update-dts-subtree.sh pull v6.8-rc6-dts".

make mrproper
make imx8mp_evk_defconfig
make

The dtb is built: dts/upstream/src/arm64/freescale/imx8mp-evk.dtb

but mkimage still looks for the old path:

 MKIMAGE u-boot.img
./tools/mkimage: Can't open arch/arm/dts/freescale/imx8mp-evk.dtb: No
such file or directory
./tools/mkimage: failed to build FIT

Any suggestions?

Thanks
Sumit Garg March 4, 2024, 6:33 a.m. UTC | #7
Hi Fabio,

On Fri, 1 Mar 2024 at 22:20, Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Sumit,
>
> On Fri, Mar 1, 2024 at 9:57 AM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, 22 Feb 2024 15:05:56 +0530, Sumit Garg wrote:
> >
> > > Changes in v6:
> > > --------------
> > > - v6_dt: https://github.com/b49020/u-boot/tree/v6_dt
> > > - Patch #3: Incorporate fix for sandbox CI failure.
> > > - Patch #6: Incorporate shell script comments from Marek.
> > > - Patch #8: Incorporate documentation review comments from Paul.
> > >
> > > [...]
> >
> > Applied to u-boot/next, thanks!
>
> I am trying to use OF_UPSTREAM with imx8mp_evk_defconfig:
>
> diff --git a/arch/arm/mach-imx/imx8m/Kconfig b/arch/arm/mach-imx/imx8m/Kconfig
> index 328c3e388051..29d0c5ce6485 100644
> --- a/arch/arm/mach-imx/imx8m/Kconfig
> +++ b/arch/arm/mach-imx/imx8m/Kconfig
> @@ -8,6 +8,7 @@ config IMX8M
>         select LTO
>         select ROM_UNIFIED_SECTIONS
>         select ARMV8_CRYPTO
> +       imply OF_UPSTREAM
>
>  config IMX8MQ
>         bool
> diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig
> index 2350d2f409b7..cdf840c683c0 100644
> --- a/configs/imx8mp_evk_defconfig
> +++ b/configs/imx8mp_evk_defconfig
> @@ -8,7 +8,7 @@ CONFIG_SPL_LIBGENERIC_SUPPORT=y
>  CONFIG_ENV_SIZE=0x1000
>  CONFIG_ENV_OFFSET=0x400000
>  CONFIG_DM_GPIO=y
> -CONFIG_DEFAULT_DEVICE_TREE="imx8mp-evk"
> +CONFIG_DEFAULT_DEVICE_TREE="freescale/imx8mp-evk"
>  CONFIG_SPL_TEXT_BASE=0x920000
>  CONFIG_TARGET_IMX8MP_EVK=y
>  CONFIG_SYS_MONITOR_LEN=524288
>
> I ran "./dts/update-dts-subtree.sh pull v6.8-rc6-dts".
>
> make mrproper
> make imx8mp_evk_defconfig
> make
>
> The dtb is built: dts/upstream/src/arm64/freescale/imx8mp-evk.dtb
>
> but mkimage still looks for the old path:
>
>  MKIMAGE u-boot.img
> ./tools/mkimage: Can't open arch/arm/dts/freescale/imx8mp-evk.dtb: No
> such file or directory
> ./tools/mkimage: failed to build FIT
>
> Any suggestions?

I think you also need Bryan's patch [1] to be incorporated as well.
Give it a try and let me know if you still see any further issues.

[1] https://patchwork.ozlabs.org/project/uboot/patch/20240205-am62px-wip-rebasing-v3-11-04cbb42eaa6f@ti.com/

-Sumit

>
> Thanks
Fabio Estevam March 4, 2024, 11:24 a.m. UTC | #8
Hi Sumit,

On Mon, Mar 4, 2024 at 3:33 AM Sumit Garg <sumit.garg@linaro.org> wrote:

> I think you also need Bryan's patch [1] to be incorporated as well.
> Give it a try and let me know if you still see any further issues.
>
> [1] https://patchwork.ozlabs.org/project/uboot/patch/20240205-am62px-wip-rebasing-v3-11-04cbb42eaa6f@ti.com/

I applied this patch, but now I get a different error:

  BINMAN  .binman_stamp
Wrote map file './itb.map' to show errors
binman: Filename 'freescale/imx8mp-evk.dtb' not found in input path
(.,.,./board/freescale/imx8mp_evk,arch/arm/dts)
(cwd='/home/fabio/denx/u-boot')
make: *** [Makefile:1126: .binman_stamp] Error 1
Sumit Garg March 4, 2024, 12:15 p.m. UTC | #9
On Mon, 4 Mar 2024 at 16:54, Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Sumit,
>
> On Mon, Mar 4, 2024 at 3:33 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > I think you also need Bryan's patch [1] to be incorporated as well.
> > Give it a try and let me know if you still see any further issues.
> >
> > [1] https://patchwork.ozlabs.org/project/uboot/patch/20240205-am62px-wip-rebasing-v3-11-04cbb42eaa6f@ti.com/
>
> I applied this patch, but now I get a different error:
>
>   BINMAN  .binman_stamp
> Wrote map file './itb.map' to show errors
> binman: Filename 'freescale/imx8mp-evk.dtb' not found in input path
> (.,.,./board/freescale/imx8mp_evk,arch/arm/dts)
> (cwd='/home/fabio/denx/u-boot')
> make: *** [Makefile:1126: .binman_stamp] Error 1

I suppose the earlier reference patch wasn't complete, can you rather
try its v4 [1] instead?

[1] https://patchwork.ozlabs.org/project/uboot/patch/20240304121257.3551104-1-sumit.garg@linaro.org/

-Sumit
Fabio Estevam March 4, 2024, 12:19 p.m. UTC | #10
On Mon, Mar 4, 2024 at 9:15 AM Sumit Garg <sumit.garg@linaro.org> wrote:

> I suppose the earlier reference patch wasn't complete, can you rather
> try its v4 [1] instead?
>
> [1] https://patchwork.ozlabs.org/project/uboot/patch/20240304121257.3551104-1-sumit.garg@linaro.org/

This one works, thanks!
Tony Dinh March 11, 2024, 3:49 a.m. UTC | #11
Hi Sumit,
Hi Tom,

On Mon, Mar 4, 2024 at 4:29 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Mon, Mar 4, 2024 at 9:15 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > I suppose the earlier reference patch wasn't complete, can you rather
> > try its v4 [1] instead?
> >
> > [1] https://patchwork.ozlabs.org/project/uboot/patch/20240304121257.3551104-1-sumit.garg@linaro.org/
>
> This one works, thanks!

I'm testing this for a Marvell Armada 385 board (Synology DS116). I'm
on the next branch, but  perhaps something is still missing.

diff --git a/configs/ds116_defconfig b/configs/ds116_defconfig
index 02ddc0e7918..1fbedcf91cf 100644
--- a/configs/ds116_defconfig
+++ b/configs/ds116_defconfig
@@ -16,7 +16,7 @@ CONFIG_SF_DEFAULT_SPEED=50000000
 CONFIG_ENV_SIZE=0x10000
 CONFIG_ENV_OFFSET=0x7E0000
 CONFIG_ENV_SECT_SIZE=0x10000
-CONFIG_DEFAULT_DEVICE_TREE="armada-385-synology-ds116"
+CONFIG_DEFAULT_DEVICE_TREE="marvell/armada-385-synology-ds116"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL_STACK=0x4002c000
@@ -29,7 +29,6 @@ CONFIG_PCI=y
 CONFIG_DEBUG_UART=y
 CONFIG_AHCI=y
 CONFIG_BOOTSTD_FULL=y
-CONFIG_BOOTSTD_DEFAULTS=y
 CONFIG_BOOTDELAY=10
 CONFIG_USE_PREBOOT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
@@ -57,6 +56,7 @@ CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:1m(u-boot),7040k(kernel),64k(u-boot-env),-(data)"
+CONFIG_OF_UPSTREAM=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_VERSION_VARIABLE=y

# make ds116_defconfig

# grep -i ds116 .config
CONFIG_SYS_BOARD="ds116"
CONFIG_SYS_CONFIG_NAME="ds116"
CONFIG_TARGET_DS116=y
CONFIG_DEFAULT_DEVICE_TREE="marvell/armada-385-synology-ds116"
CONFIG_IDENT_STRING="\nSynology DS116"
CONFIG_SYS_PROMPT="DS116> "
CONFIG_OF_LIST="marvell/armada-385-synology-ds116"
CONFIG_SPL_OF_LIST="marvell/armada-385-synology-ds116"

Built it, and looks like vendor Marvell is missing during the Make
file execution.

make -f ./scripts/Makefile.build obj=dts dtbs
make -f ./scripts/Makefile.build obj=dts/upstream/src/arm dtbs
scripts/Makefile.build:57: dts/upstream/src/arm/Makefile: No such file
or directory
make[2]: *** No rule to make target 'dts/upstream/src/arm/Makefile'.  Stop.
make[1]: *** [dts/Makefile:54: arch-dtbs] Error 2
make: *** [Makefile:1166: dts/dt.dtb] Error 2
make: *** Waiting for unfinished jobs....
make: Leaving directory '/usr/src/u-boot'

I also tried Bryan's patch like you've suggested to Fabio. But it
seems Bryan's patch was already in the next branch. Did I miss
something?

All the best ,
Tony
Sumit Garg March 11, 2024, 6:24 a.m. UTC | #12
Hi Tony,

On Mon, 11 Mar 2024 at 09:20, Tony Dinh <mibodhi@gmail.com> wrote:
>
> Hi Sumit,
> Hi Tom,
>
> On Mon, Mar 4, 2024 at 4:29 AM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > On Mon, Mar 4, 2024 at 9:15 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > > I suppose the earlier reference patch wasn't complete, can you rather
> > > try its v4 [1] instead?
> > >
> > > [1] https://patchwork.ozlabs.org/project/uboot/patch/20240304121257.3551104-1-sumit.garg@linaro.org/
> >
> > This one works, thanks!
>
> I'm testing this for a Marvell Armada 385 board (Synology DS116). I'm
> on the next branch, but  perhaps something is still missing.
>
> diff --git a/configs/ds116_defconfig b/configs/ds116_defconfig
> index 02ddc0e7918..1fbedcf91cf 100644
> --- a/configs/ds116_defconfig
> +++ b/configs/ds116_defconfig
> @@ -16,7 +16,7 @@ CONFIG_SF_DEFAULT_SPEED=50000000
>  CONFIG_ENV_SIZE=0x10000
>  CONFIG_ENV_OFFSET=0x7E0000
>  CONFIG_ENV_SECT_SIZE=0x10000
> -CONFIG_DEFAULT_DEVICE_TREE="armada-385-synology-ds116"
> +CONFIG_DEFAULT_DEVICE_TREE="marvell/armada-385-synology-ds116"
>  CONFIG_SPL_TEXT_BASE=0x40000030
>  CONFIG_SPL_SERIAL=y
>  CONFIG_SPL_STACK=0x4002c000
> @@ -29,7 +29,6 @@ CONFIG_PCI=y
>  CONFIG_DEBUG_UART=y
>  CONFIG_AHCI=y
>  CONFIG_BOOTSTD_FULL=y
> -CONFIG_BOOTSTD_DEFAULTS=y
>  CONFIG_BOOTDELAY=10
>  CONFIG_USE_PREBOOT=y
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> @@ -57,6 +56,7 @@ CONFIG_CMD_CACHE=y
>  CONFIG_CMD_TIME=y
>  CONFIG_CMD_MTDPARTS=y
>  CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:1m(u-boot),7040k(kernel),64k(u-boot-env),-(data)"
> +CONFIG_OF_UPSTREAM=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_VERSION_VARIABLE=y
>
> # make ds116_defconfig
>
> # grep -i ds116 .config
> CONFIG_SYS_BOARD="ds116"
> CONFIG_SYS_CONFIG_NAME="ds116"
> CONFIG_TARGET_DS116=y
> CONFIG_DEFAULT_DEVICE_TREE="marvell/armada-385-synology-ds116"
> CONFIG_IDENT_STRING="\nSynology DS116"
> CONFIG_SYS_PROMPT="DS116> "
> CONFIG_OF_LIST="marvell/armada-385-synology-ds116"
> CONFIG_SPL_OF_LIST="marvell/armada-385-synology-ds116"
>
> Built it, and looks like vendor Marvell is missing during the Make
> file execution.
>
> make -f ./scripts/Makefile.build obj=dts dtbs
> make -f ./scripts/Makefile.build obj=dts/upstream/src/arm dtbs
> scripts/Makefile.build:57: dts/upstream/src/arm/Makefile: No such file
> or directory

As you are building for the 32-bit Arm, you have to add a Makefile
here [1] similar to this one [2] for arm64.

[1] https://source.denx.de/u-boot/u-boot/-/tree/next/dts/upstream/src/arm?ref_type=heads
[2] https://source.denx.de/u-boot/u-boot/-/blob/next/dts/upstream/src/arm64/Makefile?ref_type=heads

-Sumit

> make[2]: *** No rule to make target 'dts/upstream/src/arm/Makefile'.  Stop.
> make[1]: *** [dts/Makefile:54: arch-dtbs] Error 2
> make: *** [Makefile:1166: dts/dt.dtb] Error 2
> make: *** Waiting for unfinished jobs....
> make: Leaving directory '/usr/src/u-boot'
>
> I also tried Bryan's patch like you've suggested to Fabio. But it
> seems Bryan's patch was already in the next branch. Did I miss
> something?
>
> All the best ,
> Tony
Tony Dinh March 11, 2024, 8:51 p.m. UTC | #13
Hi Sumit,

On Sun, Mar 10, 2024 at 11:24 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Hi Tony,
>
> On Mon, 11 Mar 2024 at 09:20, Tony Dinh <mibodhi@gmail.com> wrote:
> >
> > Hi Sumit,
> > Hi Tom,
> >
> > On Mon, Mar 4, 2024 at 4:29 AM Fabio Estevam <festevam@gmail.com> wrote:
> > >
> > > On Mon, Mar 4, 2024 at 9:15 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> > >
> > > > I suppose the earlier reference patch wasn't complete, can you rather
> > > > try its v4 [1] instead?
> > > >
> > > > [1] https://patchwork.ozlabs.org/project/uboot/patch/20240304121257.3551104-1-sumit.garg@linaro.org/
> > >
> > > This one works, thanks!
> >
> > I'm testing this for a Marvell Armada 385 board (Synology DS116). I'm
> > on the next branch, but  perhaps something is still missing.
> >
> > diff --git a/configs/ds116_defconfig b/configs/ds116_defconfig
> > index 02ddc0e7918..1fbedcf91cf 100644
> > --- a/configs/ds116_defconfig
> > +++ b/configs/ds116_defconfig
> > @@ -16,7 +16,7 @@ CONFIG_SF_DEFAULT_SPEED=50000000
> >  CONFIG_ENV_SIZE=0x10000
> >  CONFIG_ENV_OFFSET=0x7E0000
> >  CONFIG_ENV_SECT_SIZE=0x10000
> > -CONFIG_DEFAULT_DEVICE_TREE="armada-385-synology-ds116"
> > +CONFIG_DEFAULT_DEVICE_TREE="marvell/armada-385-synology-ds116"
> >  CONFIG_SPL_TEXT_BASE=0x40000030
> >  CONFIG_SPL_SERIAL=y
> >  CONFIG_SPL_STACK=0x4002c000
> > @@ -29,7 +29,6 @@ CONFIG_PCI=y
> >  CONFIG_DEBUG_UART=y
> >  CONFIG_AHCI=y
> >  CONFIG_BOOTSTD_FULL=y
> > -CONFIG_BOOTSTD_DEFAULTS=y
> >  CONFIG_BOOTDELAY=10
> >  CONFIG_USE_PREBOOT=y
> >  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> > @@ -57,6 +56,7 @@ CONFIG_CMD_CACHE=y
> >  CONFIG_CMD_TIME=y
> >  CONFIG_CMD_MTDPARTS=y
> >  CONFIG_MTDPARTS_DEFAULT="mtdparts=spi0.0:1m(u-boot),7040k(kernel),64k(u-boot-env),-(data)"
> > +CONFIG_OF_UPSTREAM=y
> >  CONFIG_ENV_OVERWRITE=y
> >  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> >  CONFIG_VERSION_VARIABLE=y
> >
> > # make ds116_defconfig
> >
> > # grep -i ds116 .config
> > CONFIG_SYS_BOARD="ds116"
> > CONFIG_SYS_CONFIG_NAME="ds116"
> > CONFIG_TARGET_DS116=y
> > CONFIG_DEFAULT_DEVICE_TREE="marvell/armada-385-synology-ds116"
> > CONFIG_IDENT_STRING="\nSynology DS116"
> > CONFIG_SYS_PROMPT="DS116> "
> > CONFIG_OF_LIST="marvell/armada-385-synology-ds116"
> > CONFIG_SPL_OF_LIST="marvell/armada-385-synology-ds116"
> >
> > Built it, and looks like vendor Marvell is missing during the Make
> > file execution.
> >
> > make -f ./scripts/Makefile.build obj=dts dtbs
> > make -f ./scripts/Makefile.build obj=dts/upstream/src/arm dtbs
> > scripts/Makefile.build:57: dts/upstream/src/arm/Makefile: No such file
> > or directory
>
> As you are building for the 32-bit Arm, you have to add a Makefile
> here [1] similar to this one [2] for arm64.
>
> [1] https://source.denx.de/u-boot/u-boot/-/tree/next/dts/upstream/src/arm?ref_type=heads
> [2] https://source.denx.de/u-boot/u-boot/-/blob/next/dts/upstream/src/arm64/Makefile?ref_type=heads

Thanks! It works great with that Make file.

All the best,
Tony

>
> -Sumit
>
> > make[2]: *** No rule to make target 'dts/upstream/src/arm/Makefile'.  Stop.
> > make[1]: *** [dts/Makefile:54: arch-dtbs] Error 2
> > make: *** [Makefile:1166: dts/dt.dtb] Error 2
> > make: *** Waiting for unfinished jobs....
> > make: Leaving directory '/usr/src/u-boot'
> >
> > I also tried Bryan's patch like you've suggested to Fabio. But it
> > seems Bryan's patch was already in the next branch. Did I miss
> > something?
> >
> > All the best ,
> > Tony