From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751633AbaHTJRb (ORCPT ); Wed, 20 Aug 2014 05:17:31 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:59675 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750819AbaHTJR3 (ORCPT ); Wed, 20 Aug 2014 05:17:29 -0400 X-AuditID: cbfee61a-f79e46d00000134f-5b-53f467a8590f From: Bartlomiej Zolnierkiewicz To: Javier Martinez Canillas Cc: Lee Jones , Marek Szyprowski , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Samuel Ortiz , Daniel Drake , Tobias Jakobi , Olof Johansson , Krzysztof Kozlowski Subject: Re: [PATCH] mfd: max77686: make interrupts support optional Date: Wed, 20 Aug 2014 11:17:24 +0200 Message-id: <3154093.ltyAb6brb9@amdc1032> User-Agent: KMail/4.8.4 (Linux/3.2.0-54-generic-pae; KDE/4.8.5; i686; ; ) In-reply-to: <53F45C73.1030304@collabora.co.uk> References: <1408516009-22106-1-git-send-email-m.szyprowski@samsung.com> <20140820081909.GH4266@lee--X1> <53F45C73.1030304@collabora.co.uk> MIME-version: 1.0 Content-transfer-encoding: 7Bit Content-type: text/plain; charset=UTF-8 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrFLMWRmVeSWpSXmKPExsVy+t9jQd0V6V+CDW6957Z4NP8xs8XR3wUW r18YWtz/epTRYtPja6wWl3fNYbPo+vmTzWLtkbvsFqeuf2azON3N6sDl8ff5dRaPRd+zPBZv 2s/mcefaHjaPeScDPTYvqfe4cqKJ1aNvyypGj8+b5AI4o7hsUlJzMstSi/TtErgyena0MRY0 SlZcvvqOtYHxn3AXIyeHhICJxLpfp9ggbDGJC/fWA9lcHEIC0xkl3s89COW0MEl8/7uKCaSK TcBKYmL7KkYQW0TATuLG6ofMIEXMAreZJKZfvgyWEBZwkXh/+SVQNwcHi4CqxOo+B5Awr4Cm xO4LP5lBbFEBT4kd21eCbeYU0Jc4emwuK8SyXkaJhQ+XsUI0CEr8mHyPBcRmFpCX2Ld/KiuE rS4xad4i5gmMArOQlM1CUjYLSdkCRuZVjKKpBckFxUnpuYZ6xYm5xaV56XrJ+bmbGMEx8kxq B+PKBotDjAIcjEo8vArZn4OFWBPLiitzDzFKcDArifAWJXwJFuJNSaysSi3Kjy8qzUktPsQo zcGiJM57oNU6UEggPbEkNTs1tSC1CCbLxMEp1cC4dn/H6Y2L97sd4PNI2vU873Dm0pNZR4Xv FopNaa5envjg+L4/3mv2nvvD4vI9gf99M8NEreKmC/znHR+GuAfybD59LCMh/8BJf4NVwY9i X3LOvOfeePr2Hs7oylNrwyIOZ8z+1y4iFXaP23jqMu+5LG4sZ+arRc85tzkxhOnntZMfpgix J2TMVGIpzkg01GIuKk4EAF50QBuNAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Wednesday, August 20, 2014 10:29:39 AM Javier Martinez Canillas wrote: > Hello Lee, Marek, > > On 08/20/2014 10:19 AM, Lee Jones wrote: > > On Wed, 20 Aug 2014, Marek Szyprowski wrote: > > > >> Commit 6f1c1e71d93 ("mfd: max77686: Convert to use regmap_irq") broke > >> support for boards, which provide no irq for MAX 77686 PMIC. In such > >> case, not all functions of PMIC are available, but the driver still > >> should at least handle voltage regulators instead of failing to init. > >> This patch restores previous behavior. > >> > >> Signed-off-by: Marek Szyprowski > >> --- > >> Hello, > >> > >> This patch fixes booting issues with recently merged support for > >> Exynos4412 based Odroid X2/U3 boards. See > >> http://www.spinics.net/lists/linux-samsung-soc/msg35773.html > >> thread for more details. > >> > >> Best regards > >> Marek Szyprowski > >> Samsung R&D Institute Poland > > I already gave my opinion about this patch on thread: > > "[PATCH v2 0/4] Add Exynos4412 based Odroid X2 and U2/U3/U3+ support" > > where the issue was reported by Olof but I'll add it here as well for > completeness since I don't know if the same people are cc'ed on both threads. > > Bartlomiej Zolnierkiewicz posted a similar patch [0] before but as I said BTW my patch a bit simpler than this one. regmap_del_irq_chip() checks for second argument being NULL so it is not needed to explicitly check for max77686->irq not being zero on probe failures before calling regmap_del_irq_chip(). > on that thread, I'm not sure if is correct to make that DT property > optional. Or rather, if we need to make it optional then we should also > change the Documentation/devicetree/bindings/mfd/max77686.txt that list > "interrupts" DT property as required. > > It's true that before converting the max77686 to use the regmap irq API, > the driver was loose on that regard but max77686_irq_init() in > max77686-irq.c was returning -ENODEV when the IRQ was not provided and > that error code was not checked in max77686_i2c_probe() (which was a bug > IMHO). > > More importantly I wonder if a design where a IRQ line is not connected to > the max77686 PMIC makes even sense. If such a design is possible then the > "interrupt" property should be made optional but if not then I think that > the driver should fail to probe in that case. Yes, it is a regression but > the Odroid DTS was not following the DT binding and was just lucky that > the driver was not checking the required DT properties. > > Daniel Drake posted the correct fix for the Odroid DTS [1]. So I tend to > say that Daniel's fix should be added as a -rc fix and we should keep both > the max77686 driver and its DT binding as it is now but as I said, if from > a hardware point of view makes sense to leave that line unconnected then I > don't oppose to this change. > > Best regards, > Javier > > [0]: https://lkml.org/lkml/2014/8/7/374 > [1]: > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/272869.html Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics