Patchwork [alsa-devel,v5,08/11] ASoC: Intel: atom: Make PCI dependency explicit

login
register
mail settings
Submitter Pierre-Louis Bossart
Date Jan. 2, 2019, 8:33 p.m.
Message ID <57ed1d94-7f89-20e8-3289-7ef7efd18c20@linux.intel.com>
Download mbox | patch
Permalink /patch/691755/
State New
Headers show

Comments

Pierre-Louis Bossart - Jan. 2, 2019, 8:33 p.m.
I have three opens with this ACPI/PCI change

1. the baseline change fails on my cross-compilation checks, see below 
the result of the attached script (simplification of the one I use to 
avoid 0day reports).

2. there are different patterns to express the dependency on PCI e.g.

  config MMC_SDHCI_ACPI
      tristate "SDHCI support for ACPI enumerated SDHCI controllers"
      depends on MMC_SDHCI && ACPI
-    select IOSF_MBI if X86
+    select IOSF_MBI if (X86 && PCI)

but

config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
      tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
      default ACPI
-    depends on X86 && ACPI
+    depends on X86 && ACPI && PCI
      select SND_SST_IPC_ACPI
      select SND_SST_ATOM_HIFI2_PLATFORM
      select SND_SOC_ACPI_INTEL_MATCH

IOSF is only needed for Baytrail-CR detection, and the code will compile 
fine without it, so maybe it'd be a better model if you used the 
following diff?

ACPI, so we expose drivers that can be selected but fail on probe since 
there are no machine drivers. I am not sure if we want to be strict and 
only expose meaningful configurations, or allow for more compilations 
tests and corner cases?

-Pierre


cross-compilation issue:

git checkout next-20190102

make CROSS_COMPILE=/opt/gcc-8.1.0-nolibc/ia64-linux/bin/ia64-linux- 
--jobs=16 allmodconfig ARCH=ia64
   HOSTCC  scripts/basic/fixdep
   HOSTCC  scripts/kconfig/conf.o
   YACC    scripts/kconfig/zconf.tab.c
   LEX     scripts/kconfig/zconf.lex.c
   HOSTCC  scripts/kconfig/confdata.o
   HOSTCC  scripts/kconfig/expr.o
   HOSTCC  scripts/kconfig/symbol.o
   YACC    scripts/kconfig/zconf.tab.h
   HOSTCC  scripts/kconfig/preprocess.o
   HOSTCC  scripts/kconfig/zconf.tab.o
   HOSTCC  scripts/kconfig/zconf.lex.o
   HOSTLD  scripts/kconfig/conf
scripts/kconfig/conf  --allmodconfig Kconfig
arch/ia64/Kconfig:128:error: recursive dependency detected!
arch/ia64/Kconfig:128:    choice <choice> contains symbol IA64_HP_SIM
arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice PM
kernel/power/Kconfig:144:    symbol PM is selected by PM_SLEEP
kernel/power/Kconfig:104:    symbol PM_SLEEP depends on HIBERNATE_CALLBACKS
kernel/power/Kconfig:31:    symbol HIBERNATE_CALLBACKS is selected by 
HIBERNATION
kernel/power/Kconfig:34:    symbol HIBERNATION depends on SWAP
init/Kconfig:250:    symbol SWAP depends on BLOCK
block/Kconfig:5:    symbol BLOCK is selected by UBIFS_FS
fs/ubifs/Kconfig:1:    symbol UBIFS_FS depends on MISC_FILESYSTEMS
fs/Kconfig:227:    symbol MISC_FILESYSTEMS is selected by ACPI_APEI
drivers/acpi/apei/Kconfig:8:    symbol ACPI_APEI depends on ACPI
drivers/acpi/Kconfig:9:    symbol ACPI depends on ARCH_SUPPORTS_ACPI
drivers/acpi/Kconfig:6:    symbol ARCH_SUPPORTS_ACPI is selected by 
IA64_HP_SIM
arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice <choice>
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
#!/bin/bash

set -e

test_fast() {

    PLATFORM="$1"
    make mrproper
    ../make.cross allmodconfig ARCH=$PLATFORM
    ../make.cross olddefconfig ARCH=$PLATFORM    
    ../make.cross ARCH=$PLATFORM modules_prepare
}

test_compile() {

    PLATFORM="$1"
    make mrproper
    
    make defconfig ARCH=$PLATFORM
    
    echo "modules first"
    ../make.cross ARCH=$PLATFORM
    if [ "$PLATFORM" = "x86_64" ]; then
	../make.cross bindeb-pkg ARCH=$PLATFORM
    fi


    echo "all yes second"
    perl -pi.bak -e 's/=m/=y/g' .config
    ../make.cross olddefconfig ARCH=$PLATFORM
    ../make.cross ARCH=$PLATFORM
    if [ "$PLATFORM" = "x86_64" ]; then
	../make.cross bindeb-pkg ARCH=$PLATFORM
    fi

}

