Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1359894 > unrolled thread

[PATCH 00/15] Add support for Tegra210 AGIC

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/15] Add support for Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
    [PATCH 05/15] irqchip: Mask the non-type/sense bits when translating an IRQ Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
    [PATCH 09/15] irqchip/gic: Don't initialise chip if mapping IO space fails Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
    [PATCH 02/15] ARM: OMAP: Correct interrupt type for ARM TWD Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
      Re: [PATCH 02/15] ARM: OMAP: Correct interrupt type for ARM TWD Grygorii Strashko <grygorii.strashko@ti.com> - 2016-03-18 16:50 +0100
        Re: [PATCH 02/15] ARM: OMAP: Correct interrupt type for ARM TWD Jon Hunter <jonathanh@nvidia.com> - 2016-03-29 16:10 +0200
          Re: [PATCH 02/15] ARM: OMAP: Correct interrupt type for ARM TWD Tony Lindgren <tony@atomide.com> - 2016-03-30 23:30 +0200
    [PATCH 10/15] irqchip/gic: Remove static irq_chip definition for eoimode1 Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
      Re: [PATCH 10/15] irqchip/gic: Remove static irq_chip definition for eoimode1 Linus Walleij <linus.walleij@linaro.org> - 2016-03-22 12:50 +0100
    [PATCH 08/15] genirq: Add runtime power management support for IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
      Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Marc Zyngier <marc.zyngier@arm.com> - 2016-03-17 16:10 +0100
        Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 16:20 +0100
          Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Marc Zyngier <marc.zyngier@arm.com> - 2016-03-17 16:30 +0100
            Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Linus Walleij <linus.walleij@linaro.org> - 2016-03-22 12:50 +0100
      Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Thomas Gleixner <tglx@linutronix.de> - 2016-03-17 16:10 +0100
        Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 16:50 +0100
      Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Grygorii Strashko <grygorii.strashko@ti.com> - 2016-03-18 12:20 +0100
        Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2016-03-18 13:30 +0100
          Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Grygorii Strashko <grygorii.strashko@ti.com> - 2016-03-18 15:30 +0100
            Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2016-03-18 15:50 +0100
              Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2016-03-18 16:00 +0100
                Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Grygorii Strashko <grygorii.strashko@ti.com> - 2016-03-18 19:00 +0100
                  Re: [PATCH 08/15] genirq: Add runtime power management support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2016-03-21 11:10 +0100
    [PATCH 12/15] irqchip/gic: Pass GIC pointer to save/restore functions Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
    [PATCH 13/15] irqchip/gic: Prepare for adding platform driver Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
      Re: [PATCH 13/15] irqchip/gic: Prepare for adding platform driver Geert Uytterhoeven <geert@linux-m68k.org> - 2016-03-29 15:10 +0200
        Re: [PATCH 13/15] irqchip/gic: Prepare for adding platform driver Jon Hunter <jonathanh@nvidia.com> - 2016-03-29 16:00 +0200
    [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
      Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type  fails Thomas Gleixner <tglx@linutronix.de> - 2016-03-17 16:00 +0100
        Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type  fails Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 16:10 +0100
          Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type  fails Jason Cooper <jason@lakedaemon.net> - 2016-03-17 16:20 +0100
            Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type  fails Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 17:30 +0100
              Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails Geert Uytterhoeven <geert@linux-m68k.org> - 2016-03-18 10:30 +0100
                Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type  fails Jon Hunter <jonathanh@nvidia.com> - 2016-03-18 11:00 +0100
                  Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails Geert Uytterhoeven <geert@linux-m68k.org> - 2016-03-18 11:30 +0100
    [PATCH 03/15] irqchip/gic: Don't unnecessarily write the IRQ configuration Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
    [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from binding document Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
      Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Rob Herring <robh+dt@kernel.org> - 2016-03-17 21:20 +0100
        Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Jon Hunter <jonathanh@nvidia.com> - 2016-03-18 09:40 +0100
      Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Geert Uytterhoeven <geert@linux-m68k.org> - 2016-03-18 10:20 +0100
        Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Jon Hunter <jonathanh@nvidia.com> - 2016-03-18 11:20 +0100
          Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Geert Uytterhoeven <geert@linux-m68k.org> - 2016-03-18 12:00 +0100
            Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Jon Hunter <jonathanh@nvidia.com> - 2016-03-18 12:00 +0100
              Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Geert Uytterhoeven <geert@linux-m68k.org> - 2016-03-18 13:10 +0100
                Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Grygorii Strashko <grygorii.strashko@ti.com> - 2016-03-18 13:50 +0100
                  Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Geert Uytterhoeven <geert@linux-m68k.org> - 2016-03-18 14:10 +0100
                    Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Grygorii Strashko <grygorii.strashko@ti.com> - 2016-03-18 19:40 +0100
                Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from  binding document Jon Hunter <jonathanh@nvidia.com> - 2016-03-18 13:50 +0100
    [PATCH 11/15] irqchip/gic: Return an error if GIC initialisation fails Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100
    [PATCH 15/15] irqchip/gic: Add support for tegra AGIC interrupt controller Jon Hunter <jonathanh@nvidia.com> - 2016-03-17 15:30 +0100

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1360714 — Re: [PATCH 08/15] genirq: Add runtime power management support for IRQ chips

On 18/03/16 14:40, Jon Hunter wrote:
> On 18/03/16 14:23, Grygorii Strashko wrote:
>> On 03/18/2016 02:27 PM, Jon Hunter wrote:
>>>
>>> On 18/03/16 11:11, Grygorii Strashko wrote:

[snip]

>> oh :( That will require updating of all drivers (and if it will be taken into account that
>> wakeup can be configured from sysfs + devm_ - it will be painful).
> 
> Will it? I know that there are a few gpio chips that have some hacked
> ways to get around the PM issue, but I wonder how many drivers this
> really impacts. What sysfs entries are you referring too?

Thinking about this some more, yes I guess it would impact all drivers
that use a gpio but don't use it for a wake-up. I could see that could
be a few drivers indeed.

>>> but it would avoid every irqchip having to
>>> handle this themselves and having a custom handler.
>>
>> irqchip like TI OMAP GPIO will need custom handling any way even if it's not expected
>> to be Powered off during Suspend or deep CPUIdle states, simply because its state
>> in suspend is unknown - PM state managed automatically (and depends on many factors)
>> and wakeup can be handled by special HW in case if GPIO bank was really switched off.
>>
>>>> I propose do not touch common/generic suspend code now. Any common code can be always
>>>> refactored later once there will be real drivers updated to use irqchip RPM
>>>> and which will support Suspend.
>>>
>>> If this is strongly opposed, I would concede to making this a pr_debug()
>>> as I think it could be useful.
>>
>> Probably yes, because most of the drivers now and IRQ PM core are not ready
>> for this approach. 
> 
> May be this calls for a new flag to not WARN if non-wakeup IRQs are not
> freed when entering suspend.

Flag or pr_debug()?

Jon

[toc] | [prev] | [next] | [standalone]

#1360846 — Re: [PATCH 08/15] genirq: Add runtime power management support for IRQ chips

On 03/18/2016 04:56 PM, Jon Hunter wrote:
> 
> On 18/03/16 14:40, Jon Hunter wrote:
>> On 18/03/16 14:23, Grygorii Strashko wrote:
>>> On 03/18/2016 02:27 PM, Jon Hunter wrote:
>>>>
>>>> On 18/03/16 11:11, Grygorii Strashko wrote:
> 
> [snip]
> 
>>> oh :( That will require updating of all drivers (and if it will be taken into account that
>>> wakeup can be configured from sysfs + devm_ - it will be painful).
>>
>> Will it? I know that there are a few gpio chips that have some hacked
>> ways to get around the PM issue, but I wonder how many drivers this
>> really impacts. What sysfs entries are you referring too?

echo enabled > /sys/devices/platform/44000000.ocp/48020000.serial/tty/ttyS2/power/wakeup

> 
> Thinking about this some more, yes I guess it would impact all drivers
> that use a gpio but don't use it for a wake-up. I could see that could
> be a few drivers indeed.

yep. I've just tested it
- gpio was requested through sysfs and configured as IRQ
- do suspend

the same is if GPIO is requested as IRQ only and not configured as wakeup source 

[  319.669760] PM: late suspend of devices complete after 0.213 msecs
[  319.671195] irq 191 has no wakeup set and has not been freed!
[  319.673453] PM: noirq suspend of devices complete after 2.258 msecs

this is very minimal configuration - the regular one is at ~30-50 devices
most of them will use IRQ and only ~10% are used as wakeup sources.


> 
>>>> but it would avoid every irqchip having to
>>>> handle this themselves and having a custom handler.
>>>
>>> irqchip like TI OMAP GPIO will need custom handling any way even if it's not expected
>>> to be Powered off during Suspend or deep CPUIdle states, simply because its state
>>> in suspend is unknown - PM state managed automatically (and depends on many factors)
>>> and wakeup can be handled by special HW in case if GPIO bank was really switched off.
>>>
>>>>> I propose do not touch common/generic suspend code now. Any common code can be always
>>>>> refactored later once there will be real drivers updated to use irqchip RPM
>>>>> and which will support Suspend.
>>>>
>>>> If this is strongly opposed, I would concede to making this a pr_debug()
>>>> as I think it could be useful.
>>>
>>> Probably yes, because most of the drivers now and IRQ PM core are not ready
>>> for this approach.
>>
>> May be this calls for a new flag to not WARN if non-wakeup IRQs are not
>> freed when entering suspend.
> 
> Flag or pr_debug()?
> 

Honestly, I don't know how to proceed - minimum is pr_debug.
My personal opinion is still the same - don't touch suspend core code now, within this series.


-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]

#1361670 — Re: [PATCH 08/15] genirq: Add runtime power management support for IRQ chips

On 18/03/16 17:52, Grygorii Strashko wrote:
> On 03/18/2016 04:56 PM, Jon Hunter wrote:
>>
>> On 18/03/16 14:40, Jon Hunter wrote:
>>> On 18/03/16 14:23, Grygorii Strashko wrote:
>>>> On 03/18/2016 02:27 PM, Jon Hunter wrote:
>>>>>
>>>>> On 18/03/16 11:11, Grygorii Strashko wrote:
>>
>> [snip]
>>
>>>> oh :( That will require updating of all drivers (and if it will be taken into account that
>>>> wakeup can be configured from sysfs + devm_ - it will be painful).
>>>
>>> Will it? I know that there are a few gpio chips that have some hacked
>>> ways to get around the PM issue, but I wonder how many drivers this
>>> really impacts. What sysfs entries are you referring too?
> 
> echo enabled > /sys/devices/platform/44000000.ocp/48020000.serial/tty/ttyS2/power/wakeup
> 
>>
>> Thinking about this some more, yes I guess it would impact all drivers
>> that use a gpio but don't use it for a wake-up. I could see that could
>> be a few drivers indeed.
> 
> yep. I've just tested it
> - gpio was requested through sysfs and configured as IRQ
> - do suspend
> 
> the same is if GPIO is requested as IRQ only and not configured as wakeup source 
> 
> [  319.669760] PM: late suspend of devices complete after 0.213 msecs
> [  319.671195] irq 191 has no wakeup set and has not been freed!
> [  319.673453] PM: noirq suspend of devices complete after 2.258 msecs
> 
> this is very minimal configuration - the regular one is at ~30-50 devices
> most of them will use IRQ and only ~10% are used as wakeup sources.

Then it is working as intended :-)

However, if this is too verbose for some irqchips, then as I mentioned
we can have a flag to avoid these messages.

Cheers
Jon

[toc] | [prev] | [next] | [standalone]

#1359900 — [PATCH 12/15] irqchip/gic: Pass GIC pointer to save/restore functions

Instead of passing the GIC index to the save/restore functions pass a
pointer to the GIC chip data. This will allow these save/restore
functions to be re-used by a platform driver where the GIC chip data
structure is allocated dynamically and so there is no applicable index
for identifying the GIC.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/irqchip/irq-gic.c | 74 +++++++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 35 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 42a1412b5186..de9f1caee648 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -519,34 +519,35 @@ int gic_cpu_if_down(unsigned int gic_nr)
  * this function, no interrupts will be delivered by the GIC, and another
  * platform-specific wakeup source must be enabled.
  */
-static void gic_dist_save(unsigned int gic_nr)
+static void gic_dist_save(struct gic_chip_data *gic)
 {
 	unsigned int gic_irqs;
 	void __iomem *dist_base;
 	int i;
 
-	BUG_ON(gic_nr >= CONFIG_ARM_GIC_MAX_NR);
+	if (WARN_ON(!gic))
+		return;
 
-	gic_irqs = gic_data[gic_nr].gic_irqs;
-	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+	gic_irqs = gic->gic_irqs;
+	dist_base = gic_data_dist_base(gic);
 
 	if (!dist_base)
 		return;
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 16); i++)
-		gic_data[gic_nr].saved_spi_conf[i] =
+		gic->saved_spi_conf[i] =
 			readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
-		gic_data[gic_nr].saved_spi_target[i] =
+		gic->saved_spi_target[i] =
 			readl_relaxed(dist_base + GIC_DIST_TARGET + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
-		gic_data[gic_nr].saved_spi_enable[i] =
+		gic->saved_spi_enable[i] =
 			readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++)
-		gic_data[gic_nr].saved_spi_active[i] =
+		gic->saved_spi_active[i] =
 			readl_relaxed(dist_base + GIC_DIST_ACTIVE_SET + i * 4);
 }
 
@@ -557,16 +558,17 @@ static void gic_dist_save(unsigned int gic_nr)
  * handled normally, but any edge interrupts that occured will not be seen by
  * the GIC and need to be handled by the platform-specific wakeup source.
  */
-static void gic_dist_restore(unsigned int gic_nr)
+static void gic_dist_restore(struct gic_chip_data *gic)
 {
 	unsigned int gic_irqs;
 	unsigned int i;
 	void __iomem *dist_base;
 
-	BUG_ON(gic_nr >= CONFIG_ARM_GIC_MAX_NR);
+	if (WARN_ON(!gic))
+		return;
 
-	gic_irqs = gic_data[gic_nr].gic_irqs;
-	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+	gic_irqs = gic->gic_irqs;
+	dist_base = gic_data_dist_base(gic);
 
 	if (!dist_base)
 		return;
@@ -574,7 +576,7 @@ static void gic_dist_restore(unsigned int gic_nr)
 	writel_relaxed(GICD_DISABLE, dist_base + GIC_DIST_CTRL);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 16); i++)
