Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-29 Thread Tomasz Nowicki

On 29.01.2015 16:29, Catalin Marinas wrote:

On Tue, Jan 27, 2015 at 04:12:08PM +, Grant Likely wrote:

On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier  wrote:

@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs 
*))
  void __init init_IRQ(void)
  {
 irqchip_init();
+
+   if (!handle_arch_irq)
+   acpi_gic_init();
+


Why isn't this called from irqchip_init? It would seem like the logical
spot to probe an interrupt controller.


What has been done here isn't an unusual choice. We've got stuff all
over the kernel that may or may not be implemented depending on what
the architecture supports. If the function call is renamed to
acpi_init_irq(), are you content?


My (full) suggestion was to do it like we've done it for DT, and I don't
think I varied much from this point of view. Yes, calling it
acpi_irq_init() would be a good start, and having the ACPI-compatible
irqchips to be self-probable even better.



Hell, if nobody beats me to it, maybe I'll just write that code, with
proper entry points in the various GIC drivers. Yes, this is
infrastructure. Maybe it is grossly overdesigned. But I really spend too
much time dealing with the crap that people are happy to pile on top of
the GIC code to be madly enthusiastic about the general "good enough"
attitude.



Or to put it in a slightly more diplomatic way: If ACPI is to be our
future, can we please make the future look a bit better?


Hi Marc,

As per our off-list discussion, I completely agree. We don't want to
be adding hack upon hack, and I will be first in line to NAK any
patches taking that approach. However, for this initial series, it
only supports exactly one GIC that can be set up by ACPI. Can we agree
to leave it as is in this series, with the agreement that it will be
replaced for v2m and v3 support with a proper pluggable initializer?


Can we at least call it acpi_init_irq() and avoid #including
gic-specific header files? IOW hide the apci_gic_init() behind some
generically named macro until the full solution is in place.



Yes, we will move away gic specific bits from here.

Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-29 Thread Catalin Marinas
On Tue, Jan 27, 2015 at 04:12:08PM +, Grant Likely wrote:
> On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier  wrote:
>  @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
>  pt_regs *))
>   void __init init_IRQ(void)
>   {
>  irqchip_init();
>  +
>  +   if (!handle_arch_irq)
>  +   acpi_gic_init();
>  +
> >>>
> >>> Why isn't this called from irqchip_init? It would seem like the logical
> >>> spot to probe an interrupt controller.
> >>
> >> What has been done here isn't an unusual choice. We've got stuff all
> >> over the kernel that may or may not be implemented depending on what
> >> the architecture supports. If the function call is renamed to
> >> acpi_init_irq(), are you content?
> >
> > My (full) suggestion was to do it like we've done it for DT, and I don't
> > think I varied much from this point of view. Yes, calling it
> > acpi_irq_init() would be a good start, and having the ACPI-compatible
> > irqchips to be self-probable even better.
> >
> > 
> >
> > Hell, if nobody beats me to it, maybe I'll just write that code, with
> > proper entry points in the various GIC drivers. Yes, this is
> > infrastructure. Maybe it is grossly overdesigned. But I really spend too
> > much time dealing with the crap that people are happy to pile on top of
> > the GIC code to be madly enthusiastic about the general "good enough"
> > attitude.
> >
> > 
> >
> > Or to put it in a slightly more diplomatic way: If ACPI is to be our
> > future, can we please make the future look a bit better?
> 
> Hi Marc,
> 
> As per our off-list discussion, I completely agree. We don't want to
> be adding hack upon hack, and I will be first in line to NAK any
> patches taking that approach. However, for this initial series, it
> only supports exactly one GIC that can be set up by ACPI. Can we agree
> to leave it as is in this series, with the agreement that it will be
> replaced for v2m and v3 support with a proper pluggable initializer?

Can we at least call it acpi_init_irq() and avoid #including
gic-specific header files? IOW hide the apci_gic_init() behind some
generically named macro until the full solution is in place.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-29 Thread Catalin Marinas
On Tue, Jan 27, 2015 at 04:12:08PM +, Grant Likely wrote:
 On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier marc.zyng...@arm.com wrote:
  @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
  pt_regs *))
   void __init init_IRQ(void)
   {
  irqchip_init();
  +
  +   if (!handle_arch_irq)
  +   acpi_gic_init();
  +
 
  Why isn't this called from irqchip_init? It would seem like the logical
  spot to probe an interrupt controller.
 
  What has been done here isn't an unusual choice. We've got stuff all
  over the kernel that may or may not be implemented depending on what
  the architecture supports. If the function call is renamed to
  acpi_init_irq(), are you content?
 
  My (full) suggestion was to do it like we've done it for DT, and I don't
  think I varied much from this point of view. Yes, calling it
  acpi_irq_init() would be a good start, and having the ACPI-compatible
  irqchips to be self-probable even better.
 
  lack-of-sleep-rant
 
  Hell, if nobody beats me to it, maybe I'll just write that code, with
  proper entry points in the various GIC drivers. Yes, this is
  infrastructure. Maybe it is grossly overdesigned. But I really spend too
  much time dealing with the crap that people are happy to pile on top of
  the GIC code to be madly enthusiastic about the general good enough
  attitude.
 
  /lack-of-sleep-rant
 
  Or to put it in a slightly more diplomatic way: If ACPI is to be our
  future, can we please make the future look a bit better?
 
 Hi Marc,
 
 As per our off-list discussion, I completely agree. We don't want to
 be adding hack upon hack, and I will be first in line to NAK any
 patches taking that approach. However, for this initial series, it
 only supports exactly one GIC that can be set up by ACPI. Can we agree
 to leave it as is in this series, with the agreement that it will be
 replaced for v2m and v3 support with a proper pluggable initializer?

Can we at least call it acpi_init_irq() and avoid #including
gic-specific header files? IOW hide the apci_gic_init() behind some
generically named macro until the full solution is in place.

-- 
Catalin
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-29 Thread Tomasz Nowicki

On 29.01.2015 16:29, Catalin Marinas wrote:

On Tue, Jan 27, 2015 at 04:12:08PM +, Grant Likely wrote:

On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier marc.zyng...@arm.com wrote:

@@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct pt_regs 
*))
  void __init init_IRQ(void)
  {
 irqchip_init();
+
+   if (!handle_arch_irq)
+   acpi_gic_init();
+


Why isn't this called from irqchip_init? It would seem like the logical
spot to probe an interrupt controller.


What has been done here isn't an unusual choice. We've got stuff all
over the kernel that may or may not be implemented depending on what
the architecture supports. If the function call is renamed to
acpi_init_irq(), are you content?


My (full) suggestion was to do it like we've done it for DT, and I don't
think I varied much from this point of view. Yes, calling it
acpi_irq_init() would be a good start, and having the ACPI-compatible
irqchips to be self-probable even better.

lack-of-sleep-rant

Hell, if nobody beats me to it, maybe I'll just write that code, with
proper entry points in the various GIC drivers. Yes, this is
infrastructure. Maybe it is grossly overdesigned. But I really spend too
much time dealing with the crap that people are happy to pile on top of
the GIC code to be madly enthusiastic about the general good enough
attitude.

/lack-of-sleep-rant

Or to put it in a slightly more diplomatic way: If ACPI is to be our
future, can we please make the future look a bit better?


Hi Marc,

As per our off-list discussion, I completely agree. We don't want to
be adding hack upon hack, and I will be first in line to NAK any
patches taking that approach. However, for this initial series, it
only supports exactly one GIC that can be set up by ACPI. Can we agree
to leave it as is in this series, with the agreement that it will be
replaced for v2m and v3 support with a proper pluggable initializer?


Can we at least call it acpi_init_irq() and avoid #including
gic-specific header files? IOW hide the apci_gic_init() behind some
generically named macro until the full solution is in place.



Yes, we will move away gic specific bits from here.

Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-27 Thread Grant Likely
On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier  wrote:
 @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
 pt_regs *))
  void __init init_IRQ(void)
  {
 irqchip_init();
 +
 +   if (!handle_arch_irq)
 +   acpi_gic_init();
 +
>>>
>>> Why isn't this called from irqchip_init? It would seem like the logical
>>> spot to probe an interrupt controller.
>>
>> What has been done here isn't an unusual choice. We've got stuff all
>> over the kernel that may or may not be implemented depending on what
>> the architecture supports. If the function call is renamed to
>> acpi_init_irq(), are you content?
>
> My (full) suggestion was to do it like we've done it for DT, and I don't
> think I varied much from this point of view. Yes, calling it
> acpi_irq_init() would be a good start, and having the ACPI-compatible
> irqchips to be self-probable even better.
>
> 
>
> Hell, if nobody beats me to it, maybe I'll just write that code, with
> proper entry points in the various GIC drivers. Yes, this is
> infrastructure. Maybe it is grossly overdesigned. But I really spend too
> much time dealing with the crap that people are happy to pile on top of
> the GIC code to be madly enthusiastic about the general "good enough"
> attitude.
>
> 
>
> Or to put it in a slightly more diplomatic way: If ACPI is to be our
> future, can we please make the future look a bit better?

Hi Marc,

As per our off-list discussion, I completely agree. We don't want to
be adding hack upon hack, and I will be first in line to NAK any
patches taking that approach. However, for this initial series, it
only supports exactly one GIC that can be set up by ACPI. Can we agree
to leave it as is in this series, with the agreement that it will be
replaced for v2m and v3 support with a proper pluggable initializer?
Tomasz is currently working on getting that change ready, but the
logistics are simpler if this series isn't blocked on that change.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-27 Thread Grant Likely
On Fri, Jan 16, 2015 at 2:37 PM, Marc Zyngier marc.zyng...@arm.com wrote:
 @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
 pt_regs *))
  void __init init_IRQ(void)
  {
 irqchip_init();
 +
 +   if (!handle_arch_irq)
 +   acpi_gic_init();
 +

 Why isn't this called from irqchip_init? It would seem like the logical
 spot to probe an interrupt controller.

 What has been done here isn't an unusual choice. We've got stuff all
 over the kernel that may or may not be implemented depending on what
 the architecture supports. If the function call is renamed to
 acpi_init_irq(), are you content?

 My (full) suggestion was to do it like we've done it for DT, and I don't
 think I varied much from this point of view. Yes, calling it
 acpi_irq_init() would be a good start, and having the ACPI-compatible
 irqchips to be self-probable even better.

 lack-of-sleep-rant

 Hell, if nobody beats me to it, maybe I'll just write that code, with
 proper entry points in the various GIC drivers. Yes, this is
 infrastructure. Maybe it is grossly overdesigned. But I really spend too
 much time dealing with the crap that people are happy to pile on top of
 the GIC code to be madly enthusiastic about the general good enough
 attitude.

 /lack-of-sleep-rant

 Or to put it in a slightly more diplomatic way: If ACPI is to be our
 future, can we please make the future look a bit better?

Hi Marc,

As per our off-list discussion, I completely agree. We don't want to
be adding hack upon hack, and I will be first in line to NAK any
patches taking that approach. However, for this initial series, it
only supports exactly one GIC that can be set up by ACPI. Can we agree
to leave it as is in this series, with the agreement that it will be
replaced for v2m and v3 support with a proper pluggable initializer?
Tomasz is currently working on getting that change ready, but the
logistics are simpler if this series isn't blocked on that change.

g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-23 Thread Hanjun Guo

On 2015年01月22日 22:46, Marc Zyngier wrote:

Hi Hanjun,

On 22/01/15 12:46, Hanjun Guo wrote:

Hi Marc,

We (Tomasz, Suravee and me) are working on supporting stacked domain on
ACPI, and rework GIC ACPI related patch, before we going further, we
need your guidance to see if we are going the right direction.

- You said that we spread GIC related code every where, so how
  about put all the ACPI related GIC init code in one file under
  drivers/irqchip/ with name irq-gic-acpi.c?


That would certainly be an improvement.


- ACPI only support one GICD for now, so we assume that there
 only one gicv2/v3 core domain and every device not using MSI
  will refer to that irqdomain in default.


That's good enough, provided that nobody comes up with any form of
chained interrupt controller (in whatever way that's implemented). ACPI
doesn't seem to cater for that anyway.

But default domains are only a quick optimization (it is only there to
cope with code that didn't know about irq domains at all). What we need
is a proper integration of the ACPI namespace in the irq domain code.
Being able to lookup a domain by ACPI table, for example (just like
irq_find_host returns the domain associated to a DT node).


I totally agree, so we have different ways to handle devices using
MSI and devices not using MSI.

  - Devices using MSI, there is a IORT table to map the dev id to ITS,
then every device can easily lookup a domain;

  - Devices not using MSI, we only present the GSI (hwirq num) used
in DSDT by this device to OS, no property to indicate its interrupt
parent, since we have only one domain for now, we can just let
those devices refer to the gic core domain, and it will work.

For x86, devices using GSI have no such problem, because every
IOAPIC have the GSI base reported and how many GSI is supported,
so with a GSI num, we can easily find a IOAPIC then pointing to
its irqdomain, can we do something similar to x86 here?



This would ensure that we can reuse most of the existing code (stacked
domains, per-device MSI domains [WIP]) without too much effort.


I agree.

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-23 Thread Hanjun Guo

On 2015年01月22日 22:46, Marc Zyngier wrote:

Hi Hanjun,

On 22/01/15 12:46, Hanjun Guo wrote:

Hi Marc,

We (Tomasz, Suravee and me) are working on supporting stacked domain on
ACPI, and rework GIC ACPI related patch, before we going further, we
need your guidance to see if we are going the right direction.

- You said that we spread GIC related code every where, so how
  about put all the ACPI related GIC init code in one file under
  drivers/irqchip/ with name irq-gic-acpi.c?


That would certainly be an improvement.


- ACPI only support one GICD for now, so we assume that there
 only one gicv2/v3 core domain and every device not using MSI
  will refer to that irqdomain in default.


That's good enough, provided that nobody comes up with any form of
chained interrupt controller (in whatever way that's implemented). ACPI
doesn't seem to cater for that anyway.

But default domains are only a quick optimization (it is only there to
cope with code that didn't know about irq domains at all). What we need
is a proper integration of the ACPI namespace in the irq domain code.
Being able to lookup a domain by ACPI table, for example (just like
irq_find_host returns the domain associated to a DT node).


I totally agree, so we have different ways to handle devices using
MSI and devices not using MSI.

  - Devices using MSI, there is a IORT table to map the dev id to ITS,
then every device can easily lookup a domain;

  - Devices not using MSI, we only present the GSI (hwirq num) used
in DSDT by this device to OS, no property to indicate its interrupt
parent, since we have only one domain for now, we can just let
those devices refer to the gic core domain, and it will work.

For x86, devices using GSI have no such problem, because every
IOAPIC have the GSI base reported and how many GSI is supported,
so with a GSI num, we can easily find a IOAPIC then pointing to
its irqdomain, can we do something similar to x86 here?



This would ensure that we can reuse most of the existing code (stacked
domains, per-device MSI domains [WIP]) without too much effort.


I agree.

Thanks
Hanjun
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-22 Thread Marc Zyngier
Hi Hanjun,

On 22/01/15 12:46, Hanjun Guo wrote:
> Hi Marc,
> 
> We (Tomasz, Suravee and me) are working on supporting stacked domain on
> ACPI, and rework GIC ACPI related patch, before we going further, we
> need your guidance to see if we are going the right direction.
> 
>- You said that we spread GIC related code every where, so how
>  about put all the ACPI related GIC init code in one file under
>  drivers/irqchip/ with name irq-gic-acpi.c?

That would certainly be an improvement.

>- ACPI only support one GICD for now, so we assume that there
> only one gicv2/v3 core domain and every device not using MSI
>  will refer to that irqdomain in default.

That's good enough, provided that nobody comes up with any form of
chained interrupt controller (in whatever way that's implemented). ACPI
doesn't seem to cater for that anyway.

But default domains are only a quick optimization (it is only there to
cope with code that didn't know about irq domains at all). What we need
is a proper integration of the ACPI namespace in the irq domain code.
Being able to lookup a domain by ACPI table, for example (just like
irq_find_host returns the domain associated to a DT node).

This would ensure that we can reuse most of the existing code (stacked
domains, per-device MSI domains [WIP]) without too much effort.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-22 Thread Hanjun Guo

Hi Marc,

We (Tomasz, Suravee and me) are working on supporting stacked domain on
ACPI, and rework GIC ACPI related patch, before we going further, we
need your guidance to see if we are going the right direction.

  - You said that we spread GIC related code every where, so how
about put all the ACPI related GIC init code in one file under
drivers/irqchip/ with name irq-gic-acpi.c?

  - ACPI only support one GICD for now, so we assume that there
   only one gicv2/v3 core domain and every device not using MSI
will refer to that irqdomain in default.

Are we going the right direction?

Thanks
Hanjun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-22 Thread Hanjun Guo

Hi Marc,

We (Tomasz, Suravee and me) are working on supporting stacked domain on
ACPI, and rework GIC ACPI related patch, before we going further, we
need your guidance to see if we are going the right direction.

  - You said that we spread GIC related code every where, so how
about put all the ACPI related GIC init code in one file under
drivers/irqchip/ with name irq-gic-acpi.c?

  - ACPI only support one GICD for now, so we assume that there
   only one gicv2/v3 core domain and every device not using MSI
will refer to that irqdomain in default.

Are we going the right direction?

Thanks
Hanjun
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-22 Thread Marc Zyngier
Hi Hanjun,

On 22/01/15 12:46, Hanjun Guo wrote:
 Hi Marc,
 
 We (Tomasz, Suravee and me) are working on supporting stacked domain on
 ACPI, and rework GIC ACPI related patch, before we going further, we
 need your guidance to see if we are going the right direction.
 
- You said that we spread GIC related code every where, so how
  about put all the ACPI related GIC init code in one file under
  drivers/irqchip/ with name irq-gic-acpi.c?

That would certainly be an improvement.

- ACPI only support one GICD for now, so we assume that there
 only one gicv2/v3 core domain and every device not using MSI
  will refer to that irqdomain in default.

That's good enough, provided that nobody comes up with any form of
chained interrupt controller (in whatever way that's implemented). ACPI
doesn't seem to cater for that anyway.

But default domains are only a quick optimization (it is only there to
cope with code that didn't know about irq domains at all). What we need
is a proper integration of the ACPI namespace in the irq domain code.
Being able to lookup a domain by ACPI table, for example (just like
irq_find_host returns the domain associated to a DT node).

This would ensure that we can reuse most of the existing code (stacked
domains, per-device MSI domains [WIP]) without too much effort.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-20 Thread Jon Masters
On 01/20/2015 05:40 AM, Tomasz Nowicki wrote:
> Hi Marc,
> 
> On 16.01.2015 12:15, Marc Zyngier wrote:
>> On 14/01/15 15:05, Hanjun Guo wrote:
>>> From: Tomasz Nowicki 
>>>
>>> ACPI kernel uses MADT table for proper GIC initialization. It needs to
>>> parse GIC related subtables, collect CPU interface and distributor
>>> addresses and call driver initialization function (which is hardware
>>> abstraction agnostic). In a similar way, FDT initialize GICv1/2.
>>>
>>> NOTE: This commit allow to initialize GICv1/2 basic functionality.
>>> GICv2 vitalization extension, GICv3/4 and ITS are considered as next
>>> steps.
>>
>> And so is GICv2m, apparently (see below).
>>
>>>
>>> Tested-by: Suravee Suthikulpanit 
>>> Tested-by: Yijing Wang 
>>> Signed-off-by: Tomasz Nowicki 
>>> Signed-off-by: Hanjun Guo 
>>> ---
>>>   arch/arm64/kernel/acpi.c |  26 +
>>>   drivers/irqchip/irq-gic.c| 108 
>>> +++
>>>   drivers/irqchip/irqchip.c|   3 +
>>>   include/linux/irqchip/arm-gic-acpi.h |  31 ++
>>>   4 files changed, 168 insertions(+)
>>>   create mode 100644 include/linux/irqchip/arm-gic-acpi.h
>>>
>>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>>> index c3e24c4..ea3c9fc 100644
>>> --- a/arch/arm64/kernel/acpi.c
>>> +++ b/arch/arm64/kernel/acpi.c
>>> @@ -23,6 +23,7 @@
>>>   #include 
>>>   #include 
>>>   #include 
>>> +#include 
>>>
>>>   #include 
>>>   #include 
>>> @@ -315,6 +316,31 @@ void __init acpi_boot_table_init(void)
>>> pr_err("Can't find FADT or error happened during parsing 
>>> FADT\n");
>>>   }
>>>
>>> +void __init acpi_gic_init(void)
>>> +{
>>> +   struct acpi_table_header *table;
>>> +   acpi_status status;
>>> +   acpi_size tbl_size;
>>> +   int err;
>>> +
>>> +   if (acpi_disabled)
>>> +   return;
>>> +
>>> +   status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, , _size);
>>> +   if (ACPI_FAILURE(status)) {
>>> +   const char *msg = acpi_format_exception(status);
>>> +
>>> +   pr_err("Failed to get MADT table, %s\n", msg);
>>> +   return;
>>> +   }
>>> +
>>> +   err = gic_v2_acpi_init(table);
>>> +   if (err)
>>> +   pr_err("Failed to initialize GIC IRQ controller");
>>> +
>>> +   early_acpi_os_unmap_memory((char *)table, tbl_size);
>>> +}
>>> +
>>>   static int __init parse_acpi(char *arg)
>>>   {
>>> if (!arg)
>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>> index d617ee5..89a8120 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -33,12 +33,14 @@
>>>   #include 
>>>   #include 
>>>   #include 
>>> +#include 
>>>   #include 
>>>   #include 
>>>   #include 
>>>   #include 
>>>   #include 
>>>   #include 
>>> +#include 
>>>
>>>   #include 
>>>   #include 
>>> @@ -1083,3 +1085,109 @@ IRQCHIP_DECLARE(msm_8660_qgic, 
>>> "qcom,msm-8660-qgic", gic_of_init);
>>>   IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>>>
>>>   #endif
>>> +
>>> +#ifdef CONFIG_ACPI
>>> +static phys_addr_t dist_phy_base, cpu_phy_base;
>>> +static int cpu_base_assigned;
>>> +
>>> +static int __init
>>> +gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
>>> +   const unsigned long end)
>>> +{
>>> +   struct acpi_madt_generic_interrupt *processor;
>>> +   phys_addr_t gic_cpu_base;
>>> +
>>> +   processor = (struct acpi_madt_generic_interrupt *)header;
>>> +
>>> +   if (BAD_MADT_ENTRY(processor, end))
>>> +   return -EINVAL;
>>> +
>>> +   /*
>>> +* There is no support for non-banked GICv1/2 register in ACPI spec.
>>> +* All CPU interface addresses have to be the same.
>>> +*/
>>> +   gic_cpu_base = processor->base_address;
>>> +   if (cpu_base_assigned && gic_cpu_base != cpu_phy_base)
>>> +   return -EFAULT;
>>
>> EFAULT? That feels weird. This error code should be returned if an
>> access would generate (or has actually generated) a fault, but this is
>> not the case here. Same for the other cases below.
> Right, will fix that and other cases too.
> 
>>
>>> +
>>> +   cpu_phy_base = gic_cpu_base;
>>> +   cpu_base_assigned = 1;
>>> +   return 0;
>>> +}
>>> +
>>> +static int __init
>>> +gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
>>> +   const unsigned long end)
>>> +{
>>> +   struct acpi_madt_generic_distributor *dist;
>>> +
>>> +   dist = (struct acpi_madt_generic_distributor *)header;
>>> +
>>> +   if (BAD_MADT_ENTRY(dist, end))
>>> +   return -EINVAL;
>>> +
>>> +   dist_phy_base = dist->base_address;
>>> +   return 0;
>>> +}
>>> +
>>> +int __init
>>> +gic_v2_acpi_init(struct acpi_table_header *table)
>>> +{
>>> +   void __iomem *cpu_base, *dist_base;
>>> +   int count;
>>> +
>>> +   /* Collect CPU base addresses */
>>> +   count = acpi_parse_entries(ACPI_SIG_MADT,
>>> +  sizeof(struct acpi_table_madt),
>>> +  gic_acpi_parse_madt_cpu, table,

Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-20 Thread Tomasz Nowicki

Hi Marc,

On 16.01.2015 12:15, Marc Zyngier wrote:

On 14/01/15 15:05, Hanjun Guo wrote:

From: Tomasz Nowicki 

ACPI kernel uses MADT table for proper GIC initialization. It needs to
parse GIC related subtables, collect CPU interface and distributor
addresses and call driver initialization function (which is hardware
abstraction agnostic). In a similar way, FDT initialize GICv1/2.

NOTE: This commit allow to initialize GICv1/2 basic functionality.
GICv2 vitalization extension, GICv3/4 and ITS are considered as next
steps.


And so is GICv2m, apparently (see below).



Tested-by: Suravee Suthikulpanit 
Tested-by: Yijing Wang 
Signed-off-by: Tomasz Nowicki 
Signed-off-by: Hanjun Guo 
---
  arch/arm64/kernel/acpi.c |  26 +
  drivers/irqchip/irq-gic.c| 108 +++
  drivers/irqchip/irqchip.c|   3 +
  include/linux/irqchip/arm-gic-acpi.h |  31 ++
  4 files changed, 168 insertions(+)
  create mode 100644 include/linux/irqchip/arm-gic-acpi.h

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index c3e24c4..ea3c9fc 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -23,6 +23,7 @@
  #include 
  #include 
  #include 
+#include 

  #include 
  #include 
@@ -315,6 +316,31 @@ void __init acpi_boot_table_init(void)
pr_err("Can't find FADT or error happened during parsing 
FADT\n");
  }

+void __init acpi_gic_init(void)
+{
+   struct acpi_table_header *table;
+   acpi_status status;
+   acpi_size tbl_size;
+   int err;
+
+   if (acpi_disabled)
+   return;
+
+   status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, , _size);
+   if (ACPI_FAILURE(status)) {
+   const char *msg = acpi_format_exception(status);
+
+   pr_err("Failed to get MADT table, %s\n", msg);
+   return;
+   }
+
+   err = gic_v2_acpi_init(table);
+   if (err)
+   pr_err("Failed to initialize GIC IRQ controller");
+
+   early_acpi_os_unmap_memory((char *)table, tbl_size);
+}
+
  static int __init parse_acpi(char *arg)
  {
if (!arg)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d617ee5..89a8120 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -33,12 +33,14 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
+#include 

  #include 
  #include 
@@ -1083,3 +1085,109 @@ IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", 
gic_of_init);
  IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);

  #endif
+
+#ifdef CONFIG_ACPI
+static phys_addr_t dist_phy_base, cpu_phy_base;
+static int cpu_base_assigned;
+
+static int __init
+gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   struct acpi_madt_generic_interrupt *processor;
+   phys_addr_t gic_cpu_base;
+
+   processor = (struct acpi_madt_generic_interrupt *)header;
+
+   if (BAD_MADT_ENTRY(processor, end))
+   return -EINVAL;
+
+   /*
+* There is no support for non-banked GICv1/2 register in ACPI spec.
+* All CPU interface addresses have to be the same.
+*/
+   gic_cpu_base = processor->base_address;
+   if (cpu_base_assigned && gic_cpu_base != cpu_phy_base)
+   return -EFAULT;


EFAULT? That feels weird. This error code should be returned if an
access would generate (or has actually generated) a fault, but this is
not the case here. Same for the other cases below.

Right, will fix that and other cases too.




+
+   cpu_phy_base = gic_cpu_base;
+   cpu_base_assigned = 1;
+   return 0;
+}
+
+static int __init
+gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   struct acpi_madt_generic_distributor *dist;
+
+   dist = (struct acpi_madt_generic_distributor *)header;
+
+   if (BAD_MADT_ENTRY(dist, end))
+   return -EINVAL;
+
+   dist_phy_base = dist->base_address;
+   return 0;
+}
+
+int __init
+gic_v2_acpi_init(struct acpi_table_header *table)
+{
+   void __iomem *cpu_base, *dist_base;
+   int count;
+
+   /* Collect CPU base addresses */
+   count = acpi_parse_entries(ACPI_SIG_MADT,
+  sizeof(struct acpi_table_madt),
+  gic_acpi_parse_madt_cpu, table,
+  ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+   if (count < 0) {
+   pr_err("Error during GICC entries parsing\n");
+   return -EFAULT;
+   } else if (!count) {
+   pr_err("No valid GICC entries exist\n");
+   return -EINVAL;
+   }
+
+   /*
+* Find distributor base address. We expect one distributor entry since
+* ACPI 5.1 spec neither support multi-GIC 

Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-20 Thread Tomasz Nowicki

Hi Marc,

On 16.01.2015 12:15, Marc Zyngier wrote:

On 14/01/15 15:05, Hanjun Guo wrote:

From: Tomasz Nowicki tomasz.nowi...@linaro.org

ACPI kernel uses MADT table for proper GIC initialization. It needs to
parse GIC related subtables, collect CPU interface and distributor
addresses and call driver initialization function (which is hardware
abstraction agnostic). In a similar way, FDT initialize GICv1/2.

NOTE: This commit allow to initialize GICv1/2 basic functionality.
GICv2 vitalization extension, GICv3/4 and ITS are considered as next
steps.


And so is GICv2m, apparently (see below).



Tested-by: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
Tested-by: Yijing Wang wangyij...@huawei.com
Signed-off-by: Tomasz Nowicki tomasz.nowi...@linaro.org
Signed-off-by: Hanjun Guo hanjun@linaro.org
---
  arch/arm64/kernel/acpi.c |  26 +
  drivers/irqchip/irq-gic.c| 108 +++
  drivers/irqchip/irqchip.c|   3 +
  include/linux/irqchip/arm-gic-acpi.h |  31 ++
  4 files changed, 168 insertions(+)
  create mode 100644 include/linux/irqchip/arm-gic-acpi.h

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index c3e24c4..ea3c9fc 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -23,6 +23,7 @@
  #include linux/irqdomain.h
  #include linux/bootmem.h
  #include linux/smp.h
+#include linux/irqchip/arm-gic-acpi.h

  #include asm/cputype.h
  #include asm/cpu_ops.h
@@ -315,6 +316,31 @@ void __init acpi_boot_table_init(void)
pr_err(Can't find FADT or error happened during parsing 
FADT\n);
  }

+void __init acpi_gic_init(void)
+{
+   struct acpi_table_header *table;
+   acpi_status status;
+   acpi_size tbl_size;
+   int err;
+
+   if (acpi_disabled)
+   return;
+
+   status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, table, tbl_size);
+   if (ACPI_FAILURE(status)) {
+   const char *msg = acpi_format_exception(status);
+
+   pr_err(Failed to get MADT table, %s\n, msg);
+   return;
+   }
+
+   err = gic_v2_acpi_init(table);
+   if (err)
+   pr_err(Failed to initialize GIC IRQ controller);
+
+   early_acpi_os_unmap_memory((char *)table, tbl_size);
+}
+
  static int __init parse_acpi(char *arg)
  {
if (!arg)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d617ee5..89a8120 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -33,12 +33,14 @@
  #include linux/of.h
  #include linux/of_address.h
  #include linux/of_irq.h
+#include linux/acpi.h
  #include linux/irqdomain.h
  #include linux/interrupt.h
  #include linux/percpu.h
  #include linux/slab.h
  #include linux/irqchip/chained_irq.h
  #include linux/irqchip/arm-gic.h
+#include linux/irqchip/arm-gic-acpi.h

  #include asm/cputype.h
  #include asm/irq.h
@@ -1083,3 +1085,109 @@ IRQCHIP_DECLARE(msm_8660_qgic, qcom,msm-8660-qgic, 
gic_of_init);
  IRQCHIP_DECLARE(msm_qgic2, qcom,msm-qgic2, gic_of_init);

  #endif
+
+#ifdef CONFIG_ACPI
+static phys_addr_t dist_phy_base, cpu_phy_base;
+static int cpu_base_assigned;
+
+static int __init
+gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   struct acpi_madt_generic_interrupt *processor;
+   phys_addr_t gic_cpu_base;
+
+   processor = (struct acpi_madt_generic_interrupt *)header;
+
+   if (BAD_MADT_ENTRY(processor, end))
+   return -EINVAL;
+
+   /*
+* There is no support for non-banked GICv1/2 register in ACPI spec.
+* All CPU interface addresses have to be the same.
+*/
+   gic_cpu_base = processor-base_address;
+   if (cpu_base_assigned  gic_cpu_base != cpu_phy_base)
+   return -EFAULT;


EFAULT? That feels weird. This error code should be returned if an
access would generate (or has actually generated) a fault, but this is
not the case here. Same for the other cases below.

Right, will fix that and other cases too.




+
+   cpu_phy_base = gic_cpu_base;
+   cpu_base_assigned = 1;
+   return 0;
+}
+
+static int __init
+gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
+   const unsigned long end)
+{
+   struct acpi_madt_generic_distributor *dist;
+
+   dist = (struct acpi_madt_generic_distributor *)header;
+
+   if (BAD_MADT_ENTRY(dist, end))
+   return -EINVAL;
+
+   dist_phy_base = dist-base_address;
+   return 0;
+}
+
+int __init
+gic_v2_acpi_init(struct acpi_table_header *table)
+{
+   void __iomem *cpu_base, *dist_base;
+   int count;
+
+   /* Collect CPU base addresses */
+   count = acpi_parse_entries(ACPI_SIG_MADT,
+  sizeof(struct acpi_table_madt),
+  gic_acpi_parse_madt_cpu, table,
+  

Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-20 Thread Jon Masters
On 01/20/2015 05:40 AM, Tomasz Nowicki wrote:
 Hi Marc,
 
 On 16.01.2015 12:15, Marc Zyngier wrote:
 On 14/01/15 15:05, Hanjun Guo wrote:
 From: Tomasz Nowicki tomasz.nowi...@linaro.org

 ACPI kernel uses MADT table for proper GIC initialization. It needs to
 parse GIC related subtables, collect CPU interface and distributor
 addresses and call driver initialization function (which is hardware
 abstraction agnostic). In a similar way, FDT initialize GICv1/2.

 NOTE: This commit allow to initialize GICv1/2 basic functionality.
 GICv2 vitalization extension, GICv3/4 and ITS are considered as next
 steps.

 And so is GICv2m, apparently (see below).


 Tested-by: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
 Tested-by: Yijing Wang wangyij...@huawei.com
 Signed-off-by: Tomasz Nowicki tomasz.nowi...@linaro.org
 Signed-off-by: Hanjun Guo hanjun@linaro.org
 ---
   arch/arm64/kernel/acpi.c |  26 +
   drivers/irqchip/irq-gic.c| 108 
 +++
   drivers/irqchip/irqchip.c|   3 +
   include/linux/irqchip/arm-gic-acpi.h |  31 ++
   4 files changed, 168 insertions(+)
   create mode 100644 include/linux/irqchip/arm-gic-acpi.h

 diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
 index c3e24c4..ea3c9fc 100644
 --- a/arch/arm64/kernel/acpi.c
 +++ b/arch/arm64/kernel/acpi.c
 @@ -23,6 +23,7 @@
   #include linux/irqdomain.h
   #include linux/bootmem.h
   #include linux/smp.h
 +#include linux/irqchip/arm-gic-acpi.h

   #include asm/cputype.h
   #include asm/cpu_ops.h
 @@ -315,6 +316,31 @@ void __init acpi_boot_table_init(void)
 pr_err(Can't find FADT or error happened during parsing 
 FADT\n);
   }

 +void __init acpi_gic_init(void)
 +{
 +   struct acpi_table_header *table;
 +   acpi_status status;
 +   acpi_size tbl_size;
 +   int err;
 +
 +   if (acpi_disabled)
 +   return;
 +
 +   status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, table, tbl_size);
 +   if (ACPI_FAILURE(status)) {
 +   const char *msg = acpi_format_exception(status);
 +
 +   pr_err(Failed to get MADT table, %s\n, msg);
 +   return;
 +   }
 +
 +   err = gic_v2_acpi_init(table);
 +   if (err)
 +   pr_err(Failed to initialize GIC IRQ controller);
 +
 +   early_acpi_os_unmap_memory((char *)table, tbl_size);
 +}
 +
   static int __init parse_acpi(char *arg)
   {
 if (!arg)
 diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
 index d617ee5..89a8120 100644
 --- a/drivers/irqchip/irq-gic.c
 +++ b/drivers/irqchip/irq-gic.c
 @@ -33,12 +33,14 @@
   #include linux/of.h
   #include linux/of_address.h
   #include linux/of_irq.h
 +#include linux/acpi.h
   #include linux/irqdomain.h
   #include linux/interrupt.h
   #include linux/percpu.h
   #include linux/slab.h
   #include linux/irqchip/chained_irq.h
   #include linux/irqchip/arm-gic.h
 +#include linux/irqchip/arm-gic-acpi.h

   #include asm/cputype.h
   #include asm/irq.h
 @@ -1083,3 +1085,109 @@ IRQCHIP_DECLARE(msm_8660_qgic, 
 qcom,msm-8660-qgic, gic_of_init);
   IRQCHIP_DECLARE(msm_qgic2, qcom,msm-qgic2, gic_of_init);

   #endif
 +
 +#ifdef CONFIG_ACPI
 +static phys_addr_t dist_phy_base, cpu_phy_base;
 +static int cpu_base_assigned;
 +
 +static int __init
 +gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
 +   const unsigned long end)
 +{
 +   struct acpi_madt_generic_interrupt *processor;
 +   phys_addr_t gic_cpu_base;
 +
 +   processor = (struct acpi_madt_generic_interrupt *)header;
 +
 +   if (BAD_MADT_ENTRY(processor, end))
 +   return -EINVAL;
 +
 +   /*
 +* There is no support for non-banked GICv1/2 register in ACPI spec.
 +* All CPU interface addresses have to be the same.
 +*/
 +   gic_cpu_base = processor-base_address;
 +   if (cpu_base_assigned  gic_cpu_base != cpu_phy_base)
 +   return -EFAULT;

 EFAULT? That feels weird. This error code should be returned if an
 access would generate (or has actually generated) a fault, but this is
 not the case here. Same for the other cases below.
 Right, will fix that and other cases too.
 

 +
 +   cpu_phy_base = gic_cpu_base;
 +   cpu_base_assigned = 1;
 +   return 0;
 +}
 +
 +static int __init
 +gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
 +   const unsigned long end)
 +{
 +   struct acpi_madt_generic_distributor *dist;
 +
 +   dist = (struct acpi_madt_generic_distributor *)header;
 +
 +   if (BAD_MADT_ENTRY(dist, end))
 +   return -EINVAL;
 +
 +   dist_phy_base = dist-base_address;
 +   return 0;
 +}
 +
 +int __init
 +gic_v2_acpi_init(struct acpi_table_header *table)
 +{
 +   void __iomem *cpu_base, *dist_base;
 +   int count;
 +
 +   /* Collect CPU base addresses */
 +   count = acpi_parse_entries(ACPI_SIG_MADT,
 +  sizeof(struct acpi_table_madt),
 +  gic_acpi_parse_madt_cpu, table,
 +  

Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-16 Thread Marc Zyngier
On 16/01/15 13:54, Grant Likely wrote:
> On Fri, Jan 16, 2015 at 11:15 AM, Marc Zyngier  wrote:
>> On 14/01/15 15:05, Hanjun Guo wrote:
>>> From: Tomasz Nowicki 
>>>
>>> ACPI kernel uses MADT table for proper GIC initialization. It needs to
>>> parse GIC related subtables, collect CPU interface and distributor
>>> addresses and call driver initialization function (which is hardware
>>> abstraction agnostic). In a similar way, FDT initialize GICv1/2.
>>>
>>> NOTE: This commit allow to initialize GICv1/2 basic functionality.
>>> GICv2 vitalization extension, GICv3/4 and ITS are considered as next
>>> steps.
>>
>> And so is GICv2m, apparently (see below).
>>
>>>
>>> Tested-by: Suravee Suthikulpanit 
>>> Tested-by: Yijing Wang 
>>> Signed-off-by: Tomasz Nowicki 
>>> Signed-off-by: Hanjun Guo 
>>> ---
>>> + /*
>>> +  * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
>>> +  * as default IRQ domain to allow for GSI registration and GSI to IRQ
>>> +  * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
>>> +  */
>>> + gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>>
>> I assume you never tried to port the GICv2m driver to ACPI, right?
>> Because the above code actively prevents the GIC domain to be defined as
>> a stacked domain, making it impossible for the v2m widget to be
>> implemented on top of GIC. But maybe legacy interrupts are enough?
> 
> It's sufficient for what is supported right now, and easily changed in
> a patch series to add GICv2m support. This shouldn't be a blocking
> issue as it isn't actively dangerous. It is merely limited, and that
> is okay.
> 
>>> @@ -26,4 +27,6 @@ extern struct of_device_id __irqchip_of_table[];
>>>  void __init irqchip_init(void)
>>>  {
>>>   of_irq_init(__irqchip_of_table);
>>> +
>>> + acpi_gic_init();
>>
>> Have you realised that this file is probably compiled on multiple
>> architecture, none of which particularly cares about ACPI or the GIC?
>> This is (still) very ugly.
> 
> "very ugly?" That's overstating things a bit. We may quibble about the
> name, but we're just talking about a setup hook function that may or
> may not be implemented. How about we put acpi_irq_init() here and make
> it an inline macro that directly calls acpi_gic_init() when ACPI is
> enabled on AARCH64? Then the function can be extended by architectures
> as needed, and default to an empty inline otherwise. When (if) we have
> more than one hook that needs to be called from it, then we can
> refactor it to be more like of_irq_init().
> 
> As for other architectures calling this function, but not caring about
> ACPI, I believe it was your suggestion to put it here!  :-)
> 
> On Mon, 01 Sep 2014 18:35:18 +0100^M, Marc Zyngier  
> wrote:
>> On 01/09/14 15:57, Hanjun Guo wrote:
>>> @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
>>> pt_regs *))
>>>  void __init init_IRQ(void)
>>>  {
>>> irqchip_init();
>>> +
>>> +   if (!handle_arch_irq)
>>> +   acpi_gic_init();
>>> +
>>
>> Why isn't this called from irqchip_init? It would seem like the logical
>> spot to probe an interrupt controller.
> 
> What has been done here isn't an unusual choice. We've got stuff all
> over the kernel that may or may not be implemented depending on what
> the architecture supports. If the function call is renamed to
> acpi_init_irq(), are you content?

My (full) suggestion was to do it like we've done it for DT, and I don't
think I varied much from this point of view. Yes, calling it
acpi_irq_init() would be a good start, and having the ACPI-compatible
irqchips to be self-probable even better.



Hell, if nobody beats me to it, maybe I'll just write that code, with
proper entry points in the various GIC drivers. Yes, this is
infrastructure. Maybe it is grossly overdesigned. But I really spend too
much time dealing with the crap that people are happy to pile on top of
the GIC code to be madly enthusiastic about the general "good enough"
attitude.



Or to put it in a slightly more diplomatic way: If ACPI is to be our
future, can we please make the future look a bit better?

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-16 Thread Grant Likely
On Fri, Jan 16, 2015 at 11:15 AM, Marc Zyngier  wrote:
> On 14/01/15 15:05, Hanjun Guo wrote:
>> From: Tomasz Nowicki 
>>
>> ACPI kernel uses MADT table for proper GIC initialization. It needs to
>> parse GIC related subtables, collect CPU interface and distributor
>> addresses and call driver initialization function (which is hardware
>> abstraction agnostic). In a similar way, FDT initialize GICv1/2.
>>
>> NOTE: This commit allow to initialize GICv1/2 basic functionality.
>> GICv2 vitalization extension, GICv3/4 and ITS are considered as next
>> steps.
>
> And so is GICv2m, apparently (see below).
>
>>
>> Tested-by: Suravee Suthikulpanit 
>> Tested-by: Yijing Wang 
>> Signed-off-by: Tomasz Nowicki 
>> Signed-off-by: Hanjun Guo 
>> ---
>> + /*
>> +  * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
>> +  * as default IRQ domain to allow for GSI registration and GSI to IRQ
>> +  * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
>> +  */
>> + gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>
> I assume you never tried to port the GICv2m driver to ACPI, right?
> Because the above code actively prevents the GIC domain to be defined as
> a stacked domain, making it impossible for the v2m widget to be
> implemented on top of GIC. But maybe legacy interrupts are enough?

It's sufficient for what is supported right now, and easily changed in
a patch series to add GICv2m support. This shouldn't be a blocking
issue as it isn't actively dangerous. It is merely limited, and that
is okay.

>> @@ -26,4 +27,6 @@ extern struct of_device_id __irqchip_of_table[];
>>  void __init irqchip_init(void)
>>  {
>>   of_irq_init(__irqchip_of_table);
>> +
>> + acpi_gic_init();
>
> Have you realised that this file is probably compiled on multiple
> architecture, none of which particularly cares about ACPI or the GIC?
> This is (still) very ugly.

"very ugly?" That's overstating things a bit. We may quibble about the
name, but we're just talking about a setup hook function that may or
may not be implemented. How about we put acpi_irq_init() here and make
it an inline macro that directly calls acpi_gic_init() when ACPI is
enabled on AARCH64? Then the function can be extended by architectures
as needed, and default to an empty inline otherwise. When (if) we have
more than one hook that needs to be called from it, then we can
refactor it to be more like of_irq_init().

As for other architectures calling this function, but not caring about
ACPI, I believe it was your suggestion to put it here!  :-)

On Mon, 01 Sep 2014 18:35:18 +0100^M, Marc Zyngier  wrote:
> On 01/09/14 15:57, Hanjun Guo wrote:
> > @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
> > pt_regs *))
> >  void __init init_IRQ(void)
> >  {
> > irqchip_init();
> > +
> > +   if (!handle_arch_irq)
> > +   acpi_gic_init();
> > +
>
> Why isn't this called from irqchip_init? It would seem like the logical
> spot to probe an interrupt controller.

What has been done here isn't an unusual choice. We've got stuff all
over the kernel that may or may not be implemented depending on what
the architecture supports. If the function call is renamed to
acpi_init_irq(), are you content?

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-16 Thread Marc Zyngier
On 14/01/15 15:05, Hanjun Guo wrote:
> From: Tomasz Nowicki 
> 
> ACPI kernel uses MADT table for proper GIC initialization. It needs to
> parse GIC related subtables, collect CPU interface and distributor
> addresses and call driver initialization function (which is hardware
> abstraction agnostic). In a similar way, FDT initialize GICv1/2.
> 
> NOTE: This commit allow to initialize GICv1/2 basic functionality.
> GICv2 vitalization extension, GICv3/4 and ITS are considered as next
> steps.

And so is GICv2m, apparently (see below).

> 
> Tested-by: Suravee Suthikulpanit 
> Tested-by: Yijing Wang 
> Signed-off-by: Tomasz Nowicki 
> Signed-off-by: Hanjun Guo 
> ---
>  arch/arm64/kernel/acpi.c |  26 +
>  drivers/irqchip/irq-gic.c| 108 
> +++
>  drivers/irqchip/irqchip.c|   3 +
>  include/linux/irqchip/arm-gic-acpi.h |  31 ++
>  4 files changed, 168 insertions(+)
>  create mode 100644 include/linux/irqchip/arm-gic-acpi.h
> 
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index c3e24c4..ea3c9fc 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -315,6 +316,31 @@ void __init acpi_boot_table_init(void)
>   pr_err("Can't find FADT or error happened during parsing 
> FADT\n");
>  }
>  
> +void __init acpi_gic_init(void)
> +{
> + struct acpi_table_header *table;
> + acpi_status status;
> + acpi_size tbl_size;
> + int err;
> +
> + if (acpi_disabled)
> + return;
> +
> + status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, , _size);
> + if (ACPI_FAILURE(status)) {
> + const char *msg = acpi_format_exception(status);
> +
> + pr_err("Failed to get MADT table, %s\n", msg);
> + return;
> + }
> +
> + err = gic_v2_acpi_init(table);
> + if (err)
> + pr_err("Failed to initialize GIC IRQ controller");
> +
> + early_acpi_os_unmap_memory((char *)table, tbl_size);
> +}
> +
>  static int __init parse_acpi(char *arg)
>  {
>   if (!arg)
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index d617ee5..89a8120 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -33,12 +33,14 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -1083,3 +1085,109 @@ IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", 
> gic_of_init);
>  IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>  
>  #endif
> +
> +#ifdef CONFIG_ACPI
> +static phys_addr_t dist_phy_base, cpu_phy_base;
> +static int cpu_base_assigned;
> +
> +static int __init
> +gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_generic_interrupt *processor;
> + phys_addr_t gic_cpu_base;
> +
> + processor = (struct acpi_madt_generic_interrupt *)header;
> +
> + if (BAD_MADT_ENTRY(processor, end))
> + return -EINVAL;
> +
> + /*
> +  * There is no support for non-banked GICv1/2 register in ACPI spec.
> +  * All CPU interface addresses have to be the same.
> +  */
> + gic_cpu_base = processor->base_address;
> + if (cpu_base_assigned && gic_cpu_base != cpu_phy_base)
> + return -EFAULT;

EFAULT? That feels weird. This error code should be returned if an
access would generate (or has actually generated) a fault, but this is
not the case here. Same for the other cases below.

> +
> + cpu_phy_base = gic_cpu_base;
> + cpu_base_assigned = 1;
> + return 0;
> +}
> +
> +static int __init
> +gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_generic_distributor *dist;
> +
> + dist = (struct acpi_madt_generic_distributor *)header;
> +
> + if (BAD_MADT_ENTRY(dist, end))
> + return -EINVAL;
> +
> + dist_phy_base = dist->base_address;
> + return 0;
> +}
> +
> +int __init
> +gic_v2_acpi_init(struct acpi_table_header *table)
> +{
> + void __iomem *cpu_base, *dist_base;
> + int count;
> +
> + /* Collect CPU base addresses */
> + count = acpi_parse_entries(ACPI_SIG_MADT,
> +sizeof(struct acpi_table_madt),
> +gic_acpi_parse_madt_cpu, table,
> +ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
> + if (count < 0) {
> + pr_err("Error during GICC entries parsing\n");
> + return -EFAULT;
> + } else if (!count) {
> + pr_err("No valid GICC entries exist\n");
> + return -EINVAL;
> + }
> +
> + /*
> +  * Find distributor base address. We expect 

Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-16 Thread Marc Zyngier
On 14/01/15 15:05, Hanjun Guo wrote:
 From: Tomasz Nowicki tomasz.nowi...@linaro.org
 
 ACPI kernel uses MADT table for proper GIC initialization. It needs to
 parse GIC related subtables, collect CPU interface and distributor
 addresses and call driver initialization function (which is hardware
 abstraction agnostic). In a similar way, FDT initialize GICv1/2.
 
 NOTE: This commit allow to initialize GICv1/2 basic functionality.
 GICv2 vitalization extension, GICv3/4 and ITS are considered as next
 steps.

And so is GICv2m, apparently (see below).

 
 Tested-by: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
 Tested-by: Yijing Wang wangyij...@huawei.com
 Signed-off-by: Tomasz Nowicki tomasz.nowi...@linaro.org
 Signed-off-by: Hanjun Guo hanjun@linaro.org
 ---
  arch/arm64/kernel/acpi.c |  26 +
  drivers/irqchip/irq-gic.c| 108 
 +++
  drivers/irqchip/irqchip.c|   3 +
  include/linux/irqchip/arm-gic-acpi.h |  31 ++
  4 files changed, 168 insertions(+)
  create mode 100644 include/linux/irqchip/arm-gic-acpi.h
 
 diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
 index c3e24c4..ea3c9fc 100644
 --- a/arch/arm64/kernel/acpi.c
 +++ b/arch/arm64/kernel/acpi.c
 @@ -23,6 +23,7 @@
  #include linux/irqdomain.h
  #include linux/bootmem.h
  #include linux/smp.h
 +#include linux/irqchip/arm-gic-acpi.h
  
  #include asm/cputype.h
  #include asm/cpu_ops.h
 @@ -315,6 +316,31 @@ void __init acpi_boot_table_init(void)
   pr_err(Can't find FADT or error happened during parsing 
 FADT\n);
  }
  
 +void __init acpi_gic_init(void)
 +{
 + struct acpi_table_header *table;
 + acpi_status status;
 + acpi_size tbl_size;
 + int err;
 +
 + if (acpi_disabled)
 + return;
 +
 + status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, table, tbl_size);
 + if (ACPI_FAILURE(status)) {
 + const char *msg = acpi_format_exception(status);
 +
 + pr_err(Failed to get MADT table, %s\n, msg);
 + return;
 + }
 +
 + err = gic_v2_acpi_init(table);
 + if (err)
 + pr_err(Failed to initialize GIC IRQ controller);
 +
 + early_acpi_os_unmap_memory((char *)table, tbl_size);
 +}
 +
  static int __init parse_acpi(char *arg)
  {
   if (!arg)
 diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
 index d617ee5..89a8120 100644
 --- a/drivers/irqchip/irq-gic.c
 +++ b/drivers/irqchip/irq-gic.c
 @@ -33,12 +33,14 @@
  #include linux/of.h
  #include linux/of_address.h
  #include linux/of_irq.h
 +#include linux/acpi.h
  #include linux/irqdomain.h
  #include linux/interrupt.h
  #include linux/percpu.h
  #include linux/slab.h
  #include linux/irqchip/chained_irq.h
  #include linux/irqchip/arm-gic.h
 +#include linux/irqchip/arm-gic-acpi.h
  
  #include asm/cputype.h
  #include asm/irq.h
 @@ -1083,3 +1085,109 @@ IRQCHIP_DECLARE(msm_8660_qgic, qcom,msm-8660-qgic, 
 gic_of_init);
  IRQCHIP_DECLARE(msm_qgic2, qcom,msm-qgic2, gic_of_init);
  
  #endif
 +
 +#ifdef CONFIG_ACPI
 +static phys_addr_t dist_phy_base, cpu_phy_base;
 +static int cpu_base_assigned;
 +
 +static int __init
 +gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
 + const unsigned long end)
 +{
 + struct acpi_madt_generic_interrupt *processor;
 + phys_addr_t gic_cpu_base;
 +
 + processor = (struct acpi_madt_generic_interrupt *)header;
 +
 + if (BAD_MADT_ENTRY(processor, end))
 + return -EINVAL;
 +
 + /*
 +  * There is no support for non-banked GICv1/2 register in ACPI spec.
 +  * All CPU interface addresses have to be the same.
 +  */
 + gic_cpu_base = processor-base_address;
 + if (cpu_base_assigned  gic_cpu_base != cpu_phy_base)
 + return -EFAULT;

EFAULT? That feels weird. This error code should be returned if an
access would generate (or has actually generated) a fault, but this is
not the case here. Same for the other cases below.

 +
 + cpu_phy_base = gic_cpu_base;
 + cpu_base_assigned = 1;
 + return 0;
 +}
 +
 +static int __init
 +gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
 + const unsigned long end)
 +{
 + struct acpi_madt_generic_distributor *dist;
 +
 + dist = (struct acpi_madt_generic_distributor *)header;
 +
 + if (BAD_MADT_ENTRY(dist, end))
 + return -EINVAL;
 +
 + dist_phy_base = dist-base_address;
 + return 0;
 +}
 +
 +int __init
 +gic_v2_acpi_init(struct acpi_table_header *table)
 +{
 + void __iomem *cpu_base, *dist_base;
 + int count;
 +
 + /* Collect CPU base addresses */
 + count = acpi_parse_entries(ACPI_SIG_MADT,
 +sizeof(struct acpi_table_madt),
 +gic_acpi_parse_madt_cpu, table,
 +ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
 + if (count  0) {
 + 

Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-16 Thread Grant Likely
On Fri, Jan 16, 2015 at 11:15 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 14/01/15 15:05, Hanjun Guo wrote:
 From: Tomasz Nowicki tomasz.nowi...@linaro.org

 ACPI kernel uses MADT table for proper GIC initialization. It needs to
 parse GIC related subtables, collect CPU interface and distributor
 addresses and call driver initialization function (which is hardware
 abstraction agnostic). In a similar way, FDT initialize GICv1/2.

 NOTE: This commit allow to initialize GICv1/2 basic functionality.
 GICv2 vitalization extension, GICv3/4 and ITS are considered as next
 steps.

 And so is GICv2m, apparently (see below).


 Tested-by: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
 Tested-by: Yijing Wang wangyij...@huawei.com
 Signed-off-by: Tomasz Nowicki tomasz.nowi...@linaro.org
 Signed-off-by: Hanjun Guo hanjun@linaro.org
 ---
 + /*
 +  * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
 +  * as default IRQ domain to allow for GSI registration and GSI to IRQ
 +  * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
 +  */
 + gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);

 I assume you never tried to port the GICv2m driver to ACPI, right?
 Because the above code actively prevents the GIC domain to be defined as
 a stacked domain, making it impossible for the v2m widget to be
 implemented on top of GIC. But maybe legacy interrupts are enough?

It's sufficient for what is supported right now, and easily changed in
a patch series to add GICv2m support. This shouldn't be a blocking
issue as it isn't actively dangerous. It is merely limited, and that
is okay.

 @@ -26,4 +27,6 @@ extern struct of_device_id __irqchip_of_table[];
  void __init irqchip_init(void)
  {
   of_irq_init(__irqchip_of_table);
 +
 + acpi_gic_init();

 Have you realised that this file is probably compiled on multiple
 architecture, none of which particularly cares about ACPI or the GIC?
 This is (still) very ugly.

very ugly? That's overstating things a bit. We may quibble about the
name, but we're just talking about a setup hook function that may or
may not be implemented. How about we put acpi_irq_init() here and make
it an inline macro that directly calls acpi_gic_init() when ACPI is
enabled on AARCH64? Then the function can be extended by architectures
as needed, and default to an empty inline otherwise. When (if) we have
more than one hook that needs to be called from it, then we can
refactor it to be more like of_irq_init().

As for other architectures calling this function, but not caring about
ACPI, I believe it was your suggestion to put it here!  :-)

On Mon, 01 Sep 2014 18:35:18 +0100^M, Marc Zyngier marc.zyng...@arm.com wrote:
 On 01/09/14 15:57, Hanjun Guo wrote:
  @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
  pt_regs *))
   void __init init_IRQ(void)
   {
  irqchip_init();
  +
  +   if (!handle_arch_irq)
  +   acpi_gic_init();
  +

 Why isn't this called from irqchip_init? It would seem like the logical
 spot to probe an interrupt controller.

What has been done here isn't an unusual choice. We've got stuff all
over the kernel that may or may not be implemented depending on what
the architecture supports. If the function call is renamed to
acpi_init_irq(), are you content?

g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-16 Thread Marc Zyngier
On 16/01/15 13:54, Grant Likely wrote:
 On Fri, Jan 16, 2015 at 11:15 AM, Marc Zyngier marc.zyng...@arm.com wrote:
 On 14/01/15 15:05, Hanjun Guo wrote:
 From: Tomasz Nowicki tomasz.nowi...@linaro.org

 ACPI kernel uses MADT table for proper GIC initialization. It needs to
 parse GIC related subtables, collect CPU interface and distributor
 addresses and call driver initialization function (which is hardware
 abstraction agnostic). In a similar way, FDT initialize GICv1/2.

 NOTE: This commit allow to initialize GICv1/2 basic functionality.
 GICv2 vitalization extension, GICv3/4 and ITS are considered as next
 steps.

 And so is GICv2m, apparently (see below).


 Tested-by: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
 Tested-by: Yijing Wang wangyij...@huawei.com
 Signed-off-by: Tomasz Nowicki tomasz.nowi...@linaro.org
 Signed-off-by: Hanjun Guo hanjun@linaro.org
 ---
 + /*
 +  * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
 +  * as default IRQ domain to allow for GSI registration and GSI to IRQ
 +  * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
 +  */
 + gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);

 I assume you never tried to port the GICv2m driver to ACPI, right?
 Because the above code actively prevents the GIC domain to be defined as
 a stacked domain, making it impossible for the v2m widget to be
 implemented on top of GIC. But maybe legacy interrupts are enough?
 
 It's sufficient for what is supported right now, and easily changed in
 a patch series to add GICv2m support. This shouldn't be a blocking
 issue as it isn't actively dangerous. It is merely limited, and that
 is okay.
 
 @@ -26,4 +27,6 @@ extern struct of_device_id __irqchip_of_table[];
  void __init irqchip_init(void)
  {
   of_irq_init(__irqchip_of_table);
 +
 + acpi_gic_init();

 Have you realised that this file is probably compiled on multiple
 architecture, none of which particularly cares about ACPI or the GIC?
 This is (still) very ugly.
 
 very ugly? That's overstating things a bit. We may quibble about the
 name, but we're just talking about a setup hook function that may or
 may not be implemented. How about we put acpi_irq_init() here and make
 it an inline macro that directly calls acpi_gic_init() when ACPI is
 enabled on AARCH64? Then the function can be extended by architectures
 as needed, and default to an empty inline otherwise. When (if) we have
 more than one hook that needs to be called from it, then we can
 refactor it to be more like of_irq_init().
 
 As for other architectures calling this function, but not caring about
 ACPI, I believe it was your suggestion to put it here!  :-)
 
 On Mon, 01 Sep 2014 18:35:18 +0100^M, Marc Zyngier marc.zyng...@arm.com 
 wrote:
 On 01/09/14 15:57, Hanjun Guo wrote:
 @@ -78,6 +79,10 @@ void __init set_handle_irq(void (*handle_irq)(struct 
 pt_regs *))
  void __init init_IRQ(void)
  {
 irqchip_init();
 +
 +   if (!handle_arch_irq)
 +   acpi_gic_init();
 +

 Why isn't this called from irqchip_init? It would seem like the logical
 spot to probe an interrupt controller.
 
 What has been done here isn't an unusual choice. We've got stuff all
 over the kernel that may or may not be implemented depending on what
 the architecture supports. If the function call is renamed to
 acpi_init_irq(), are you content?

My (full) suggestion was to do it like we've done it for DT, and I don't
think I varied much from this point of view. Yes, calling it
acpi_irq_init() would be a good start, and having the ACPI-compatible
irqchips to be self-probable even better.

lack-of-sleep-rant

Hell, if nobody beats me to it, maybe I'll just write that code, with
proper entry points in the various GIC drivers. Yes, this is
infrastructure. Maybe it is grossly overdesigned. But I really spend too
much time dealing with the crap that people are happy to pile on top of
the GIC code to be madly enthusiastic about the general good enough
attitude.

/lack-of-sleep-rant

Or to put it in a slightly more diplomatic way: If ACPI is to be our
future, can we please make the future look a bit better?

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-15 Thread Mark Langsdorf

On 01/14/2015 09:05 AM, Hanjun Guo wrote:

From: Tomasz Nowicki 

ACPI kernel uses MADT table for proper GIC initialization. It needs to
parse GIC related subtables, collect CPU interface and distributor
addresses and call driver initialization function (which is hardware
abstraction agnostic). In a similar way, FDT initialize GICv1/2.

NOTE: This commit allow to initialize GICv1/2 basic functionality.
GICv2 vitalization extension, GICv3/4 and ITS are considered as next
steps.

Tested-by: Suravee Suthikulpanit 
Tested-by: Yijing Wang 
Signed-off-by: Tomasz Nowicki 
Signed-off-by: Hanjun Guo 
---

Tested-by: Mark Langsdorf 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v7 13/17] ARM64 / ACPI: Add GICv2 specific ACPI boot support

2015-01-15 Thread Mark Langsdorf

On 01/14/2015 09:05 AM, Hanjun Guo wrote:

From: Tomasz Nowicki tomasz.nowi...@linaro.org

ACPI kernel uses MADT table for proper GIC initialization. It needs to
parse GIC related subtables, collect CPU interface and distributor
addresses and call driver initialization function (which is hardware
abstraction agnostic). In a similar way, FDT initialize GICv1/2.

NOTE: This commit allow to initialize GICv1/2 basic functionality.
GICv2 vitalization extension, GICv3/4 and ITS are considered as next
steps.

Tested-by: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
Tested-by: Yijing Wang wangyij...@huawei.com
Signed-off-by: Tomasz Nowicki tomasz.nowi...@linaro.org
Signed-off-by: Hanjun Guo hanjun@linaro.org
---

Tested-by: Mark Langsdorf mlang...@redhat.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/