test_platform() {

    PLATFORM="$1"
       
    test_fast    $PLATFORM
#    test_compile $PLATFORM
}

test_platform x86_64
test_platform i386
test_platform ia64
test_platform arm
test_platform arm64
test_platform sh
test_platform mips
test_platform s390
test_platform openrisc
test_platform sparc
test_platform sparc64
test_platform m68k

echo "X-compilation check PASS"
Rafael J. Wysocki - Jan. 2, 2019, 10:01 p.m.
On Wed, Jan 2, 2019 at 9:33 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
> I have three opens with this ACPI/PCI change
>
> 1. the baseline change fails on my cross-compilation checks, see below
> the result of the attached script (simplification of the one I use to
> avoid 0day reports).

What baseline change?

That failure is not related to PCI if I'm not missing anything.

> 2. there are different patterns to express the dependency on PCI e.g.
>
>   config MMC_SDHCI_ACPI
>       tristate "SDHCI support for ACPI enumerated SDHCI controllers"
>       depends on MMC_SDHCI && ACPI
> -    select IOSF_MBI if X86
> +    select IOSF_MBI if (X86 && PCI)
>
> but
>
> config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
>       tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
>       default ACPI
> -    depends on X86 && ACPI
> +    depends on X86 && ACPI && PCI
>       select SND_SST_IPC_ACPI
>       select SND_SST_ATOM_HIFI2_PLATFORM
>       select SND_SOC_ACPI_INTEL_MATCH
>
> IOSF is only needed for Baytrail-CR detection, and the code will compile
> fine without it, so maybe it'd be a better model if you used the
> following diff?
>
> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> index 2fd1b61e8331..68af0ea5c96c 100644
> --- a/sound/soc/intel/Kconfig
> +++ b/sound/soc/intel/Kconfig
> @@ -95,7 +95,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
>          select SND_SST_IPC_ACPI
>          select SND_SST_ATOM_HIFI2_PLATFORM
>          select SND_SOC_ACPI_INTEL_MATCH
> -       select IOSF_MBI
> +       select IOSF_MBI if PCI

Well, does it actually make sense to ever set
SND_SST_ATOM_HIFI2_PLATFORM_ACPI without PCI?

> 3. All the Intel machine drivers depend on X86_INTEL_LPSS which depends
> on PCI. But for Baytrail/Haswell/Broadwell we have only a dependency on
> ACPI, so we expose drivers that can be selected but fail on probe since
> there are no machine drivers. I am not sure if we want to be strict and
> only expose meaningful configurations, or allow for more compilations
> tests and corner cases?

I would only expose meaningful configurations to start with and then
*maybe* relax that going forward as long as the benefit is worth it.

Cheers,
Rafael
Sinan Kaya - Jan. 2, 2019, 10:09 p.m.
On Wed, Jan 2, 2019 at 8:33 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
> I have three opens with this ACPI/PCI change
>
> 1. the baseline change fails on my cross-compilation checks, see below
> the result of the attached script (simplification of the one I use to
> avoid 0day reports).
>

This is pointing to a kconfig issue on ia64 arch.

arch/ia64/Kconfig:128:error: recursive dependency detected!
arch/ia64/Kconfig:128:    choice <choice> contains symbol IA64_HP_SIM
arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice PM

IA64_HP_SIM is both a choice and is selected.

I did allyesconfig and disabled PCI afterwards to find all the issues
on this patchset.

> 2. there are different patterns to express the dependency on PCI e.g.
>
>   config MMC_SDHCI_ACPI
>       tristate "SDHCI support for ACPI enumerated SDHCI controllers"
>       depends on MMC_SDHCI && ACPI
> -    select IOSF_MBI if X86
> +    select IOSF_MBI if (X86 && PCI)
>
> but
>
> config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
>       tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
>       default ACPI
> -    depends on X86 && ACPI
> +    depends on X86 && ACPI && PCI
>       select SND_SST_IPC_ACPI
>       select SND_SST_ATOM_HIFI2_PLATFORM
>       select SND_SOC_ACPI_INTEL_MATCH
>

I matched depends line to

depends on X86 && ACPI && PCI

for MMC_SDHCI_ACPI per feedback from Rafael on V5. This should resolve
the inconsistency.


> IOSF is only needed for Baytrail-CR detection, and the code will compile
> fine without it, so maybe it'd be a better model if you used the
> following diff?
>
> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> index 2fd1b61e8331..68af0ea5c96c 100644
> --- a/sound/soc/intel/Kconfig
> +++ b/sound/soc/intel/Kconfig
> @@ -95,7 +95,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
>          select SND_SST_IPC_ACPI
>          select SND_SST_ATOM_HIFI2_PLATFORM
>          select SND_SOC_ACPI_INTEL_MATCH
> -       select IOSF_MBI
> +       select IOSF_MBI if PCI
>
> 3. All the Intel machine drivers depend on X86_INTEL_LPSS which depends
> on PCI. But for Baytrail/Haswell/Broadwell we have only a dependency on
> ACPI, so we expose drivers that can be selected but fail on probe since
> there are no machine drivers. I am not sure if we want to be strict and
> only expose meaningful configurations, or allow for more compilations
> tests and corner cases?

