From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933612AbcKIPF7 (ORCPT ); Wed, 9 Nov 2016 10:05:59 -0500 Received: from mail-wm0-f45.google.com ([74.125.82.45]:38097 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752876AbcKIPF5 (ORCPT ); Wed, 9 Nov 2016 10:05:57 -0500 Date: Wed, 9 Nov 2016 15:08:42 +0000 From: Lee Jones To: Guenter Roeck Cc: Thomas Gleixner , Tony Lindgren , linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org, Marcin Niestroj , Arnd Bergmann Subject: Re: [PATCH] mfd: tps65217: Drop call to irq_set_parent() Message-ID: <20161109150842.GF13127@dell> References: <1476479886-10208-1-git-send-email-linux@roeck-us.net> <20161026123516.GC11267@dell> <20161026125854.GA12618@dell> <3b280a00-271a-c743-d076-4f55813fe43f@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <3b280a00-271a-c743-d076-4f55813fe43f@roeck-us.net> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 26 Oct 2016, Guenter Roeck wrote: > On 10/26/2016 05:58 AM, Lee Jones wrote: > > On Wed, 26 Oct 2016, Thomas Gleixner wrote: > > > > > On Wed, 26 Oct 2016, Lee Jones wrote: > > > > On Fri, 14 Oct 2016, Guenter Roeck wrote: > > > > > > > > > The call to irq_set_parent() causes the following build error if tps65217 > > > > > is built as module. > > > > > > > > > > ERROR: ".irq_set_parent" [drivers/mfd/tps65217.ko] undefined! > > > > > > > > > > The problem was introduced with commit 6556bdacf646f ("mfd: tps65217: Add > > > > > support for IRQs"). > > > > > > > > > > The author states: "I have added irq_set_parent() similarly as in > > > > > drivers/base/regmap/regmap-irq.c. But to be honest I am not sure what it > > > > > really does in case of tps65217." > > > > > > > > > > So let's drop it. > > > > > > > > > > Fixes: 6556bdacf646f ("mfd: tps65217: Add support for IRQs") > > > > > Cc: Marcin Niestroj > > > > > Cc: Arnd Bergmann > > > > > Cc: Thomas Gleixner > > > > > Signed-off-by: Guenter Roeck > > > > > --- > > > > > drivers/mfd/tps65217.c | 1 - > > > > > 1 file changed, 1 deletion(-) > > > > > > > > This has been fixed now. > > > > > > It was not fixed. The export was a work around as everyone was bitching > > > about the build robots failing forever. > > > > > > So if the irq_set_parent() call is not required for functionality of the > > > driver then it should not be there in the first place. > > > > Ah, I thought this was just another one of the many hacks I received > > in response to the auto-builder's complains. I've just been NACKing > > them out of habit. > > > > Well, it was, in a way. However, with the driver author being silent, > and with irq_set_parent() not that well documented, I considered it > a better solution than blindly exporting the function. > > Having said that, I do suspect that its use might possibly be warranted > in this case, since the driver uses edge triggered interrupts and calls > handle_nested_irq(). But then many other drivers do the same and don't > call irq_set_parent(), so who knows. The use case for irq_set_parent() > isn't exactly well explained. Final call; am I taking this patch or not? > FWIW, since everyone seems to be bitching about auto-builders: You may not > care, but problems like this end up hiding other problems, can make > bisecting a pain, and can end up costing a lot of time in the future. > I have worked for companies where the common attitude was "who cares about > any builds but ours". Sounds great, until one needs to enable one more > configuration option and everything falls apart. > > If you don't care about a driver being buildable as module, make it boolean. > Please. > > Thanks, > Guenter > -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog