[RFCv1] doc: develop: Describe using CONFIG vs CFG for values

Document how and when to use CONFIG or CFG namespace for options.  There
are times where Kconfig is not a great fit for our needs, so we want to
use the CFG namespace instead.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
RFCv1:
- This is essentially my idea on how to better handle the problem of
  CONFIG values that just don't fit in Kconfig because it makes much
  more sense to define them statically for a given SoC or calculate them
  from other values, and so on.  One example here would be to revert
  c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
  re-name these to CFG_SYS_.. instead. Another big example here would be
  a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
  all tool-generated. Not all CONFIG_SYS_ options would get this as
  boolean choices are well handled in Kconfig, and that may not be clear
  enough in what I wrote here?
---
 doc/develop/build_configuration.rst | 36 +++++++++++++++++++++++++++++
 doc/develop/index.rst               |  1 +
 2 files changed, 37 insertions(+)
 create mode 100644 doc/develop/build_configuration.rst

On Thursday 28 July 2022 09:20:46 Tom Rini wrote:
> Document how and when to use CONFIG or CFG namespace for options.  There
> are times where Kconfig is not a great fit for our needs, so we want to
> use the CFG namespace instead.
> 
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> RFCv1:
> - This is essentially my idea on how to better handle the problem of
>   CONFIG values that just don't fit in Kconfig because it makes much
>   more sense to define them statically for a given SoC or calculate them
>   from other values, and so on.  One example here would be to revert
>   c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
>   re-name these to CFG_SYS_.. instead. Another big example here would be
>   a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
>   all tool-generated. Not all CONFIG_SYS_ options would get this as
>   boolean choices are well handled in Kconfig, and that may not be clear
>   enough in what I wrote here?

In README is also written:

  * Configuration _OPTIONS_:
    These are selectable by the user and have names beginning with
      "CONFIG_".

  * Configuration _SETTINGS_:
    These depend on the hardware etc. and should not be meddled with if
      you don't know what you're doing; they have names beginning with
        "CONFIG_SYS_".

So based on this statement, proposed CFG_* options should be of second
category - not selectable by user - right?

And this reminds me, that CONFIG_SYS_* option in Kconfig should also not
be possible to select / change by user or in user's defconfig. This can
be achieved in Kconfig by _not_ specifying short description (after
"bool" or "hex" keyword). So maybe this should be also fixed? Or at
least do not introduce new Kconfig options with SYS_* which can be
changed?