Hopefully, v5 resolves this too with

depends on X86 && ACPI && PCI

Let me know otherwise.
Pierre-Louis Bossart - Jan. 2, 2019, 10:50 p.m.
> This is pointing to a kconfig issue on ia64 arch.
>
> arch/ia64/Kconfig:128:error: recursive dependency detected!
> arch/ia64/Kconfig:128:    choice <choice> contains symbol IA64_HP_SIM
> arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice PM
>
> IA64_HP_SIM is both a choice and is selected.
>
> I did allyesconfig and disabled PCI afterwards to find all the issues
> on this patchset.

Are you saying there's a newer series that fixes this issue for both 
allyesconfig and allmodconfig?

if yes, then we're good.

>
>> 2. there are different patterns to express the dependency on PCI e.g.
>>
>>    config MMC_SDHCI_ACPI
>>        tristate "SDHCI support for ACPI enumerated SDHCI controllers"
>>        depends on MMC_SDHCI && ACPI
>> -    select IOSF_MBI if X86
>> +    select IOSF_MBI if (X86 && PCI)
>>
>> but
>>
>> config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
>>        tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
>>        default ACPI
>> -    depends on X86 && ACPI
>> +    depends on X86 && ACPI && PCI
>>        select SND_SST_IPC_ACPI
>>        select SND_SST_ATOM_HIFI2_PLATFORM
>>        select SND_SOC_ACPI_INTEL_MATCH
>>
> I matched depends line to
>
> depends on X86 && ACPI && PCI
>
> for MMC_SDHCI_ACPI per feedback from Rafael on V5. This should resolve
> the inconsistency.
ok, I didn't see the delta
>
>
>> IOSF is only needed for Baytrail-CR detection, and the code will compile
>> fine without it, so maybe it'd be a better model if you used the
>> following diff?
>>
>> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
>> index 2fd1b61e8331..68af0ea5c96c 100644
>> --- a/sound/soc/intel/Kconfig
>> +++ b/sound/soc/intel/Kconfig
>> @@ -95,7 +95,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
>>           select SND_SST_IPC_ACPI
>>           select SND_SST_ATOM_HIFI2_PLATFORM
>>           select SND_SOC_ACPI_INTEL_MATCH
>> -       select IOSF_MBI
>> +       select IOSF_MBI if PCI
>>
>> 3. All the Intel machine drivers depend on X86_INTEL_LPSS which depends
>> on PCI. But for Baytrail/Haswell/Broadwell we have only a dependency on
>> ACPI, so we expose drivers that can be selected but fail on probe since
>> there are no machine drivers. I am not sure if we want to be strict and
>> only expose meaningful configurations, or allow for more compilations
>> tests and corner cases?
> Hopefully, v5 resolves this too with
>
> depends on X86 && ACPI && PCI
>
> Let me know otherwise.

it doesn't but that's not a good enough reason to lay on the tracks. 
I'll follow-up with a cleanup for the Intel audio parts when this series 
is merged. The PCI dependency could be moved to the top-level since it's 
pretty much required for all platforms except for compilation tests, and 
there are multiple dependencies that repeated for no good reason, so FWIW

Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Sinan Kaya - Jan. 2, 2019, 10:58 p.m.
On Wed, Jan 2, 2019 at 10:50 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> > This is pointing to a kconfig issue on ia64 arch.
> >
> > arch/ia64/Kconfig:128:error: recursive dependency detected!
> > arch/ia64/Kconfig:128:    choice <choice> contains symbol IA64_HP_SIM
> > arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice PM
> >
> > IA64_HP_SIM is both a choice and is selected.
> >
> > I did allyesconfig and disabled PCI afterwards to find all the issues
> > on this patchset.
>
> Are you saying there's a newer series that fixes this issue for both
> allyesconfig and allmodconfig?
>
> if yes, then we're good.


No, I haven't fixed ia64 kconfig issue. That's somebody else's job. I
used allyesconfig to find out all compilation issues on x86 arch to
come up with this patchset.


