diff mbox series

[13/14] Update u-boot.cfg to include CFG also

Message ID 20240623203033.1566505-14-sjg@chromium.org
State Deferred
Delegated to: Tom Rini
Headers show
Series testb: Various tweaks and fixes for Labgrid | expand

Commit Message

Simon Glass June 23, 2024, 8:30 p.m. UTC
Some configuration is now in variables with a CFG_ prefix. Add these to
the .cfg file so that we can see everything in one place. Sort the
options so they are easier to find and compare.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options

 scripts/Makefile.autoconf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Rini June 24, 2024, 6:29 p.m. UTC | #1
On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:

> Some configuration is now in variables with a CFG_ prefix. Add these to
> the .cfg file so that we can see everything in one place. Sort the
> options so they are easier to find and compare.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Add new patch to update u-boot.cfg with CFG_... options
> 
>  scripts/Makefile.autoconf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> index b42f9b525fe..65ff11ea508 100644
> --- a/scripts/Makefile.autoconf
> +++ b/scripts/Makefile.autoconf
> @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
>  quiet_cmd_u_boot_cfg = CFG     $@
>        cmd_u_boot_cfg = \
>  	$(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> -		grep 'define CONFIG_' $@.tmp | \
> +		egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
>  			sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
>  		rm $@.tmp;						\
>  	} || {								\

I don't like this because whereas "CONFIG_" is enforced to be set only
by Kconfig and so always all reliably set and found via a single header,
CFG_ stuff is not.
Simon Glass June 25, 2024, 12:38 p.m. UTC | #2
Hi Tom,

On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
>
> > Some configuration is now in variables with a CFG_ prefix. Add these to
> > the .cfg file so that we can see everything in one place. Sort the
> > options so they are easier to find and compare.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add new patch to update u-boot.cfg with CFG_... options
> >
> >  scripts/Makefile.autoconf | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > index b42f9b525fe..65ff11ea508 100644
> > --- a/scripts/Makefile.autoconf
> > +++ b/scripts/Makefile.autoconf
> > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> >  quiet_cmd_u_boot_cfg = CFG     $@
> >        cmd_u_boot_cfg = \
> >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > -             grep 'define CONFIG_' $@.tmp | \
> > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> >               rm $@.tmp;                                              \
> >       } || {                                                          \
>
> I don't like this because whereas "CONFIG_" is enforced to be set only
> by Kconfig and so always all reliably set and found via a single header,
> CFG_ stuff is not.

OK, so how are CFG_ options found? I hit this when trying to find the
SDRAM size on rockchip 3399 and I could not find any way of figuring
it out.

Regards,
Simon
Tom Rini June 25, 2024, 2:14 p.m. UTC | #3
On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> >
> > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > the .cfg file so that we can see everything in one place. Sort the
> > > options so they are easier to find and compare.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > > Changes in v2:
> > > - Add new patch to update u-boot.cfg with CFG_... options
> > >
> > >  scripts/Makefile.autoconf | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > index b42f9b525fe..65ff11ea508 100644
> > > --- a/scripts/Makefile.autoconf
> > > +++ b/scripts/Makefile.autoconf
> > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > >  quiet_cmd_u_boot_cfg = CFG     $@
> > >        cmd_u_boot_cfg = \
> > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > -             grep 'define CONFIG_' $@.tmp | \
> > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > >               rm $@.tmp;                                              \
> > >       } || {                                                          \
> >
> > I don't like this because whereas "CONFIG_" is enforced to be set only
> > by Kconfig and so always all reliably set and found via a single header,
> > CFG_ stuff is not.
> 
> OK, so how are CFG_ options found? I hit this when trying to find the
> SDRAM size on rockchip 3399 and I could not find any way of figuring
> it out.

It's just another define, there's no uniformity to it. For some of the
SDRAM values really we need some build time way to grab some information
out of the default device tree.
Simon Glass June 26, 2024, 8 a.m. UTC | #4
Hi Tom,

On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > >
> > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > the .cfg file so that we can see everything in one place. Sort the
> > > > options so they are easier to find and compare.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > >
> > > >  scripts/Makefile.autoconf | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > index b42f9b525fe..65ff11ea508 100644
> > > > --- a/scripts/Makefile.autoconf
> > > > +++ b/scripts/Makefile.autoconf
> > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > >        cmd_u_boot_cfg = \
> > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > >               rm $@.tmp;                                              \
> > > >       } || {                                                          \
> > >
> > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > by Kconfig and so always all reliably set and found via a single header,
> > > CFG_ stuff is not.
> >
> > OK, so how are CFG_ options found? I hit this when trying to find the
> > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > it out.
>
> It's just another define, there's no uniformity to it. For some of the
> SDRAM values really we need some build time way to grab some information
> out of the default device tree.

Can you give an example of a board that could use this? I looked at
the devicetree for chromebook_kevin and don't see a memory range in
ther.

Regards,
Simon
Tom Rini June 26, 2024, 2:07 p.m. UTC | #5
On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > >
> > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > options so they are easier to find and compare.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > >
> > > > >  scripts/Makefile.autoconf | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > --- a/scripts/Makefile.autoconf
> > > > > +++ b/scripts/Makefile.autoconf
> > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > >        cmd_u_boot_cfg = \
> > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > >               rm $@.tmp;                                              \
> > > > >       } || {                                                          \
> > > >
> > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > by Kconfig and so always all reliably set and found via a single header,
> > > > CFG_ stuff is not.
> > >
> > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > it out.
> >
> > It's just another define, there's no uniformity to it. For some of the
> > SDRAM values really we need some build time way to grab some information
> > out of the default device tree.
> 
> Can you give an example of a board that could use this? I looked at
> the devicetree for chromebook_kevin and don't see a memory range in
> ther.

OK, wow, I didn't realize /memory was optional now. But indeed, I don't
see it in the dtb file. That removes that option then, sadly.
Simon Glass June 27, 2024, 8:37 a.m. UTC | #6
Hi Tom,

On Wed, 26 Jun 2024 at 15:07, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > > >
> > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > > options so they are easier to find and compare.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > >
> > > > > > Changes in v2:
> > > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > > >
> > > > > >  scripts/Makefile.autoconf | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > > --- a/scripts/Makefile.autoconf
> > > > > > +++ b/scripts/Makefile.autoconf
> > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > > >        cmd_u_boot_cfg = \
> > > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > > >               rm $@.tmp;                                              \
> > > > > >       } || {                                                          \
> > > > >
> > > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > > by Kconfig and so always all reliably set and found via a single header,
> > > > > CFG_ stuff is not.
> > > >
> > > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > > it out.
> > >
> > > It's just another define, there's no uniformity to it. For some of the
> > > SDRAM values really we need some build time way to grab some information
> > > out of the default device tree.
> >
> > Can you give an example of a board that could use this? I looked at
> > the devicetree for chromebook_kevin and don't see a memory range in
> > ther.
>
> OK, wow, I didn't realize /memory was optional now. But indeed, I don't
> see it in the dtb file. That removes that option then, sadly.

Well, we can still require it, so long as an error is produced if the
property is needed but does not exist.

Regards,
Simon
Tom Rini June 27, 2024, 2:42 p.m. UTC | #7
On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 26 Jun 2024 at 15:07, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > > > >
> > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > > > options so they are easier to find and compare.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > ---
> > > > > > >
> > > > > > > Changes in v2:
> > > > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > > > >
> > > > > > >  scripts/Makefile.autoconf | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > > > --- a/scripts/Makefile.autoconf
> > > > > > > +++ b/scripts/Makefile.autoconf
> > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > > > >        cmd_u_boot_cfg = \
> > > > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > > > >               rm $@.tmp;                                              \
> > > > > > >       } || {                                                          \
> > > > > >
> > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > > > by Kconfig and so always all reliably set and found via a single header,
> > > > > > CFG_ stuff is not.
> > > > >
> > > > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > > > it out.
> > > >
> > > > It's just another define, there's no uniformity to it. For some of the
> > > > SDRAM values really we need some build time way to grab some information
> > > > out of the default device tree.
> > >
> > > Can you give an example of a board that could use this? I looked at
> > > the devicetree for chromebook_kevin and don't see a memory range in
> > > ther.
> >
> > OK, wow, I didn't realize /memory was optional now. But indeed, I don't
> > see it in the dtb file. That removes that option then, sadly.
> 
> Well, we can still require it, so long as an error is produced if the
> property is needed but does not exist.

"We" who? I don't feel like we'll have a lot of traction with linux
kernel folks in requiring /memory to be added to the dts files on
however many platforms don't have it today because I'm going to guess
it's added at run time, possibly by us, with the correct size and we'd
be asking for statically adding things half-wrong like a lot of
platforms used to do (and in turn rely on U-Boot to correct the size).
Simon Glass June 28, 2024, 7:33 a.m. UTC | #8
Hi Tom,

On Thu, 27 Jun 2024 at 15:42, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 26 Jun 2024 at 15:07, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > > > > >
> > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > > > > options so they are easier to find and compare.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > ---
> > > > > > > >
> > > > > > > > Changes in v2:
> > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > > > > >
> > > > > > > >  scripts/Makefile.autoconf | 2 +-
> > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > > > > --- a/scripts/Makefile.autoconf
> > > > > > > > +++ b/scripts/Makefile.autoconf
> > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > > > > >        cmd_u_boot_cfg = \
> > > > > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > > > > >               rm $@.tmp;                                              \
> > > > > > > >       } || {                                                          \
> > > > > > >
> > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > > > > by Kconfig and so always all reliably set and found via a single header,
> > > > > > > CFG_ stuff is not.
> > > > > >
> > > > > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > > > > it out.
> > > > >
> > > > > It's just another define, there's no uniformity to it. For some of the
> > > > > SDRAM values really we need some build time way to grab some information
> > > > > out of the default device tree.
> > > >
> > > > Can you give an example of a board that could use this? I looked at
> > > > the devicetree for chromebook_kevin and don't see a memory range in
> > > > ther.
> > >
> > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't
> > > see it in the dtb file. That removes that option then, sadly.
> >
> > Well, we can still require it, so long as an error is produced if the
> > property is needed but does not exist.
>
> "We" who? I don't feel like we'll have a lot of traction with linux
> kernel folks in requiring /memory to be added to the dts files on
> however many platforms don't have it today because I'm going to guess
> it's added at run time, possibly by us, with the correct size and we'd
> be asking for statically adding things half-wrong like a lot of
> platforms used to do (and in turn rely on U-Boot to correct the size).

Hmm yes of course, the firmware is supposed to add these
properties...that's how it gets in there. So we need to stick with CFG
(and perhaps the RAM-size prober) for now.

Regards,
Simon
Simon Glass July 28, 2024, 7:36 p.m. UTC | #9
Hi Tom,

On Fri, 28 Jun 2024 at 01:33, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tom,
>
> On Thu, 27 Jun 2024 at 15:42, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 26 Jun 2024 at 15:07, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > > > > > >
> > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > > > > > options so they are easier to find and compare.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > Changes in v2:
> > > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > > > > > >
> > > > > > > > >  scripts/Makefile.autoconf | 2 +-
> > > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > >
> > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > > > > > --- a/scripts/Makefile.autoconf
> > > > > > > > > +++ b/scripts/Makefile.autoconf
> > > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > > > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > > > > > >        cmd_u_boot_cfg = \
> > > > > > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > > > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > > > > > >               rm $@.tmp;                                              \
> > > > > > > > >       } || {                                                          \
> > > > > > > >
> > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > > > > > by Kconfig and so always all reliably set and found via a single header,
> > > > > > > > CFG_ stuff is not.
> > > > > > >
> > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > > > > > it out.
> > > > > >
> > > > > > It's just another define, there's no uniformity to it. For some of the
> > > > > > SDRAM values really we need some build time way to grab some information
> > > > > > out of the default device tree.
> > > > >
> > > > > Can you give an example of a board that could use this? I looked at
> > > > > the devicetree for chromebook_kevin and don't see a memory range in
> > > > > ther.
> > > >
> > > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't
> > > > see it in the dtb file. That removes that option then, sadly.
> > >
> > > Well, we can still require it, so long as an error is produced if the
> > > property is needed but does not exist.
> >
> > "We" who? I don't feel like we'll have a lot of traction with linux
> > kernel folks in requiring /memory to be added to the dts files on
> > however many platforms don't have it today because I'm going to guess
> > it's added at run time, possibly by us, with the correct size and we'd
> > be asking for statically adding things half-wrong like a lot of
> > platforms used to do (and in turn rely on U-Boot to correct the size).
>
> Hmm yes of course, the firmware is supposed to add these
> properties...that's how it gets in there. So we need to stick with CFG
> (and perhaps the RAM-size prober) for now.

Coming back to this patch, can we apply it? It provides a way to find
out the value of these CFG options, which otherwise involves chasing
around header files.

Regards,
SImon
Tom Rini July 29, 2024, 6:17 p.m. UTC | #10
On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 28 Jun 2024 at 01:33, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tom,
> >
> > On Thu, 27 Jun 2024 at 15:42, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Wed, 26 Jun 2024 at 15:07, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > > > > > > >
> > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > > > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > > > > > > options so they are easier to find and compare.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > > Changes in v2:
> > > > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > > > > > > >
> > > > > > > > > >  scripts/Makefile.autoconf | 2 +-
> > > > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > > > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > > > > > > --- a/scripts/Makefile.autoconf
> > > > > > > > > > +++ b/scripts/Makefile.autoconf
> > > > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > > > > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > > > > > > >        cmd_u_boot_cfg = \
> > > > > > > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > > > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > > > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > > > > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > > > > > > >               rm $@.tmp;                                              \
> > > > > > > > > >       } || {                                                          \
> > > > > > > > >
> > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > > > > > > by Kconfig and so always all reliably set and found via a single header,
> > > > > > > > > CFG_ stuff is not.
> > > > > > > >
> > > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > > > > > > it out.
> > > > > > >
> > > > > > > It's just another define, there's no uniformity to it. For some of the
> > > > > > > SDRAM values really we need some build time way to grab some information
> > > > > > > out of the default device tree.
> > > > > >
> > > > > > Can you give an example of a board that could use this? I looked at
> > > > > > the devicetree for chromebook_kevin and don't see a memory range in
> > > > > > ther.
> > > > >
> > > > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't
> > > > > see it in the dtb file. That removes that option then, sadly.
> > > >
> > > > Well, we can still require it, so long as an error is produced if the
> > > > property is needed but does not exist.
> > >
> > > "We" who? I don't feel like we'll have a lot of traction with linux
> > > kernel folks in requiring /memory to be added to the dts files on
> > > however many platforms don't have it today because I'm going to guess
> > > it's added at run time, possibly by us, with the correct size and we'd
> > > be asking for statically adding things half-wrong like a lot of
> > > platforms used to do (and in turn rely on U-Boot to correct the size).
> >
> > Hmm yes of course, the firmware is supposed to add these
> > properties...that's how it gets in there. So we need to stick with CFG
> > (and perhaps the RAM-size prober) for now.
> 
> Coming back to this patch, can we apply it? It provides a way to find
> out the value of these CFG options, which otherwise involves chasing
> around header files.

No, because it implies that there's a consistent way to know what a
given CFG value will be when there is not. There is no equivalent header
to include like for CONFIG symbols to know that you got them. You're
likely better off trying out "ripgrep" which I have found to be much
faster than "git grep".
Simon Glass July 31, 2024, 2:39 p.m. UTC | #11
Hi Tom,

On Mon, 29 Jul 2024 at 12:17, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 28 Jun 2024 at 01:33, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Tom,
> > >
> > > On Thu, 27 Jun 2024 at 15:42, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Wed, 26 Jun 2024 at 15:07, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > > > > > > > Hi Tom,
> > > > > > > > >
> > > > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > > > > > > > >
> > > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > > > > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > > > > > > > options so they are easier to find and compare.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > > Changes in v2:
> > > > > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > > > > > > > >
> > > > > > > > > > >  scripts/Makefile.autoconf | 2 +-
> > > > > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > > > > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > > > > > > > --- a/scripts/Makefile.autoconf
> > > > > > > > > > > +++ b/scripts/Makefile.autoconf
> > > > > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > > > > > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > > > > > > > >        cmd_u_boot_cfg = \
> > > > > > > > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > > > > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > > > > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > > > > > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > > > > > > > >               rm $@.tmp;                                              \
> > > > > > > > > > >       } || {                                                          \
> > > > > > > > > >
> > > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > > > > > > > by Kconfig and so always all reliably set and found via a single header,
> > > > > > > > > > CFG_ stuff is not.
> > > > > > > > >
> > > > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > > > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > > > > > > > it out.
> > > > > > > >
> > > > > > > > It's just another define, there's no uniformity to it. For some of the
> > > > > > > > SDRAM values really we need some build time way to grab some information
> > > > > > > > out of the default device tree.
> > > > > > >
> > > > > > > Can you give an example of a board that could use this? I looked at
> > > > > > > the devicetree for chromebook_kevin and don't see a memory range in
> > > > > > > ther.
> > > > > >
> > > > > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't
> > > > > > see it in the dtb file. That removes that option then, sadly.
> > > > >
> > > > > Well, we can still require it, so long as an error is produced if the
> > > > > property is needed but does not exist.
> > > >
> > > > "We" who? I don't feel like we'll have a lot of traction with linux
> > > > kernel folks in requiring /memory to be added to the dts files on
> > > > however many platforms don't have it today because I'm going to guess
> > > > it's added at run time, possibly by us, with the correct size and we'd
> > > > be asking for statically adding things half-wrong like a lot of
> > > > platforms used to do (and in turn rely on U-Boot to correct the size).
> > >
> > > Hmm yes of course, the firmware is supposed to add these
> > > properties...that's how it gets in there. So we need to stick with CFG
> > > (and perhaps the RAM-size prober) for now.
> >
> > Coming back to this patch, can we apply it? It provides a way to find
> > out the value of these CFG options, which otherwise involves chasing
> > around header files.
>
> No, because it implies that there's a consistent way to know what a
> given CFG value will be when there is not. There is no equivalent header
> to include like for CONFIG symbols to know that you got them. You're
> likely better off trying out "ripgrep" which I have found to be much
> faster than "git grep".

Hmm it is really hard to know which files to grep!

Could we require that config.h includes all the CFG values? I'm just
trying to find a way to bring a bit more order to this area.

Regards,
Simon
Tom Rini July 31, 2024, 5:17 p.m. UTC | #12
On Wed, Jul 31, 2024 at 08:39:30AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 29 Jul 2024 at 12:17, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 28 Jun 2024 at 01:33, Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Tom,
> > > >
> > > > On Thu, 27 Jun 2024 at 15:42, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Wed, 26 Jun 2024 at 15:07, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > > > > > > > > Hi Tom,
> > > > > > > > > >
> > > > > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > > > > > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > > > > > > > > options so they are easier to find and compare.
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > > > ---
> > > > > > > > > > > >
> > > > > > > > > > > > Changes in v2:
> > > > > > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > > > > > > > > >
> > > > > > > > > > > >  scripts/Makefile.autoconf | 2 +-
> > > > > > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > > > > > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > > > > > > > > --- a/scripts/Makefile.autoconf
> > > > > > > > > > > > +++ b/scripts/Makefile.autoconf
> > > > > > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > > > > > > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > > > > > > > > >        cmd_u_boot_cfg = \
> > > > > > > > > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > > > > > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > > > > > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > > > > > > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > > > > > > > > >               rm $@.tmp;                                              \
> > > > > > > > > > > >       } || {                                                          \
> > > > > > > > > > >
> > > > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > > > > > > > > by Kconfig and so always all reliably set and found via a single header,
> > > > > > > > > > > CFG_ stuff is not.
> > > > > > > > > >
> > > > > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > > > > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > > > > > > > > it out.
> > > > > > > > >
> > > > > > > > > It's just another define, there's no uniformity to it. For some of the
> > > > > > > > > SDRAM values really we need some build time way to grab some information
> > > > > > > > > out of the default device tree.
> > > > > > > >
> > > > > > > > Can you give an example of a board that could use this? I looked at
> > > > > > > > the devicetree for chromebook_kevin and don't see a memory range in
> > > > > > > > ther.
> > > > > > >
> > > > > > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't
> > > > > > > see it in the dtb file. That removes that option then, sadly.
> > > > > >
> > > > > > Well, we can still require it, so long as an error is produced if the
> > > > > > property is needed but does not exist.
> > > > >
> > > > > "We" who? I don't feel like we'll have a lot of traction with linux
> > > > > kernel folks in requiring /memory to be added to the dts files on
> > > > > however many platforms don't have it today because I'm going to guess
> > > > > it's added at run time, possibly by us, with the correct size and we'd
> > > > > be asking for statically adding things half-wrong like a lot of
> > > > > platforms used to do (and in turn rely on U-Boot to correct the size).
> > > >
> > > > Hmm yes of course, the firmware is supposed to add these
> > > > properties...that's how it gets in there. So we need to stick with CFG
> > > > (and perhaps the RAM-size prober) for now.
> > >
> > > Coming back to this patch, can we apply it? It provides a way to find
> > > out the value of these CFG options, which otherwise involves chasing
> > > around header files.
> >
> > No, because it implies that there's a consistent way to know what a
> > given CFG value will be when there is not. There is no equivalent header
> > to include like for CONFIG symbols to know that you got them. You're
> > likely better off trying out "ripgrep" which I have found to be much
> > faster than "git grep".
> 
> Hmm it is really hard to know which files to grep!

It's super easy with ripgrep:
$ rg -g *.h CFG_SYS_SDRAM_SIZE

> Could we require that config.h includes all the CFG values? I'm just
> trying to find a way to bring a bit more order to this area.

Well, some of them could perhaps be Kconfig symbols instead, it just got
too tedious to untangle some of them. For others (not
CFG_SYS_SDRAM_SIZE/BASE, sadly) it goes back to my idea about seeing
what can be pulled at build time from the default device tree.
Simon Glass Sept. 19, 2024, 2:13 p.m. UTC | #13
Hi Tom,

On Wed, 31 Jul 2024 at 19:17, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Jul 31, 2024 at 08:39:30AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 29 Jul 2024 at 12:17, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Fri, 28 Jun 2024 at 01:33, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 27 Jun 2024 at 15:42, Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Wed, 26 Jun 2024 at 15:07, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
> > > > > > > > > Hi Tom,
> > > > > > > > >
> > > > > > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
> > > > > > > > > > > Hi Tom,
> > > > > > > > > > >
> > > > > > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to
> > > > > > > > > > > > > the .cfg file so that we can see everything in one place. Sort the
> > > > > > > > > > > > > options so they are easier to find and compare.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > > > > ---
> > > > > > > > > > > > >
> > > > > > > > > > > > > Changes in v2:
> > > > > > > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options
> > > > > > > > > > > > >
> > > > > > > > > > > > >  scripts/Makefile.autoconf | 2 +-
> > > > > > > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > > > > >
> > > > > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
> > > > > > > > > > > > > index b42f9b525fe..65ff11ea508 100644
> > > > > > > > > > > > > --- a/scripts/Makefile.autoconf
> > > > > > > > > > > > > +++ b/scripts/Makefile.autoconf
> > > > > > > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN     $@
> > > > > > > > > > > > >  quiet_cmd_u_boot_cfg = CFG     $@
> > > > > > > > > > > > >        cmd_u_boot_cfg = \
> > > > > > > > > > > > >       $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
> > > > > > > > > > > > > -             grep 'define CONFIG_' $@.tmp | \
> > > > > > > > > > > > > +             egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
> > > > > > > > > > > > >                       sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
> > > > > > > > > > > > >               rm $@.tmp;                                              \
> > > > > > > > > > > > >       } || {                                                          \
> > > > > > > > > > > >
> > > > > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only
> > > > > > > > > > > > by Kconfig and so always all reliably set and found via a single header,
> > > > > > > > > > > > CFG_ stuff is not.
> > > > > > > > > > >
> > > > > > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the
> > > > > > > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring
> > > > > > > > > > > it out.
> > > > > > > > > >
> > > > > > > > > > It's just another define, there's no uniformity to it. For some of the
> > > > > > > > > > SDRAM values really we need some build time way to grab some information
> > > > > > > > > > out of the default device tree.
> > > > > > > > >
> > > > > > > > > Can you give an example of a board that could use this? I looked at
> > > > > > > > > the devicetree for chromebook_kevin and don't see a memory range in
> > > > > > > > > ther.
> > > > > > > >
> > > > > > > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't
> > > > > > > > see it in the dtb file. That removes that option then, sadly.
> > > > > > >
> > > > > > > Well, we can still require it, so long as an error is produced if the
> > > > > > > property is needed but does not exist.
> > > > > >
> > > > > > "We" who? I don't feel like we'll have a lot of traction with linux
> > > > > > kernel folks in requiring /memory to be added to the dts files on
> > > > > > however many platforms don't have it today because I'm going to guess
> > > > > > it's added at run time, possibly by us, with the correct size and we'd
> > > > > > be asking for statically adding things half-wrong like a lot of
> > > > > > platforms used to do (and in turn rely on U-Boot to correct the size).
> > > > >
> > > > > Hmm yes of course, the firmware is supposed to add these
> > > > > properties...that's how it gets in there. So we need to stick with CFG
> > > > > (and perhaps the RAM-size prober) for now.
> > > >
> > > > Coming back to this patch, can we apply it? It provides a way to find
> > > > out the value of these CFG options, which otherwise involves chasing
> > > > around header files.
> > >
> > > No, because it implies that there's a consistent way to know what a
> > > given CFG value will be when there is not. There is no equivalent header
> > > to include like for CONFIG symbols to know that you got them. You're
> > > likely better off trying out "ripgrep" which I have found to be much
> > > faster than "git grep".
> >
> > Hmm it is really hard to know which files to grep!
>
> It's super easy with ripgrep:
> $ rg -g *.h CFG_SYS_SDRAM_SIZE

All that does is show me lots of matches. I'm not sure which board
they relate to. Some of them are obvious, but quite a few have header
files common to many boards.

>
> > Could we require that config.h includes all the CFG values? I'm just
> > trying to find a way to bring a bit more order to this area.
>
> Well, some of them could perhaps be Kconfig symbols instead, it just got
> too tedious to untangle some of them. For others (not
> CFG_SYS_SDRAM_SIZE/BASE, sadly) it goes back to my idea about seeing
> what can be pulled at build time from the default device tree.

Hmm, yes. I am not sure how many are in that category. Linux often
seems to use tables of data in the code, since it doesn't care so much
about code size. So finding bindings for some of this data may be
tricky.

I suppose the SDRAM base/size could go in the DT if it had its own
binding, e.g. in /options/u-boot - but still many boards would want to
determine the size at runtime.

With text environment and a solution to SDRAM, perhaps we can require
that new boards not use CFG_...?

Regards,
Simon
diff mbox series

Patch

diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf
index b42f9b525fe..65ff11ea508 100644
--- a/scripts/Makefile.autoconf
+++ b/scripts/Makefile.autoconf
@@ -71,7 +71,7 @@  quiet_cmd_autoconf = GEN     $@
 quiet_cmd_u_boot_cfg = CFG     $@
       cmd_u_boot_cfg = \
 	$(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
-		grep 'define CONFIG_' $@.tmp | \
+		egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \
 			sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
 		rm $@.tmp;						\
 	} || {								\