Patchwork [v4,08/11] ASoC: Intel: atom: Make PCI dependency explicit

login
register
mail settings
Submitter Sinan Kaya
Date Dec. 30, 2018, 7:56 p.m.
Message ID <20181230195612.6657-9-okaya@kernel.org>
Download mbox | patch
Permalink /patch/691005/
State New
Headers show

Comments

Sinan Kaya - Dec. 30, 2018, 7:56 p.m.
Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on
CONFIG_PCI set but this is not specified anywhere.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 sound/soc/intel/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Mark Brown - Dec. 31, 2018, 5:46 p.m.
On Sun, Dec 30, 2018 at 07:56:09PM +0000, Sinan Kaya wrote:
> Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on
> CONFIG_PCI set but this is not specified anywhere.

I don't have the cover letter or anything for this series, what's going
on with dependencies?
Sinan Kaya - Dec. 31, 2018, 5:52 p.m.
On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Sun, Dec 30, 2018 at 07:56:09PM +0000, Sinan Kaya wrote:
> > Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on
> > CONFIG_PCI set but this is not specified anywhere.
>
> I don't have the cover letter or anything for this series, what's going
> on with dependencies?

Here is the executive summary:

I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI
used to select PCI. This is no longer true.

You can build an ACPI system without any PCI devices.
Mark Brown - Dec. 31, 2018, 7:30 p.m.
On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote:
> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote:

> > I don't have the cover letter or anything for this series, what's going
> > on with dependencies?

> Here is the executive summary:

> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI
> used to select PCI. This is no longer true.

> You can build an ACPI system without any PCI devices.

So there's no dependency and I can just apply this?
Sinan Kaya - Dec. 31, 2018, 7:35 p.m.
On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote:
> > On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote:
>
> > > I don't have the cover letter or anything for this series, what's going
> > > on with dependencies?
>
> > Here is the executive summary:
>
> > I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI
> > used to select PCI. This is no longer true.
>
> > You can build an ACPI system without any PCI devices.
>
> So there's no dependency and I can just apply this?

The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch.
Pierre-Louis Bossart - Dec. 31, 2018, 8:29 p.m.
On 12/31/18 1:35 PM, Sinan Kaya wrote:
> On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote:
>> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote:
>>> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote:
>>>> I don't have the cover letter or anything for this series, what's going
>>>> on with dependencies?
>>> Here is the executive summary:
>>> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI
>>> used to select PCI. This is no longer true.
>>> You can build an ACPI system without any PCI devices.
>> So there's no dependency and I can just apply this?
> The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch.
Anytime we change the Kconfig settings for audio, we get all kinds of 
problems with randconfig and 0day/kbuild due to depend/select issues. 
I'd like to give this a spin first, can you share a link to the entire 
series? Thanks!
Sinan Kaya - Dec. 31, 2018, 8:35 p.m.
On Mon, Dec 31, 2018 at 11:29 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> On 12/31/18 1:35 PM, Sinan Kaya wrote:
> > On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote:
> >> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote:
> >>> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote:
> >>>> I don't have the cover letter or anything for this series, what's going
> >>>> on with dependencies?
> >>> Here is the executive summary:
> >>> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI
> >>> used to select PCI. This is no longer true.
> >>> You can build an ACPI system without any PCI devices.
> >> So there's no dependency and I can just apply this?
> > The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch.
> Anytime we change the Kconfig settings for audio, we get all kinds of
> problems with randconfig and 0day/kbuild due to depend/select issues.
> I'd like to give this a spin first, can you share a link to the entire
> series? Thanks!

Sure,

You can find them here

https://lore.kernel.org/patchwork/patch/1028330/

Click related.
Pierre-Louis Bossart - Dec. 31, 2018, 9:42 p.m.
On 12/31/18 2:35 PM, Sinan Kaya wrote:
> On Mon, Dec 31, 2018 at 11:29 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>>
>> On 12/31/18 1:35 PM, Sinan Kaya wrote:
>>> On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote:
>>>> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote:
>>>>> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote:
>>>>>> I don't have the cover letter or anything for this series, what's going
>>>>>> on with dependencies?
>>>>> Here is the executive summary:
>>>>> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI
>>>>> used to select PCI. This is no longer true.
>>>>> You can build an ACPI system without any PCI devices.
>>>> So there's no dependency and I can just apply this?
>>> The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch.
>> Anytime we change the Kconfig settings for audio, we get all kinds of
>> problems with randconfig and 0day/kbuild due to depend/select issues.
>> I'd like to give this a spin first, can you share a link to the entire
>> series? Thanks!
> Sure,
>
> You can find them here
>
> https://lore.kernel.org/patchwork/patch/1028330/
>
> Click related.

Something must be missing, I get compilation errors when PCI is not 
defined? And I see tons of references to pci stuff in drivers/acpi.