>
> >
> >> 2. there are different patterns to express the dependency on PCI e.g.
> >>
> >>    config MMC_SDHCI_ACPI
> >>        tristate "SDHCI support for ACPI enumerated SDHCI controllers"
> >>        depends on MMC_SDHCI && ACPI
> >> -    select IOSF_MBI if X86
> >> +    select IOSF_MBI if (X86 && PCI)
> >>
> >> but
> >>
> >> config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
> >>        tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
> >>        default ACPI
> >> -    depends on X86 && ACPI
> >> +    depends on X86 && ACPI && PCI
> >>        select SND_SST_IPC_ACPI
> >>        select SND_SST_ATOM_HIFI2_PLATFORM
> >>        select SND_SOC_ACPI_INTEL_MATCH
> >>
> > I matched depends line to
> >
> > depends on X86 && ACPI && PCI
> >
> > for MMC_SDHCI_ACPI per feedback from Rafael on V5. This should resolve
> > the inconsistency.
> ok, I didn't see the delta
> >
> >
> >> IOSF is only needed for Baytrail-CR detection, and the code will compile
> >> fine without it, so maybe it'd be a better model if you used the
> >> following diff?
> >>
> >> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> >> index 2fd1b61e8331..68af0ea5c96c 100644
> >> --- a/sound/soc/intel/Kconfig
> >> +++ b/sound/soc/intel/Kconfig
> >> @@ -95,7 +95,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
> >>           select SND_SST_IPC_ACPI
> >>           select SND_SST_ATOM_HIFI2_PLATFORM
> >>           select SND_SOC_ACPI_INTEL_MATCH
> >> -       select IOSF_MBI
> >> +       select IOSF_MBI if PCI
> >>
> >> 3. All the Intel machine drivers depend on X86_INTEL_LPSS which depends
> >> on PCI. But for Baytrail/Haswell/Broadwell we have only a dependency on
> >> ACPI, so we expose drivers that can be selected but fail on probe since
> >> there are no machine drivers. I am not sure if we want to be strict and
> >> only expose meaningful configurations, or allow for more compilations
> >> tests and corner cases?
> > Hopefully, v5 resolves this too with
> >
> > depends on X86 && ACPI && PCI
> >
> > Let me know otherwise.
>
> it doesn't but that's not a good enough reason to lay on the tracks.
> I'll follow-up with a cleanup for the Intel audio parts when this series
> is merged. The PCI dependency could be moved to the top-level since it's
> pretty much required for all platforms except for compilation tests, and
> there are multiple dependencies that repeated for no good reason, so FWIW
>
> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>
Pierre-Louis Bossart - Jan. 2, 2019, 11:50 p.m.
On 1/2/19 4:58 PM, Sinan Kaya wrote:
> On Wed, Jan 2, 2019 at 10:50 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>>
>>> This is pointing to a kconfig issue on ia64 arch.
>>>
>>> arch/ia64/Kconfig:128:error: recursive dependency detected!
>>> arch/ia64/Kconfig:128:    choice <choice> contains symbol IA64_HP_SIM
>>> arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice PM
>>>
>>> IA64_HP_SIM is both a choice and is selected.
>>>
>>> I did allyesconfig and disabled PCI afterwards to find all the issues
>>> on this patchset.
>> Are you saying there's a newer series that fixes this issue for both
>> allyesconfig and allmodconfig?
>>
>> if yes, then we're good.
>
> No, I haven't fixed ia64 kconfig issue. That's somebody else's job. I
> used allyesconfig to find out all compilation issues on x86 arch to
> come up with this patchset.

Nothing makes me cringe more than "somebody else's job" statements. In 
this case, there is obviously a correlation with your ACPI changes since 
the circular dependency happens because of the ACPI symbol.

arch/ia64/Kconfig:126:error: recursive dependency detected!
arch/ia64/Kconfig:126:    choice <choice> contains symbol IA64_HP_SIM
arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice PM
kernel/power/Kconfig:144:    symbol PM is selected by PM_SLEEP
kernel/power/Kconfig:104:    symbol PM_SLEEP depends on HIBERNATE_CALLBACKS
kernel/power/Kconfig:31:    symbol HIBERNATE_CALLBACKS is selected by 
HIBERNATION
kernel/power/Kconfig:34:    symbol HIBERNATION depends on SWAP
init/Kconfig:250:    symbol SWAP depends on BLOCK
block/Kconfig:5:    symbol BLOCK is selected by UBIFS_FS
fs/ubifs/Kconfig:1:    symbol UBIFS_FS depends on MISC_FILESYSTEMS
fs/Kconfig:220:    symbol MISC_FILESYSTEMS is selected by ACPI_APEI
drivers/acpi/apei/Kconfig:8:    symbol ACPI_APEI depends on ACPI
drivers/acpi/Kconfig:9:    symbol ACPI depends on ARCH_SUPPORTS_ACPI 
<<<< LOOK HERE
drivers/acpi/Kconfig:6:    symbol ARCH_SUPPORTS_ACPI is selected by 
IA64_HP_SIM
arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice <choice>

At any rate, a 3 mn git bisect tells me the circular dependency is 
exposed by this change:

