Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1324849

Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for ACT8945A

Show all headers | View raw

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 <broonie@kernel.org>
>> Cc: Yang, Wenyou <Wenyou.Yang@atmel.com>; Liam Girdwood
>> <lgirdwood@gmail.com>; Rob Herring <robh+dt@kernel.org>; Pawel Moll
>> <pawel.moll@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>; Kumar
>> Gala <galak@codeaurora.org>; Krzysztof Kozlowski
>> <k.kozlowski@samsung.com>; Javier Martinez Canillas <javier@dowhile0.org>;
>> Lee Jones <lee.jones@linaro.org>; Peter Korsgaard <jacmet@gmail.com>; Ferre,
>> Nicolas <Nicolas.FERRE@atmel.com>; 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 <broonie@kernel.org> 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

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A Mark Brown <broonie@kernel.org> - 2016-01-29 01:20 +0100
  RE: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A "Yang, Wenyou" <Wenyou.Yang@atmel.com> - 2016-01-29 03:20 +0100
    Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A Mark Brown <broonie@kernel.org> - 2016-01-29 12:40 +0100
      Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for ACT8945A Javier Martinez Canillas <javier@dowhile0.org> - 2016-01-29 13:00 +0100
        Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A Mark Brown <broonie@kernel.org> - 2016-01-29 13:10 +0100
        RE: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A "Yang, Wenyou" <Wenyou.Yang@atmel.com> - 2016-02-01 07:00 +0100
          Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for ACT8945A Javier Martinez Canillas <javier@dowhile0.org> - 2016-02-01 14:20 +0100
            RE: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A "Yang, Wenyou" <Wenyou.Yang@atmel.com> - 2016-02-02 03:30 +0100
            RE: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A "Yang, Wenyou" <Wenyou.Yang@atmel.com> - 2016-02-03 03:40 +0100
              Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A Mark Brown <broonie@kernel.org> - 2016-02-03 18:50 +0100
                RE: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A "Yang, Wenyou" <Wenyou.Yang@atmel.com> - 2016-02-04 14:30 +0100
      Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for ACT8945A Peter Korsgaard <peter@korsgaard.com> - 2016-02-02 18:50 +0100
        RE: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A "Yang, Wenyou" <Wenyou.Yang@atmel.com> - 2016-02-03 03:30 +0100
          Re: [PATCH v4 1/2] regulator: act8945a: add regulator driver for  ACT8945A Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-02-03 03:50 +0100

csiph-web