diff mbox series

[bug#57070] bootloader: extlinux: support for optional FDTDIR

Message ID 20220828124938.0289bd68@pantherx.org
State Accepted
Headers show
Series [bug#57070] bootloader: extlinux: support for optional FDTDIR | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Reza Alizadeh Majd Aug. 28, 2022, 8:19 a.m. UTC
Hi Mathieu,


On Thu, 25 Aug 2022 19:35:45 +0200
Mathieu Othacehe <othacehe@gnu.org> wrote:

>
>> * gnu/bootloader.scm (<bootloader>)[device-tree-support?]: new
>> field.  
>
>You need to document this new field in the "Bootloader Configuration"
>documentation section.
>

I just checked the "Bootloader Configuration" section. it describes the
"bootloader-configuration" record itself, but the proposed patch adds
the "device-tree-supports?" field to the "bootloader" record. 

unfortunately I couldn't find the section describing the "bootloader"
record fields. so I added the documentations as a note for the
"bootloader" field of "bootloader-configuration" record. 


>> * gnu/tests/bootloader.scm: add tests for FDTDIR modification.  
>
>This will test for regressions on an x86_64-linux machine that will
>probably never use this FDTDIR thing. As those tests are expensive to
>run an maintain we can probably remove the test.
>

OK, I removed the test from recent patch. 


Best,
Reza

Comments

Mathieu Othacehe Aug. 28, 2022, 3:49 p.m. UTC | #1
Hey Reza,

Thanks for the updated version!

> I just checked the "Bootloader Configuration" section. it describes the
> "bootloader-configuration" record itself, but the proposed patch adds
> the "device-tree-supports?" field to the "bootloader" record. 

About that, any reason not to have this "device-tree-supports?" field in
the <bootloader-configuration> record?

The <bootloader> record is about how to install the bootloader while
<bootloader-configuration> is about its configuration. So maybe it would
be a better fit?

> OK, I removed the test from recent patch. 

Good. Let me know what you think about the proposal and we should be
good to proceed.

Mathieu
diff mbox series

Patch

From cbeba372def25a957f5d8942f01fbde6cdc03704 Mon Sep 17 00:00:00 2001
From: Reza Alizadeh Majd <r.majd@pantherx.org>
Date: Sun, 28 Aug 2022 12:34:46 +0430
Subject: [PATCH] bootloader: extlinux: support for optional FDTDIR

There are situations that u-boot doesn't have to load from the device tree.
some provide the device tree using a vendor bootloader (like what raspberry-pi
does) or with an external bootloader that chainloads the u-boot (what Asahi
does for m1n1 bootloader).

Unfortunately we couldn't find any reliable document to enforce u-boot to pass
the device tree via `extlinux.conf`, however during our tests, we found that
removing the `FDTDIR` line from the `extlinux.conf` tend us to do so.

There is also no reliable way to guess if u-boot bootloader should load device
tree or not on a specific hardware. in addition, there are hardware that can
be booted with both firmware device tree on some kernels and with special
device tree on other (modified) kernels.

the following changes provided to define an optional parameter in <bootloader>
record, called <device-tree-support?>  which by default is set to #t to keep
the current behavior unchanged. if this paramter is set to #f, the FDTDIR line
will be discarded from the <extlinux.conf> and u-boot doesn't load the device
tree automatically.

* gnu/bootloader.scm (<bootloader>)[device-tree-support?]: new field.
* gnu/bootloader/extlinux.scm (extlinux-configuration-file): add FDTDIR line
based on bootloader <device-tree-support?> field of <bootloader>.
* doc/guix.texi (Bootloader Configuration)[bootloader]: Add note about
bootloader's optional FDTDIR support.
---
 doc/guix.texi               |  9 +++++++++
 gnu/bootloader.scm          |  6 +++++-
 gnu/bootloader/extlinux.scm | 13 +++++++++++--
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 023b48ae35..25b336e958 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -36814,6 +36814,15 @@  modules.  In particular, @code{(gnu bootloader u-boot)} contains definitions
 of bootloaders for a wide range of ARM and AArch64 systems, using the
 @uref{https://www.denx.de/wiki/U-Boot/, U-Boot bootloader}.
 
+@quotation Note
+There are situations when @code{u-boot} shouldn't load the device tree from
+the file system. because it is either unnecessary for the platform being used
+or has already been loaded into RAM earlier in the boot process. in such cases
+you can set the @code{device-tree-support?} field of the @code{bootloader}
+record to @code{#f}. setting this flag to @code{#f} removes the @code{FDTDIR}
+from the @file{/boot/extlinux/extlinux.conf}.
+@end quotation
+
 @vindex grub-efi-bootloader
 @code{grub-efi-bootloader} allows to boot on modern systems using the
 @dfn{Unified Extensible Firmware Interface} (UEFI).  This is what you should
diff --git a/gnu/bootloader.scm b/gnu/bootloader.scm
index 70e1836179..32fd7f0c2e 100644
--- a/gnu/bootloader.scm
+++ b/gnu/bootloader.scm
@@ -4,6 +4,7 @@ 
 ;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2019, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
+;;; Copyright © 2022 Reza Alizadeh Majd <r.majd@pantherx.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -54,6 +55,7 @@  (define-module (gnu bootloader)
             bootloader-disk-image-installer
             bootloader-configuration-file
             bootloader-configuration-file-generator
+            bootloader-device-tree-support?
 
             bootloader-configuration
             bootloader-configuration?
@@ -172,7 +174,9 @@  (define-record-type* <bootloader>
   (disk-image-installer            bootloader-disk-image-installer
                                    (default #f))
   (configuration-file              bootloader-configuration-file)
-  (configuration-file-generator    bootloader-configuration-file-generator))
+  (configuration-file-generator    bootloader-configuration-file-generator)
+  (device-tree-support?            bootloader-device-tree-support?
+                                   (default #t)))
 
 
 ;;;
diff --git a/gnu/bootloader/extlinux.scm b/gnu/bootloader/extlinux.scm
index 6b5ff298e7..f3d69c0cc0 100644
--- a/gnu/bootloader/extlinux.scm
+++ b/gnu/bootloader/extlinux.scm
@@ -1,6 +1,7 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017 David Craven <david@craven.ch>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2022 Reza Alizadeh Majd <r.majd@pantherx.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -38,6 +39,10 @@  (define* (extlinux-configuration-file config entries
   (define all-entries
     (append entries (bootloader-configuration-menu-entries config)))
 
+  (define with-fdtdir?
+    (let ((bootloader (bootloader-configuration-bootloader config)))
+      (bootloader-device-tree-support? bootloader)))
+
   (define (menu-entry->gexp entry)
     (let ((label (menu-entry-label entry))
           (kernel (menu-entry-linux entry))
@@ -46,12 +51,16 @@  (define (menu-entry->gexp entry)
       #~(format port "LABEL ~a
   MENU LABEL ~a
   KERNEL ~a
-  FDTDIR ~a/lib/dtbs
+  ~a
   INITRD ~a
   APPEND ~a
 ~%"
                 #$label #$label
-                #$kernel (dirname #$kernel) #$initrd
+                #$kernel
+                (if #$with-fdtdir?
+                    (string-append "FDTDIR " (dirname #$kernel) "/lib/dtbs")
+                    "")
+                #$initrd
                 (string-join (list #$@kernel-arguments)))))
 
   (define builder
-- 
2.37.1