f3fd6cd74fedf99b6060f75df00943fda13b65f2 is the first bad commit
commit f3fd6cd74fedf99b6060f75df00943fda13b65f2
Author: Chandan Rajendra <chandan@linux.vnet.ibm.com>
Date:   Sat Dec 8 12:21:38 2018 +0530

     fscrypt: remove filesystem specific build config option

     In order to have a common code base for fscrypt "post read" processing
     for all filesystems which support encryption, this commit removes
     filesystem specific build config option (e.g. 
CONFIG_EXT4_FS_ENCRYPTION)
     and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose
     value affects all the filesystems making use of fscrypt.

     Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
     Signed-off-by: Theodore Ts'o <tytso@mit.edu>

-Pierre
Sinan Kaya - Jan. 3, 2019, 3:28 a.m.
+Tony

On Wed, Jan 2, 2019 at 11:50 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> On 1/2/19 4:58 PM, Sinan Kaya wrote:
> > On Wed, Jan 2, 2019 at 10:50 PM Pierre-Louis Bossart
> > <pierre-louis.bossart@linux.intel.com> wrote:
> >>
> >>> This is pointing to a kconfig issue on ia64 arch.
> >>>
> >>> arch/ia64/Kconfig:128:error: recursive dependency detected!
> >>> arch/ia64/Kconfig:128:    choice <choice> contains symbol IA64_HP_SIM
> >>> arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice PM
> >>>
> >>> IA64_HP_SIM is both a choice and is selected.
> >>>
> >>> I did allyesconfig and disabled PCI afterwards to find all the issues
> >>> on this patchset.
> >> Are you saying there's a newer series that fixes this issue for both
> >> allyesconfig and allmodconfig?
> >>
> >> if yes, then we're good.
> >
> > No, I haven't fixed ia64 kconfig issue. That's somebody else's job. I
> > used allyesconfig to find out all compilation issues on x86 arch to
> > come up with this patchset.
>
> Nothing makes me cringe more than "somebody else's job" statements. In
> this case, there is obviously a correlation with your ACPI changes since
> the circular dependency happens because of the ACPI symbol.
>

I agree that it needs to be fixed. I am not convinced that the issue
is related to my change.
Your log is pointing to an inconsistency in ia64 kconfig.

> arch/ia64/Kconfig:126:error: recursive dependency detected!
> arch/ia64/Kconfig:126:    choice <choice> contains symbol IA64_HP_SIM
> arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice PM
> kernel/power/Kconfig:144:    symbol PM is selected by PM_SLEEP
> kernel/power/Kconfig:104:    symbol PM_SLEEP depends on HIBERNATE_CALLBACKS
> kernel/power/Kconfig:31:    symbol HIBERNATE_CALLBACKS is selected by
> HIBERNATION
> kernel/power/Kconfig:34:    symbol HIBERNATION depends on SWAP
> init/Kconfig:250:    symbol SWAP depends on BLOCK
> block/Kconfig:5:    symbol BLOCK is selected by UBIFS_FS
> fs/ubifs/Kconfig:1:    symbol UBIFS_FS depends on MISC_FILESYSTEMS
> fs/Kconfig:220:    symbol MISC_FILESYSTEMS is selected by ACPI_APEI
> drivers/acpi/apei/Kconfig:8:    symbol ACPI_APEI depends on ACPI
> drivers/acpi/Kconfig:9:    symbol ACPI depends on ARCH_SUPPORTS_ACPI
> <<<< LOOK HERE

I am still having hard time seeing how this issue is exposed by
removing PCI dependency from ACPi.

Rafael, Tony:

can you help me?

