From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751784AbcEJSAw (ORCPT ); Tue, 10 May 2016 14:00:52 -0400 Received: from hqemgate16.nvidia.com ([216.228.121.65]:16255 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751288AbcEJSAv (ORCPT ); Tue, 10 May 2016 14:00:51 -0400 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Tue, 10 May 2016 10:59:16 -0700 Subject: Re: [PATCH 02/11] irqdomain: Warn if we fail to set the IRQ type To: Marc Zyngier References: <1462893285-13515-1-git-send-email-jonathanh@nvidia.com> <1462893285-13515-3-git-send-email-jonathanh@nvidia.com> <57321998.2060008@arm.com> CC: Thomas Gleixner , Jason Cooper , , X-Nvconfidentiality: public From: Jon Hunter Message-ID: <573221CC.2080102@nvidia.com> Date: Tue, 10 May 2016 19:00:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <57321998.2060008@arm.com> X-Originating-IP: [10.21.132.102] X-ClientProxiedBy: UKMAIL101.nvidia.com (10.26.138.13) To UKMAIL101.nvidia.com (10.26.138.13) Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/05/16 18:25, Marc Zyngier wrote: > On 10/05/16 16:14, Jon Hunter wrote: >> When setting the IRQ type we don't check the return value to see if it >> is set correctly. Due to this, failures to set the IRQ type have gone >> unnoticed and because these failures were not catastrophic have not had >> an impact on the system. >> >> Ideally, we should return an error if we fail to set the type, however, >> this could cause non-catastrophic failures to prevent devices from >> working. Therefore, for now add a warning so that any bad interrupt >> configurations can be corrected. >> >> Signed-off-by: Jon Hunter >> --- >> kernel/irq/irqdomain.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c >> index 8798b6c9e945..09060072cc28 100644 >> --- a/kernel/irq/irqdomain.c >> +++ b/kernel/irq/irqdomain.c >> @@ -610,7 +610,8 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec) >> /* Set type if specified and different than the current one */ >> if (type != IRQ_TYPE_NONE && >> type != irq_get_trigger_type(virq)) >> - irq_set_irq_type(virq, type); >> + if (irq_set_irq_type(virq, type)) >> + pr_warn("failed to set type for irq %d\n", virq); > > This warning triggers on all per-cpu interrupts, because > irq_set_irq_type() uses IRQ_GET_DESC_CHECK_GLOBAL and not > IRQ_GET_DESC_CHECK_PERCPU. Which sort of makes sense because the trigger > is per-cpu and not global. We'd need some similar check in > enable_percpu_irq, but at that stage, we've already lost the context > coming from the firmware. > > Which only proves one thing: per-cpu interrupts have never been > configured on the allocation path, and we've been living pretty > dangerously so far. They do work (at least on ARM) because of the > following reasons: > > 1) the triggers are already configured (firmware, read-only...) > 2) the handle_percpu_devid_irq handler doesn't distinguish between flows > > It is probably broken on all other architectures, which kind of sucks. > At this point, I'm really tempted to drop this patch and to aim towards > something similar to what you had in patches 5 and 6 in your previous > series. I'll have a think tonight. OK. I will hold off on posting the other patches for the minute. Cheers Jon