All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: Rob Herring <robh@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>,
	linux-kernel@vger.kernel.org, patches@lists.linux.dev,
	linux-um@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, kunit-dev@googlegroups.com,
	linux-kselftest@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 4/6] of: Create of_root if no dtb provided by firmware
Date: Tue, 16 Jan 2024 17:18:15 -0800	[thread overview]
Message-ID: <cdaadf62222a705cda198dd96dc7c73d.sboyd@kernel.org> (raw)
In-Reply-To: <20240115203230.GA1439771-robh@kernel.org>

Quoting Rob Herring (2024-01-15 12:32:30)
> On Fri, Jan 12, 2024 at 12:07:47PM -0800, Stephen Boyd wrote:
> > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > index da9826accb1b..9628e48baa15 100644
> > --- a/drivers/of/Kconfig
> > +++ b/drivers/of/Kconfig
> > @@ -54,9 +54,14 @@ config OF_FLATTREE
> >       select CRC32
> >  
> >  config OF_EARLY_FLATTREE
> > -     bool
> > +     bool "Functions for accessing Flat Devicetree (FDT) early in boot"
> 
> I think we could instead just get rid of this kconfig option. Or 
> always enable with CONFIG_OF (except on Sparc). The only cost of 
> enabling it is init section functions which get freed anyways.

Getting rid of it is a more massive change. It can be the default and
kept hidden instead? If it can't be selected on Sparc then it should be
hidden there anyway.

> 
> >       select DMA_DECLARE_COHERENT if HAS_DMA && HAS_IOMEM
> >       select OF_FLATTREE
> > +     help
> > +       Normally selected by platforms that process an FDT that has been
> > +       passed to the kernel by the bootloader.  If the bootloader does not
> > +       pass an FDT to the kernel and you need an empty devicetree that
> > +       contains only a root node to exist, then say Y here.
> >  
> >  config OF_PROMTREE
> >       bool
[...]
> > @@ -195,6 +191,17 @@ static inline int of_node_check_flag(const struct device_node *n, unsigned long
> >       return test_bit(flag, &n->_flags);
> >  }
> >  
> > +/**
> > + * of_have_populated_dt() - Has DT been populated by bootloader
> > + *
> > + * Return: True if a DTB has been populated by the bootloader and it isn't the
> > + * empty builtin one. False otherwise.
> > + */
> > +static inline bool of_have_populated_dt(void)
> > +{
> > +     return of_root != NULL && !of_node_check_flag(of_root, OF_EMPTY_ROOT);
> 
> Just a side comment, but I think many/all callers of this function could 
> just be removed.
> 
> I don't love new flags. Another possible way to handle this would be 
> checking for "compatible" being present in the root node. I guess this 
> is fine as-is for now at least.

Ok. I can add a check for a compatible property. That's probably better
anyway. Should there be a compatible property there to signal that this
DT isn't compatible with anything? I worry about DT overlays injecting a
compatible string into the root node, but maybe that is already
prevented.

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@kernel.org>
To: Rob Herring <robh@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>,
	linux-kernel@vger.kernel.org, patches@lists.linux.dev,
	linux-um@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, kunit-dev@googlegroups.com,
	linux-kselftest@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 4/6] of: Create of_root if no dtb provided by firmware
Date: Tue, 16 Jan 2024 17:18:15 -0800	[thread overview]
Message-ID: <cdaadf62222a705cda198dd96dc7c73d.sboyd@kernel.org> (raw)
In-Reply-To: <20240115203230.GA1439771-robh@kernel.org>

Quoting Rob Herring (2024-01-15 12:32:30)
> On Fri, Jan 12, 2024 at 12:07:47PM -0800, Stephen Boyd wrote:
> > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> > index da9826accb1b..9628e48baa15 100644
> > --- a/drivers/of/Kconfig
> > +++ b/drivers/of/Kconfig
> > @@ -54,9 +54,14 @@ config OF_FLATTREE
> >       select CRC32
> >  
> >  config OF_EARLY_FLATTREE
> > -     bool
> > +     bool "Functions for accessing Flat Devicetree (FDT) early in boot"
> 
> I think we could instead just get rid of this kconfig option. Or 
> always enable with CONFIG_OF (except on Sparc). The only cost of 
> enabling it is init section functions which get freed anyways.

Getting rid of it is a more massive change. It can be the default and
kept hidden instead? If it can't be selected on Sparc then it should be
hidden there anyway.

