2019-01-02 20:32:00

by Sinan Kaya

[permalink] [raw]
Subject: [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly. This code relies on IOSF_MBI and IOSF_MBI depends
on PCI. For this reason, add a direct dependency on CONFIG_PCI to the
IOSF_MBI driver.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya <[email protected]>
---
sound/soc/intel/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 2fd1b61e8331..b0764b2fe001 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -91,7 +91,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_PCI
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
--
2.19.0



2019-01-02 22:59:50

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

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?

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?

-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"


Attachments:
cross-check._sh (1.05 kB)

2019-01-03 02:18:33

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

On Wed, Jan 2, 2019 at 9:33 PM Pierre-Louis Bossart
<[email protected]> 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

2019-01-03 02:37:57

by Sinan Kaya

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

On Wed, Jan 2, 2019 at 8:33 PM Pierre-Louis Bossart
<[email protected]> 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.

2019-01-03 03:02:07

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit


On 1/2/19 4:58 PM, Sinan Kaya wrote:
> On Wed, Jan 2, 2019 at 10:50 PM Pierre-Louis Bossart
> <[email protected]> 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 <[email protected]>
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 <[email protected]>
    Signed-off-by: Theodore Ts'o <[email protected]>

-Pierre



2019-01-03 03:54:53

by Sinan Kaya

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

On Wed, Jan 2, 2019 at 10:50 PM Pierre-Louis Bossart
<[email protected]> 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 <[email protected]>
>

2019-01-03 03:54:54

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit


> 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 <[email protected]>


2019-01-03 08:44:13

by Sinan Kaya

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

+Tony

On Wed, Jan 2, 2019 at 11:50 PM Pierre-Louis Bossart
<[email protected]> wrote:
>
>
> On 1/2/19 4:58 PM, Sinan Kaya wrote:
> > On Wed, Jan 2, 2019 at 10:50 PM Pierre-Louis Bossart
> > <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Theodore Ts'o <[email protected]>
>
> -Pierre
>
>

2019-01-03 09:17:40

by Chandan Rajendra

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

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
> > <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Theodore Ts'o <[email protected]>
>

FWIW, The patch at https://patchwork.kernel.org/patch/10725883/ fixes this
problem by removing "select BLOCK if FS_ENCRYPTION" from fs/ubifs/Kconfig.

--
chandan




2019-01-03 13:56:47

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

On Thu, Jan 3, 2019 at 5:08 AM Chandan Rajendra <[email protected]> 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
> > > <[email protected]> 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 <[email protected]>
> > 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 <[email protected]>
> > Signed-off-by: Theodore Ts'o <[email protected]>
> >
>
> 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?

2019-01-03 16:22:39

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

On Wed, Jan 02, 2019 at 06:10:35PM +0000, Sinan Kaya wrote:

> After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
> CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
> satisfied implicitly through dependencies on CONFIG_ACPI have to be
> specified directly. This code relies on IOSF_MBI and IOSF_MBI depends
> on PCI. For this reason, add a direct dependency on CONFIG_PCI to the
> IOSF_MBI driver.

I still don't understand what's going on with dependencies here and
still don't have the cover letter or anything :( . As far as I can tell
the above commit is in Linus' tree so I'd expect I can just apply it
directly but you were saying that this needs to go via some other tree
so I'm a bit confused as to what's going on here.


Attachments:
(No filename) (780.00 B)
signature.asc (499.00 B)
Download all attachments

2019-01-03 16:22:54

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

On Thu, Jan 3, 2019 at 1:34 PM Mark Brown <[email protected]> wrote:
>
> On Wed, Jan 02, 2019 at 06:10:35PM +0000, Sinan Kaya wrote:
>
> > After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
> > CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
> > satisfied implicitly through dependencies on CONFIG_ACPI have to be
> > specified directly. This code relies on IOSF_MBI and IOSF_MBI depends
> > on PCI. For this reason, add a direct dependency on CONFIG_PCI to the
> > IOSF_MBI driver.
>
> I still don't understand what's going on with dependencies here and
> still don't have the cover letter or anything :( . As far as I can tell
> the above commit is in Linus' tree so I'd expect I can just apply it
> directly

Yes, you can.

> but you were saying that this needs to go via some other tree
> so I'm a bit confused as to what's going on here.

I guess Sinan wanted it to go in via the ACPI tree like 5d32a66541c4,
but that of course isn't necessary.

If you want to pick up anything from this series, please do. I will
pick up what's left. :-)

2019-01-03 22:14:55

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit


>>> 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 <[email protected]>
>>> 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 <[email protected]>
>>> Signed-off-by: Theodore Ts'o <[email protected]>
>>>
>> 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.


2019-01-04 12:40:23

by Chandan Rajendra

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH v5 08/11] ASoC: Intel: atom: Make PCI dependency explicit

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 <[email protected]>
> >>> 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 <[email protected]>
> >>> Signed-off-by: Theodore Ts'o <[email protected]>
> >>>
> >> 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.

--
chandan