> drivers/acpi/Kconfig:6:    symbol ARCH_SUPPORTS_ACPI is selected by
> IA64_HP_SIM
> arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice <choice>
>
> At any rate, a 3 mn git bisect tells me the circular dependency is
> exposed by this change:
>
> f3fd6cd74fedf99b6060f75df00943fda13b65f2 is the first bad commit
> commit f3fd6cd74fedf99b6060f75df00943fda13b65f2
> Author: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> Date:   Sat Dec 8 12:21:38 2018 +0530
>
>      fscrypt: remove filesystem specific build config option
>
>      In order to have a common code base for fscrypt "post read" processing
>      for all filesystems which support encryption, this commit removes
>      filesystem specific build config option (e.g.
> CONFIG_EXT4_FS_ENCRYPTION)
>      and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose
>      value affects all the filesystems making use of fscrypt.
>
>      Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
>      Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>
> -Pierre
>
>
Chandan Rajendra - Jan. 3, 2019, 4:08 a.m.
On Thursday, January 3, 2019 5:20:27 AM IST Pierre-Louis Bossart wrote:
> 
> On 1/2/19 4:58 PM, Sinan Kaya wrote:
> > On Wed, Jan 2, 2019 at 10:50 PM Pierre-Louis Bossart
> > <pierre-louis.bossart@linux.intel.com> wrote:
> >>
> >>> This is pointing to a kconfig issue on ia64 arch.
> >>>
> >>> arch/ia64/Kconfig:128:error: recursive dependency detected!
> >>> arch/ia64/Kconfig:128:    choice <choice> contains symbol IA64_HP_SIM
> >>> arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice PM
> >>>
> >>> IA64_HP_SIM is both a choice and is selected.
> >>>
> >>> I did allyesconfig and disabled PCI afterwards to find all the issues
> >>> on this patchset.
> >> Are you saying there's a newer series that fixes this issue for both
> >> allyesconfig and allmodconfig?
> >>
> >> if yes, then we're good.
> >
> > No, I haven't fixed ia64 kconfig issue. That's somebody else's job. I
> > used allyesconfig to find out all compilation issues on x86 arch to
> > come up with this patchset.
> 
> Nothing makes me cringe more than "somebody else's job" statements. In 
> this case, there is obviously a correlation with your ACPI changes since 
> the circular dependency happens because of the ACPI symbol.
> 
> arch/ia64/Kconfig:126:error: recursive dependency detected!
> arch/ia64/Kconfig:126:    choice <choice> contains symbol IA64_HP_SIM
> arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice PM
> kernel/power/Kconfig:144:    symbol PM is selected by PM_SLEEP
> kernel/power/Kconfig:104:    symbol PM_SLEEP depends on HIBERNATE_CALLBACKS
> kernel/power/Kconfig:31:    symbol HIBERNATE_CALLBACKS is selected by 
> HIBERNATION
> kernel/power/Kconfig:34:    symbol HIBERNATION depends on SWAP
> init/Kconfig:250:    symbol SWAP depends on BLOCK
> block/Kconfig:5:    symbol BLOCK is selected by UBIFS_FS
> fs/ubifs/Kconfig:1:    symbol UBIFS_FS depends on MISC_FILESYSTEMS
> fs/Kconfig:220:    symbol MISC_FILESYSTEMS is selected by ACPI_APEI
> drivers/acpi/apei/Kconfig:8:    symbol ACPI_APEI depends on ACPI
> drivers/acpi/Kconfig:9:    symbol ACPI depends on ARCH_SUPPORTS_ACPI 
> <<<< LOOK HERE
> drivers/acpi/Kconfig:6:    symbol ARCH_SUPPORTS_ACPI is selected by 
> IA64_HP_SIM
> arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice <choice>
> 
> At any rate, a 3 mn git bisect tells me the circular dependency is 
> exposed by this change:
> 
> f3fd6cd74fedf99b6060f75df00943fda13b65f2 is the first bad commit
> commit f3fd6cd74fedf99b6060f75df00943fda13b65f2
> Author: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> Date:   Sat Dec 8 12:21:38 2018 +0530
> 
>      fscrypt: remove filesystem specific build config option
> 
>      In order to have a common code base for fscrypt "post read" processing
>      for all filesystems which support encryption, this commit removes
>      filesystem specific build config option (e.g. 
> CONFIG_EXT4_FS_ENCRYPTION)
>      and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose
>      value affects all the filesystems making use of fscrypt.
> 
>      Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
>      Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> 

