| Message ID | 20230829221457.101469-3-afd@ti.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Allow defconfigs defined from fragments | expand |
On 17:14-20230829, Andrew Davis wrote: > Add am62x_beagleplay_r5_defconfig for R5 SPL and > am62x_beagleplay_a53_defconfig for A53 SPL and U-Boot support. > > These defconfigs are composite defconfigs built from the config fragment > board/ti/am62x/beagleplay_*.config applied onto the base > am62x_evm_*_defconfig. > > Signed-off-by: Andrew Davis <afd@ti.com> > --- > configs/am62x_beagleplay_a53_defconfig | 3 +++ > configs/am62x_beagleplay_r5_defconfig | 3 +++ > 2 files changed, 6 insertions(+) > create mode 100644 configs/am62x_beagleplay_a53_defconfig > create mode 100644 configs/am62x_beagleplay_r5_defconfig > > diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig > new file mode 100644 > index 00000000000..ad708e15397 > --- /dev/null > +++ b/configs/am62x_beagleplay_a53_defconfig > @@ -0,0 +1,3 @@ > +// The BeaglePlay defconfig for A53 core > +#include "configs/am62x_evm_a53_defconfig" > +#include "board/ti/am62x/beagleplay_a53.config" > diff --git a/configs/am62x_beagleplay_r5_defconfig b/configs/am62x_beagleplay_r5_defconfig > new file mode 100644 > index 00000000000..276b1f81a3e > --- /dev/null > +++ b/configs/am62x_beagleplay_r5_defconfig > @@ -0,0 +1,3 @@ > +// The BeaglePlay defconfig for R5 core > +#include "configs/am62x_evm_r5_defconfig" > +#include "board/ti/am62x/beagleplay_r5.config" > -- > 2.39.2 > my only complaint is that if we add lets say board/ti/am62x/dfu.config, Then: R5: 1. am62x_evm_r5_defconfig = am62x_evm_r5_defconfig 2. am62x_beagleplay_r5_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config 3. am62x_evm_r5_dfu_defconfig = am62x_evm_r5_defconfig + dfu.config 4. am62x_beagleplay_r5_dfu_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config + dfu.config This information can be in a single txt file Rather than have a defconfig file for each combination.
On Wed, Aug 30, 2023 at 07:31:51AM -0500, Nishanth Menon wrote: > On 17:14-20230829, Andrew Davis wrote: > > Add am62x_beagleplay_r5_defconfig for R5 SPL and > > am62x_beagleplay_a53_defconfig for A53 SPL and U-Boot support. > > > > These defconfigs are composite defconfigs built from the config fragment > > board/ti/am62x/beagleplay_*.config applied onto the base > > am62x_evm_*_defconfig. > > > > Signed-off-by: Andrew Davis <afd@ti.com> > > --- > > configs/am62x_beagleplay_a53_defconfig | 3 +++ > > configs/am62x_beagleplay_r5_defconfig | 3 +++ > > 2 files changed, 6 insertions(+) > > create mode 100644 configs/am62x_beagleplay_a53_defconfig > > create mode 100644 configs/am62x_beagleplay_r5_defconfig > > > > diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig > > new file mode 100644 > > index 00000000000..ad708e15397 > > --- /dev/null > > +++ b/configs/am62x_beagleplay_a53_defconfig > > @@ -0,0 +1,3 @@ > > +// The BeaglePlay defconfig for A53 core > > +#include "configs/am62x_evm_a53_defconfig" > > +#include "board/ti/am62x/beagleplay_a53.config" > > diff --git a/configs/am62x_beagleplay_r5_defconfig b/configs/am62x_beagleplay_r5_defconfig > > new file mode 100644 > > index 00000000000..276b1f81a3e > > --- /dev/null > > +++ b/configs/am62x_beagleplay_r5_defconfig > > @@ -0,0 +1,3 @@ > > +// The BeaglePlay defconfig for R5 core > > +#include "configs/am62x_evm_r5_defconfig" > > +#include "board/ti/am62x/beagleplay_r5.config" > > -- > > 2.39.2 > > > > my only complaint is that if we add lets say > board/ti/am62x/dfu.config, Then: > > R5: > 1. am62x_evm_r5_defconfig = am62x_evm_r5_defconfig > 2. am62x_beagleplay_r5_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config > 3. am62x_evm_r5_dfu_defconfig = am62x_evm_r5_defconfig + dfu.config > 4. am62x_beagleplay_r5_dfu_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config + dfu.config > > This information can be in a single txt file Rather than have a > defconfig file for each combination. I know Andrew is trying to do what Simon's asked. But yes, this is why I don't think the right approach is to have some file that says "here are all of the valid combinations". We aren't going to be breaking "make am62x_beagleplay_r5_dfu_defconfig local-fragment.config" in the above example. And we don't need every possible combination in CI, we just need to make sure unique paths are tested. So long as something builds all of the dts files, the Tegra example in-tree now is fine because we're checking all of those. The beagle examples here too should be fine.
On 8/30/23 7:31 AM, Nishanth Menon wrote: > On 17:14-20230829, Andrew Davis wrote: >> Add am62x_beagleplay_r5_defconfig for R5 SPL and >> am62x_beagleplay_a53_defconfig for A53 SPL and U-Boot support. >> >> These defconfigs are composite defconfigs built from the config fragment >> board/ti/am62x/beagleplay_*.config applied onto the base >> am62x_evm_*_defconfig. >> >> Signed-off-by: Andrew Davis <afd@ti.com> >> --- >> configs/am62x_beagleplay_a53_defconfig | 3 +++ >> configs/am62x_beagleplay_r5_defconfig | 3 +++ >> 2 files changed, 6 insertions(+) >> create mode 100644 configs/am62x_beagleplay_a53_defconfig >> create mode 100644 configs/am62x_beagleplay_r5_defconfig >> >> diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig >> new file mode 100644 >> index 00000000000..ad708e15397 >> --- /dev/null >> +++ b/configs/am62x_beagleplay_a53_defconfig >> @@ -0,0 +1,3 @@ >> +// The BeaglePlay defconfig for A53 core >> +#include "configs/am62x_evm_a53_defconfig" >> +#include "board/ti/am62x/beagleplay_a53.config" >> diff --git a/configs/am62x_beagleplay_r5_defconfig b/configs/am62x_beagleplay_r5_defconfig >> new file mode 100644 >> index 00000000000..276b1f81a3e >> --- /dev/null >> +++ b/configs/am62x_beagleplay_r5_defconfig >> @@ -0,0 +1,3 @@ >> +// The BeaglePlay defconfig for R5 core >> +#include "configs/am62x_evm_r5_defconfig" >> +#include "board/ti/am62x/beagleplay_r5.config" >> -- >> 2.39.2 >> > > my only complaint is that if we add lets say > board/ti/am62x/dfu.config, Then: > > R5: > 1. am62x_evm_r5_defconfig = am62x_evm_r5_defconfig > 2. am62x_beagleplay_r5_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config > 3. am62x_evm_r5_dfu_defconfig = am62x_evm_r5_defconfig + dfu.config > 4. am62x_beagleplay_r5_dfu_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config + dfu.config > > This information can be in a single txt file Rather than have a > defconfig file for each combination. > Having every combination in a text file vs in a directory of files doesn't seem like much difference to me. `cat combinations.txt` vs `ls -l configs/`. But using a file would mean extra tooling and non-standard usage. Let's simply try to avoid these combinatorial problems by avoiding adding too many fragments that apply broadly. That adds testing burden. When features need added/removed, folks can use menuconfig or similar. We shouldn't need a defconfig fragment for DFU.. Andrew
On 09:31-20230830, Andrew Davis wrote: > On 8/30/23 7:31 AM, Nishanth Menon wrote: > > On 17:14-20230829, Andrew Davis wrote: > > > Add am62x_beagleplay_r5_defconfig for R5 SPL and > > > am62x_beagleplay_a53_defconfig for A53 SPL and U-Boot support. > > > > > > These defconfigs are composite defconfigs built from the config fragment > > > board/ti/am62x/beagleplay_*.config applied onto the base > > > am62x_evm_*_defconfig. > > > > > > Signed-off-by: Andrew Davis <afd@ti.com> > > > --- > > > configs/am62x_beagleplay_a53_defconfig | 3 +++ > > > configs/am62x_beagleplay_r5_defconfig | 3 +++ > > > 2 files changed, 6 insertions(+) > > > create mode 100644 configs/am62x_beagleplay_a53_defconfig > > > create mode 100644 configs/am62x_beagleplay_r5_defconfig > > > > > > diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig > > > new file mode 100644 > > > index 00000000000..ad708e15397 > > > --- /dev/null > > > +++ b/configs/am62x_beagleplay_a53_defconfig > > > @@ -0,0 +1,3 @@ > > > +// The BeaglePlay defconfig for A53 core > > > +#include "configs/am62x_evm_a53_defconfig" > > > +#include "board/ti/am62x/beagleplay_a53.config" > > > diff --git a/configs/am62x_beagleplay_r5_defconfig b/configs/am62x_beagleplay_r5_defconfig > > > new file mode 100644 > > > index 00000000000..276b1f81a3e > > > --- /dev/null > > > +++ b/configs/am62x_beagleplay_r5_defconfig > > > @@ -0,0 +1,3 @@ > > > +// The BeaglePlay defconfig for R5 core > > > +#include "configs/am62x_evm_r5_defconfig" > > > +#include "board/ti/am62x/beagleplay_r5.config" > > > -- > > > 2.39.2 > > > > > > > my only complaint is that if we add lets say > > board/ti/am62x/dfu.config, Then: > > > > R5: > > 1. am62x_evm_r5_defconfig = am62x_evm_r5_defconfig > > 2. am62x_beagleplay_r5_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config > > 3. am62x_evm_r5_dfu_defconfig = am62x_evm_r5_defconfig + dfu.config > > 4. am62x_beagleplay_r5_dfu_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config + dfu.config > > > > This information can be in a single txt file Rather than have a > > defconfig file for each combination. > > > > Having every combination in a text file vs in a directory of files doesn't > seem like much difference to me. `cat combinations.txt` vs `ls -l configs/`. > But using a file would mean extra tooling and non-standard usage. The .config usage is a standard already in kernel - nothing new there. What we are attempting to solve is CI build coverage and test aspect of things. Thinking aloud here: some sort of board/<vendor>/<board>/ci.conf yaml could probably be a better approach with description of build, automated test information, potentially board revisions etc. > Let's simply try to avoid these combinatorial problems by avoiding adding > too many fragments that apply broadly. That adds testing burden. When features The combinations will be valid since the intent is a supported configuration.
On 8/30/23 9:59 AM, Nishanth Menon wrote: > On 09:31-20230830, Andrew Davis wrote: >> On 8/30/23 7:31 AM, Nishanth Menon wrote: >>> On 17:14-20230829, Andrew Davis wrote: >>>> Add am62x_beagleplay_r5_defconfig for R5 SPL and >>>> am62x_beagleplay_a53_defconfig for A53 SPL and U-Boot support. >>>> >>>> These defconfigs are composite defconfigs built from the config fragment >>>> board/ti/am62x/beagleplay_*.config applied onto the base >>>> am62x_evm_*_defconfig. >>>> >>>> Signed-off-by: Andrew Davis <afd@ti.com> >>>> --- >>>> configs/am62x_beagleplay_a53_defconfig | 3 +++ >>>> configs/am62x_beagleplay_r5_defconfig | 3 +++ >>>> 2 files changed, 6 insertions(+) >>>> create mode 100644 configs/am62x_beagleplay_a53_defconfig >>>> create mode 100644 configs/am62x_beagleplay_r5_defconfig >>>> >>>> diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig >>>> new file mode 100644 >>>> index 00000000000..ad708e15397 >>>> --- /dev/null >>>> +++ b/configs/am62x_beagleplay_a53_defconfig >>>> @@ -0,0 +1,3 @@ >>>> +// The BeaglePlay defconfig for A53 core >>>> +#include "configs/am62x_evm_a53_defconfig" >>>> +#include "board/ti/am62x/beagleplay_a53.config" >>>> diff --git a/configs/am62x_beagleplay_r5_defconfig b/configs/am62x_beagleplay_r5_defconfig >>>> new file mode 100644 >>>> index 00000000000..276b1f81a3e >>>> --- /dev/null >>>> +++ b/configs/am62x_beagleplay_r5_defconfig >>>> @@ -0,0 +1,3 @@ >>>> +// The BeaglePlay defconfig for R5 core >>>> +#include "configs/am62x_evm_r5_defconfig" >>>> +#include "board/ti/am62x/beagleplay_r5.config" >>>> -- >>>> 2.39.2 >>>> >>> >>> my only complaint is that if we add lets say >>> board/ti/am62x/dfu.config, Then: >>> >>> R5: >>> 1. am62x_evm_r5_defconfig = am62x_evm_r5_defconfig >>> 2. am62x_beagleplay_r5_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config >>> 3. am62x_evm_r5_dfu_defconfig = am62x_evm_r5_defconfig + dfu.config >>> 4. am62x_beagleplay_r5_dfu_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config + dfu.config >>> >>> This information can be in a single txt file Rather than have a >>> defconfig file for each combination. >>> >> >> Having every combination in a text file vs in a directory of files doesn't >> seem like much difference to me. `cat combinations.txt` vs `ls -l configs/`. >> But using a file would mean extra tooling and non-standard usage. > > The .config usage is a standard already in kernel - nothing new there. > > What we are attempting to solve is CI build coverage and test aspect of > things. > Exactly, when I say "standard" I mean CI standard, which is to take all the configs/* and build them. No parsing these new combination files needed. Just add a new configs/xx_defconfig for a combination you want to be CI tested and you are done. > Thinking aloud here: > some sort of board/<vendor>/<board>/ci.conf yaml could probably be a better > approach with description of build, automated test information, > potentially board revisions etc. > >> Let's simply try to avoid these combinatorial problems by avoiding adding >> too many fragments that apply broadly. That adds testing burden. When features > > The combinations will be valid since the intent is a supported > configuration. > In theory anything you do in menuconfig should result in a valid configuration (if we have our kconfig symbol dependencies in order). And randomconfig testing can handle that. The combinations we want always tested should be limited, and making each have a dedicated configs/ file does that. Andrew
Hi, On Wed, 30 Aug 2023 at 09:17, Andrew Davis <afd@ti.com> wrote: > > On 8/30/23 9:59 AM, Nishanth Menon wrote: > > On 09:31-20230830, Andrew Davis wrote: > >> On 8/30/23 7:31 AM, Nishanth Menon wrote: > >>> On 17:14-20230829, Andrew Davis wrote: > >>>> Add am62x_beagleplay_r5_defconfig for R5 SPL and > >>>> am62x_beagleplay_a53_defconfig for A53 SPL and U-Boot support. > >>>> > >>>> These defconfigs are composite defconfigs built from the config fragment > >>>> board/ti/am62x/beagleplay_*.config applied onto the base > >>>> am62x_evm_*_defconfig. > >>>> > >>>> Signed-off-by: Andrew Davis <afd@ti.com> > >>>> --- > >>>> configs/am62x_beagleplay_a53_defconfig | 3 +++ > >>>> configs/am62x_beagleplay_r5_defconfig | 3 +++ > >>>> 2 files changed, 6 insertions(+) > >>>> create mode 100644 configs/am62x_beagleplay_a53_defconfig > >>>> create mode 100644 configs/am62x_beagleplay_r5_defconfig > >>>> > >>>> diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig > >>>> new file mode 100644 > >>>> index 00000000000..ad708e15397 > >>>> --- /dev/null > >>>> +++ b/configs/am62x_beagleplay_a53_defconfig > >>>> @@ -0,0 +1,3 @@ > >>>> +// The BeaglePlay defconfig for A53 core > >>>> +#include "configs/am62x_evm_a53_defconfig" > >>>> +#include "board/ti/am62x/beagleplay_a53.config" > >>>> diff --git a/configs/am62x_beagleplay_r5_defconfig b/configs/am62x_beagleplay_r5_defconfig > >>>> new file mode 100644 > >>>> index 00000000000..276b1f81a3e > >>>> --- /dev/null > >>>> +++ b/configs/am62x_beagleplay_r5_defconfig > >>>> @@ -0,0 +1,3 @@ > >>>> +// The BeaglePlay defconfig for R5 core > >>>> +#include "configs/am62x_evm_r5_defconfig" > >>>> +#include "board/ti/am62x/beagleplay_r5.config" > >>>> -- > >>>> 2.39.2 > >>>> > >>> > >>> my only complaint is that if we add lets say > >>> board/ti/am62x/dfu.config, Then: > >>> > >>> R5: > >>> 1. am62x_evm_r5_defconfig = am62x_evm_r5_defconfig > >>> 2. am62x_beagleplay_r5_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config > >>> 3. am62x_evm_r5_dfu_defconfig = am62x_evm_r5_defconfig + dfu.config > >>> 4. am62x_beagleplay_r5_dfu_defconfig = am62x_evm_r5_defconfig + beagleplay_r5.config + dfu.config > >>> > >>> This information can be in a single txt file Rather than have a > >>> defconfig file for each combination. > >>> > >> > >> Having every combination in a text file vs in a directory of files doesn't > >> seem like much difference to me. `cat combinations.txt` vs `ls -l configs/`. > >> But using a file would mean extra tooling and non-standard usage. > > > > The .config usage is a standard already in kernel - nothing new there. > > > > What we are attempting to solve is CI build coverage and test aspect of > > things. > > > > Exactly, when I say "standard" I mean CI standard, which is to take all > the configs/* and build them. No parsing these new combination files needed. > Just add a new configs/xx_defconfig for a combination you want to be > CI tested and you are done. > > > Thinking aloud here: > > some sort of board/<vendor>/<board>/ci.conf yaml could probably be a better > > approach with description of build, automated test information, > > potentially board revisions etc. > > > >> Let's simply try to avoid these combinatorial problems by avoiding adding > >> too many fragments that apply broadly. That adds testing burden. When features > > > > The combinations will be valid since the intent is a supported > > configuration. > > > > In theory anything you do in menuconfig should result in a valid configuration > (if we have our kconfig symbol dependencies in order). And randomconfig testing > can handle that. The combinations we want always tested should be limited, and > making each have a dedicated configs/ file does that. I think this is a reasonable solution to what I see as the problem (we don't know what we can build) but I am very open to others. Reviewed-by: Simon Glass <sjg@chromium.org> Regards, Simon
diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig new file mode 100644 index 00000000000..ad708e15397 --- /dev/null +++ b/configs/am62x_beagleplay_a53_defconfig @@ -0,0 +1,3 @@ +// The BeaglePlay defconfig for A53 core +#include "configs/am62x_evm_a53_defconfig" +#include "board/ti/am62x/beagleplay_a53.config" diff --git a/configs/am62x_beagleplay_r5_defconfig b/configs/am62x_beagleplay_r5_defconfig new file mode 100644 index 00000000000..276b1f81a3e --- /dev/null +++ b/configs/am62x_beagleplay_r5_defconfig @@ -0,0 +1,3 @@ +// The BeaglePlay defconfig for R5 core +#include "configs/am62x_evm_r5_defconfig" +#include "board/ti/am62x/beagleplay_r5.config"
Add am62x_beagleplay_r5_defconfig for R5 SPL and am62x_beagleplay_a53_defconfig for A53 SPL and U-Boot support. These defconfigs are composite defconfigs built from the config fragment board/ti/am62x/beagleplay_*.config applied onto the base am62x_evm_*_defconfig. Signed-off-by: Andrew Davis <afd@ti.com> --- configs/am62x_beagleplay_a53_defconfig | 3 +++ configs/am62x_beagleplay_r5_defconfig | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 configs/am62x_beagleplay_a53_defconfig create mode 100644 configs/am62x_beagleplay_r5_defconfig