> ---
>  doc/develop/build_configuration.rst | 36 +++++++++++++++++++++++++++++
>  doc/develop/index.rst               |  1 +
>  2 files changed, 37 insertions(+)
>  create mode 100644 doc/develop/build_configuration.rst
> 
> diff --git a/doc/develop/build_configuration.rst b/doc/develop/build_configuration.rst
> new file mode 100644
> index 000000000000..541ce528b36a
> --- /dev/null
> +++ b/doc/develop/build_configuration.rst
> @@ -0,0 +1,36 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +Using CONFIG or CFG to configure the build
> +==========================================
> +
> +There are two namespaces used to control the build-time configuration of
> +U-Boot, ``CONFIG`` and ``CFG``. These are used when it is either not possible
> +or not practical to make a run-time determination about some functionality of
> +the hardware or a required software feature or similar. Each of these has their
> +own places where they are better suited than the other for use.
> +
> +The first of these, ``CONFIG``` is managed in the `Kconfig language
> +<https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html>`_ that is
> +most commonly associated with the Linux kernel and the Kconfig files found
> +throughout our sources. Adding options to this namespace is the preferred way of
> +exposing new configuration options as there are a number of ways for both users
> +and system integrators to manage and change these options. Some common examples
> +here are to enable a specific command within U-Boot or even a whole subsystem
> +such as NAND flash or network connectivity.
> +
> +The second of these, ``CFG`` is implemented directly as C preprocessor values or
> +macros, depending on what they are in turn describing. The nature of U-Boot
> +means that while we have some functionality that is very reasonable to expose to
> +the end user to enable or disable we have other places where we need to describe
> +things such as register locations or values, memory map ranges and so on. When
> +practical, we should be getting these values from the :doc:`device tree
> +<devicetree/control>`. However, there are cases where this is either not
> +practical due to when we need the information and may not have a device tree yet
> +or due to legacy reasons code has not been rewritten. As this information tends
> +to be specific to a given SoC (or family of SoCs) or if board specific something
> +that is mandated by the physical design rather than truly user configurable, it
> +is acceptable to ``#define`` these values in the ``CFG`` namespace. It is
> +strongly encouraged to place these in the architecture header files, if they are
> +generic to a given SoC, or under the board directory if board specific. Placing
> +them under the board.h file in the *include/configs/* directory should be seen
> +as a last resort.
> diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> index 73741ceb6a2f..c8ec7c210d35 100644
> --- a/doc/develop/index.rst
> +++ b/doc/develop/index.rst
> @@ -9,6 +9,7 @@ General
>  .. toctree::
>     :maxdepth: 1
>  
> +   build_configuration
>     codingstyle
>     designprinciples
>     process
> -- 
> 2.25.1
> 

This change looks good. But I'm not sure how to correctly do it. It
would be nice to see example, e.g. how to handle it for revert of
c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig").
Those are board specific options which are exported and required by both
u-boot drivers and u-boot arch/cpu code.

On Thu, Jul 28, 2022 at 03:34:43PM +0200, Pali Rohár wrote:
> On Thursday 28 July 2022 09:20:46 Tom Rini wrote:
> > Document how and when to use CONFIG or CFG namespace for options.  There
> > are times where Kconfig is not a great fit for our needs, so we want to
> > use the CFG namespace instead.
> > 
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> > RFCv1:
> > - This is essentially my idea on how to better handle the problem of
> >   CONFIG values that just don't fit in Kconfig because it makes much
> >   more sense to define them statically for a given SoC or calculate them
> >   from other values, and so on.  One example here would be to revert
> >   c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
> >   re-name these to CFG_SYS_.. instead. Another big example here would be
> >   a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
> >   all tool-generated. Not all CONFIG_SYS_ options would get this as
> >   boolean choices are well handled in Kconfig, and that may not be clear
> >   enough in what I wrote here?
> 
> In README is also written:

Ah yes, I forgot to see if the README had anything here.  I'll remove
and incorporate in RFCv2.

>   * Configuration _OPTIONS_:
>     These are selectable by the user and have names beginning with
>       "CONFIG_".
> 
>   * Configuration _SETTINGS_:
>     These depend on the hardware etc. and should not be meddled with if
>       you don't know what you're doing; they have names beginning with
>         "CONFIG_SYS_".
> 
> So based on this statement, proposed CFG_* options should be of second
> category - not selectable by user - right?

Right.

> And this reminds me, that CONFIG_SYS_* option in Kconfig should also not
> be possible to select / change by user or in user's defconfig. This can
> be achieved in Kconfig by _not_ specifying short description (after
> "bool" or "hex" keyword). So maybe this should be also fixed? Or at
> least do not introduce new Kconfig options with SYS_* which can be
> changed?

There is a lot of possible follow-up cleanup work to do, yes.  I gave
one example of an unmigration that should be done, but there's certainly
more.  I'll try and make it clearer in RFCv2 about when it is
appropriate to add CONFIG settings but that aren't configurable.

[snip]
> This change looks good. But I'm not sure how to correctly do it. It
> would be nice to see example, e.g. how to handle it for revert of
> c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig").
> Those are board specific options which are exported and required by both
> u-boot drivers and u-boot arch/cpu code.

I'll probably just start with a revert + sed, and leave further clean-up
to whomever has more time and interest on the legacy platforms, for that
specific example.  For all of the SYS_INIT_SP_ADDR as another example,
I'll probably go with renaming to CFG_SYS_INIT_SP_ADDR and
CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR being select'd and
CONFIG_CUSTOM_SYS_INIT_SP_ADDR becoming CFG_SYS_INIT_SP_ADDR directly,
and in board.h until someone has the time and desire to further rectify
something that really shouldn't be board, but rather SoC (and adjusted
based on further options) specific.

Am 28. Juli 2022 15:20:46 MESZ schrieb Tom Rini <trini@konsulko.com>:
>Document how and when to use CONFIG or CFG namespace for options.  There
>are times where Kconfig is not a great fit for our needs, so we want to
>use the CFG namespace instead.
>
>Signed-off-by: Tom Rini <trini@konsulko.com>
>---
>RFCv1:
>- This is essentially my idea on how to better handle the problem of
>  CONFIG values that just don't fit in Kconfig because it makes much
>  more sense to define them statically for a given SoC or calculate them
>  from other values, and so on.  One example here would be to revert
>  c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
>  re-name these to CFG_SYS_.. instead. Another big example here would be
>  a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
>  all tool-generated. Not all CONFIG_SYS_ options would get this as
>  boolean choices are well handled in Kconfig, and that may not be clear
>  enough in what I wrote here?
>---
> doc/develop/build_configuration.rst | 36 +++++++++++++++++++++++++++++
> doc/develop/index.rst               |  1 +
> 2 files changed, 37 insertions(+)
> create mode 100644 doc/develop/build_configuration.rst
>
>diff --git a/doc/develop/build_configuration.rst b/doc/develop/build_configuration.rst
>new file mode 100644
>index 000000000000..541ce528b36a
>--- /dev/null
>+++ b/doc/develop/build_configuration.rst
>@@ -0,0 +1,36 @@
>+.. SPDX-License-Identifier: GPL-2.0+
>+
>+Using CONFIG or CFG to configure the build
>+==========================================
>+
>+There are two namespaces used to control the build-time configuration of
>+U-Boot, ``CONFIG`` and ``CFG``. These are used when it is either not possible
>+or not practical to make a run-time determination about some functionality of
>+the hardware or a required software feature or similar. Each of these has their
>+own places where they are better suited than the other for use.
>+
>+The first of these, ``CONFIG``` is managed in the `Kconfig language
>+<https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html>`_ that is
>+most commonly associated with the Linux kernel and the Kconfig files found
>+throughout our sources. Adding options to this namespace is the preferred way of
>+exposing new configuration options as there are a number of ways for both users
>+and system integrators to manage and change these options. Some common examples
>+here are to enable a specific command within U-Boot or even a whole subsystem
>+such as NAND flash or network connectivity.
>+
>+The second of these, ``CFG`` is implemented directly as C preprocessor values or
>+macros, depending on what they are in turn describing. The nature of U-Boot
>+means that while we have some functionality that is very reasonable to expose to
>+the end user to enable or disable we have other places where we need to describe
>+things such as register locations or values, memory map ranges and so on. When
>+practical, we should be getting these values from the :doc:`device tree
>+<devicetree/control>`. However, there are cases where this is either not
>+practical due to when we need the information and may not have a device tree yet
>+or due to legacy reasons code has not been rewritten. As this information tends
>+to be specific to a given SoC (or family of SoCs) or if board specific something
>+that is mandated by the physical design rather than truly user configurable, it
>+is acceptable to ``#define`` these values in the ``CFG`` namespace. It is
>+strongly encouraged to place these in the architecture header files, if they are
>+generic to a given SoC, or under the board directory if board specific. Placing
>+them under the board.h file in the *include/configs/* directory should be seen
>+as a last resort.

I would prefer to have numbered list of priorities:

Get property
1) by runtime probing if the U-Boot binary supports multiple boards
2) from Kconfig CONFIG value
3) from DT at runtime
4) from SoC specific CFG constant value
5) from board specific CFG constant value

I am not sure about the sequence of 2 and 3. We don't want to replace defconfig by DT. But we wouldn't want register addresses in Kconfig either. So this needs mire elaboration.

Best regards

Heinrich 

>diff --git a/doc/develop/index.rst b/doc/develop/index.rst
>index 73741ceb6a2f..c8ec7c210d35 100644
>--- a/doc/develop/index.rst
>+++ b/doc/develop/index.rst
>@@ -9,6 +9,7 @@ General
> .. toctree::
>    :maxdepth: 1
> 
>+   build_configuration
>    codingstyle
>    designprinciples
>    process

On Thu, Jul 28, 2022 at 06:44:56PM +0200, Heinrich Schuchardt wrote:
> 
> 
> Am 28. Juli 2022 15:20:46 MESZ schrieb Tom Rini <trini@konsulko.com>:
> >Document how and when to use CONFIG or CFG namespace for options.  There
> >are times where Kconfig is not a great fit for our needs, so we want to
> >use the CFG namespace instead.
> >
> >Signed-off-by: Tom Rini <trini@konsulko.com>
> >---
> >RFCv1:
> >- This is essentially my idea on how to better handle the problem of
> >  CONFIG values that just don't fit in Kconfig because it makes much
> >  more sense to define them statically for a given SoC or calculate them
> >  from other values, and so on.  One example here would be to revert
> >  c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
> >  re-name these to CFG_SYS_.. instead. Another big example here would be
> >  a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
> >  all tool-generated. Not all CONFIG_SYS_ options would get this as
> >  boolean choices are well handled in Kconfig, and that may not be clear
> >  enough in what I wrote here?
> >---
> > doc/develop/build_configuration.rst | 36 +++++++++++++++++++++++++++++
> > doc/develop/index.rst               |  1 +
> > 2 files changed, 37 insertions(+)
> > create mode 100644 doc/develop/build_configuration.rst
> >
> >diff --git a/doc/develop/build_configuration.rst b/doc/develop/build_configuration.rst
> >new file mode 100644
> >index 000000000000..541ce528b36a
> >--- /dev/null
> >+++ b/doc/develop/build_configuration.rst
> >@@ -0,0 +1,36 @@
> >+.. SPDX-License-Identifier: GPL-2.0+
> >+
> >+Using CONFIG or CFG to configure the build
> >+==========================================
> >+
> >+There are two namespaces used to control the build-time configuration of
> >+U-Boot, ``CONFIG`` and ``CFG``. These are used when it is either not possible
> >+or not practical to make a run-time determination about some functionality of
> >+the hardware or a required software feature or similar. Each of these has their
> >+own places where they are better suited than the other for use.
> >+
> >+The first of these, ``CONFIG``` is managed in the `Kconfig language
> >+<https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html>`_ that is
> >+most commonly associated with the Linux kernel and the Kconfig files found
> >+throughout our sources. Adding options to this namespace is the preferred way of
> >+exposing new configuration options as there are a number of ways for both users
> >+and system integrators to manage and change these options. Some common examples
> >+here are to enable a specific command within U-Boot or even a whole subsystem
> >+such as NAND flash or network connectivity.
> >+
> >+The second of these, ``CFG`` is implemented directly as C preprocessor values or
> >+macros, depending on what they are in turn describing. The nature of U-Boot
> >+means that while we have some functionality that is very reasonable to expose to
> >+the end user to enable or disable we have other places where we need to describe
> >+things such as register locations or values, memory map ranges and so on. When
> >+practical, we should be getting these values from the :doc:`device tree
> >+<devicetree/control>`. However, there are cases where this is either not
> >+practical due to when we need the information and may not have a device tree yet
> >+or due to legacy reasons code has not been rewritten. As this information tends
> >+to be specific to a given SoC (or family of SoCs) or if board specific something
> >+that is mandated by the physical design rather than truly user configurable, it
> >+is acceptable to ``#define`` these values in the ``CFG`` namespace. It is
> >+strongly encouraged to place these in the architecture header files, if they are
> >+generic to a given SoC, or under the board directory if board specific. Placing
> >+them under the board.h file in the *include/configs/* directory should be seen
> >+as a last resort.
> 
> I would prefer to have numbered list of priorities:
> 
> Get property
> 1) by runtime probing if the U-Boot binary supports multiple boards
> 2) from Kconfig CONFIG value
> 3) from DT at runtime
> 4) from SoC specific CFG constant value
> 5) from board specific CFG constant value
> 
> I am not sure about the sequence of 2 and 3. We don't want to replace defconfig by DT. But we wouldn't want register addresses in Kconfig either. So this needs mire elaboration.

I've rewritten things a bit so far to try and be clearer on CONFIG vs
CFG.  Maybe I should rename it to system_configuration.rst and try and
talk more about when to get something at run-time?

Am 28. Juli 2022 19:53:34 MESZ schrieb Tom Rini <trini@konsulko.com>:
>On Thu, Jul 28, 2022 at 06:44:56PM +0200, Heinrich Schuchardt wrote:
>> 
>> 
>> Am 28. Juli 2022 15:20:46 MESZ schrieb Tom Rini <trini@konsulko.com>:
>> >Document how and when to use CONFIG or CFG namespace for options.  There
>> >are times where Kconfig is not a great fit for our needs, so we want to
>> >use the CFG namespace instead.
>> >
>> >Signed-off-by: Tom Rini <trini@konsulko.com>
>> >---
>> >RFCv1:
>> >- This is essentially my idea on how to better handle the problem of
>> >  CONFIG values that just don't fit in Kconfig because it makes much
>> >  more sense to define them statically for a given SoC or calculate them
>> >  from other values, and so on.  One example here would be to revert
>> >  c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
>> >  re-name these to CFG_SYS_.. instead. Another big example here would be
>> >  a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
>> >  all tool-generated. Not all CONFIG_SYS_ options would get this as
>> >  boolean choices are well handled in Kconfig, and that may not be clear
>> >  enough in what I wrote here?
>> >---
>> > doc/develop/build_configuration.rst | 36 +++++++++++++++++++++++++++++
>> > doc/develop/index.rst               |  1 +
>> > 2 files changed, 37 insertions(+)
>> > create mode 100644 doc/develop/build_configuration.rst
>> >
>> >diff --git a/doc/develop/build_configuration.rst b/doc/develop/build_configuration.rst
>> >new file mode 100644
>> >index 000000000000..541ce528b36a
>> >--- /dev/null
>> >+++ b/doc/develop/build_configuration.rst
>> >@@ -0,0 +1,36 @@
>> >+.. SPDX-License-Identifier: GPL-2.0+
>> >+
>> >+Using CONFIG or CFG to configure the build
>> >+==========================================
>> >+
>> >+There are two namespaces used to control the build-time configuration of
>> >+U-Boot, ``CONFIG`` and ``CFG``. These are used when it is either not possible
>> >+or not practical to make a run-time determination about some functionality of
>> >+the hardware or a required software feature or similar. Each of these has their
>> >+own places where they are better suited than the other for use.
>> >+
>> >+The first of these, ``CONFIG``` is managed in the `Kconfig language
>> >+<https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html>`_ that is
>> >+most commonly associated with the Linux kernel and the Kconfig files found
>> >+throughout our sources. Adding options to this namespace is the preferred way of
>> >+exposing new configuration options as there are a number of ways for both users
>> >+and system integrators to manage and change these options. Some common examples
>> >+here are to enable a specific command within U-Boot or even a whole subsystem
>> >+such as NAND flash or network connectivity.
>> >+
>> >+The second of these, ``CFG`` is implemented directly as C preprocessor values or
>> >+macros, depending on what they are in turn describing. The nature of U-Boot
>> >+means that while we have some functionality that is very reasonable to expose to
>> >+the end user to enable or disable we have other places where we need to describe
>> >+things such as register locations or values, memory map ranges and so on. When
>> >+practical, we should be getting these values from the :doc:`device tree
>> >+<devicetree/control>`. However, there are cases where this is either not
>> >+practical due to when we need the information and may not have a device tree yet
>> >+or due to legacy reasons code has not been rewritten. As this information tends
>> >+to be specific to a given SoC (or family of SoCs) or if board specific something
>> >+that is mandated by the physical design rather than truly user configurable, it
>> >+is acceptable to ``#define`` these values in the ``CFG`` namespace. It is
>> >+strongly encouraged to place these in the architecture header files, if they are
>> >+generic to a given SoC, or under the board directory if board specific. Placing
>> >+them under the board.h file in the *include/configs/* directory should be seen
>> >+as a last resort.
>> 
>> I would prefer to have numbered list of priorities:
>> 
>> Get property
>> 1) by runtime probing if the U-Boot binary supports multiple boards
>> 2) from Kconfig CONFIG value
>> 3) from DT at runtime
>> 4) from SoC specific CFG constant value
>> 5) from board specific CFG constant value
>> 
>> I am not sure about the sequence of 2 and 3. We don't want to replace defconfig by DT. But we wouldn't want register addresses in Kconfig either. So this needs mire elaboration.
>
>I've rewritten things a bit so far to try and be clearer on CONFIG vs
>CFG.  Maybe I should rename it to system_configuration.rst and try and
>talk more about when to get something at run-time?

Such a broadened scope would be helpful for future developers.

Best regards

Heinrich

Hi Tom,

On Thu, 28 Jul 2022 at 07:20, Tom Rini <trini@konsulko.com> wrote:
>
> Document how and when to use CONFIG or CFG namespace for options.  There
> are times where Kconfig is not a great fit for our needs, so we want to
> use the CFG namespace instead.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> RFCv1:
> - This is essentially my idea on how to better handle the problem of
>   CONFIG values that just don't fit in Kconfig because it makes much
>   more sense to define them statically for a given SoC or calculate them
>   from other values, and so on.  One example here would be to revert
>   c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
>   re-name these to CFG_SYS_.. instead. Another big example here would be
>   a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
>   all tool-generated. Not all CONFIG_SYS_ options would get this as
>   boolean choices are well handled in Kconfig, and that may not be clear
>   enough in what I wrote here?
> ---
>  doc/develop/build_configuration.rst | 36 +++++++++++++++++++++++++++++
>  doc/develop/index.rst               |  1 +
>  2 files changed, 37 insertions(+)
>  create mode 100644 doc/develop/build_configuration.rst

I worry that CFG is confusing as it is too similar to CONFIG. Could we
have an entirely different prefix, e.g. SYS_? Also, why is a prefix
needed?

I have certainly seen things that are named CONFIG_... but are really
just SoC values. If they are addresses they should be in the
devicetree, but perhaps for early code in SPL that is not practical.

Re the revert, why? Are you worried that we are bloating Kconfig too
much with all these internal SoC settings?

What is the policy on using such things for a board? I would very much
like to avoid board-specific #defines like this.

Is your hope to drop all remaining ad-hoc CONFIGs in this way?

Regards,
Simon

On Sat, Jul 30, 2022 at 07:27:31PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 28 Jul 2022 at 07:20, Tom Rini <trini@konsulko.com> wrote:
> >
> > Document how and when to use CONFIG or CFG namespace for options.  There
> > are times where Kconfig is not a great fit for our needs, so we want to
> > use the CFG namespace instead.
> >
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> > RFCv1:
> > - This is essentially my idea on how to better handle the problem of
> >   CONFIG values that just don't fit in Kconfig because it makes much
> >   more sense to define them statically for a given SoC or calculate them
> >   from other values, and so on.  One example here would be to revert
> >   c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
> >   re-name these to CFG_SYS_.. instead. Another big example here would be
> >   a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
> >   all tool-generated. Not all CONFIG_SYS_ options would get this as
> >   boolean choices are well handled in Kconfig, and that may not be clear
> >   enough in what I wrote here?
> > ---
> >  doc/develop/build_configuration.rst | 36 +++++++++++++++++++++++++++++
> >  doc/develop/index.rst               |  1 +
> >  2 files changed, 37 insertions(+)
> >  create mode 100644 doc/develop/build_configuration.rst
> 
> I worry that CFG is confusing as it is too similar to CONFIG. Could we
> have an entirely different prefix, e.g. SYS_? Also, why is a prefix
> needed?

We need a prefix here for consistency.  I don't want to use just SYS_
here as that's not always used for some of the values to be converted.

> I have certainly seen things that are named CONFIG_... but are really
> just SoC values. If they are addresses they should be in the
> devicetree, but perhaps for early code in SPL that is not practical.

I tried to elaborate on this more in the later revs of the document.
But yes, we have some legacy code that's just not going to get a massive
rewrite to use device tree more, but also has an active enough user base
that we aren't just going to drop it.  So long as it's not going to make
life harder in the rest of the code base, this feels like a good enough
compromise.

> Re the revert, why? Are you worried that we are bloating Kconfig too
> much with all these internal SoC settings?

As Pali explained in other threads, there was a problem with the values
in that commit, prior to conversion.  But turning the series of ORs and
shifts into a single magic value made it much harder to see and address.
Yes, if it was written to pull from the device tree, it would have been
easier to read and fix there.  But no one is going to rewrite all of the
core PowerPC code.

> What is the policy on using such things for a board? I would very much
> like to avoid board-specific #defines like this.

The policy is better laid out in later revs of the document I think.

> Is your hope to drop all remaining ad-hoc CONFIGs in this way?

A large chunk, yes.  I'm auditing CONFIG_SYS_* still, but there's
probably going a few hundred symbols that just get resolved this way.

Hi Tom,

On Sun, 31 Jul 2022 at 05:46, Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Jul 30, 2022 at 07:27:31PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 28 Jul 2022 at 07:20, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > Document how and when to use CONFIG or CFG namespace for options.  There
> > > are times where Kconfig is not a great fit for our needs, so we want to
> > > use the CFG namespace instead.
> > >
> > > Signed-off-by: Tom Rini <trini@konsulko.com>
> > > ---
> > > RFCv1:
> > > - This is essentially my idea on how to better handle the problem of
> > >   CONFIG values that just don't fit in Kconfig because it makes much
> > >   more sense to define them statically for a given SoC or calculate them
> > >   from other values, and so on.  One example here would be to revert
> > >   c7fad78ec0ee ("Convert CONFIG_SYS_BR0_PRELIM et al to Kconfig") and
> > >   re-name these to CFG_SYS_.. instead. Another big example here would be
> > >   a global search-and-replace of 's/CONFIG_HPS_/CFG_HPS_/g' as that's
> > >   all tool-generated. Not all CONFIG_SYS_ options would get this as
> > >   boolean choices are well handled in Kconfig, and that may not be clear
> > >   enough in what I wrote here?
> > > ---
> > >  doc/develop/build_configuration.rst | 36 +++++++++++++++++++++++++++++
> > >  doc/develop/index.rst               |  1 +
> > >  2 files changed, 37 insertions(+)
> > >  create mode 100644 doc/develop/build_configuration.rst
> >
> > I worry that CFG is confusing as it is too similar to CONFIG. Could we
> > have an entirely different prefix, e.g. SYS_? Also, why is a prefix
> > needed?
>
> We need a prefix here for consistency.  I don't want to use just SYS_
> here as that's not always used for some of the values to be converted.

We could add it. I feel that people new to the code base will find it
confusing that we have CONFIG and CFG.

>
> > I have certainly seen things that are named CONFIG_... but are really
> > just SoC values. If they are addresses they should be in the
> > devicetree, but perhaps for early code in SPL that is not practical.
>
> I tried to elaborate on this more in the later revs of the document.
> But yes, we have some legacy code that's just not going to get a massive
> rewrite to use device tree more, but also has an active enough user base
> that we aren't just going to drop it.  So long as it's not going to make
> life harder in the rest of the code base, this feels like a good enough
> compromise.

OK I see.

>
> > Re the revert, why? Are you worried that we are bloating Kconfig too
> > much with all these internal SoC settings?
>
> As Pali explained in other threads, there was a problem with the values
> in that commit, prior to conversion.  But turning the series of ORs and
> shifts into a single magic value made it much harder to see and address.
> Yes, if it was written to pull from the device tree, it would have been
> easier to read and fix there.  But no one is going to rewrite all of the
> core PowerPC code.

OK.

>
> > What is the policy on using such things for a board? I would very much
> > like to avoid board-specific #defines like this.
>
> The policy is better laid out in later revs of the document I think.
>
> > Is your hope to drop all remaining ad-hoc CONFIGs in this way?
>
> A large chunk, yes.  I'm auditing CONFIG_SYS_* still, but there's
> probably going a few hundred symbols that just get resolved this way.

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

Regards,
Simon