FWIW, The patch at https://patchwork.kernel.org/patch/10725883/ fixes this
problem by removing "select BLOCK if FS_ENCRYPTION" from fs/ubifs/Kconfig.
Rafael J. Wysocki - Jan. 3, 2019, 9:52 a.m.
On Thu, Jan 3, 2019 at 5:08 AM Chandan Rajendra <chandan@linux.ibm.com> wrote:
>
> On Thursday, January 3, 2019 5:20:27 AM IST Pierre-Louis Bossart wrote:
> >
> > On 1/2/19 4:58 PM, Sinan Kaya wrote:
> > > On Wed, Jan 2, 2019 at 10:50 PM Pierre-Louis Bossart
> > > <pierre-louis.bossart@linux.intel.com> wrote:
> > >>
> > >>> This is pointing to a kconfig issue on ia64 arch.
> > >>>
> > >>> arch/ia64/Kconfig:128:error: recursive dependency detected!
> > >>> arch/ia64/Kconfig:128:    choice <choice> contains symbol IA64_HP_SIM
> > >>> arch/ia64/Kconfig:202:    symbol IA64_HP_SIM is part of choice PM
> > >>>
> > >>> IA64_HP_SIM is both a choice and is selected.
> > >>>
> > >>> I did allyesconfig and disabled PCI afterwards to find all the issues
> > >>> on this patchset.
> > >> Are you saying there's a newer series that fixes this issue for both
> > >> allyesconfig and allmodconfig?
> > >>
> > >> if yes, then we're good.
> > >
> > > No, I haven't fixed ia64 kconfig issue. That's somebody else's job. I
> > > used allyesconfig to find out all compilation issues on x86 arch to
> > > come up with this patchset.
> >
> > Nothing makes me cringe more than "somebody else's job" statements. In
> > this case, there is obviously a correlation with your ACPI changes since
> > the circular dependency happens because of the ACPI symbol.
> >
> > arch/ia64/Kconfig:126:error: recursive dependency detected!
> > arch/ia64/Kconfig:126:    choice <choice> contains symbol IA64_HP_SIM
> > arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice PM
> > kernel/power/Kconfig:144:    symbol PM is selected by PM_SLEEP
> > kernel/power/Kconfig:104:    symbol PM_SLEEP depends on HIBERNATE_CALLBACKS
> > kernel/power/Kconfig:31:    symbol HIBERNATE_CALLBACKS is selected by
> > HIBERNATION
> > kernel/power/Kconfig:34:    symbol HIBERNATION depends on SWAP
> > init/Kconfig:250:    symbol SWAP depends on BLOCK
> > block/Kconfig:5:    symbol BLOCK is selected by UBIFS_FS
> > fs/ubifs/Kconfig:1:    symbol UBIFS_FS depends on MISC_FILESYSTEMS
> > fs/Kconfig:220:    symbol MISC_FILESYSTEMS is selected by ACPI_APEI
> > drivers/acpi/apei/Kconfig:8:    symbol ACPI_APEI depends on ACPI
> > drivers/acpi/Kconfig:9:    symbol ACPI depends on ARCH_SUPPORTS_ACPI
> > <<<< LOOK HERE
> > drivers/acpi/Kconfig:6:    symbol ARCH_SUPPORTS_ACPI is selected by
> > IA64_HP_SIM
> > arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice <choice>
> >
> > At any rate, a 3 mn git bisect tells me the circular dependency is
> > exposed by this change:
> >
> > f3fd6cd74fedf99b6060f75df00943fda13b65f2 is the first bad commit
> > commit f3fd6cd74fedf99b6060f75df00943fda13b65f2
> > Author: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> > Date:   Sat Dec 8 12:21:38 2018 +0530
> >
> >      fscrypt: remove filesystem specific build config option
> >
> >      In order to have a common code base for fscrypt "post read" processing
> >      for all filesystems which support encryption, this commit removes
> >      filesystem specific build config option (e.g.
> > CONFIG_EXT4_FS_ENCRYPTION)
> >      and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose
> >      value affects all the filesystems making use of fscrypt.
> >
> >      Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> >      Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> >
>
> FWIW, The patch at https://patchwork.kernel.org/patch/10725883/ fixes this
> problem by removing "select BLOCK if FS_ENCRYPTION" from fs/ubifs/Kconfig.

OK

Pierre-Louis, can you check if this patch makes the issue go away for
you, please?
Pierre-Louis Bossart - Jan. 3, 2019, 4:28 p.m.
>>> arch/ia64/Kconfig:126:error: recursive dependency detected!
>>> arch/ia64/Kconfig:126:    choice <choice> contains symbol IA64_HP_SIM
>>> arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice PM
>>> kernel/power/Kconfig:144:    symbol PM is selected by PM_SLEEP
>>> kernel/power/Kconfig:104:    symbol PM_SLEEP depends on HIBERNATE_CALLBACKS
>>> kernel/power/Kconfig:31:    symbol HIBERNATE_CALLBACKS is selected by
>>> HIBERNATION
>>> kernel/power/Kconfig:34:    symbol HIBERNATION depends on SWAP
>>> init/Kconfig:250:    symbol SWAP depends on BLOCK
>>> block/Kconfig:5:    symbol BLOCK is selected by UBIFS_FS
>>> fs/ubifs/Kconfig:1:    symbol UBIFS_FS depends on MISC_FILESYSTEMS
>>> fs/Kconfig:220:    symbol MISC_FILESYSTEMS is selected by ACPI_APEI
>>> drivers/acpi/apei/Kconfig:8:    symbol ACPI_APEI depends on ACPI
>>> drivers/acpi/Kconfig:9:    symbol ACPI depends on ARCH_SUPPORTS_ACPI
>>> <<<< LOOK HERE
>>> drivers/acpi/Kconfig:6:    symbol ARCH_SUPPORTS_ACPI is selected by
>>> IA64_HP_SIM
>>> arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice <choice>
>>>
>>> At any rate, a 3 mn git bisect tells me the circular dependency is
>>> exposed by this change:
>>>
>>> f3fd6cd74fedf99b6060f75df00943fda13b65f2 is the first bad commit
>>> commit f3fd6cd74fedf99b6060f75df00943fda13b65f2
>>> Author: Chandan Rajendra <chandan@linux.vnet.ibm.com>
>>> Date:   Sat Dec 8 12:21:38 2018 +0530
>>>
>>>       fscrypt: remove filesystem specific build config option
>>>
>>>       In order to have a common code base for fscrypt "post read" processing
>>>       for all filesystems which support encryption, this commit removes
>>>       filesystem specific build config option (e.g.
>>> CONFIG_EXT4_FS_ENCRYPTION)
>>>       and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose
>>>       value affects all the filesystems making use of fscrypt.
>>>
>>>       Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
>>>       Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>>>
>> FWIW, The patch at https://patchwork.kernel.org/patch/10725883/ fixes this
>> problem by removing "select BLOCK if FS_ENCRYPTION" from fs/ubifs/Kconfig.
> OK
>
> Pierre-Louis, can you check if this patch makes the issue go away for
> you, please?

