From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752819AbcBOXVE (ORCPT ); Mon, 15 Feb 2016 18:21:04 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:46673 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752614AbcBOXVA (ORCPT ); Mon, 15 Feb 2016 18:21:00 -0500 X-AuditID: cbfec7f5-f79b16d000005389-3c-56c25d58d76e Subject: Re: [PATCH 2/4] mfd: max77686: Use module_i2c_driver() instead of subsys initcall To: Javier Martinez Canillas , linux-kernel@vger.kernel.org References: <1455251419-8919-1-git-send-email-javier@osg.samsung.com> <1455251419-8919-3-git-send-email-javier@osg.samsung.com> <56C1762E.9040208@samsung.com> <56C1ED07.6090008@osg.samsung.com> Cc: Andi Shyti , linux-samsung-soc@vger.kernel.org, Lee Jones , Laxman Dewangan , Marek Szyprowski From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <56C25D55.5040208@samsung.com> Date: Tue, 16 Feb 2016 08:20:53 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: <56C1ED07.6090008@osg.samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrFLMWRmVeSWpSXmKPExsVy+t/xq7oRsYfCDGbPkrbYfuQZq8Wbt2uY LF6/MLRYum81i8X9r0cZLS7vmsNmMeP8PiaLtUfusjtweNy5tofNo7f5HZvHlv677B59W1Yx enzeJBfAGsVlk5Kak1mWWqRvl8CVMb3lFFPBHJmKZf08DYzXxLoYOTkkBEwkXvyawQphi0lc uLeerYuRi0NIYCmjxMvL+9ghnKeMEu/vnmHuYuTgEBaIkXjV7QjSICIQKvHv4m1GiJqjjBId f6ewgjjMIM787T3sIFVsAsYSm5cvYYNYISfR2z2JBcTmFdCS2DJrDSOIzSKgKrFz5yuwelGB CInDnV3sEDWCEj8m3wOr5xTQl3h06B07yBHMAnoS9y9qgYSZBeQlNq95yzyBUXAWko5ZCFWz kFQtYGRexSiaWppcUJyUnmukV5yYW1yal66XnJ+7iRESA193MC49ZnWIUYCDUYmHd4ProTAh 1sSy4srcQ4wSHMxKIrxXw4BCvCmJlVWpRfnxRaU5qcWHGKU5WJTEeWfueh8iJJCeWJKanZpa kFoEk2Xi4JRqYIw7FvYyX3jZjypWw8/K1wtthL/ueiHttPDttAcmk5q+bXi8/PdG65SCRVea p7yvy7Hc8czpOPNelhPMpUz73RmPsO9t3liy6BbXLaku28b+qvM9k/wX2xzfdYA1RZ5rC+Pl xW0Zins2dbWtiSq8s/5XqqHfC7ZreYsncdR5fdvNqF3JFnlxvrwSS3FGoqEWc1FxIgB6QaQB fQIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 16.02.2016 00:21, Javier Martinez Canillas wrote: > Hello Krzysztof, > > On 02/15/2016 03:54 AM, Krzysztof Kozlowski wrote: >> On 12.02.2016 13:30, Javier Martinez Canillas wrote: >>> The driver's init and exit function don't do anything besides adding and >>> deleting the I2C driver so the module_i2c_driver() macro could be used. >>> >>> Currently is not being used because the driver is initialized at subsys >>> initcall level, claiming that this is done to allow consumers devices to >>> use the resources provided by this driver. But dependencies should be in >>> the DT and consumers drivers should not rely in the registration order. >>> >>> Signed-off-by: Javier Martinez Canillas >>> --- >>> >>> drivers/mfd/max77686.c | 13 +------------ >>> 1 file changed, 1 insertion(+), 12 deletions(-) >>> >> >> In the past not all dependencies supported deferred probing so such >> ordering was required. >> >> I don't like the "dependencies should be in DT" reason for the change... >> because it is kind of wishful thinking. Yeah, the dependencies should be >> in DT, but are they? >> >> Instead *please check it* and write: >> "Dependencies are in DT so manual ordering of init calls is not >> necessary any more". >> > > For the max77802 I know that's the case since the only two DTS in mainline > that use it are the Peach Pit and Pi and I'm very familiar with those two. > > But I wonder how can I check that this is the case for the max77686. Most > DTS in mainline have nodes that use some clocks and regulators provided by > the PMIC, only arch/arm/boot/dts/exynos5250-smdk5250.dts doesn't have one > of the regulators as input supply or clock consumer defined. +Cc Marek Szyprowski, who may know a lot more about dependencies between these. I wouldn't care for drivers not taking references to regulators/clocks. Most of necessary regulators and clocks are turned on by bootloader or by default values in PMIC. This means that later probing of PMIC shouldn't influence drivers which are not using it. The remaining problem was unsupported deferred probing by some of the drivers using regulators/clocks (drivers being consumers of regulators or clocks). AFAIR one of example was USB OTG. By "please check" in this case I mean - look if every regulator/clock consumer using stuff exposed by PMIC, supports properly deferred probing. > For the clock, I guess the RTC is just broken since it's using the s3c6410 > controller that requires a source clock and this is not defined. > > Now the question is if it doesn't really need the regulators or is that > the DTS isn't correctly defined and some drivers were relying on the MFD > and regulator drivers to be registered at subsys initcall level? I suspect the consumers are not defined in DTS. However I wouldn't care about such issue. If there is no consumer, then probe order shouldn't matter... > >> My fast tests of this patch shown that it works good... but some more >> thorough tests should be done. >> > > What do you suggest? The drivers now support deferred probing but as said, > I don't know how I can be sure that drivers aren't missing input supplies > and relying in regulators being registered early and marked as always-on. So test it... You are posting a small improvement without any important benefit but in the same time it might broke existing platforms. Perfect is the enemy of the good (or if it ain't broken, don't touch it), so please be sure that max77686 still works. :) Best regards, Krzysztof