From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932779AbcBCCqE (ORCPT ); Tue, 2 Feb 2016 21:46:04 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:43286 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932642AbcBCCqB (ORCPT ); Tue, 2 Feb 2016 21:46:01 -0500 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: cbfec7f5-f79b16d000005389-1b-56b169e5996f Content-transfer-encoding: 8BIT Subject: Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for ACT8945A To: "Yang, Wenyou" , Peter Korsgaard , Mark Brown References: <1453863463-30515-1-git-send-email-wenyou.yang@atmel.com> <1453863463-30515-2-git-send-email-wenyou.yang@atmel.com> <20160129001603.GM4130@sirena.org.uk> <20160129113518.GQ4130@sirena.org.uk> <87wpqn3t72.fsf@dell.be.48ers.dk> Cc: Liam Girdwood , Rob Herring , Pawel Moll , Ian Campbell , Kumar Gala , Javier Martinez Canillas , Lee Jones , Peter Korsgaard , "Ferre, Nicolas" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <56B169E4.9030801@samsung.com> Date: Wed, 03 Feb 2016 11:45:56 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 In-reply-to: X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrIIsWRmVeSWpSXmKPExsVy+t/xy7rPMjeGGZwKt5j68Ambxfwj51gt +t8sZLU492olo0Xv1wPMFtd+z2CzeP3C0OL+16OMFt+udDBZbHp8jdXi8q45bBbfLjezW0yY vpbFYnX3ARaL1r1H2C1u/9rA4iDgsWbeGkaPBb+2snhc7utl8vg7u5XZY+esu+weK5d/YfPY tKqTzePr17mMHneu7WHz2Lyk3qNvyypGj8+b5AJ4orhsUlJzMstSi/TtErgyep+cYy94IVJx umkyWwNjm2AXIyeHhICJROvSJawQtpjEhXvr2boYuTiEBJYySsztvM0GkuAVEJT4MfkeSxcj BwezgLzEkUvZIGFmAXWJSfMWMYPYQgJPGSXuba8AsYUFQiT+nXsONlNEoERi1dPFLBA195kk Nj0TAJnPLLCNReLHiRNg89kEjCU2L1/CBnGEnERv9ySwXbwCWhJ7T/qChFkEVCWOn5nNDmKL CkRIHO7sYgcp4RRwkLhyQn8Co+AsJIfOQjh0FpJDFzAyr2IUTS1NLihOSs810itOzC0uzUvX S87P3cQIicivOxiXHrM6xCjAwajEwxvxa0OYEGtiWXFl7iFGCQ5mJRFe44yNYUK8KYmVValF +fFFpTmpxYcYpTlYlMR5Z+56HyIkkJ5YkpqdmlqQWgSTZeLglGpgXKfL0bAtfYfJd8u7OxjS 3Z1tF84SO37x26yofYlLLr5acLf65NVjPhbfOJZtCSiSeH5147dZ9zoyJfZ7J65vkHgcmNMR oRYZ1964uf6af9EX/l/34gw0FgRONJHNn2n4zCQws4Gbe2m1K4s/e0e05ET7ttN9SxgZtNN8 9HqMVQVFr0lMOfJBiaU4I9FQi7moOBEA7M7dJMQCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03.02.2016 11:29, Yang, Wenyou wrote: > Hi Peter, > >> -----Original Message----- >> From: Peter Korsgaard [mailto:jacmet@gmail.com] On Behalf Of Peter Korsgaard >> Sent: 2016年2月3日 1:42 >> To: Mark Brown >> Cc: Yang, Wenyou ; Liam Girdwood >> ; Rob Herring ; Pawel Moll >> ; Ian Campbell ; Kumar >> Gala ; Krzysztof Kozlowski >> ; Javier Martinez Canillas ; >> Lee Jones ; Peter Korsgaard ; Ferre, >> Nicolas ; linux-arm-kernel@lists.infradead.org; >> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org >> Subject: Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for >> ACT8945A >> >>>>>>> "Mark" == Mark Brown writes: >> >> > On Fri, Jan 29, 2016 at 01:20:08AM +0000, Yang, Wenyou wrote: >> >> > > +static const struct of_device_id act8945a_pmic_of_match[] = { >> >> > > + { .compatible = "active-semi,act8945a-regulator" }, >> >> > > + { }, >> >> > > +}; >> >> > > +MODULE_DEVICE_TABLE(of, act8945a_pmic_of_match); >> >> >> > This seems mostly OK but why do we have a compatible string here - >> shouldn't >> > the MFD be able to instantiate the regulator function without >> needing this? >> >> >> Because I got feedback from Javier for the act8945a-charger patches of this >> MFD series, >> He said missing the OF match table will cause the module >> autoloading broken. >> >> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/398113.html >> >> >> What do you think about it? >> >> > If then device is not being loaded from the DT (and it shouldn't be, the > device >> looks like it should be instantiated directly by the MFD as it > can't exist >> separately to that MFD) an OF table will do nothing. >> >> To add to the confusion, the regulator part of the chip is actually identical to >> act8865, so it could use the existing regulator driver / compatible, except that it >> binds to the platform bus instead of i2c. > > Thank you for your opinion. > > But I think It is better to make it a separate driver, the driver is simpler. Do I understand correctly that you are creating a new device driver for the same logical device (regulator block)? By duplicating the code you are not making kernel simpler. Maybe your new driver will be simple but still this adds a NEW driver instead of re-using existing code. Best regards, Krzysztof