public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: Suravee.Suthikulpanit@amd•com (Suravee Suthikulpanit)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 08/10] ACPI: GIC: Add ACPI helper functions to query irq-domain tokens for for GIC MSI and ITS
Date: Sun, 9 Aug 2015 15:02:33 +0700	[thread overview]
Message-ID: <55C70919.2050805@amd.com> (raw)
In-Reply-To: <55C0C5DD.6090607@arm.com>

Hi Marc,

Sorry for late reply. Please see my comments below.

On 8/4/15 21:02, Marc Zyngier wrote:
> On 29/07/15 11:08, Hanjun Guo wrote:
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd•com>
>>
>> This patch introduces acpi_gic_get_msi_token(), which returns irq-domain
>> token that can be used to look up MSI doamin of a device.
>> In both GIC MSI and ITS cases, the base_address specified in the GIC MSI
>> or GIC ITS structure is used as a token for MSI domain.
>>
>> In addition, this patch also provides low-level helper functions to parse
>> and query GIC MSI structure and GIC ITS from MADT. Once parsed, it keeps
>> a copy of the structure for use in subsequent queries to avoid having
>> to map and parse MADT multiple times.
>>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd•com>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro•org>
>> ---
>>   drivers/acpi/Makefile   |   1 +
>>   drivers/acpi/acpi_gic.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/acpi/acpi_gic.h |  23 +++++
>>   include/linux/acpi.h    |   1 +
>>   4 files changed, 259 insertions(+)
>>   create mode 100644 drivers/acpi/acpi_gic.c
>>   create mode 100644 include/acpi/acpi_gic.h
>>
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index 8321430..def54b9 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -54,6 +54,7 @@ acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
>>   acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>>   acpi-y				+= acpi_lpat.o
>>   acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
>> +acpi-$(CONFIG_ARM_GIC_ACPI)	+= acpi_gic.o
>>
>>   # These are (potentially) separate modules
>>
>> diff --git a/drivers/acpi/acpi_gic.c b/drivers/acpi/acpi_gic.c
>> new file mode 100644
>> index 0000000..11ee4eb
>> --- /dev/null
>> +++ b/drivers/acpi/acpi_gic.c
>
> I think this is starting badly. If this is GIC specific, it lives in
> drivers/irqchip. Nothing in drivers/acpi should be interrupt-controller
> specific at all. If there are things you need to expose through the ACPI
> layer, add some indirections.

OK, originally, I intended for this to be an intermediate layer b/w ACPI 
and irqchip, but I guess that's still not what you are looking for. I'll 
rework this again.

[....]
>> +
>> +#endif /*__ACPI_GIC_H__*/
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 04dd0bb..5d58b61 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -44,6 +44,7 @@
>>
>>   #include <acpi/acpi_bus.h>
>>   #include <acpi/acpi_drivers.h>
>> +#include <acpi/acpi_gic.h>
>
> Ah! No.
>
>>   #include <acpi/acpi_numa.h>
>>   #include <acpi/acpi_io.h>
>>   #include <asm/acpi.h>
>>
>
> Right. Very little of what is above belongs to the ACPI layer. What
> would belong here is a generic acpi_get_msi_domain_token(dev) that would
> call into a controller-specific function to parse the various tables and
> find out which GICv2m frame (or which ITS) is serving the given device.
> This would include parsing of the IORT structures if they are available.

The problem here is we would need to figure out the hook into *a 
controller-specific function* from a generic layer (ACPI in this case).

> For GICv2m, it should be simplistic: just return the domain_token of the
> v2m widget. For the ITS, it is slightly more complex, and there should
> be some specific backend for that. There is no ACPI support for the ITS
> yet, so that shouldn't be your concern at this point in time.
>
> Overall, drivers/acpi should be hardware agnostic (or at least aim for
> it), just like drivers/of is.

I think I understand how you don't like the current approach. Lemme try 
a different approach and send out another revision.