Wondering if Chandan provided the right pointer, I wasn't able to apply 
this patch, but commenting out "select BLOCK if FS_ENCRYPTION" in 
fs/ubifs/Kconfig makes the circular dependency go away. All good for me.
Chandan Rajendra - Jan. 4, 2019, 10:55 a.m.
On Thursday, January 3, 2019 9:58:28 PM IST Pierre-Louis Bossart wrote:
> 
> >>> arch/ia64/Kconfig:126:error: recursive dependency detected!
> >>> arch/ia64/Kconfig:126:    choice <choice> contains symbol IA64_HP_SIM
> >>> arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice PM
> >>> kernel/power/Kconfig:144:    symbol PM is selected by PM_SLEEP
> >>> kernel/power/Kconfig:104:    symbol PM_SLEEP depends on HIBERNATE_CALLBACKS
> >>> kernel/power/Kconfig:31:    symbol HIBERNATE_CALLBACKS is selected by
> >>> HIBERNATION
> >>> kernel/power/Kconfig:34:    symbol HIBERNATION depends on SWAP
> >>> init/Kconfig:250:    symbol SWAP depends on BLOCK
> >>> block/Kconfig:5:    symbol BLOCK is selected by UBIFS_FS
> >>> fs/ubifs/Kconfig:1:    symbol UBIFS_FS depends on MISC_FILESYSTEMS
> >>> fs/Kconfig:220:    symbol MISC_FILESYSTEMS is selected by ACPI_APEI
> >>> drivers/acpi/apei/Kconfig:8:    symbol ACPI_APEI depends on ACPI
> >>> drivers/acpi/Kconfig:9:    symbol ACPI depends on ARCH_SUPPORTS_ACPI
> >>> <<<< LOOK HERE
> >>> drivers/acpi/Kconfig:6:    symbol ARCH_SUPPORTS_ACPI is selected by
> >>> IA64_HP_SIM
> >>> arch/ia64/Kconfig:200:    symbol IA64_HP_SIM is part of choice <choice>
> >>>
> >>> At any rate, a 3 mn git bisect tells me the circular dependency is
> >>> exposed by this change:
> >>>
> >>> f3fd6cd74fedf99b6060f75df00943fda13b65f2 is the first bad commit
> >>> commit f3fd6cd74fedf99b6060f75df00943fda13b65f2
> >>> Author: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> >>> Date:   Sat Dec 8 12:21:38 2018 +0530
> >>>
> >>>       fscrypt: remove filesystem specific build config option
> >>>
> >>>       In order to have a common code base for fscrypt "post read" processing
> >>>       for all filesystems which support encryption, this commit removes
> >>>       filesystem specific build config option (e.g.
> >>> CONFIG_EXT4_FS_ENCRYPTION)
> >>>       and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose
> >>>       value affects all the filesystems making use of fscrypt.
> >>>
> >>>       Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> >>>       Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> >>>
> >> FWIW, The patch at https://patchwork.kernel.org/patch/10725883/ fixes this
> >> problem by removing "select BLOCK if FS_ENCRYPTION" from fs/ubifs/Kconfig.
> > OK
> >
> > Pierre-Louis, can you check if this patch makes the issue go away for
> > you, please?
> 
> Wondering if Chandan provided the right pointer, I wasn't able to apply 
> this patch, but commenting out "select BLOCK if FS_ENCRYPTION" in 
> fs/ubifs/Kconfig makes the circular dependency go away. All good for me.
> 

V4 version of the patchset (which is part of linux-next) had introduced the
"select BLOCK if FS_ENCRYPTION". The V5 patchset omits that line. Hence the
patch did not apply cleanly. However just commenting out that line was the
right thing to do.

Patch

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 2fd1b61e8331..68af0ea5c96c 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -95,7 +95,7 @@  config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
         select SND_SST_IPC_ACPI
         select SND_SST_ATOM_HIFI2_PLATFORM
         select SND_SOC_ACPI_INTEL_MATCH
-       select IOSF_MBI
+       select IOSF_MBI if PCI

3. All the Intel machine drivers depend on X86_INTEL_LPSS which depends 
on PCI. But for Baytrail/Haswell/Broadwell we have only a dependency on