drivers/acpi/reboot.c: In function ‘acpi_reboot’:
drivers/acpi/reboot.c:37:10: error: implicit declaration of function 
‘pci_find_bus’; did you mean ‘pci_find_next_bus’? 
[-Werror=implicit-function-declaration]
    bus0 = pci_find_bus(0, 0);
           ^~~~~~~~~~~~
           pci_find_next_bus
drivers/acpi/reboot.c:37:8: warning: assignment makes pointer from 
integer without a cast [-Wint-conversion]
    bus0 = pci_find_bus(0, 0);
         ^
drivers/acpi/reboot.c:45:3: error: implicit declaration of function 
‘pci_bus_write_config_byte’; did you mean ‘pci_write_config_byte’? 
[-Werror=implicit-function-declaration]
    pci_bus_write_config_byte(bus0, devfn,
    ^~~~~~~~~~~~~~~~~~~~~~~~~
    pci_write_config_byte
Sinan Kaya - Jan. 1, 2019, 1:07 a.m.
On Tue, Jan 1, 2019 at 12:42 AM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> On 12/31/18 2:35 PM, Sinan Kaya wrote:
> > On Mon, Dec 31, 2018 at 11:29 PM Pierre-Louis Bossart
> > <pierre-louis.bossart@linux.intel.com> wrote:
> >>
> >> On 12/31/18 1:35 PM, Sinan Kaya wrote:
> >>> On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote:
> >>>> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote:
> >>>>> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote:
> >>>>>> I don't have the cover letter or anything for this series, what's going
> >>>>>> on with dependencies?
> >>>>> Here is the executive summary:
> >>>>> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI
> >>>>> used to select PCI. This is no longer true.
> >>>>> You can build an ACPI system without any PCI devices.
> >>>> So there's no dependency and I can just apply this?
> >>> The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch.
> >> Anytime we change the Kconfig settings for audio, we get all kinds of
> >> problems with randconfig and 0day/kbuild due to depend/select issues.
> >> I'd like to give this a spin first, can you share a link to the entire
> >> series? Thanks!
> > Sure,
> >
> > You can find them here
> >
> > https://lore.kernel.org/patchwork/patch/1028330/
> >
> > Click related.
>
> Something must be missing, I get compilation errors when PCI is not
> defined? And I see tons of references to pci stuff in drivers/acpi.
>
> drivers/acpi/reboot.c: In function ‘acpi_reboot’:
> drivers/acpi/reboot.c:37:10: error: implicit declaration of function
> ‘pci_find_bus’; did you mean ‘pci_find_next_bus’?
> [-Werror=implicit-function-declaration]
>     bus0 = pci_find_bus(0, 0);
>            ^~~~~~~~~~~~
>            pci_find_next_bus
> drivers/acpi/reboot.c:37:8: warning: assignment makes pointer from
> integer without a cast [-Wint-conversion]
>     bus0 = pci_find_bus(0, 0);
>          ^
> drivers/acpi/reboot.c:45:3: error: implicit declaration of function
> ‘pci_bus_write_config_byte’; did you mean ‘pci_write_config_byte’?
> [-Werror=implicit-function-declaration]
>     pci_bus_write_config_byte(bus0, devfn,
>     ^~~~~~~~~~~~~~~~~~~~~~~~~
>     pci_write_config_byte
>
>

Please check out this tag next-20181224 and apply the patches afterwards.
Rafael J. Wysocki - Jan. 2, 2019, 9:34 a.m.
On Sun, Dec 30, 2018 at 8:56 PM Sinan Kaya <okaya@kernel.org> wrote:
>
> Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on
> CONFIG_PCI set but this is not specified anywhere.

IMO it would be better to say

"After commit 5d32a66541c46 (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.  For this reason, add a direct dependency on
CONFIG_PCI to the IOSF_MBI driver."

If you did that, the reviewers would know upfront what this was about
and that might save at least one back-and-forth e-mail exchange in
each case.
Rafael J. Wysocki - Jan. 2, 2019, 9:36 a.m.
On Wed, Jan 2, 2019 at 10:34 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sun, Dec 30, 2018 at 8:56 PM Sinan Kaya <okaya@kernel.org> wrote:
> >
> > Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on
> > CONFIG_PCI set but this is not specified anywhere.
>
> IMO it would be better to say
>
> "After commit 5d32a66541c46 (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.  For this reason, add a direct dependency on
> CONFIG_PCI to the IOSF_MBI driver."
>
> If you did that, the reviewers would know upfront what this was about
> and that might save at least one back-and-forth e-mail exchange in
> each case.

But, of course, the changelog doesn't even match the patch contents in
this particular case.  Please fix that.
Pierre-Louis Bossart - Jan. 2, 2019, 4:34 p.m.
> Please check out this tag next-20181224 and apply the patches afterwards.
Thanks, will do. I think this patchset will uncover additional 
inconsistencies, e.g. for legacy Haswell/Broadwell/Baytrail the machine 
drivers depend on X86_INTEL_LPSS, which depends in turn on PCI, but the 
platform drivers only depend on ACPI, so there is a risk of creating a 
config that makes no sense (or should only be used for COMPILE_TEST)

Patch

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