diff mbox series

[v2] devicetree.bbclass: Allow selection of dts files to build

Message ID 20230421084052.2675963-1-kubiznak@2n.com
State Accepted, archived
Commit a6164c384a5bf3980a7a6c7f23927af9aca93b85
Headers show
Series [v2] devicetree.bbclass: Allow selection of dts files to build | expand

Commit Message

Petr Kubizňák - 2N April 21, 2023, 8:40 a.m. UTC
Add DT_FILES variable to allow the user of the class to select specific
dts files to build. This is useful for packages featuring dts files
for multiple machines.

Since many machine configs contain a list of dtb files
(e.g. KERNEL_DEVICETREE), DT_FILES works with both dts and dtb files.

Signed-off-by: Petr Kubizňák <kubiznak@2n.com>
---
 meta/classes-recipe/devicetree.bbclass | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Nathan Rossi April 21, 2023, 11:44 a.m. UTC | #1
On Fri, 21 Apr 2023 at 18:41, Petr Kubizňák <kubiznak@2n.com> wrote:
>
> Add DT_FILES variable to allow the user of the class to select specific
> dts files to build. This is useful for packages featuring dts files
> for multiple machines.
>
> Since many machine configs contain a list of dtb files
> (e.g. KERNEL_DEVICETREE), DT_FILES works with both dts and dtb files.
>
> Signed-off-by: Petr Kubizňák <kubiznak@2n.com>
> ---
>  meta/classes-recipe/devicetree.bbclass | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes-recipe/devicetree.bbclass b/meta/classes-recipe/devicetree.bbclass
> index ed2a92e447..8f348f1370 100644
> --- a/meta/classes-recipe/devicetree.bbclass
> +++ b/meta/classes-recipe/devicetree.bbclass
> @@ -53,8 +53,10 @@ KERNEL_INCLUDE ??= " \
>
>  DT_INCLUDE[doc] = "Search paths to be made available to both the device tree compiler and preprocessor for inclusion."
>  DT_INCLUDE ?= "${DT_FILES_PATH} ${KERNEL_INCLUDE}"
> -DT_FILES_PATH[doc] = "Defaults to source directory, can be used to select dts files that are not in source (e.g. generated)."
> +DT_FILES_PATH[doc] = "Path to the directory containing dts files to build. Defaults to source directory."
>  DT_FILES_PATH ?= "${S}"
> +DT_FILES[doc] = "Space-separated list of dts or dtb files to build. If empty, all dts files are built."

Worth noting in the doc string that DT_FILES paths are relative to
DT_FILES_PATH.

