Message ID | 1453863463-30515-2-git-send-email-wenyou.yang@atmel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Wed, Jan 27, 2016 at 10:57:42AM +0800, Wenyou Yang 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?
Hi Mark, > -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: 2016?1?29? 8:16 > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > Cc: 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 > > On Wed, Jan 27, 2016 at 10:57:42AM +0800, Wenyou Yang 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? Best Regards, Wenyou Yang
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.
Hello Mark, On Fri, Jan 29, 2016 at 8:35 AM, Mark Brown <broonie@kernel.org> wrote: > 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. Then he should remove the .of_compatible from the MFD cell definition. Otherwise the MFD core will assign an of_node to this device in mfd_add_device(), making the platform core to believe the device was instantiated by OF. So an OF modalias uevent will be reported to load the module and the OF table should be used for matching the device with the driver. IOW, the MFD and regulator driver have to agree on this. Either everything should have OF or platform information, both work but mixing these as it was on a previous version doesn't. Best regards, Javier
On Fri, Jan 29, 2016 at 08:50:19AM -0300, Javier Martinez Canillas wrote:
> Then he should remove the .of_compatible from the MFD cell definition.
Yes, obviously.
Hi Javier & Mark, I am educated. Thank you all. > -----Original Message----- > From: Javier Martinez Canillas [mailto:javier@dowhile0.org] > Sent: 2016?1?29? 19:50 > 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>; 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 > > Hello Mark, > > On Fri, Jan 29, 2016 at 8:35 AM, Mark Brown <broonie@kernel.org> wrote: > > 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/39 > >> 8113.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. > > Then he should remove the .of_compatible from the MFD cell definition. I tried it, But if removed this .of_compatible, and reserved the OF table. the &pdev->dev->of_node is NULL, the driver fails to get the configuration value from DT, It seems the OF table still doesn't work. Where is wrong? Could you help give some suggestion? > > Otherwise the MFD core will assign an of_node to this device in mfd_add_device(), > making the platform core to believe the device was instantiated by OF. So an OF > modalias uevent will be reported to load the module and the OF table should be > used for matching the device with the driver. > > IOW, the MFD and regulator driver have to agree on this. Either everything should > have OF or platform information, both work but mixing these as it was on a > previous version doesn't. > > Best regards, > Javier Best Regards, Wenyou Yang
Hello Wenyou, [snip] >> > >> > 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. >> >> Then he should remove the .of_compatible from the MFD cell definition. > > I tried it, > > But if removed this .of_compatible, and reserved the OF table. What we said is that you should remove both the mfd cell .of_compatible field and the OF table. Of course removing one but leaving the other is not correct. > the &pdev->dev->of_node is NULL, the driver fails to get the configuration value from DT, > I see, you mean that you are not able to lookup the "active-semi,vsel-high" property. > It seems the OF table still doesn't work. Where is wrong? > I think the problem is with your DT binding. You have a PMIC dev node with compatible "active-semi,act8945a" that has a child node with compatible "active-semi,act8945a-regulator" which in turn has the "regulators" node. I believe what Mark says is that there shouldn't be a compatible and node for the regulators IC since is part of the PMIC. IOW, you can't have it as a standalone node in the DTS. > Could you help give some suggestion? > I would just remove the "active-semi,act8945a-regulator" node and make "active-semi,vsel-high" a property of the "active-semi,act8945a" node. That way you can remove the mfd cell .of_compatible and OF table in the regulator driver and read the "active-semi,vsel-high" using the platform device's parent of_node. But it's better if you wait for Mark's opinions before re-spining your patches. Best regards, Javier
Hi Javier, > -----Original Message----- > From: Javier Martinez Canillas [mailto:javier@dowhile0.org] > Sent: 2016?2?1? 21:15 > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > Cc: Mark Brown <broonie@kernel.org>; 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>; 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 > > Hello Wenyou, > > [snip] > > >> > > >> > 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. > >> > >> Then he should remove the .of_compatible from the MFD cell definition. > > > > I tried it, > > > > But if removed this .of_compatible, and reserved the OF table. > > What we said is that you should remove both the mfd cell .of_compatible field and > the OF table. Of course removing one but leaving the other is not correct. I am confused :) > > > the &pdev->dev->of_node is NULL, the driver fails to get the > > configuration value from DT, > > > > I see, you mean that you are not able to lookup the "active-semi,vsel-high" > property. No only this property, The regulator init_datas are also not gotten from DT due to the &pdev->dev->of_node is NULL. > > > It seems the OF table still doesn't work. Where is wrong? > > > > I think the problem is with your DT binding. You have a PMIC dev node with > compatible "active-semi,act8945a" that has a child node with compatible "active- > semi,act8945a-regulator" which in turn has the "regulators" node. > > I believe what Mark says is that there shouldn't be a compatible and node for the > regulators IC since is part of the PMIC. IOW, you can't have it as a standalone > node in the DTS. > > > Could you help give some suggestion? > > > > I would just remove the "active-semi,act8945a-regulator" node and make "active- > semi,vsel-high" a property of the "active-semi,act8945a" node. > That way you can remove the mfd cell .of_compatible and OF table in the > regulator driver and read the "active-semi,vsel-high" using the platform device's > parent of_node. > > But it's better if you wait for Mark's opinions before re-spining your patches. All right. > > Best regards, > Javier Thanks Best Regards, Wenyou Yang
>>>>> "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.
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. > > -- > Bye, Peter Korsgaard Best Regards, Wenyou Yang
> -----Original Message----- > From: Javier Martinez Canillas [mailto:javier@dowhile0.org] > Sent: 2016?2?1? 21:15 > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > Cc: Mark Brown <broonie@kernel.org>; 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>; 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 > > Hello Wenyou, > > [snip] > > >> > > >> > 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. > >> > >> Then he should remove the .of_compatible from the MFD cell definition. > > > > I tried it, > > > > But if removed this .of_compatible, and reserved the OF table. > > What we said is that you should remove both the mfd cell .of_compatible field and > the OF table. Of course removing one but leaving the other is not correct. > > > the &pdev->dev->of_node is NULL, the driver fails to get the > > configuration value from DT, > > > > I see, you mean that you are not able to lookup the "active-semi,vsel-high" > property. > > > It seems the OF table still doesn't work. Where is wrong? > > > > I think the problem is with your DT binding. You have a PMIC dev node with > compatible "active-semi,act8945a" that has a child node with compatible "active- > semi,act8945a-regulator" which in turn has the "regulators" node. > > I believe what Mark says is that there shouldn't be a compatible and node for the > regulators IC since is part of the PMIC. IOW, you can't have it as a standalone > node in the DTS. > > > Could you help give some suggestion? > > > > I would just remove the "active-semi,act8945a-regulator" node and make "active- > semi,vsel-high" a property of the "active-semi,act8945a" node. > That way you can remove the mfd cell .of_compatible and OF table in the > regulator driver and read the "active-semi,vsel-high" using the platform device's > parent of_node. > > But it's better if you wait for Mark's opinions before re-spining your patches. Hi Mark, what is your opinions? > > Best regards, > Javier Best Regards, Wenyou Yang
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
On Wed, Feb 03, 2016 at 02:33:11AM +0000, Yang, Wenyou wrote: > > I would just remove the "active-semi,act8945a-regulator" node and make "active- > > semi,vsel-high" a property of the "active-semi,act8945a" node. > > That way you can remove the mfd cell .of_compatible and OF table in the > > regulator driver and read the "active-semi,vsel-high" using the platform device's > > parent of_node. > > But it's better if you wait for Mark's opinions before re-spining your patches. > Hi Mark, what is your opinions? Like Javier says you should make your DT binding look like the DT bindings for other MFDs and not use of_compatible.
> -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: 2016?2?4? 1:48 > To: Yang, Wenyou <Wenyou.Yang@atmel.com> > Cc: Javier Martinez Canillas <javier@dowhile0.org>; 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>; 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 > > On Wed, Feb 03, 2016 at 02:33:11AM +0000, Yang, Wenyou wrote: > > > > I would just remove the "active-semi,act8945a-regulator" node and > > > make "active- semi,vsel-high" a property of the "active-semi,act8945a" node. > > > That way you can remove the mfd cell .of_compatible and OF table in > > > the regulator driver and read the "active-semi,vsel-high" using the > > > platform device's parent of_node. > > > > But it's better if you wait for Mark's opinions before re-spining your patches. > > > Hi Mark, what is your opinions? > > Like Javier says you should make your DT binding look like the DT bindings for > other MFDs and not use of_compatible. All right. Thank you, Mark, Javier. Best Regards, Wenyou Yang
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig index 8155e80..74a6354 100644 --- a/drivers/regulator/Kconfig +++ b/drivers/regulator/Kconfig @@ -78,6 +78,15 @@ config REGULATOR_ACT8865 This driver controls a active-semi act8865 voltage output regulator via I2C bus. +config REGULATOR_ACT8945A + tristate "Active-semi ACT8945A voltage regulator" + depends on MFD_ACT8945A + help + This driver controls a active-semi ACT8945A voltage regulator + via I2C bus. The ACT8945A features three step-down DC/DC converters + and four low-dropout linear regulators, along with a ActivePath + battery charger. + config REGULATOR_AD5398 tristate "Analog Devices AD5398/AD5821 regulators" depends on I2C diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile index 980b194..348cfd7 100644 --- a/drivers/regulator/Makefile +++ b/drivers/regulator/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_REGULATOR_AAT2870) += aat2870-regulator.o obj-$(CONFIG_REGULATOR_AB3100) += ab3100.o obj-$(CONFIG_REGULATOR_AB8500) += ab8500-ext.o ab8500.o obj-$(CONFIG_REGULATOR_ACT8865) += act8865-regulator.o +obj-$(CONFIG_REGULATOR_ACT8945A) += act8945a-regulator.o obj-$(CONFIG_REGULATOR_AD5398) += ad5398.o obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o obj-$(CONFIG_REGULATOR_ARIZONA) += arizona-micsupp.o arizona-ldo1.o diff --git a/drivers/regulator/act8945a-regulator.c b/drivers/regulator/act8945a-regulator.c new file mode 100644 index 0000000..990f0b7 --- /dev/null +++ b/drivers/regulator/act8945a-regulator.c @@ -0,0 +1,172 @@ +/* + * Voltage regulation driver for active-semi ACT8945A PMIC + * + * Copyright (C) 2015 Atmel Corporation + * + * Author: Wenyou Yang <wenyou.yang@atmel.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + */ + +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/regulator/driver.h> +#include <linux/regulator/machine.h> + +/** + * ACT8945A Global Register Map. + */ +#define ACT8945A_SYS_MODE 0x00 +#define ACT8945A_SYS_CTRL 0x01 +#define ACT8945A_DCDC1_VSET1 0x20 +#define ACT8945A_DCDC1_VSET2 0x21 +#define ACT8945A_DCDC1_CTRL 0x22 +#define ACT8945A_DCDC2_VSET1 0x30 +#define ACT8945A_DCDC2_VSET2 0x31 +#define ACT8945A_DCDC2_CTRL 0x32 +#define ACT8945A_DCDC3_VSET1 0x40 +#define ACT8945A_DCDC3_VSET2 0x41 +#define ACT8945A_DCDC3_CTRL 0x42 +#define ACT8945A_LDO1_VSET 0x50 +#define ACT8945A_LDO1_CTRL 0x51 +#define ACT8945A_LDO2_VSET 0x54 +#define ACT8945A_LDO2_CTRL 0x55 +#define ACT8945A_LDO3_VSET 0x60 +#define ACT8945A_LDO3_CTRL 0x61 +#define ACT8945A_LDO4_VSET 0x64 +#define ACT8945A_LDO4_CTRL 0x65 + +/** + * Field Definitions. + */ +#define ACT8945A_ENA 0x80 /* ON - [7] */ +#define ACT8945A_VSEL_MASK 0x3F /* VSET - [5:0] */ + +/** + * ACT8945A Voltage Number + */ +#define ACT8945A_VOLTAGE_NUM 64 + +enum { + ACT8945A_ID_DCDC1, + ACT8945A_ID_DCDC2, + ACT8945A_ID_DCDC3, + ACT8945A_ID_LDO1, + ACT8945A_ID_LDO2, + ACT8945A_ID_LDO3, + ACT8945A_ID_LDO4, + ACT8945A_REG_NUM, +}; + +static const struct regulator_linear_range act8945a_voltage_ranges[] = { + REGULATOR_LINEAR_RANGE(600000, 0, 23, 25000), + REGULATOR_LINEAR_RANGE(1200000, 24, 47, 50000), + REGULATOR_LINEAR_RANGE(2400000, 48, 63, 100000), +}; + +static struct regulator_ops act8945a_ops = { + .list_voltage = regulator_list_voltage_linear_range, + .map_voltage = regulator_map_voltage_linear_range, + .get_voltage_sel = regulator_get_voltage_sel_regmap, + .set_voltage_sel = regulator_set_voltage_sel_regmap, + .enable = regulator_enable_regmap, + .disable = regulator_disable_regmap, + .is_enabled = regulator_is_enabled_regmap, +}; + +#define ACT89xx_REG(_name, _family, _id, _vsel_reg, _supply) \ + [_family##_ID_##_id] = { \ + .name = _name, \ + .supply_name = _supply, \ + .of_match = of_match_ptr("REG_"#_id), \ + .regulators_node = of_match_ptr("regulators"), \ + .id = _family##_ID_##_id, \ + .type = REGULATOR_VOLTAGE, \ + .ops = &act8945a_ops, \ + .n_voltages = ACT8945A_VOLTAGE_NUM, \ + .linear_ranges = act8945a_voltage_ranges, \ + .n_linear_ranges = ARRAY_SIZE(act8945a_voltage_ranges), \ + .vsel_reg = _family##_##_id##_##_vsel_reg, \ + .vsel_mask = ACT8945A_VSEL_MASK, \ + .enable_reg = _family##_##_id##_CTRL, \ + .enable_mask = ACT8945A_ENA, \ + .owner = THIS_MODULE, \ + } + +static const struct regulator_desc act8945a_regulators[] = { + ACT89xx_REG("DCDC_REG1", ACT8945A, DCDC1, VSET1, "vp1"), + ACT89xx_REG("DCDC_REG2", ACT8945A, DCDC2, VSET1, "vp2"), + ACT89xx_REG("DCDC_REG3", ACT8945A, DCDC3, VSET1, "vp3"), + ACT89xx_REG("LDO_REG1", ACT8945A, LDO1, VSET, "inl45"), + ACT89xx_REG("LDO_REG2", ACT8945A, LDO2, VSET, "inl45"), + ACT89xx_REG("LDO_REG3", ACT8945A, LDO3, VSET, "inl67"), + ACT89xx_REG("LDO_REG4", ACT8945A, LDO4, VSET, "inl67"), +}; + +static const struct regulator_desc act8945a_alt_regulators[] = { + ACT89xx_REG("DCDC_REG1", ACT8945A, DCDC1, VSET2, "vp1"), + ACT89xx_REG("DCDC_REG2", ACT8945A, DCDC2, VSET2, "vp2"), + ACT89xx_REG("DCDC_REG3", ACT8945A, DCDC3, VSET2, "vp3"), + ACT89xx_REG("LDO_REG1", ACT8945A, LDO1, VSET, "inl45"), + ACT89xx_REG("LDO_REG2", ACT8945A, LDO2, VSET, "inl45"), + ACT89xx_REG("LDO_REG3", ACT8945A, LDO3, VSET, "inl67"), + ACT89xx_REG("LDO_REG4", ACT8945A, LDO4, VSET, "inl67"), +}; + +static int act8945a_pmic_probe(struct platform_device *pdev) +{ + struct regulator_config config = { }; + const struct regulator_desc *regulators; + struct regulator_dev *rdev; + int i, num_regulators; + bool voltage_select; + + voltage_select = of_property_read_bool(pdev->dev.of_node, + "active-semi,vsel-high"); + + if (voltage_select) { + regulators = act8945a_alt_regulators; + num_regulators = ARRAY_SIZE(act8945a_alt_regulators); + } else { + regulators = act8945a_regulators; + num_regulators = ARRAY_SIZE(act8945a_regulators); + } + + for (i = 0; i < num_regulators; i++) { + config.dev = &pdev->dev; + + rdev = devm_regulator_register(&pdev->dev, ®ulators[i], &config); + if (IS_ERR(rdev)) { + dev_err(&pdev->dev, + "failed to register %s regulator\n", + regulators[i].name); + return PTR_ERR(rdev); + } + } + + return 0; +} + +static const struct of_device_id act8945a_pmic_of_match[] = { + { .compatible = "active-semi,act8945a-regulator" }, + { }, +}; +MODULE_DEVICE_TABLE(of, act8945a_pmic_of_match); + +static struct platform_driver act8945a_pmic_driver = { + .driver = { + .name = "act8945a-regulator", + .of_match_table = of_match_ptr(act8945a_pmic_of_match), + }, + .probe = act8945a_pmic_probe, +}; +module_platform_driver(act8945a_pmic_driver); + +MODULE_DESCRIPTION("Active-semi ACT8945A voltage regulator driver"); +MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>"); +MODULE_LICENSE("GPL");
This patch adds new regulator driver to support ACT8945A MFD chip's regulators. The ACT8945A has three step-down DC/DC converters and four low-dropout regulators. Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> --- Changes in v4: - change the name "act8945a-pmic" to "act8945a-regulator" to align mfd subdevice's name. Changes in v3: - use the core regulators_node and of_match implementation, remove the redundant code. - use the core dev_get_regmap(dev->parent) code, remove the redundant code. - change the regulator name to easily implement macro. - use a space after #define, not tabs. - add OF match table. Changes in v2: - add more help information in Kconfig. - update the file header with short version license and author line. - remove unused structure, struct act8945a_pmic. - substitute of_property_read_bool() for of_get_property(). - due to removing the member of stuct act8945a_dev, dev. - remove the unnecessary print out. - use module_platform_driver(), instead of subsys_initcall(). - substitute MODULE_LICENSE("GPL") for MODULE_LICENSE("GPL v2"). drivers/regulator/Kconfig | 9 ++ drivers/regulator/Makefile | 1 + drivers/regulator/act8945a-regulator.c | 172 ++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+) create mode 100644 drivers/regulator/act8945a-regulator.c