On 12/04/16 09:50, Jon Hunter wrote: > > On 11/04/16 16:39, Marc Zyngier wrote: >> On 11/04/16 16:31, Jon Hunter wrote: >>> On 09/04/16 11:58, Marc Zyngier wrote: > > [snip] > >>>> I think we need to phase things. Let's start with warning people for a >>>> few kernel releases. Actively maintained platforms will quickly address >>>> the issue (fixing their DT). As I see it, this issue seems rather >>>> widespread (even kvmtool outputs a DT with the wrong triggering >>>> information). >>>> >>>> Once we've fixed the bulk of the platforms and virtual environments, we >>>> can start thinking about making it fail harder. >>> >>> Ok, so are you OK with this patch as-is? If so, can I add your ACK? >> >> It depends where you plan to handle the error. Ideally, I'd keep on >> returning the error (because that's the right thing to do), and move the >> WARN_ON() into the core code. We'd keep on ignoring the error as we're >> doing today, but we'd scream about it. >> >> After a couple of releases, we'd turn the WARN_ON into a hard fail. >> >> Thoughts? > > I agree that would be best/ideal, but looking at it, I don't believe it > is possible and this is why I have not done that so far. > > If we were to add the WARN to the core code, then we would need to add a > warning everywhere __irq_set_trigger() is called. One of the places it > is called today is from __setup_irq() and today this does the right > thing and handle any error returned. The problem is that in > irq_create_fwspec_mapping() we have never checked the return code from > irq_set_irq_type() (which calls __irq_set_trigger()) or attempted to > handle any errors. So the problem is that depending on the path through > which the type is programmed, errors may or may not be detected. This is > the actual headache :-( > > Given that this problem so far only pertains to GIC PPI interrupts and > that it is a not a catastrophic error (interrupts still work fine), I > was thinking we add the warning to the GIC driver. > > May be a less severe change would be to only return an error if > configuring an SPI fails and if it is a PPI then simply WARN and > carry-on as we assume we cannot change it. I'd take that. Limiting it to PPIs would be a minimal change, and the warning would hopefully make people realize their DT is wrong. Failing to program an SPI is really not expected, and should definitely explode. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