> +DT_FILES ?= ""
>
>  DT_PADDING_SIZE[doc] = "Size of padding on the device tree blob, used as extra space typically for additional properties during boot."
>  DT_PADDING_SIZE ??= "0x3000"
> @@ -125,7 +127,10 @@ def devicetree_compile(dtspath, includes, d):
>      subprocess.run(dtcargs, check = True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
>
>  python devicetree_do_compile() {
> +    import re
>      includes = expand_includes("DT_INCLUDE", d)
> +    dtfiles = d.getVar("DT_FILES")

If this populates the variable dtfiles with DT_FILES already split it
can fall back to os.listdir if empty, this avoids the need for the "if
filter..." logic within the loop:

dtfiles = d.getVar("DT_FILES").split()
dtfiles = [<renaming for dtb> for dtfiles in dtfiles]
for dts in dtfiles or os.listdir(listpath):

> +    filter = [ re.sub("dtbo?", "dts", dtfile) for dtfile in dtfiles.split() ]

This regex is a bit problematic, it will substitute any part of the
filename and not just the extension suffix. E.g. "test-dtbo.dts" ->
"test-dts.dts". The expression might need to be something similar to
"\.dtbo?$".

I am also curious why you need to handle this mapping of .dtb -> .dts.
Do you have an example recipe that is using this class in this way?
Because the ".dtb" in KERNEL_DEVICETREE is specifically because the
KERNEL_DEVICETREE variable is pointing at kernel makefile targets.

Regards,
Nathan

>      listpath = d.getVar("DT_FILES_PATH")
>      for dts in os.listdir(listpath):
>          dtspath = os.path.join(listpath, dts)
> @@ -134,6 +139,9 @@ python devicetree_do_compile() {
>                  continue # skip non-.dts files and non-overlay files
>          except:
>              continue # skip if can't determine if overlay
> +        if filter:
> +            if not(dts in filter):
> +                continue # skip if dts not in defined filter
>          devicetree_compile(dtspath, includes, d)
>  }
>
> --
> 2.30.2
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#180281): https://lists.openembedded.org/g/openembedded-core/message/180281
> Mute This Topic: https://lists.openembedded.org/mt/98406619/3616963
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [nathan@nathanrossi.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Petr Kubizňák - 2N April 21, 2023, 12:30 p.m. UTC | #2
Hi Nathan,

Thank you for great suggestions. All of them are valid and I will update the patch once we discuss the .dtb -> .dts mapping.

> I am also curious why you need to handle this mapping of .dtb -> .dts.
> Do you have an example recipe that is using this class in this way?
> Because the ".dtb" in KERNEL_DEVICETREE is specifically because the
> KERNEL_DEVICETREE variable is pointing at kernel makefile targets.

I don't have an example of a particular recipe, but how about EXTERNAL_KERNEL_DEVICETREE variable, used in kernel-fitimage.bbclass? Albeit I don't use this variable in my recipes, I believe it could be used for both classes now with simple assignment DT_FILES = EXTERNAL_KERNEL_DEVICETREE.

And if for whatever reason someone uses a custom variable (like me), they would still expect dtb's rather than dts's. I didn't find any example of using lists of dts files. Contrary, kernel-devicetree.bbclass does the same translation dtb->dts and it even warns about dts being provided (see normalize_dtb()). I didn't want to be that strict here (if someone has a list of dts files, while not to use it directly) but in general, I expect a list of dtb files will be much more common.

Kind Regards,
Petr
Petr Kubizňák - 2N April 21, 2023, 12:47 p.m. UTC | #3
> kernel-devicetree.bbclass does the same translation dtb->dts

I mean dts->dtb, of course. The point is dtb's are expected on the input.

Petr
Nathan Rossi April 22, 2023, 4:22 a.m. UTC | #4
On Fri, 21 Apr 2023 at 22:47, Petr Kubizňák - 2N <Kubiznak@2n.com> wrote:
>
> > kernel-devicetree.bbclass does the same translation dtb->dts
>
> I mean dts->dtb, of course. The point is dtb's are expected on the input.
>
> Petr
>
> ________________________________________
> From: Petr Kubizňák - 2N <Kubiznak@2n.com>
> Sent: Friday, April 21, 2023 2:30 PM
> To: Nathan Rossi
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core][PATCH v2] devicetree.bbclass: Allow selection of dts files to build
>
> Hi Nathan,
>
> Thank you for great suggestions. All of them are valid and I will update the patch once we discuss the .dtb -> .dts mapping.
>
> > I am also curious why you need to handle this mapping of .dtb -> .dts.
> > Do you have an example recipe that is using this class in this way?
> > Because the ".dtb" in KERNEL_DEVICETREE is specifically because the
> > KERNEL_DEVICETREE variable is pointing at kernel makefile targets.
>
> I don't have an example of a particular recipe, but how about EXTERNAL_KERNEL_DEVICETREE variable, used in kernel-fitimage.bbclass? Albeit I don't use this variable in my recipes, I believe it could be used for both classes now with simple assignment DT_FILES = EXTERNAL_KERNEL_DEVICETREE.
>
> And if for whatever reason someone uses a custom variable (like me), they would still expect dtb's rather than dts's. I didn't find any example of using lists of dts files. Contrary, kernel-devicetree.bbclass does the same translation dtb->dts and it even warns about dts being provided (see normalize_dtb()). I didn't want to be that strict here (if someone has a list of dts files, while not to use it directly) but in general, I expect a list of dtb files will be much more common.

So just trying to make the DT_FILES somewhat consistent with the
KERNEL_DEVICETREE, seems fine. I was only curious in case you had some
special path handling that might be better solved by overriding the
devicetree_do_compile and calling the associated compile functions
directly.

The v3 patch looks good :).

Thanks,
Nathan


>
> Kind Regards,
> Petr
>
> ________________________________________
> From: Nathan Rossi <nathan@nathanrossi.com>
> Sent: Friday, April 21, 2023 1:44 PM
> To: Petr Kubizňák - 2N
> Cc: openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core][PATCH v2] devicetree.bbclass: Allow selection of dts files to build
>
> On Fri, 21 Apr 2023 at 18:41, Petr Kubizňák <kubiznak@2n.com> wrote:
> >
> > Add DT_FILES variable to allow the user of the class to select specific
> > dts files to build. This is useful for packages featuring dts files
> > for multiple machines.
> >
> > Since many machine configs contain a list of dtb files
> > (e.g. KERNEL_DEVICETREE), DT_FILES works with both dts and dtb files.
> >
> > Signed-off-by: Petr Kubizňák <kubiznak@2n.com>
> > ---
> >  meta/classes-recipe/devicetree.bbclass | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/meta/classes-recipe/devicetree.bbclass b/meta/classes-recipe/devicetree.bbclass
> > index ed2a92e447..8f348f1370 100644
> > --- a/meta/classes-recipe/devicetree.bbclass
> > +++ b/meta/classes-recipe/devicetree.bbclass
> > @@ -53,8 +53,10 @@ KERNEL_INCLUDE ??= " \
> >
> >  DT_INCLUDE[doc] = "Search paths to be made available to both the device tree compiler and preprocessor for inclusion."
> >  DT_INCLUDE ?= "${DT_FILES_PATH} ${KERNEL_INCLUDE}"
> > -DT_FILES_PATH[doc] = "Defaults to source directory, can be used to select dts files that are not in source (e.g. generated)."
> > +DT_FILES_PATH[doc] = "Path to the directory containing dts files to build. Defaults to source directory."
> >  DT_FILES_PATH ?= "${S}"
> > +DT_FILES[doc] = "Space-separated list of dts or dtb files to build. If empty, all dts files are built."
>
> Worth noting in the doc string that DT_FILES paths are relative to
> DT_FILES_PATH.
>
> > +DT_FILES ?= ""
> >
> >  DT_PADDING_SIZE[doc] = "Size of padding on the device tree blob, used as extra space typically for additional properties during boot."
> >  DT_PADDING_SIZE ??= "0x3000"
> > @@ -125,7 +127,10 @@ def devicetree_compile(dtspath, includes, d):
> >      subprocess.run(dtcargs, check = True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
> >
> >  python devicetree_do_compile() {
> > +    import re
> >      includes = expand_includes("DT_INCLUDE", d)
> > +    dtfiles = d.getVar("DT_FILES")
>
> If this populates the variable dtfiles with DT_FILES already split it
> can fall back to os.listdir if empty, this avoids the need for the "if
> filter..." logic within the loop:
>
> dtfiles = d.getVar("DT_FILES").split()
> dtfiles = [<renaming for dtb> for dtfiles in dtfiles]
> for dts in dtfiles or os.listdir(listpath):
>
> > +    filter = [ re.sub("dtbo?", "dts", dtfile) for dtfile in dtfiles.split() ]
>
> This regex is a bit problematic, it will substitute any part of the
> filename and not just the extension suffix. E.g. "test-dtbo.dts" ->
> "test-dts.dts". The expression might need to be something similar to
> "\.dtbo?$".
>
> I am also curious why you need to handle this mapping of .dtb -> .dts.
> Do you have an example recipe that is using this class in this way?
> Because the ".dtb" in KERNEL_DEVICETREE is specifically because the
> KERNEL_DEVICETREE variable is pointing at kernel makefile targets.
>
> Regards,
> Nathan
>
> >      listpath = d.getVar("DT_FILES_PATH")
> >      for dts in os.listdir(listpath):
> >          dtspath = os.path.join(listpath, dts)
> > @@ -134,6 +139,9 @@ python devicetree_do_compile() {
> >                  continue # skip non-.dts files and non-overlay files
> >          except:
> >              continue # skip if can't determine if overlay
> > +        if filter:
> > +            if not(dts in filter):
> > +                continue # skip if dts not in defined filter
> >          devicetree_compile(dtspath, includes, d)
> >  }
> >
> > --
> > 2.30.2
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#180281): https://lists.openembedded.org/g/openembedded-core/message/180281
> > Mute This Topic: https://lists.openembedded.org/mt/98406619/3616963
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [nathan@nathanrossi.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
diff mbox series

Patch

diff --git a/meta/classes-recipe/devicetree.bbclass b/meta/classes-recipe/devicetree.bbclass
index ed2a92e447..8f348f1370 100644
--- a/meta/classes-recipe/devicetree.bbclass
+++ b/meta/classes-recipe/devicetree.bbclass
@@ -53,8 +53,10 @@  KERNEL_INCLUDE ??= " \
 
 DT_INCLUDE[doc] = "Search paths to be made available to both the device tree compiler and preprocessor for inclusion."
 DT_INCLUDE ?= "${DT_FILES_PATH} ${KERNEL_INCLUDE}"
-DT_FILES_PATH[doc] = "Defaults to source directory, can be used to select dts files that are not in source (e.g. generated)."
+DT_FILES_PATH[doc] = "Path to the directory containing dts files to build. Defaults to source directory."
 DT_FILES_PATH ?= "${S}"
+DT_FILES[doc] = "Space-separated list of dts or dtb files to build. If empty, all dts files are built."
+DT_FILES ?= ""
 
 DT_PADDING_SIZE[doc] = "Size of padding on the device tree blob, used as extra space typically for additional properties during boot."
 DT_PADDING_SIZE ??= "0x3000"
@@ -125,7 +127,10 @@  def devicetree_compile(dtspath, includes, d):
     subprocess.run(dtcargs, check = True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
 
 python devicetree_do_compile() {
+    import re
     includes = expand_includes("DT_INCLUDE", d)
+    dtfiles = d.getVar("DT_FILES")
+    filter = [ re.sub("dtbo?", "dts", dtfile) for dtfile in dtfiles.split() ]
     listpath = d.getVar("DT_FILES_PATH")
     for dts in os.listdir(listpath):
         dtspath = os.path.join(listpath, dts)
@@ -134,6 +139,9 @@  python devicetree_do_compile() {
                 continue # skip non-.dts files and non-overlay files
         except:
             continue # skip if can't determine if overlay
+        if filter:
+            if not(dts in filter):
+                continue # skip if dts not in defined filter
         devicetree_compile(dtspath, includes, d)
 }