-		writel_relaxed(gic_data[gic_nr].saved_spi_conf[i],
+		writel_relaxed(gic->saved_spi_conf[i],
 			dist_base + GIC_DIST_CONFIG + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
@@ -582,85 +584,87 @@ static void gic_dist_restore(unsigned int gic_nr)
 			dist_base + GIC_DIST_PRI + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 4); i++)
-		writel_relaxed(gic_data[gic_nr].saved_spi_target[i],
+		writel_relaxed(gic->saved_spi_target[i],
 			dist_base + GIC_DIST_TARGET + i * 4);
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {
 		writel_relaxed(GICD_INT_EN_CLR_X32,
 			dist_base + GIC_DIST_ENABLE_CLEAR + i * 4);
-		writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
+		writel_relaxed(gic->saved_spi_enable[i],
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
 	}
 
 	for (i = 0; i < DIV_ROUND_UP(gic_irqs, 32); i++) {
 		writel_relaxed(GICD_INT_EN_CLR_X32,
 			dist_base + GIC_DIST_ACTIVE_CLEAR + i * 4);
-		writel_relaxed(gic_data[gic_nr].saved_spi_active[i],
+		writel_relaxed(gic->saved_spi_active[i],
 			dist_base + GIC_DIST_ACTIVE_SET + i * 4);
 	}
 
 	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
 }
 
-static void gic_cpu_save(unsigned int gic_nr)
+static void gic_cpu_save(struct gic_chip_data *gic)
 {
 	int i;
 	u32 *ptr;
 	void __iomem *dist_base;
 	void __iomem *cpu_base;
 
-	BUG_ON(gic_nr >= CONFIG_ARM_GIC_MAX_NR);
+	if (WARN_ON(!gic))
+		return;
 
-	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
-	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
+	dist_base = gic_data_dist_base(gic);
+	cpu_base = gic_data_cpu_base(gic);
 
 	if (!dist_base || !cpu_base)
 		return;
 
-	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_enable);
+	ptr = raw_cpu_ptr(gic->saved_ppi_enable);
 	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ENABLE_SET + i * 4);
 
-	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_active);
+	ptr = raw_cpu_ptr(gic->saved_ppi_active);
 	for (i = 0; i < DIV_ROUND_UP(32, 32); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_ACTIVE_SET + i * 4);
 
