From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935898AbcIFQ2X (ORCPT ); Tue, 6 Sep 2016 12:28:23 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:46153 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933357AbcIFQ2V (ORCPT ); Tue, 6 Sep 2016 12:28:21 -0400 Date: Tue, 6 Sep 2016 17:28:08 +0100 From: Russell King - ARM Linux To: Lee Jones Cc: Arnd Bergmann , Linus Walleij , Hans-Christian Egtvedt , Dmitry Torokhov , Thomas Gleixner , linux-kernel@vger.kernel.org Subject: Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check Message-ID: <20160906162807.GA1041@n2100.armlinux.org.uk> References: <20160906130409.1691750-1-arnd@arndb.de> <20160906131730.GV1041@n2100.armlinux.org.uk> <20160906154530.GW4921@dell> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160906154530.GW4921@dell> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 06, 2016 at 04:45:30PM +0100, Lee Jones wrote: > On Tue, 06 Sep 2016, Russell King - ARM Linux wrote: > > > You need to send this _to_ me as I need to merge it with my other > > changes. This patch on its own does not make sense - it only makes > > sense with the rest of my SA11x0 patch stack. > > > > NAK for Lee to merge this. > > So if I were to accept this patch, would anything break? In other > words, is there an ordering issue where this this change relies on > something you have in your tree? 1) This is my driver which I've maintained in the past myself, including handling all patches for. This pre-dates your decision to take over the mfd stuff. I'm still maintaining this driver and I have not passed the responsibility to you. 2) If you review the driver and consider the effect of the change (which, as you don't know the driver, you should have done before replying if you're wanting to claim to be responsible for it) you would have noticed that the patch on its own subsitutes one form of brokenness with a different form of brokenness - ucb1x00_detect_irq() can return NO_IRQ. So checking for !ucb->irq breaks when ucb1x00_detect_irq() does so. 3) I already have a patch which removes that NO_IRQ usage, so the only way that _this_ patch makes sense is when combined with my patch. 4) I find it annoying that you've picked up on this patch in the way that you have (as a result of my NAK), yet you have failed to make any comment what so ever on _my_ patch, which you were copied with on the 30th August. Moreover, I reserve the right to keep it in my tree as the driver author, possibly publishing it in linux-next. I reserve the right to review patches to my driver. I reserve the right to NAK patches to my driver for whatever reason I deem appropriate. This is part of the problem I have here with your attitude with mfd: you decided yourself to become maintainer of everything in drivers/mfd, riding rough-shod over your fellow maintainers. And when your fellow maintainers try to reassert control, you get upset about it. Sorry, you can't have it both ways. Now, if you had discussed my patch from the 30th _first_ then I would not be NAKing this patch, and I probably would not be making such a big deal about this. But let's put that aside and stick to the technicalities. There are two dependencies here. Right now, the probe_irq_on() returning zero on SA11x0 platforms, causing ucb1x00_detect_irq() to return NO_IRQ, which then causes the probe function to fail. Whether that happens on PXA or not, I don't know and I have no way to test anymore as I donated all my PXA platforms to Robert. Applying Arnd's patch on its own, or applying my patch on its own will cause ucb1x00 to initialise with either NO_IRQ or 0 in ucb->irq, which causes the device to have no interrupts - or worse, it screws up any configuration of IRQ0 that the platform may have (eg, PXA). Applying both together results in the probe function continuing to fail. My patch is not supposed to be applied on its own; it is supposed to be applied with the third patch already in place - a GPIO patch. So yes, there _were_ dependencies here. That dependency can be solved by taking _both_ my patch (first) and Arnd's patch. However, given that each patch introduces its own different form of breakage, it makes sense to combine the two patches to avoid that breakage. So, when I'm less busy sorting out other SA11x0 stuff, I want to discuss with Arnd about combining them into a single patch. _Then_ we need to have a discussion about how to handle the patch. So, it's not as simple as you can see, because you don't have the knowledge to make the decisions for this driver. I hope this is clear enough. Thanks. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.