Hi Stefan, On Tue, 4 Aug 2020 at 08:03, Stefan Roese <s...@denx.de> wrote: > > Hi Simon, > > On 31.07.20 20:44, Simon Glass wrote: > > Hi Stefan, > > > > On Thu, 30 Jul 2020 at 09:35, Stefan Roese <s...@denx.de> wrote: > >> > >> Hi Simon, > >> > >> On 28.07.20 21:01, Simon Glass wrote: > >>> Hi Stefan, > >>> > >>> On Fri, 24 Jul 2020 at 04:09, Stefan Roese <s...@denx.de> wrote: > >>>> > >>>> From: Suneel Garapati <sgarap...@marvell.com> > >>>> > >>>> Enable PCI memory regions in ranges property to be of multiple entry. > >>>> This helps to add support for SoC's like OcteonTX/TX2 where every > >>>> peripheral is on PCI bus. > >>>> > >>>> Signed-off-by: Suneel Garapati <sgarap...@marvell.com> > >>>> Cc: Simon Glass <s...@chromium.org> > >>>> Cc: Bin Meng <bmeng...@gmail.com> > >>>> > >>>> Signed-off-by: Stefan Roese <s...@denx.de> > >>>> --- > >>>> > >>>> Changes in v1: > >>>> - Change patch subject > >>>> - Enhance Kconfig help descrition > >>>> - Use if() instead of #if > >>>> > >>>> drivers/pci/Kconfig | 10 ++++++++++ > >>>> drivers/pci/pci-uclass.c | 9 ++++++--- > >>>> 2 files changed, 16 insertions(+), 3 deletions(-) > >>> > >>> This needs an update to a sandbox test to handle this behaviour. > >> > >> Okay. But how should I handle all these defconfig changes with regard > >> to the other patches in this series, introducing multiple new PCI > >> related Kconfig options. With 3 new Kconfig options, all permutations > >> would lead to 8 (2 ^ 3) different defconfig files. This does not > >> scale. > >> > >> I might be missing something here though - perhaps this is easier to > >> achieve. > > > > For sandbox, turn on all options and then add a new PCI bus that uses > > this functionality. If there are lots of combinations you could add 8 > > new buses, but I'm hoping that isn't necessary? > > If I turn on all new options, sandbox will run with these new options > enabled. I don't know with with implications, as it usually runs with > the "normal" PCI related Kconfig options. Also the "normal" PCI > defconfig (e.g. CONFIG_PCI_REGION_MULTI_ENTRY etc disabled) will not > be tested any more via the sandbox tests. So you get either a test for > the new Kconfig option enabled or disabled this way. > > Do you really want me to do this?
So the Kconfig completely changes the implementation of PCI? That doesn't make it very testable, as you say. Instead, I think the Kconfig should enable the option, then use one of three ways to select the option: - a device tree property (on sandbox particularly) - compatible string (where the property is not appropriate - setting a flag in PCI bus (where a driver requires the option be selected) That way you can write a test for the new feature in sandbox, without deleting all the other tests. Regards, SImon