-	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
+	ptr = raw_cpu_ptr(gic->saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
 		ptr[i] = readl_relaxed(dist_base + GIC_DIST_CONFIG + i * 4);
 
 }
 
-static void gic_cpu_restore(unsigned int gic_nr)
+static void gic_cpu_restore(struct gic_chip_data *gic)
 {
 	int i;
 	u32 *ptr;
 	void __iomem *dist_base;
 	void __iomem *cpu_base;
 
-	BUG_ON(gic_nr >= CONFIG_ARM_GIC_MAX_NR);
+	if (WARN_ON(!gic))
+		return;
 
-	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
-	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
+	dist_base = gic_data_dist_base(gic);
+	cpu_base = gic_data_cpu_base(gic);
 
 	if (!dist_base || !cpu_base)
 		return;
 
-	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_enable);
+	ptr = raw_cpu_ptr(gic->saved_ppi_enable);
 	for (i = 0; i < DIV_ROUND_UP(32, 32); i++) {
 		writel_relaxed(GICD_INT_EN_CLR_X32,
 			       dist_base + GIC_DIST_ENABLE_CLEAR + i * 4);
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);
 	}
 
-	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_active);
+	ptr = raw_cpu_ptr(gic->saved_ppi_active);
 	for (i = 0; i < DIV_ROUND_UP(32, 32); i++) {
 		writel_relaxed(GICD_INT_EN_CLR_X32,
 			       dist_base + GIC_DIST_ACTIVE_CLEAR + i * 4);
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_ACTIVE_SET + i * 4);
 	}
 
-	ptr = raw_cpu_ptr(gic_data[gic_nr].saved_ppi_conf);
+	ptr = raw_cpu_ptr(gic->saved_ppi_conf);
 	for (i = 0; i < DIV_ROUND_UP(32, 16); i++)
 		writel_relaxed(ptr[i], dist_base + GIC_DIST_CONFIG + i * 4);
 
@@ -669,7 +673,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
 					dist_base + GIC_DIST_PRI + i * 4);
 
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, cpu_base + GIC_CPU_PRIMASK);
-	gic_cpu_if_up(&gic_data[gic_nr]);
+	gic_cpu_if_up(gic);
 }
 
 static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
@@ -684,18 +688,18 @@ static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
 #endif
 		switch (cmd) {
 		case CPU_PM_ENTER:
-			gic_cpu_save(i);
+			gic_cpu_save(&gic_data[i]);
 			break;
 		case CPU_PM_ENTER_FAILED:
 		case CPU_PM_EXIT:
-			gic_cpu_restore(i);
+			gic_cpu_restore(&gic_data[i]);
 			break;
 		case CPU_CLUSTER_PM_ENTER:
-			gic_dist_save(i);
+			gic_dist_save(&gic_data[i]);
 			break;
 		case CPU_CLUSTER_PM_ENTER_FAILED:
 		case CPU_CLUSTER_PM_EXIT:
-			gic_dist_restore(i);
+			gic_dist_restore(&gic_data[i]);
 			break;
 		}
 	}
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1359901 — [PATCH 13/15] irqchip/gic: Prepare for adding platform driver

To support GIC chips located in power-domains outside of the CPU subsystem
it is necessary to add a platform driver for these chips, so that the
probing of the chip can be deferred if resources, such as a power-domain,
is not yet available.

To re-use the code that initialises the GIC (found in __gic_init_bases()),
from within the platform driver, it is necessary to move the code from the
__init section so that it is always present and not removed. Unfortunately,
it is not possible to simply drop the __init from the function declaration
for __gic_init_bases() because it contains calls to set_smp_cross_call()
and set_handle_irq() which are both located in the __init section.
Fortunately, these calls are only required for the root controller and
because the platform driver will only support non-root controllers that
can be initialised later in the boot process, we can move these calls to
another function. Move the bulk of the code from __gic_init_bases() to a
new function called gic_init_bases() which is not located in the __init
section and can be used by the platform driver. Update __gic_init_bases()
to call gic_init_bases() and if necessary, set_smp_cross_call() and
set_handle_irq().

The function, gic_init_bases(), references the GIC via a pointer to the
GIC chip data structure instead of an index so that it can be used by the
platform driver and statically declared GICs. This means that the name
must be passed to gic_init_bases() as well, because the name will not be
passed upon an index for platform devices.

Drop the __init section from the gic_dist_config(), gic_dist_init() and
gic_pm_init() so these can be re-used by the platform driver as well.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/irqchip/irq-gic-common.c |  4 +-
 drivers/irqchip/irq-gic.c        | 80 +++++++++++++++++++++++++++-------------
 2 files changed, 57 insertions(+), 27 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 423a345d7b75..646fc0f4a3bc 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -64,8 +64,8 @@ void gic_configure_irq(unsigned int irq, unsigned int type,
 		sync_access();
 }
 
-void __init gic_dist_config(void __iomem *base, int gic_irqs,
-			    void (*sync_access)(void))
+void gic_dist_config(void __iomem *base, int gic_irqs,
+		     void (*sync_access)(void))
 {
 	unsigned int i;
 
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index de9f1caee648..7cc5380db298 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -438,7 +438,7 @@ static void gic_cpu_if_up(struct gic_chip_data *gic)
 }
 
 
