From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH V7 07/11] pci, acpi: Handle ACPI companion assignment. Date: Tue, 10 May 2016 20:43:42 +0200 Message-ID: References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-8-git-send-email-tn@semihalf.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: Sender: linux-pci-owner@vger.kernel.org To: "Rafael J. Wysocki" Cc: Tomasz Nowicki , Bjorn Helgaas , Arnd Bergmann , Will Deacon , Catalin Marinas , Hanjun Guo , Lorenzo Pieralisi , Sinan Kaya , jchandra@broadcom.com, robert.richter@caviumnetworks.com, Marcin Wojtas , Liviu.Dudau@arm.com, David Daney , wangyijing@huawei.com, Suravee Suthikulanit , Mark Salter , Linux PCI , "linux-arm-kernel@lists.infradead.org" , ACPI Devel Maling List , Linux Kernel Mailing List , "linaro-acpi@lists.linaro.org" , Jon Masters List-Id: linux-acpi@vger.kernel.org On Tue, May 10, 2016 at 8:37 PM, Rafael J. Wysocki wrote: > On Tue, May 10, 2016 at 5:19 PM, Tomasz Nowicki wrote: >> This patch provides a way to set the ACPI companion in PCI code. >> We define acpi_pci_set_companion() to set the ACPI companion pointer and >> call it from PCI core code. The function is stub for now. >> >> Signed-off-by: Jayachandran C >> Signed-off-by: Tomasz Nowicki >> --- >> drivers/pci/probe.c | 2 ++ >> include/linux/pci-acpi.h | 4 ++++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 8004f67..fb0b752 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -12,6 +12,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -2141,6 +2142,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, >> bridge->dev.parent = parent; >> bridge->dev.release = pci_release_host_bridge_dev; >> dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); >> + acpi_pci_set_companion(bridge); > > Yes, we'll probably add something similar here. > > Do I think now is the right time to do that? No. > >> error = pcibios_root_bridge_prepare(bridge); >> if (error) { >> kfree(bridge); >> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h >> index 09f9f02..1baa515 100644 >> --- a/include/linux/pci-acpi.h >> +++ b/include/linux/pci-acpi.h >> @@ -111,6 +111,10 @@ static inline void acpi_pci_add_bus(struct pci_bus *bus) { } >> static inline void acpi_pci_remove_bus(struct pci_bus *bus) { } >> #endif /* CONFIG_ACPI */ >> >> +static inline void acpi_pci_set_companion(struct pci_host_bridge *bridge) >> +{ >> +} >> + >> static inline int acpi_pci_bus_domain_nr(struct pci_bus *bus) >> { >> return 0; >> -- > > Honestly, to me it looks like this series is trying very hard to avoid > doing any PCI host bridge configuration stuff from arch/arm64/ > although (a) that might be simpler and (b) it would allow us to > identify the code that's common between *all* architectures using ACPI > support for host bridge configuration and to move *that* to a common > place later. As done here it seems to be following the "ARM64 is > generic and the rest of the world is special" line which isn't really > helpful. Speaking of which, at least one of the reasons why the ACPI PCI host bridge thing on x86 and ia64 went to the arch code was to avoid explicit references to ACPI-specific data types and related #ifdeffery in the generic PCI code and data structures. If you are going to add those references now anyway, that reason is not relevant any more and all of that can just be reworked to refer to ACPI explicitly. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751466AbcEJSnq (ORCPT ); Tue, 10 May 2016 14:43:46 -0400 Received: from mail-lf0-f65.google.com ([209.85.215.65]:35794 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750922AbcEJSno (ORCPT ); Tue, 10 May 2016 14:43:44 -0400 MIME-Version: 1.0 In-Reply-To: References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-8-git-send-email-tn@semihalf.com> Date: Tue, 10 May 2016 20:43:42 +0200 X-Google-Sender-Auth: cnmWqFAyDfCzLxG0yeGL-NRfriY Message-ID: Subject: Re: [PATCH V7 07/11] pci, acpi: Handle ACPI companion assignment. From: "Rafael J. Wysocki" To: "Rafael J. Wysocki" Cc: Tomasz Nowicki , Bjorn Helgaas , Arnd Bergmann , Will Deacon , Catalin Marinas , Hanjun Guo , Lorenzo Pieralisi , Sinan Kaya , jchandra@broadcom.com, robert.richter@caviumnetworks.com, Marcin Wojtas , Liviu.Dudau@arm.com, David Daney , wangyijing@huawei.com, Suravee Suthikulanit , Mark Salter , Linux PCI , "linux-arm-kernel@lists.infradead.org" , ACPI Devel Maling List , Linux Kernel Mailing List , "linaro-acpi@lists.linaro.org" , Jon Masters , Andrea Gallo , Duc Dang , jeremy.linton@arm.com, liudongdong3@huawei.com, Christopher Covington Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 10, 2016 at 8:37 PM, Rafael J. Wysocki wrote: > On Tue, May 10, 2016 at 5:19 PM, Tomasz Nowicki wrote: >> This patch provides a way to set the ACPI companion in PCI code. >> We define acpi_pci_set_companion() to set the ACPI companion pointer and >> call it from PCI core code. The function is stub for now. >> >> Signed-off-by: Jayachandran C >> Signed-off-by: Tomasz Nowicki >> --- >> drivers/pci/probe.c | 2 ++ >> include/linux/pci-acpi.h | 4 ++++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 8004f67..fb0b752 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -12,6 +12,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -2141,6 +2142,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, >> bridge->dev.parent = parent; >> bridge->dev.release = pci_release_host_bridge_dev; >> dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); >> + acpi_pci_set_companion(bridge); > > Yes, we'll probably add something similar here. > > Do I think now is the right time to do that? No. > >> error = pcibios_root_bridge_prepare(bridge); >> if (error) { >> kfree(bridge); >> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h >> index 09f9f02..1baa515 100644 >> --- a/include/linux/pci-acpi.h >> +++ b/include/linux/pci-acpi.h >> @@ -111,6 +111,10 @@ static inline void acpi_pci_add_bus(struct pci_bus *bus) { } >> static inline void acpi_pci_remove_bus(struct pci_bus *bus) { } >> #endif /* CONFIG_ACPI */ >> >> +static inline void acpi_pci_set_companion(struct pci_host_bridge *bridge) >> +{ >> +} >> + >> static inline int acpi_pci_bus_domain_nr(struct pci_bus *bus) >> { >> return 0; >> -- > > Honestly, to me it looks like this series is trying very hard to avoid > doing any PCI host bridge configuration stuff from arch/arm64/ > although (a) that might be simpler and (b) it would allow us to > identify the code that's common between *all* architectures using ACPI > support for host bridge configuration and to move *that* to a common > place later. As done here it seems to be following the "ARM64 is > generic and the rest of the world is special" line which isn't really > helpful. Speaking of which, at least one of the reasons why the ACPI PCI host bridge thing on x86 and ia64 went to the arch code was to avoid explicit references to ACPI-specific data types and related #ifdeffery in the generic PCI code and data structures. If you are going to add those references now anyway, that reason is not relevant any more and all of that can just be reworked to refer to ACPI explicitly. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: MIME-Version: 1.0 Sender: rjwysocki@gmail.com In-Reply-To: References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-8-git-send-email-tn@semihalf.com> Date: Tue, 10 May 2016 20:43:42 +0200 Message-ID: Subject: Re: [PATCH V7 07/11] pci, acpi: Handle ACPI companion assignment. From: "Rafael J. Wysocki" To: "Rafael J. Wysocki" Cc: Tomasz Nowicki , Bjorn Helgaas , Arnd Bergmann , Will Deacon , Catalin Marinas , Hanjun Guo , Lorenzo Pieralisi , Sinan Kaya , jchandra@broadcom.com, robert.richter@caviumnetworks.com, Marcin Wojtas , Liviu.Dudau@arm.com, David Daney , wangyijing@huawei.com, Suravee Suthikulanit , Mark Salter , Linux PCI , "linux-arm-kernel@lists.infradead.org" , ACPI Devel Maling List , Linux Kernel Mailing List , "linaro-acpi@lists.linaro.org" , Jon Masters , Andrea Gallo , Duc Dang , jeremy.linton@arm.com, liudongdong3@huawei.com, Christopher Covington Content-Type: text/plain; charset=UTF-8 List-ID: On Tue, May 10, 2016 at 8:37 PM, Rafael J. Wysocki wrote: > On Tue, May 10, 2016 at 5:19 PM, Tomasz Nowicki wrote: >> This patch provides a way to set the ACPI companion in PCI code. >> We define acpi_pci_set_companion() to set the ACPI companion pointer and >> call it from PCI core code. The function is stub for now. >> >> Signed-off-by: Jayachandran C >> Signed-off-by: Tomasz Nowicki >> --- >> drivers/pci/probe.c | 2 ++ >> include/linux/pci-acpi.h | 4 ++++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 8004f67..fb0b752 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -12,6 +12,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -2141,6 +2142,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, >> bridge->dev.parent = parent; >> bridge->dev.release = pci_release_host_bridge_dev; >> dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); >> + acpi_pci_set_companion(bridge); > > Yes, we'll probably add something similar here. > > Do I think now is the right time to do that? No. > >> error = pcibios_root_bridge_prepare(bridge); >> if (error) { >> kfree(bridge); >> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h >> index 09f9f02..1baa515 100644 >> --- a/include/linux/pci-acpi.h >> +++ b/include/linux/pci-acpi.h >> @@ -111,6 +111,10 @@ static inline void acpi_pci_add_bus(struct pci_bus *bus) { } >> static inline void acpi_pci_remove_bus(struct pci_bus *bus) { } >> #endif /* CONFIG_ACPI */ >> >> +static inline void acpi_pci_set_companion(struct pci_host_bridge *bridge) >> +{ >> +} >> + >> static inline int acpi_pci_bus_domain_nr(struct pci_bus *bus) >> { >> return 0; >> -- > > Honestly, to me it looks like this series is trying very hard to avoid > doing any PCI host bridge configuration stuff from arch/arm64/ > although (a) that might be simpler and (b) it would allow us to > identify the code that's common between *all* architectures using ACPI > support for host bridge configuration and to move *that* to a common > place later. As done here it seems to be following the "ARM64 is > generic and the rest of the world is special" line which isn't really > helpful. Speaking of which, at least one of the reasons why the ACPI PCI host bridge thing on x86 and ia64 went to the arch code was to avoid explicit references to ACPI-specific data types and related #ifdeffery in the generic PCI code and data structures. If you are going to add those references now anyway, that reason is not relevant any more and all of that can just be reworked to refer to ACPI explicitly. From mboxrd@z Thu Jan 1 00:00:00 1970 From: rafael@kernel.org (Rafael J. Wysocki) Date: Tue, 10 May 2016 20:43:42 +0200 Subject: [PATCH V7 07/11] pci, acpi: Handle ACPI companion assignment. In-Reply-To: References: <1462893601-8937-1-git-send-email-tn@semihalf.com> <1462893601-8937-8-git-send-email-tn@semihalf.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, May 10, 2016 at 8:37 PM, Rafael J. Wysocki wrote: > On Tue, May 10, 2016 at 5:19 PM, Tomasz Nowicki wrote: >> This patch provides a way to set the ACPI companion in PCI code. >> We define acpi_pci_set_companion() to set the ACPI companion pointer and >> call it from PCI core code. The function is stub for now. >> >> Signed-off-by: Jayachandran C >> Signed-off-by: Tomasz Nowicki >> --- >> drivers/pci/probe.c | 2 ++ >> include/linux/pci-acpi.h | 4 ++++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 8004f67..fb0b752 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -12,6 +12,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -2141,6 +2142,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus, >> bridge->dev.parent = parent; >> bridge->dev.release = pci_release_host_bridge_dev; >> dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus); >> + acpi_pci_set_companion(bridge); > > Yes, we'll probably add something similar here. > > Do I think now is the right time to do that? No. > >> error = pcibios_root_bridge_prepare(bridge); >> if (error) { >> kfree(bridge); >> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h >> index 09f9f02..1baa515 100644 >> --- a/include/linux/pci-acpi.h >> +++ b/include/linux/pci-acpi.h >> @@ -111,6 +111,10 @@ static inline void acpi_pci_add_bus(struct pci_bus *bus) { } >> static inline void acpi_pci_remove_bus(struct pci_bus *bus) { } >> #endif /* CONFIG_ACPI */ >> >> +static inline void acpi_pci_set_companion(struct pci_host_bridge *bridge) >> +{ >> +} >> + >> static inline int acpi_pci_bus_domain_nr(struct pci_bus *bus) >> { >> return 0; >> -- > > Honestly, to me it looks like this series is trying very hard to avoid > doing any PCI host bridge configuration stuff from arch/arm64/ > although (a) that might be simpler and (b) it would allow us to > identify the code that's common between *all* architectures using ACPI > support for host bridge configuration and to move *that* to a common > place later. As done here it seems to be following the "ARM64 is > generic and the rest of the world is special" line which isn't really > helpful. Speaking of which, at least one of the reasons why the ACPI PCI host bridge thing on x86 and ia64 went to the arch code was to avoid explicit references to ACPI-specific data types and related #ifdeffery in the generic PCI code and data structures. If you are going to add those references now anyway, that reason is not relevant any more and all of that can just be reworked to refer to ACPI explicitly.