Thanks,
Suravee

  reply	other threads:[~2015-08-09  8:02 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-29 10:08 [PATCH v4 00/10] ACPI GIC Self-probing, GICv2m and GICv3 support Hanjun Guo
2015-07-29 10:08 ` [PATCH v4 01/10] irqchip / GIC: Add GIC version support in ACPI MADT Hanjun Guo
2015-08-04 12:06   ` Marc Zyngier
2015-08-05 12:40     ` Hanjun Guo
2015-08-05 12:57       ` Marc Zyngier
2015-08-05 13:11         ` Hanjun Guo
2015-07-29 10:08 ` [PATCH v4 02/10] ACPI / irqchip: Add self-probe infrastructure to initialize IRQ controller Hanjun Guo
2015-08-04 12:27   ` Marc Zyngier
2015-08-05 13:24     ` Hanjun Guo
2015-08-06 16:29       ` Marc Zyngier
2015-07-29 10:08 ` [PATCH v4 03/10] irqchip / GIC / ACPI: Use IRQCHIP_ACPI_DECLARE to simplify GICv2 init code Hanjun Guo
2015-07-29 10:08 ` [PATCH v4 04/10] irqchip / GICv3: Refactor gic_of_init() for GICv3 driver Hanjun Guo
2015-07-29 10:08 ` [PATCH v4 05/10] irqchip / GICv3: remove the useless comparision of device node in xlate Hanjun Guo
2015-07-29 10:08 ` [PATCH v4 06/10] irqchip / GICv3: Add ACPI support for GICv3+ initialization Hanjun Guo
2015-08-04 13:17   ` Marc Zyngier
2015-08-05 14:00     ` Hanjun Guo
2015-08-06 16:42       ` Marc Zyngier
2015-08-11  7:19         ` Hanjun Guo
2015-07-29 10:08 ` [PATCH v4 07/10] irqchip / GICv3 / ACPI: Add GICR support via GICC structures Hanjun Guo
2015-08-04 13:37   ` Marc Zyngier
2015-08-05 14:11     ` Hanjun Guo
2015-08-06 16:42       ` Marc Zyngier
2015-07-29 10:08 ` [PATCH v4 08/10] ACPI: GIC: Add ACPI helper functions to query irq-domain tokens for for GIC MSI and ITS Hanjun Guo
2015-08-04 14:02   ` Marc Zyngier
2015-08-09  8:02     ` Suravee Suthikulpanit [this message]
2015-07-29 10:08 ` [PATCH v4 09/10] PCI: ACPI: Bind GIC MSI frame to PCI host bridge Hanjun Guo
2015-08-04 14:04   ` Marc Zyngier
2015-08-07  8:42     ` Hanjun Guo
2015-08-09  8:02     ` Suravee Suthikulpanit
2015-08-07 10:03   ` Tomasz Nowicki
2015-08-07 10:48     ` Mark Brown
2015-08-07 12:06     ` Marc Zyngier
2015-07-29 10:08 ` [PATCH v4 10/10] irqchip / gicv2m: Introducing gicv2m_acpi_init() Hanjun Guo
2015-08-04 14:23   ` Marc Zyngier
2015-08-09  8:04     ` Suravee Suthikulpanit
2015-08-11 22:01 ` [PATCH v4 00/10] ACPI GIC Self-probing, GICv2m and GICv3 support Timur Tabi
2015-08-11 22:24   ` [Linaro-acpi] " G Gregory
2015-08-11 22:25   ` Marc Zyngier
2015-08-11 22:36     ` Timur Tabi
2015-08-11 22:48       ` Marc Zyngier
2015-08-11 23:33         ` Timur Tabi
2015-08-12  7:21           ` Marc Zyngier
2015-08-12 19:20             ` Timur Tabi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55C70919.2050805@amd.com \
    --to=suravee.suthikulpanit@amd$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox