On Thu, Sep 19, 2024 at 04:13:57PM +0200, Simon Glass wrote:
> Hi Tom,
>
> On Wed, 31 Jul 2024 at 19:17, Tom Rini <tr...@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 <tr...@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 <s...@chromium.org> wrote:
> > > > > >
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Thu, 27 Jun 2024 at 15:42, Tom Rini <tr...@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 <tr...@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 <tr...@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
> > > > > > > > > > > > <tr...@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 <s...@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_...?I don't see doing anything further to CFG_ as particularly important right now, honestly. The majority of symbols are in legacy things and then it's just a few symbols that no, really, there's not a good way to get this information other than a define. -- Tom
signature.asc
Description: PGP signature
