Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751658AbdG0SR0 (ORCPT ); Thu, 27 Jul 2017 14:17:26 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:34452 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751458AbdG0SRZ (ORCPT ); Thu, 27 Jul 2017 14:17:25 -0400 Subject: Re: [PATCH v3] irqchip/tango: Don't use incorrect irq_mask_ack callback To: =?UTF-8?B?TcOlbnMgUnVsbGfDpXJk?= , Marc Gonzalez , Mason Cc: Doug Berger , Thomas Gleixner , Marc Zyngier , Jason Cooper , LKML , Linux ARM References: <20170719190734.18566-1-opendmb@gmail.com> <20170719190734.18566-3-opendmb@gmail.com> <7a51555f-8191-9ebd-1f30-7c20f6db9d3f@sigmadesigns.com> <8d29fec9-35b8-c33b-3091-3e9a51c99ed7@gmail.com> <6f0092f7-692f-4a15-1d95-40f4e59c8585@sigmadesigns.com> <3b858e14-0da1-d4aa-eb84-f136ece8c2a6@gmail.com> From: Florian Fainelli Message-ID: <48734beb-0e6b-3a8f-ebf4-b1cec63322e5@gmail.com> Date: Thu, 27 Jul 2017 11:17:18 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2314 Lines: 52 On 07/26/2017 12:13 PM, Måns Rullgård wrote: > Florian Fainelli writes: > >> On 07/25/2017 06:29 AM, Måns Rullgård wrote: >>> Marc Gonzalez writes: >>> >>>> On 25/07/2017 15:16, Måns Rullgård wrote: >>>> >>>>> What happened to the patch adding the proper combined function? >>>> >>>> It appears you're not CCed on v2. >>>> >>>> https://patchwork.kernel.org/patch/9859799/ >>>> >>>> Doug wrote: >>>>> Yes, you understand correctly. The irq_mask_ack method is entirely >>>>> optional and I assume that is why this issue went undetected for so >>>>> long; however, it is slightly more efficient to combine the functions >>>>> (even if the ack is unnecessary) which is why I chose to do so for my >>>>> changes to the irqchip-brcmstb-l2 driver where I first discovered this >>>>> issue. How much value the improved efficiency has is certainly >>>>> debatable, but interrupt handling is one area where people might care >>>>> about such a small difference. As the irqchip-tango driver maintainer >>>>> you are welcome to decide whether or not the irq_mask_ack method makes >>>>> sense to you. >>>> >>>> My preference goes to leaving the irq_mask_ack callback undefined, >>>> and let the irqchip framework use irq_mask and irq_ack instead. >>> >>> Why would you prefer the less efficient way? >>> >> >> Same question here, that does not really make sense to me. >> >> The whole point of this patch series is to have a set of efficient and >> bugfree (or nearly) helper functions that drivers can rely on, are you >> saying that somehow using irq_mask_and_ack is exposing a bug in the >> tango irqchip driver and using the separate functions does not expose >> this bug? > > There is currently a bug in that the function used doesn't do what its > name implies which can't be good. Using the separate mask and ack > functions obviously works, but combining them saves a lock/unlock > sequence. The correct combined function has already been written, so I > see no reason not to use it. Marc/Mason, are you intending to get this patch accepted in order to provide a quick bugfix targeting earlier kernels with the tango irqchip driver or is this how you think the correct fix for the tango irqchip driver is as opposed to using Doug's fix? -- Florian