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

From	Jason Cooper <jason@lakedaemon.net>
Newsgroups	linux.kernel
Subject	Re: [PATCH 04/15] irqchip/gic: WARN if setting the interrupt type fails
Date	2016-03-17 16:20 +0100
Message-ID	<rdHSq-Dd-9@gated-at.bofh.it> (permalink)
References	<rdGWl-8uZ-9@gated-at.bofh.it> <rdH63-6P-35@gated-at.bofh.it> <rdHz3-gE-5@gated-at.bofh.it> <rdHIJ-zS-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

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.

Thread

[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

csiph-web