From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiang Liu Subject: Re: [Patch v5 1/6] ACPI/PCI: Enhance ACPI core to support sparse IO space Date: Wed, 9 Sep 2015 14:38:02 +0800 Message-ID: <55EFD3CA.3050301@linux.intel.com> References: <1433780448-18636-1-git-send-email-jiang.liu@linux.intel.com> <1433780448-18636-2-git-send-email-jiang.liu@linux.intel.com> <20150729203741.GC9640@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com ([192.55.52.88]:24406 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752694AbbIIGiH (ORCPT ); Wed, 9 Sep 2015 02:38:07 -0400 In-Reply-To: <20150729203741.GC9640@google.com> Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Bjorn Helgaas Cc: Lorenzo Pieralisi , "Rafael J . Wysocki" , Marc Zyngier , Hanjun Guo , Liviu Dudau , Yijing Wang , Len Brown , Vivek Goyal , Thierry Reding , Jakub Sitnicki , Lv Zheng , LKML , linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org, "x86 @ kernel . org" , linux-arm-kernel@lists.infradead.org On 2015/7/30 4:37, Bjorn Helgaas wrote: > On Tue, Jun 09, 2015 at 12:20:43AM +0800, Jiang Liu wrote: >> Enhance ACPI resource parsing interfaces to support sparse IO space, >> which will be used to share common code between x86 and IA64 later. >> >> Tested-by: Tony Luck >> Signed-off-by: Jiang Liu >> Reviewed-by: Hanjun Guo >> --- >> drivers/acpi/resource.c | 9 ++++++--- >> include/linux/ioport.h | 1 + >> 2 files changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c >> index 8244f013f210..fdcc73dad2c1 100644 >> --- a/drivers/acpi/resource.c >> +++ b/drivers/acpi/resource.c >> @@ -123,7 +123,7 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res) >> EXPORT_SYMBOL_GPL(acpi_dev_resource_memory); >> >> static void acpi_dev_ioresource_flags(struct resource *res, u64 len, >> - u8 io_decode) >> + u8 io_decode, u8 translation_type) >> { >> res->flags = IORESOURCE_IO; >> >> @@ -135,6 +135,8 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len, >> >> if (io_decode == ACPI_DECODE_16) >> res->flags |= IORESOURCE_IO_16BIT_ADDR; >> + if (translation_type == ACPI_SPARSE_TRANSLATION) >> + res->flags |= IORESOURCE_IO_SPARSE; >> } >> >> static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len, >> @@ -142,7 +144,7 @@ static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len, >> { >> res->start = start; >> res->end = start + len - 1; >> - acpi_dev_ioresource_flags(res, len, io_decode); >> + acpi_dev_ioresource_flags(res, len, io_decode, 0); >> } >> >> /** >> @@ -227,7 +229,8 @@ static bool acpi_decode_space(struct resource_win *win, >> acpi_dev_memresource_flags(res, len, wp); >> break; >> case ACPI_IO_RANGE: >> - acpi_dev_ioresource_flags(res, len, iodec); >> + acpi_dev_ioresource_flags(res, len, iodec, >> + addr->info.io.translation_type); >> break; >> case ACPI_BUS_NUMBER_RANGE: >> res->flags = IORESOURCE_BUS; >> diff --git a/include/linux/ioport.h b/include/linux/ioport.h >> index 388e3ae94f7a..24bea087e7af 100644 >> --- a/include/linux/ioport.h >> +++ b/include/linux/ioport.h >> @@ -94,6 +94,7 @@ struct resource { >> /* PnP I/O specific bits (IORESOURCE_BITS) */ >> #define IORESOURCE_IO_16BIT_ADDR (1<<0) >> #define IORESOURCE_IO_FIXED (1<<1) >> +#define IORESOURCE_IO_SPARSE (1<<2) > > I don't really like this bit. We adding a new generic IORESOURCE_* bit > just for a special case, and it's only used in one place, for one arch, > during enumeration. Hi Bjorn, Instead of defining a formal flag IORESOURCE_IO_SPARSE, we may reuse other field in struct resource to pass back the SPARSE flag, but that's a little dirty. For example, we may reuse res->name field to carry the SPARSE flag for the IA64 special case. Is that OK? Thanks! Gerry > > Don't we have a similar problem (struct resource can't express all the > information from an ACPI resource) for the _TRA value for bridge windows? > If it's a similar issue, we should solve it in a similar way. Per my understanding, we just ignore ACPI address space information which can't be expressed by struct resource. Thanks! Gerry > >> /* PCI ROM control bits (IORESOURCE_BITS) */ >> #define IORESOURCE_ROM_ENABLE (1<<0) /* ROM is enabled, same as PCI_ROM_ADDRESS_ENABLE */ >> -- >> 1.7.10.4 >> From mboxrd@z Thu Jan 1 00:00:00 1970 From: jiang.liu@linux.intel.com (Jiang Liu) Date: Wed, 9 Sep 2015 14:38:02 +0800 Subject: [Patch v5 1/6] ACPI/PCI: Enhance ACPI core to support sparse IO space In-Reply-To: <20150729203741.GC9640@google.com> References: <1433780448-18636-1-git-send-email-jiang.liu@linux.intel.com> <1433780448-18636-2-git-send-email-jiang.liu@linux.intel.com> <20150729203741.GC9640@google.com> Message-ID: <55EFD3CA.3050301@linux.intel.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2015/7/30 4:37, Bjorn Helgaas wrote: > On Tue, Jun 09, 2015 at 12:20:43AM +0800, Jiang Liu wrote: >> Enhance ACPI resource parsing interfaces to support sparse IO space, >> which will be used to share common code between x86 and IA64 later. >> >> Tested-by: Tony Luck >> Signed-off-by: Jiang Liu >> Reviewed-by: Hanjun Guo >> --- >> drivers/acpi/resource.c | 9 ++++++--- >> include/linux/ioport.h | 1 + >> 2 files changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c >> index 8244f013f210..fdcc73dad2c1 100644 >> --- a/drivers/acpi/resource.c >> +++ b/drivers/acpi/resource.c >> @@ -123,7 +123,7 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res) >> EXPORT_SYMBOL_GPL(acpi_dev_resource_memory); >> >> static void acpi_dev_ioresource_flags(struct resource *res, u64 len, >> - u8 io_decode) >> + u8 io_decode, u8 translation_type) >> { >> res->flags = IORESOURCE_IO; >> >> @@ -135,6 +135,8 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len, >> >> if (io_decode == ACPI_DECODE_16) >> res->flags |= IORESOURCE_IO_16BIT_ADDR; >> + if (translation_type == ACPI_SPARSE_TRANSLATION) >> + res->flags |= IORESOURCE_IO_SPARSE; >> } >> >> static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len, >> @@ -142,7 +144,7 @@ static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len, >> { >> res->start = start; >> res->end = start + len - 1; >> - acpi_dev_ioresource_flags(res, len, io_decode); >> + acpi_dev_ioresource_flags(res, len, io_decode, 0); >> } >> >> /** >> @@ -227,7 +229,8 @@ static bool acpi_decode_space(struct resource_win *win, >> acpi_dev_memresource_flags(res, len, wp); >> break; >> case ACPI_IO_RANGE: >> - acpi_dev_ioresource_flags(res, len, iodec); >> + acpi_dev_ioresource_flags(res, len, iodec, >> + addr->info.io.translation_type); >> break; >> case ACPI_BUS_NUMBER_RANGE: >> res->flags = IORESOURCE_BUS; >> diff --git a/include/linux/ioport.h b/include/linux/ioport.h >> index 388e3ae94f7a..24bea087e7af 100644 >> --- a/include/linux/ioport.h >> +++ b/include/linux/ioport.h >> @@ -94,6 +94,7 @@ struct resource { >> /* PnP I/O specific bits (IORESOURCE_BITS) */ >> #define IORESOURCE_IO_16BIT_ADDR (1<<0) >> #define IORESOURCE_IO_FIXED (1<<1) >> +#define IORESOURCE_IO_SPARSE (1<<2) > > I don't really like this bit. We adding a new generic IORESOURCE_* bit > just for a special case, and it's only used in one place, for one arch, > during enumeration. Hi Bjorn, Instead of defining a formal flag IORESOURCE_IO_SPARSE, we may reuse other field in struct resource to pass back the SPARSE flag, but that's a little dirty. For example, we may reuse res->name field to carry the SPARSE flag for the IA64 special case. Is that OK? Thanks! Gerry > > Don't we have a similar problem (struct resource can't express all the > information from an ACPI resource) for the _TRA value for bridge windows? > If it's a similar issue, we should solve it in a similar way. Per my understanding, we just ignore ACPI address space information which can't be expressed by struct resource. Thanks! Gerry > >> /* PCI ROM control bits (IORESOURCE_BITS) */ >> #define IORESOURCE_ROM_ENABLE (1<<0) /* ROM is enabled, same as PCI_ROM_ADDRESS_ENABLE */ >> -- >> 1.7.10.4 >>