-static void __init gic_dist_init(struct gic_chip_data *gic)
+static void gic_dist_init(struct gic_chip_data *gic)
 {
 	unsigned int i;
 	u32 cpumask;
@@ -711,7 +711,7 @@ static struct notifier_block gic_notifier_block = {
 	.notifier_call = gic_notifier,
 };
 
-static void __init gic_pm_init(struct gic_chip_data *gic)
+static void gic_pm_init(struct gic_chip_data *gic)
 {
 	gic->saved_ppi_enable = __alloc_percpu(DIV_ROUND_UP(32, 32) * 4,
 		sizeof(u32));
@@ -729,7 +729,7 @@ static void __init gic_pm_init(struct gic_chip_data *gic)
 		cpu_pm_register_notifier(&gic_notifier_block);
 }
 #else
-static void __init gic_pm_init(struct gic_chip_data *gic)
+static void gic_pm_init(struct gic_chip_data *gic)
 {
 }
 #endif
@@ -1003,34 +1003,31 @@ static const struct irq_domain_ops gic_irq_domain_ops = {
 	.unmap = gic_irq_domain_unmap,
 };
 
-static int __init __gic_init_bases(unsigned int gic_nr, int irq_start,
-			   void __iomem *dist_base, void __iomem *cpu_base,
-			   u32 percpu_offset, struct fwnode_handle *handle)
+static int gic_init_bases(struct gic_chip_data *gic, int irq_start,
+			  void __iomem *dist_base, void __iomem *cpu_base,
+			  u32 percpu_offset, struct fwnode_handle *handle,
+			  const char *name)
 {
 	irq_hw_number_t hwirq_base;
-	struct gic_chip_data *gic;
 	int gic_irqs, irq_base, i, ret;
 
-	BUG_ON(gic_nr >= CONFIG_ARM_GIC_MAX_NR);
+	if (WARN_ON(!gic || gic->domain))
+		return -EINVAL;
 
 	gic_check_cpu_features();
 
-	gic = &gic_data[gic_nr];
-
 	/* Initialize irq_chip */
 	gic->chip = gic_chip;
+	gic->chip.name = name;
 
-	if (static_key_true(&supports_deactivate) && gic_nr == 0) {
+	if (static_key_true(&supports_deactivate) && gic == &gic_data[0]) {
 		gic->chip.irq_mask = gic_eoimode1_mask_irq;
 		gic->chip.irq_eoi = gic_eoimode1_eoi_irq;
 		gic->chip.irq_set_vcpu_affinity = gic_irq_set_vcpu_affinity;
-		gic->chip.name = kasprintf(GFP_KERNEL, "GICv2");
-	} else {
-		gic->chip.name = kasprintf(GFP_KERNEL, "GIC-%d", gic_nr);
 	}
 
 #ifdef CONFIG_SMP
-	if (gic_nr == 0)
+	if (gic == &gic_data[0])
 		gic->chip.irq_set_affinity = gic_set_affinity;
 #endif
 
@@ -1084,7 +1081,7 @@ static int __init __gic_init_bases(unsigned int gic_nr, int irq_start,
 		 * For primary GICs, skip over SGIs.
 		 * For secondary GICs, skip over PPIs, too.
 		 */
-		if (gic_nr == 0 && (irq_start & 31) > 0) {
+		if (gic == &gic_data[0] && (irq_start & 31) > 0) {
 			hwirq_base = 16;
 			if (irq_start != -1)
 				irq_start = (irq_start & ~31) + 16;
@@ -1111,7 +1108,7 @@ static int __init __gic_init_bases(unsigned int gic_nr, int irq_start,
 		goto error;
 	}
 
-	if (gic_nr == 0) {
+	if (gic == &gic_data[0]) {
 		/*
 		 * Initialize the CPU interface map to all CPUs.
 		 * It will be refined as each CPU probes its ID.
@@ -1119,13 +1116,6 @@ static int __init __gic_init_bases(unsigned int gic_nr, int irq_start,
 		 */
 		for (i = 0; i < NR_GIC_CPU_IF; i++)
 			gic_cpu_map[i] = 0xff;
-#ifdef CONFIG_SMP
-		set_smp_cross_call(gic_raise_softirq);
-		register_cpu_notifier(&gic_cpu_notifier);
-#endif
-		set_handle_irq(gic_handle_irq);
-		if (static_key_true(&supports_deactivate))
-			pr_info("GIC: Using split EOI/Deactivate mode\n");
 	}
 
 	gic_dist_init(gic);
@@ -1140,7 +1130,47 @@ error:
 		free_percpu(gic->cpu_base.percpu_base);
 	}
 
-	kfree(gic->chip.name);
+	return ret;
+}
+
+static int __init __gic_init_bases(unsigned int gic_nr, int irq_start,
+				   void __iomem *dist_base,
+				   void __iomem *cpu_base,
+				   u32 percpu_offset,
+				   struct fwnode_handle *handle)
+{
+	struct gic_chip_data *gic;
+	char *name;
+	int ret;
+
+	if (WARN_ON(gic_nr >= CONFIG_ARM_GIC_MAX_NR))
+		return -EINVAL;
+
+	gic = &gic_data[gic_nr];
+
+	if (static_key_true(&supports_deactivate) && gic_nr == 0)
+		name = kasprintf(GFP_KERNEL, "GICv2");
+	else
+		name = kasprintf(GFP_KERNEL, "GIC-%d", gic_nr);
+
+	ret = gic_init_bases(gic, irq_start, dist_base, cpu_base,
+			     percpu_offset, handle, name);
+	if (ret) {
+		kfree(gic->chip.name);
+		return ret;
+	}
+
+	if (gic_nr == 0) {
+		if (IS_ENABLED(CONFIG_SMP)) {
+			set_smp_cross_call(gic_raise_softirq);
+			register_cpu_notifier(&gic_cpu_notifier);
+		}
+
+		set_handle_irq(gic_handle_irq);
+
+		if (static_key_true(&supports_deactivate))
+			pr_info("GIC: Using split EOI/Deactivate mode\n");
+	}
 
 	return ret;
 }
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1366253 — Re: [PATCH 13/15] irqchip/gic: Prepare for adding platform driver

Hi Jon,

On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c

> -#ifdef CONFIG_SMP
> -               set_smp_cross_call(gic_raise_softirq);
> -               register_cpu_notifier(&gic_cpu_notifier);
> -#endif


> +       if (gic_nr == 0) {
> +               if (IS_ENABLED(CONFIG_SMP)) {
> +                       set_smp_cross_call(gic_raise_softirq);

If CONFIG_SMP=n:

drivers/irqchip/irq-gic.c: In function '__gic_init_bases':
drivers/irqchip/irq-gic.c:1165:4: error: implicit declaration of
function 'set_smp_cross_call' [-Werror=implicit-function-declaration]
    set_smp_cross_call(gic_raise_softirq);
    ^

There's no dummy for the CONFIG_SMP=n case, so you either have to
keep the #ifdef, or add a dummy.

> +                       register_cpu_notifier(&gic_cpu_notifier);
> +               }




-- 
Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1366303 — Re: [PATCH 13/15] irqchip/gic: Prepare for adding platform driver

Hi Geert,

On 29/03/16 14:04, Geert Uytterhoeven wrote:
> Hi Jon,
> 
> On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
> 
>> -#ifdef CONFIG_SMP
>> -               set_smp_cross_call(gic_raise_softirq);
>> -               register_cpu_notifier(&gic_cpu_notifier);
>> -#endif
> 
> 
>> +       if (gic_nr == 0) {
>> +               if (IS_ENABLED(CONFIG_SMP)) {
>> +                       set_smp_cross_call(gic_raise_softirq);
> 
> If CONFIG_SMP=n:
> 
> drivers/irqchip/irq-gic.c: In function '__gic_init_bases':
> drivers/irqchip/irq-gic.c:1165:4: error: implicit declaration of
> function 'set_smp_cross_call' [-Werror=implicit-function-declaration]
>     set_smp_cross_call(gic_raise_softirq);
>     ^
> 
> There's no dummy for the CONFIG_SMP=n case, so you either have to
> keep the #ifdef, or add a dummy.
> 
>> +                       register_cpu_notifier(&gic_cpu_notifier);
>> +               }

Thanks. I have already fixed this in my tree locally.

Cheers
Jon

[toc] | [prev] | [next] | [standalone]

#1359902 — [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails

Setting the interrupt type for private peripheral interrupts (PPIs) may
not be supported by a given GIC because it is IMPLEMENTATION DEFINED
whether this is allowed. There is no way to know if setting the type is
supported for a given GIC and so the value written is read back to
verify it matches the desired configuration. If it does not match then
an error is return.

There are cases where the interrupt configuration read from firmware
(such as a device-tree blob), has been incorrect and hence
gic_configure_irq() has returned an error. This error has gone
undetected because the error code returned was ignored but the interrupt
still worked fine because the configuration for the interrupt could not
be overwritten.

Given that this has done undetected and we should only fail to set the
type for PPIs whose configuration cannot be changed anyway, don't return
an error and simply WARN if this fails. This will allows us to fix up any
places in the kernel where we should be checking the return status and
maintain back compatibility with firmware images that may have incorrect
interrupt configurations.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/irqchip/irq-gic-common.c | 13 ++++---------
 drivers/irqchip/irq-gic-common.h |  2 +-
 drivers/irqchip/irq-gic-v3.c     |  4 +++-
 drivers/irqchip/irq-gic.c        |  4 +++-
 drivers/irqchip/irq-hip04.c      |  5 ++---
 5 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index ffff5a45f1e3..423a345d7b75 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -32,13 +32,12 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks,
 	}
 }
 
-int gic_configure_irq(unsigned int irq, unsigned int type,
+void gic_configure_irq(unsigned int irq, unsigned int type,
 		       void __iomem *base, void (*sync_access)(void))
 {
 	u32 confmask = 0x2 << ((irq % 16) * 2);
 	u32 confoff = (irq / 16) * 4;
 	u32 val, oldval;
-	int ret = 0;
 
 	/*
 	 * Read current configuration register, and insert the config
@@ -52,21 +51,17 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
 
 	/* If the current configuration is the same, then we are done */
 	if (val == oldval)
-		return 0;
+		return;
 
 	/*
 	 * Write back the new configuration, and possibly re-enable
-	 * the interrupt. If we fail to write a new configuration,
-	 * return an error.
+	 * the interrupt. WARN if we fail to write a new configuration.
 	 */
 	writel_relaxed(val, base + GIC_DIST_CONFIG + confoff);
-	if (readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val)
-		ret = -EINVAL;
+	WARN_ON(readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val);
 
 	if (sync_access)
 		sync_access();
-
-	return ret;
 }
 
 void __init gic_dist_config(void __iomem *base, int gic_irqs,
diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
index fff697db8e22..73dee3bc6bba 100644
--- a/drivers/irqchip/irq-gic-common.h
+++ b/drivers/irqchip/irq-gic-common.h
@@ -27,7 +27,7 @@ struct gic_quirk {
 	u32 mask;
 };
 
-int gic_configure_irq(unsigned int irq, unsigned int type,
+void gic_configure_irq(unsigned int irq, unsigned int type,
                        void __iomem *base, void (*sync_access)(void));
 void gic_dist_config(void __iomem *base, int gic_irqs,
 		     void (*sync_access)(void));
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 5b7d3c2129d8..c569c466fa31 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -310,7 +310,9 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
 		rwp_wait = gic_dist_wait_for_rwp;
 	}
 
-	return gic_configure_irq(irq, type, base, rwp_wait);
+	gic_configure_irq(irq, type, base, rwp_wait);
+
+	return 0;
 }
 
 static int gic_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 282344b95ec2..6c555a2c5315 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -279,7 +279,9 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
 			    type != IRQ_TYPE_EDGE_RISING)
 		return -EINVAL;
 
-	return gic_configure_irq(gicirq, type, base, NULL);
+	gic_configure_irq(gicirq, type, base, NULL);
+
+	return 0;
 }
 
 static int gic_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
index 9688d2e2a636..12be5906e91a 100644
--- a/drivers/irqchip/irq-hip04.c
+++ b/drivers/irqchip/irq-hip04.c
@@ -120,7 +120,6 @@ static int hip04_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	void __iomem *base = hip04_dist_base(d);
 	unsigned int irq = hip04_irq(d);
-	int ret;
 
 	/* Interrupt configuration for SGIs can't be changed */
 	if (irq < 16)
@@ -133,11 +132,11 @@ static int hip04_irq_set_type(struct irq_data *d, unsigned int type)
 
 	raw_spin_lock(&irq_controller_lock);
 
-	ret = gic_configure_irq(irq, type, base, NULL);
+	gic_configure_irq(irq, type, base, NULL);
 
 	raw_spin_unlock(&irq_controller_lock);
 
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_SMP
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1359923 — Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails

On Thu, 17 Mar 2016, Jon Hunter wrote:

> Setting the interrupt type for private peripheral interrupts (PPIs) may
> not be supported by a given GIC because it is IMPLEMENTATION DEFINED
> whether this is allowed. There is no way to know if setting the type is
> supported for a given GIC and so the value written is read back to
> verify it matches the desired configuration. If it does not match then
> an error is return.
> 
> There are cases where the interrupt configuration read from firmware
> (such as a device-tree blob), has been incorrect and hence
> gic_configure_irq() has returned an error. This error has gone
> undetected because the error code returned was ignored but the interrupt
> still worked fine because the configuration for the interrupt could not
> be overwritten.
> 
> Given that this has done undetected and we should only fail to set the
> type for PPIs whose configuration cannot be changed anyway, don't return
> an error and simply WARN if this fails. This will allows us to fix up any
> places in the kernel where we should be checking the return status and
> maintain back compatibility with firmware images that may have incorrect
> interrupt configurations.

Though silently returning 0 is really the wrong thing to do. You can add the
warn, but why do you want to return success?

Thanks,

	tglx

 

[toc] | [prev] | [next] | [standalone]

#1359931 — Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails

On 17/03/16 14:51, Thomas Gleixner wrote:
> On Thu, 17 Mar 2016, Jon Hunter wrote:
> 
>> Setting the interrupt type for private peripheral interrupts (PPIs) may
>> not be supported by a given GIC because it is IMPLEMENTATION DEFINED
>> whether this is allowed. There is no way to know if setting the type is
>> supported for a given GIC and so the value written is read back to
>> verify it matches the desired configuration. If it does not match then
>> an error is return.
>>
>> There are cases where the interrupt configuration read from firmware
>> (such as a device-tree blob), has been incorrect and hence
>> gic_configure_irq() has returned an error. This error has gone
>> undetected because the error code returned was ignored but the interrupt
>> still worked fine because the configuration for the interrupt could not
>> be overwritten.
>>
>> Given that this has done undetected and we should only fail to set the
>> type for PPIs whose configuration cannot be changed anyway, don't return
>> an error and simply WARN if this fails. This will allows us to fix up any
>> places in the kernel where we should be checking the return status and
>> maintain back compatibility with firmware images that may have incorrect
>> interrupt configurations.
> 
> Though silently returning 0 is really the wrong thing to do. You can add the
> warn, but why do you want to return success?

Yes that would be the correct thing to do I agree. However, the problem
is that if we do this, then after the patch "irqdomain: Don't set type
when mapping an IRQ" is applied, we may break interrupts for some
existing device-tree binaries that have bad configuration (such as omap4
and tegra20/30 ... see patches 1 and 2) that have gone unnoticed. So it
is a back compatibility issue.

If you are wondering why these interrupts break after "irqdomain: Don't
set type when mapping an IRQ", it is because today
irq_create_fwspec_mapping() does not check the return code from setting
the type, but if we defer setting the type until __setup_irq() which
does check the return code, then all of a sudden interrupts that were
working (even with bad configurations) start to fail.

The reason why I opted not to return an error code from
gic_configure_irq() is it really can't fail. The failure being reported
does not prevent the interrupt from working, but tells you your
configuration does not match the hardware setting which you cannot
overwrite.

So to maintain back compatibility and avoid any silent errors, I opted
to make it a WARN and not return an error.

If people are ok with potentially breaking interrupts for device-tree
binaries with bad settings, then I am ok to return an error here.

Cheers
Jon

[toc] | [prev] | [next] | [standalone]

#1359943 — Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails

On Thu, Mar 17, 2016 at 03:04:01PM +0000, Jon Hunter wrote:
> 
> On 17/03/16 14:51, Thomas Gleixner wrote:
> > On Thu, 17 Mar 2016, Jon Hunter wrote:
> > 
> >> Setting the interrupt type for private peripheral interrupts (PPIs) may
> >> not be supported by a given GIC because it is IMPLEMENTATION DEFINED
> >> whether this is allowed. There is no way to know if setting the type is
> >> supported for a given GIC and so the value written is read back to
> >> verify it matches the desired configuration. If it does not match then
> >> an error is return.
> >>
> >> There are cases where the interrupt configuration read from firmware
> >> (such as a device-tree blob), has been incorrect and hence
> >> gic_configure_irq() has returned an error. This error has gone
> >> undetected because the error code returned was ignored but the interrupt
> >> still worked fine because the configuration for the interrupt could not
> >> be overwritten.
> >>
> >> Given that this has done undetected and we should only fail to set the
> >> type for PPIs whose configuration cannot be changed anyway, don't return
> >> an error and simply WARN if this fails. This will allows us to fix up any
> >> places in the kernel where we should be checking the return status and
> >> maintain back compatibility with firmware images that may have incorrect
> >> interrupt configurations.
> > 
> > Though silently returning 0 is really the wrong thing to do. You can add the
> > warn, but why do you want to return success?
> 
> Yes that would be the correct thing to do I agree. However, the problem
> is that if we do this, then after the patch "irqdomain: Don't set type
> when mapping an IRQ" is applied, we may break interrupts for some
> existing device-tree binaries that have bad configuration (such as omap4
> and tegra20/30 ... see patches 1 and 2) that have gone unnoticed. So it
> is a back compatibility issue.

This sounds like a textbook case for adding a boolean dt property.  If
"can-set-ppi-type" is absent (old DT blobs and new blobs without the
ability), warn and return zero.  If it's present, the driver can set the
type, returning errors as encountered.

thx,

Jason.

[toc] | [prev] | [next] | [standalone]

#1360001 — Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails

On 17/03/16 15:18, Jason Cooper wrote:
> On Thu, Mar 17, 2016 at 03:04:01PM +0000, Jon Hunter wrote:
>>
>> On 17/03/16 14:51, Thomas Gleixner wrote:
>>> On Thu, 17 Mar 2016, Jon Hunter wrote:
>>>
>>>> Setting the interrupt type for private peripheral interrupts (PPIs) may
>>>> not be supported by a given GIC because it is IMPLEMENTATION DEFINED
>>>> whether this is allowed. There is no way to know if setting the type is
>>>> supported for a given GIC and so the value written is read back to
>>>> verify it matches the desired configuration. If it does not match then
>>>> an error is return.
>>>>
>>>> There are cases where the interrupt configuration read from firmware
>>>> (such as a device-tree blob), has been incorrect and hence
>>>> gic_configure_irq() has returned an error. This error has gone
>>>> undetected because the error code returned was ignored but the interrupt
>>>> still worked fine because the configuration for the interrupt could not
>>>> be overwritten.
>>>>
>>>> Given that this has done undetected and we should only fail to set the
>>>> type for PPIs whose configuration cannot be changed anyway, don't return
>>>> an error and simply WARN if this fails. This will allows us to fix up any
>>>> places in the kernel where we should be checking the return status and
>>>> maintain back compatibility with firmware images that may have incorrect
>>>> interrupt configurations.
>>>
>>> Though silently returning 0 is really the wrong thing to do. You can add the
>>> warn, but why do you want to return success?
>>
>> Yes that would be the correct thing to do I agree. However, the problem
>> is that if we do this, then after the patch "irqdomain: Don't set type
>> when mapping an IRQ" is applied, we may break interrupts for some
>> existing device-tree binaries that have bad configuration (such as omap4
>> and tegra20/30 ... see patches 1 and 2) that have gone unnoticed. So it
>> is a back compatibility issue.
> 
> This sounds like a textbook case for adding a boolean dt property.  If
> "can-set-ppi-type" is absent (old DT blobs and new blobs without the
> ability), warn and return zero.  If it's present, the driver can set the
> type, returning errors as encountered.

True. However, if we did have this "can-set-ppi-type" property set for a
device, it really should never fail (unless someone specified it
incorrectly). So I am trying to understand the value in adding a new DT
property.

Please note that gic_configure_irq() never used to return an error and
only when adding support for setting the type of PPIs was this added.
However, given that this has gone unnoticed and does not have a real
functional impact on the device behaviour, I wonder now if this function
should return an error? Yes, ideally, it should, but does it still make
sense?

Cheers
Jon

[toc] | [prev] | [next] | [standalone]

#1360461 — Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails

On Thu, Mar 17, 2016 at 5:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> On 17/03/16 15:18, Jason Cooper wrote:
>> On Thu, Mar 17, 2016 at 03:04:01PM +0000, Jon Hunter wrote:
>>> On 17/03/16 14:51, Thomas Gleixner wrote:
>>>> On Thu, 17 Mar 2016, Jon Hunter wrote:
>>>>> Setting the interrupt type for private peripheral interrupts (PPIs) may
>>>>> not be supported by a given GIC because it is IMPLEMENTATION DEFINED
>>>>> whether this is allowed. There is no way to know if setting the type is
>>>>> supported for a given GIC and so the value written is read back to
>>>>> verify it matches the desired configuration. If it does not match then
>>>>> an error is return.
>>>>>
>>>>> There are cases where the interrupt configuration read from firmware
>>>>> (such as a device-tree blob), has been incorrect and hence
>>>>> gic_configure_irq() has returned an error. This error has gone
>>>>> undetected because the error code returned was ignored but the interrupt
>>>>> still worked fine because the configuration for the interrupt could not
>>>>> be overwritten.
>>>>>
>>>>> Given that this has done undetected and we should only fail to set the
>>>>> type for PPIs whose configuration cannot be changed anyway, don't return
>>>>> an error and simply WARN if this fails. This will allows us to fix up any
>>>>> places in the kernel where we should be checking the return status and
>>>>> maintain back compatibility with firmware images that may have incorrect
>>>>> interrupt configurations.
>>>>
>>>> Though silently returning 0 is really the wrong thing to do. You can add the
>>>> warn, but why do you want to return success?
>>>
>>> Yes that would be the correct thing to do I agree. However, the problem
>>> is that if we do this, then after the patch "irqdomain: Don't set type
>>> when mapping an IRQ" is applied, we may break interrupts for some
>>> existing device-tree binaries that have bad configuration (such as omap4
>>> and tegra20/30 ... see patches 1 and 2) that have gone unnoticed. So it
>>> is a back compatibility issue.

Indeed (also for sh73a0 and r8a7779).

>> This sounds like a textbook case for adding a boolean dt property.  If
>> "can-set-ppi-type" is absent (old DT blobs and new blobs without the
>> ability), warn and return zero.  If it's present, the driver can set the
>> type, returning errors as encountered.
>
> True. However, if we did have this "can-set-ppi-type" property set for a
> device, it really should never fail (unless someone specified it
> incorrectly). So I am trying to understand the value in adding a new DT
> property.

Do we really want to add properties that basically indicate that a description
in DT is correct?

Alternatively, it can be fixed in the kernel in a DT quirk (if SoC == xxx then
fix TWD).

> Please note that gic_configure_irq() never used to return an error and
> only when adding support for setting the type of PPIs was this added.
> However, given that this has gone unnoticed and does not have a real
> functional impact on the device behaviour, I wonder now if this function
> should return an error? Yes, ideally, it should, but does it still make
> sense?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1360482 — Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails

On 18/03/16 09:20, Geert Uytterhoeven wrote:
> On Thu, Mar 17, 2016 at 5:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>> On 17/03/16 15:18, Jason Cooper wrote:
>>> On Thu, Mar 17, 2016 at 03:04:01PM +0000, Jon Hunter wrote:
>>>> On 17/03/16 14:51, Thomas Gleixner wrote:
>>>>> On Thu, 17 Mar 2016, Jon Hunter wrote:
>>>>>> Setting the interrupt type for private peripheral interrupts (PPIs) may
>>>>>> not be supported by a given GIC because it is IMPLEMENTATION DEFINED
>>>>>> whether this is allowed. There is no way to know if setting the type is
>>>>>> supported for a given GIC and so the value written is read back to
>>>>>> verify it matches the desired configuration. If it does not match then
>>>>>> an error is return.
>>>>>>
>>>>>> There are cases where the interrupt configuration read from firmware
>>>>>> (such as a device-tree blob), has been incorrect and hence
>>>>>> gic_configure_irq() has returned an error. This error has gone
>>>>>> undetected because the error code returned was ignored but the interrupt
>>>>>> still worked fine because the configuration for the interrupt could not
>>>>>> be overwritten.
>>>>>>
>>>>>> Given that this has done undetected and we should only fail to set the
>>>>>> type for PPIs whose configuration cannot be changed anyway, don't return
>>>>>> an error and simply WARN if this fails. This will allows us to fix up any
>>>>>> places in the kernel where we should be checking the return status and
>>>>>> maintain back compatibility with firmware images that may have incorrect
>>>>>> interrupt configurations.
>>>>>
>>>>> Though silently returning 0 is really the wrong thing to do. You can add the
>>>>> warn, but why do you want to return success?
>>>>
>>>> Yes that would be the correct thing to do I agree. However, the problem
>>>> is that if we do this, then after the patch "irqdomain: Don't set type
>>>> when mapping an IRQ" is applied, we may break interrupts for some
>>>> existing device-tree binaries that have bad configuration (such as omap4
>>>> and tegra20/30 ... see patches 1 and 2) that have gone unnoticed. So it
>>>> is a back compatibility issue.
> 
> Indeed (also for sh73a0 and r8a7779).

Thanks. I was wondering if there are others. Do you know what the
correct setting should be? Ie. should it be IRQ_TYPE_EDGE_RISING as
well? I can then include this with OMAP and Tegra.

>>> This sounds like a textbook case for adding a boolean dt property.  If
>>> "can-set-ppi-type" is absent (old DT blobs and new blobs without the
>>> ability), warn and return zero.  If it's present, the driver can set the
>>> type, returning errors as encountered.
>>
>> True. However, if we did have this "can-set-ppi-type" property set for a
>> device, it really should never fail (unless someone specified it
>> incorrectly). So I am trying to understand the value in adding a new DT
>> property.
> 
> Do we really want to add properties that basically indicate that a description
> in DT is correct?
> 
> Alternatively, it can be fixed in the kernel in a DT quirk (if SoC == xxx then
> fix TWD).

I am not sure I fully understand your proposal, but please note that it
may not be just limited to the TWD (although this does appear to be the
one client that is wrong for a lot of SoCs). PPIs are also used for the
armv7/8 timers as well.

The problem is that we have a lot of SoCs with twd-timers and I have no
way to test all of these to know which could be a problem. So I thought
that warning would be a good first step to fixing them.

However, I am still trying to see the real value in returning an error
in this case. May be I am the only one with that perspective?

Cheers
Jon

[toc] | [prev] | [next] | [standalone]

#1360501 — Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails

Hi Jon,

On Fri, Mar 18, 2016 at 10:54 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
> On 18/03/16 09:20, Geert Uytterhoeven wrote:
>> On Thu, Mar 17, 2016 at 5:20 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
>>> On 17/03/16 15:18, Jason Cooper wrote:
>>>> On Thu, Mar 17, 2016 at 03:04:01PM +0000, Jon Hunter wrote:
>>>>> On 17/03/16 14:51, Thomas Gleixner wrote:
>>>>>> On Thu, 17 Mar 2016, Jon Hunter wrote:
>>>>>>> Setting the interrupt type for private peripheral interrupts (PPIs) may
>>>>>>> not be supported by a given GIC because it is IMPLEMENTATION DEFINED
>>>>>>> whether this is allowed. There is no way to know if setting the type is
>>>>>>> supported for a given GIC and so the value written is read back to
>>>>>>> verify it matches the desired configuration. If it does not match then
>>>>>>> an error is return.
>>>>>>>
>>>>>>> There are cases where the interrupt configuration read from firmware
>>>>>>> (such as a device-tree blob), has been incorrect and hence
>>>>>>> gic_configure_irq() has returned an error. This error has gone
>>>>>>> undetected because the error code returned was ignored but the interrupt
>>>>>>> still worked fine because the configuration for the interrupt could not
>>>>>>> be overwritten.
>>>>>>>
>>>>>>> Given that this has done undetected and we should only fail to set the
>>>>>>> type for PPIs whose configuration cannot be changed anyway, don't return
>>>>>>> an error and simply WARN if this fails. This will allows us to fix up any
>>>>>>> places in the kernel where we should be checking the return status and
>>>>>>> maintain back compatibility with firmware images that may have incorrect
>>>>>>> interrupt configurations.
>>>>>>
>>>>>> Though silently returning 0 is really the wrong thing to do. You can add the
>>>>>> warn, but why do you want to return success?
>>>>>
>>>>> Yes that would be the correct thing to do I agree. However, the problem
>>>>> is that if we do this, then after the patch "irqdomain: Don't set type
>>>>> when mapping an IRQ" is applied, we may break interrupts for some
>>>>> existing device-tree binaries that have bad configuration (such as omap4
>>>>> and tegra20/30 ... see patches 1 and 2) that have gone unnoticed. So it
>>>>> is a back compatibility issue.
>>
>> Indeed (also for sh73a0 and r8a7779).
>
> Thanks. I was wondering if there are others. Do you know what the
> correct setting should be? Ie. should it be IRQ_TYPE_EDGE_RISING as
> well? I can then include this with OMAP and Tegra.

The warnings went away when using IRQ_TYPE_EDGE_RISING.
Patches sent.

>>>> This sounds like a textbook case for adding a boolean dt property.  If
>>>> "can-set-ppi-type" is absent (old DT blobs and new blobs without the
>>>> ability), warn and return zero.  If it's present, the driver can set the
>>>> type, returning errors as encountered.
>>>
>>> True. However, if we did have this "can-set-ppi-type" property set for a
>>> device, it really should never fail (unless someone specified it
>>> incorrectly). So I am trying to understand the value in adding a new DT
>>> property.
>>
>> Do we really want to add properties that basically indicate that a description
>> in DT is correct?
>>
>> Alternatively, it can be fixed in the kernel in a DT quirk (if SoC == xxx then
>> fix TWD).
>
> I am not sure I fully understand your proposal, but please note that it

The kernel can modify the DT in early startup code if it detects that the
TWD's interrupt type is wrong.

> may not be just limited to the TWD (although this does appear to be the
> one client that is wrong for a lot of SoCs). PPIs are also used for the
> armv7/8 timers as well.

True.

> The problem is that we have a lot of SoCs with twd-timers and I have no
> way to test all of these to know which could be a problem. So I thought
> that warning would be a good first step to fixing them.

Definitely.

> However, I am still trying to see the real value in returning an error
> in this case. May be I am the only one with that perspective?

Given the above, we cannot start returning an error until all problems are
fixed.

Even after that, we have to care about stable DT ABIs. In this case it's not
really an ABI change, but a real buggy description in DTS.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1359905 — [PATCH 03/15] irqchip/gic: Don't unnecessarily write the IRQ configuration

If the interrupt configuration matches the current configuration, then
don't bother writing the configuration again.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/irqchip/irq-gic-common.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index f174ce0ca361..ffff5a45f1e3 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -50,13 +50,17 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
 	else if (type & IRQ_TYPE_EDGE_BOTH)
 		val |= confmask;
 
+	/* If the current configuration is the same, then we are done */
+	if (val == oldval)
+		return 0;
+
 	/*
 	 * Write back the new configuration, and possibly re-enable
-	 * the interrupt. If we tried to write a new configuration and failed,
+	 * the interrupt. If we fail to write a new configuration,
 	 * return an error.
 	 */
 	writel_relaxed(val, base + GIC_DIST_CONFIG + confoff);
-	if (readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val && val != oldval)
+	if (readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val)
 		ret = -EINVAL;
 
 	if (sync_access)
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1359906 — [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from binding document

Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power
Domain properties") documented optional clock and power-dmoain properties
for the ARM GIC. Currently, there are no users of these and for the
Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a
functional clock and interface clock, that need to be enabled.

To allow flexibility, drop the 'clock-names' from the GIC binding and
just provide a list of clocks which the driver can parse. It is assumed
that any clocks that are listed, need to be enabled in order to access
the GIC.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

---

Please note that I am not sure if this will be popular, but I am trying
to come up with a generic way to handle multiple clocks that may be
required for accessing a GIC.

 .../devicetree/bindings/interrupt-controller/arm,gic.txt      | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
index 793c20ff8fcc..c471d1a7a8ea 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt
@@ -61,15 +61,8 @@ Optional
   regions, used when the GIC doesn't have banked registers. The offset is
   cpu-offset * cpu-nr.
 
-- clocks        : List of phandle and clock-specific pairs, one for each entry
-  in clock-names.
-- clock-names   : List of names for the GIC clock input(s). Valid clock names
-  depend on the GIC variant:
-	"ic_clk" (for "arm,arm11mp-gic")
-	"PERIPHCLKEN" (for "arm,cortex-a15-gic")
-	"PERIPHCLK", "PERIPHCLKEN" (for "arm,cortex-a9-gic")
-	"clk" (for "arm,gic-400")
-	"gclk" (for "arm,pl390")
+- clocks        : List of phandle and clock-specific pairs required for
+		  accessing the GIC.
 
 - power-domains : A phandle and PM domain specifier as defined by bindings of
 		  the power controller specified by phandle, used when the GIC
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1360168 — Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from binding document

On Thu, Mar 17, 2016 at 9:19 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power
> Domain properties") documented optional clock and power-dmoain properties
> for the ARM GIC. Currently, there are no users of these and for the
> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a
> functional clock and interface clock, that need to be enabled.
>
> To allow flexibility, drop the 'clock-names' from the GIC binding and
> just provide a list of clocks which the driver can parse. It is assumed
> that any clocks that are listed, need to be enabled in order to access
> the GIC.
>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>
> ---
>
> Please note that I am not sure if this will be popular, but I am trying
> to come up with a generic way to handle multiple clocks that may be
> required for accessing a GIC.

It's not. :)

We need to specify the number and order of clocks by compatible string
at a minimum. Sadly, ARM's GICs are well documented and include clock
names, so you can't just make up genericish names either which is
probably often the case.

Rob

[toc] | [prev] | [next] | [standalone]

#1360421 — Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from binding document

On 17/03/16 20:14, Rob Herring wrote:
> On Thu, Mar 17, 2016 at 9:19 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
>> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power
>> Domain properties") documented optional clock and power-dmoain properties
>> for the ARM GIC. Currently, there are no users of these and for the
>> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a
>> functional clock and interface clock, that need to be enabled.
>>
>> To allow flexibility, drop the 'clock-names' from the GIC binding and
>> just provide a list of clocks which the driver can parse. It is assumed
>> that any clocks that are listed, need to be enabled in order to access
>> the GIC.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>
>> ---
>>
>> Please note that I am not sure if this will be popular, but I am trying
>> to come up with a generic way to handle multiple clocks that may be
>> required for accessing a GIC.
> 
> It's not. :)
> 
> We need to specify the number and order of clocks by compatible string
> at a minimum. Sadly, ARM's GICs are well documented and include clock
> names, so you can't just make up genericish names either which is
> probably often the case.

Do you have any suggestions then?

I have had a look at the ARM TRMs and although I see that they do show
the functional clock, there is no mention of whether there are any other
clocks need in order to interface to the GIC (ie. bus clock). I know
that for other SoCs such as OMAP it is common to have both a functional
clock and interface clock. So I believe this is fairly common.

Cheers
Jon

[toc] | [prev] | [next] | [standalone]

#1360455 — Re: [PATCH 14/15] dt-bindings: arm-gic: Drop 'clock-names' from binding document

Hi Jon,

On Thu, Mar 17, 2016 at 3:19 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> Commit afbbd2338176 ("irqchip/gic: Document optional Clock and Power
> Domain properties") documented optional clock and power-dmoain properties
> for the ARM GIC. Currently, there are no users of these and for the
> Tegra210 Audio GIC (based upon the GIC-400) there are two clocks, a
> functional clock and interface clock, that need to be enabled.

The reason that there are no users for this is twofold:
  1. The GIC driver doesn't have Runtime PM support yet,
  2. There was no clean way to prevent the GIC's clock from being disabled.
Due to this, adding the clocks to the DTSes would mean that they will be
disabled during boot up as unused clocks, leading to a system lock-up.

I had hoped your series would fix part 1. I gave it a try on r8a7791/koelsch,
but unfortunately it seems the platform driver only supports non-root
controllers, while the r8a7791 GIC is the primary one...

Alternatively, part 2 can to be fixed by "clk: introduce CLK_ENABLE_HAND_OFF
flag", combined with the clock driver setting the flag when needed.
Unfortunately that patch is not yet upstream, and not even in -next.
Note that drivers/clk/renesas/renesas-cpg-mssr.c already handles
CLK_ENABLE_HAND_OFF if present, and else just ignores the clock.
So I could already add the clock to r8a7795.dtsi, which uses that driver.

For older SoCs, the module clocks are described in the dtsi, and I would need a
crude hack to enable CLK_ENABLE_HAND_OFF in the clock driver.

> To allow flexibility, drop the 'clock-names' from the GIC binding and
> just provide a list of clocks which the driver can parse. It is assumed
> that any clocks that are listed, need to be enabled in order to access
> the GIC.

Originally I just wanted to have "clocks", and let the details be handled by
SoC-specific code. However, Mark Rutland insisted on using the clock naming
from the GIC TRMs, as the number of clocks and their names depend on the
GIC variant.

Apparently they also depend on the SoC...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web