> 
> >       select DMA_DECLARE_COHERENT if HAS_DMA && HAS_IOMEM
> >       select OF_FLATTREE
> > +     help
> > +       Normally selected by platforms that process an FDT that has been
> > +       passed to the kernel by the bootloader.  If the bootloader does not
> > +       pass an FDT to the kernel and you need an empty devicetree that
> > +       contains only a root node to exist, then say Y here.
> >  
> >  config OF_PROMTREE
> >       bool
[...]
> > @@ -195,6 +191,17 @@ static inline int of_node_check_flag(const struct device_node *n, unsigned long
> >       return test_bit(flag, &n->_flags);
> >  }
> >  
> > +/**
> > + * of_have_populated_dt() - Has DT been populated by bootloader
> > + *
> > + * Return: True if a DTB has been populated by the bootloader and it isn't the
> > + * empty builtin one. False otherwise.
> > + */
> > +static inline bool of_have_populated_dt(void)
> > +{
> > +     return of_root != NULL && !of_node_check_flag(of_root, OF_EMPTY_ROOT);
> 
> Just a side comment, but I think many/all callers of this function could 
> just be removed.
> 
> I don't love new flags. Another possible way to handle this would be 
> checking for "compatible" being present in the root node. I guess this 
> is fine as-is for now at least.

Ok. I can add a check for a compatible property. That's probably better
anyway. Should there be a compatible property there to signal that this
DT isn't compatible with anything? I worry about DT overlays injecting a
compatible string into the root node, but maybe that is already
prevented.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-01-17  1:18 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-12 20:07 [PATCH 0/6] of: populate of_root node if bootloader doesn't Stephen Boyd
2024-01-12 20:07 ` Stephen Boyd
2024-01-12 20:07 ` [PATCH 1/6] arm64: Unconditionally call unflatten_device_tree() Stephen Boyd
2024-01-12 20:07   ` Stephen Boyd
2024-01-15 17:57   ` Rob Herring
2024-01-15 17:57     ` Rob Herring
2024-01-16 11:51   ` Mark Rutland
2024-01-16 11:51     ` Mark Rutland
2024-01-16 14:13     ` Geert Uytterhoeven
2024-01-16 14:13       ` Geert Uytterhoeven
2024-01-18 15:23       ` Mark Rutland
2024-01-18 15:23         ` Mark Rutland
2024-01-18 16:22         ` Geert Uytterhoeven
2024-01-18 16:22           ` Geert Uytterhoeven
2024-01-17  1:27     ` Stephen Boyd
2024-01-17  1:27       ` Stephen Boyd
2024-01-17 17:54       ` Rob Herring
2024-01-17 17:54         ` Rob Herring
2024-01-17 23:00         ` Stephen Boyd
2024-01-17 23:00           ` Stephen Boyd
2024-01-18 15:26       ` Mark Rutland
2024-01-18 15:26         ` Mark Rutland
2024-01-18 16:23         ` Geert Uytterhoeven
2024-01-18 16:23           ` Geert Uytterhoeven
2024-01-19 23:10         ` Rob Herring
2024-01-19 23:10           ` Rob Herring
2024-01-12 20:07 ` [PATCH 2/6] um: " Stephen Boyd
2024-01-12 20:07   ` Stephen Boyd
2024-01-12 20:07 ` [PATCH 3/6] of: Always unflatten in unflatten_and_copy_device_tree() Stephen Boyd
2024-01-12 20:07   ` Stephen Boyd
2024-01-12 20:07 ` [PATCH 4/6] of: Create of_root if no dtb provided by firmware Stephen Boyd
2024-01-12 20:07   ` Stephen Boyd
2024-01-15 20:32   ` Rob Herring
2024-01-15 20:32     ` Rob Herring
2024-01-17  1:18     ` Stephen Boyd [this message]
2024-01-17  1:18       ` Stephen Boyd
2024-01-17 17:41       ` Rob Herring
2024-01-17 17:41         ` Rob Herring
2024-01-18  8:45         ` Geert Uytterhoeven
2024-01-18  8:45           ` Geert Uytterhoeven
2024-01-18 13:44           ` Rob Herring
2024-01-18 13:44             ` Rob Herring
2024-01-12 20:07 ` [PATCH 5/6] of: unittest: treat missing of_root as error instead of fixing up Stephen Boyd
2024-01-12 20:07   ` Stephen Boyd
2024-01-12 20:07 ` [PATCH 6/6] of: Add KUnit test to confirm DTB is loaded Stephen Boyd
2024-01-12 20:07   ` Stephen Boyd
2024-01-16  5:03   ` David Gow
2024-01-16  5:03     ` David Gow
2024-01-22 22:48     ` Stephen Boyd
2024-01-22 22:48       ` Stephen Boyd
2024-01-24  7:25       ` David Gow
2024-01-24  7:25         ` David Gow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cdaadf62222a705cda198dd96dc7c73d.sboyd@kernel.org \
    --to=sboyd@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-um@lists.infradead.org \
    --cc=patches@lists.linux.dev \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.