| Message ID | 20170526063518.21246-1-guodong.xu@linaro.org |
|---|---|
| Headers | show |
| Series | MFD: add driver for HiSilicon Hi6421v530 PMIC | expand |
On Fri, May 26, 2017 at 4:33 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Fri, May 26, 2017 at 8:35 AM, Guodong Xu <guodong.xu@linaro.org> wrote: >> Move hi6421_regmap_config definition from c code to common header: >> - include/linux/mfd/hi6421-pmic.h >> >> This is to improve code re-use for upcoming hi6421 series of MFD driver. >> >> Signed-off-by: Guodong Xu <guodong.xu@linaro.org> > >> diff --git a/include/linux/mfd/hi6421-pmic.h b/include/linux/mfd/hi6421-pmic.h >> index 587273e..f4674ff 100644 >> --- a/include/linux/mfd/hi6421-pmic.h >> +++ b/include/linux/mfd/hi6421-pmic.h >> @@ -38,4 +38,10 @@ struct hi6421_pmic { >> struct regmap *regmap; >> }; >> >> +static const struct regmap_config hi6421_regmap_config = { >> + .reg_bits = 32, >> + .reg_stride = 4, >> + .val_bits = 8, >> + .max_register = HI6421_REG_TO_BUS_ADDR(HI6421_REG_MAX), >> +}; >> #endif /* __HI6421_PMIC_H */ > > Header files should not have static variables in general, it will cause warnings > about unused variables when you include the header from another file > (depending on compiler version and warning options, I think older gcc > versions don't warn about this, but clang and latest gcc do). > I will fix that. > How about adding the new code into the existing > drivers/mfd/hi6421-pmic-core.c file, and splitting out the part that differs > (the regmap_update_bits is the only difference I see) Yes, indeed. > into a callback > that you reference through the of_device_id->data pointer? > Thanks Arnd. I will look into this. I'll drop hi6421v530-pmic.c and resend this patchset. -Guodong > Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 30, 2017 at 3:36 PM, Lee Jones <lee.jones@linaro.org> wrote: > On Fri, 26 May 2017, Guodong Xu wrote: > >> Add support for HiSilicon Hi6421v530 PMIC. Hi6421v530 communicates with >> main SoC via memory-mapped I/O. >> >> Hi6421v530 and Hi6421 are PMIC chips from the same vendor, HiSilicon, but >> at different revisions. They share the same memory-mapped I/O design. They >> differ in regulator details, eg. LDO voltage points. >> >> Signed-off-by: Guodong Xu <guodong.xu@linaro.org> >> Signed-off-by: Wang Xiaoyin <hw.wangxiaoyin@hisilicon.com> >> --- >> drivers/mfd/Kconfig | 9 +++++ >> drivers/mfd/Makefile | 1 + >> drivers/mfd/hi6421v530-pmic.c | 92 +++++++++++++++++++++++++++++++++++++++++++ > > As previously discussed, this support should be added to the existing > driver. Yes, I will do that. > >> 3 files changed, 102 insertions(+) >> create mode 100644 drivers/mfd/hi6421v530-pmic.c >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> index 3eb5c93..bdc8304 100644 >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -388,6 +388,15 @@ config MFD_HI6421_PMIC >> menus in order to enable them. >> We communicate with the Hi6421 via memory-mapped I/O. >> >> +config MFD_HI6421V530_PMIC >> + tristate "HiSilicon Hi6421v530 PMU/Codec IC" >> + depends on OF >> + select MFD_CORE >> + select REGMAP_MMIO >> + help >> + Add support for HiSilicon Hi6421v530 PMIC. Hi6421v530 communicates >> + with main SoC via memory-mapped I/O. >> + >> config MFD_HI655X_PMIC >> tristate "HiSilicon Hi655X series PMU/Codec IC" >> depends on ARCH_HISI || COMPILE_TEST >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >> index c16bf1e..cde62fc 100644 >> --- a/drivers/mfd/Makefile >> +++ b/drivers/mfd/Makefile >> @@ -206,6 +206,7 @@ obj-$(CONFIG_MFD_STW481X) += stw481x.o >> obj-$(CONFIG_MFD_IPAQ_MICRO) += ipaq-micro.o >> obj-$(CONFIG_MFD_MENF21BMC) += menf21bmc.o >> obj-$(CONFIG_MFD_HI6421_PMIC) += hi6421-pmic-core.o >> +obj-$(CONFIG_MFD_HI6421V530_PMIC) += hi6421v530-pmic.o >> obj-$(CONFIG_MFD_HI655X_PMIC) += hi655x-pmic.o >> obj-$(CONFIG_MFD_DLN2) += dln2.o >> obj-$(CONFIG_MFD_RT5033) += rt5033.o >> diff --git a/drivers/mfd/hi6421v530-pmic.c b/drivers/mfd/hi6421v530-pmic.c >> new file mode 100644 >> index 0000000..651659a >> --- /dev/null >> +++ b/drivers/mfd/hi6421v530-pmic.c >> @@ -0,0 +1,92 @@ >> +/* >> + * Device driver for Hi6421 IC >> + * >> + * Copyright (c) <2017> HiSilicon Technologies Co., Ltd. >> + * http://www.hisilicon.com >> + * Copyright (c) <2017> Linaro Ltd. >> + * http://www.linaro.org >> + * >> + * Author: Wang Xiaoyin <hw.wangxiaoyin@hisilicon.com> >> + * Guodong Xu <guodong.xu@linaro.org> >> + * >> + * 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. >> + * >> + * This program is distributed in the hope it will be useful, but WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> + * more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. > > Can you use the shorter licence script? > >> + */ >> + >> +#include <linux/device.h> >> +#include <linux/err.h> >> +#include <linux/mfd/core.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> +#include <linux/mfd/hi6421-pmic.h> > > Alphabetical. > I will update that in the existing drivers/mfd/hi6421-pmic-core.c together with my other changes. >> +static const struct mfd_cell hi6421v530_devs[] = { >> + { .name = "hi6421v530-regulator", }, >> +}; > > Chen Feng promised me that he would add other devices to the original > driver nearly 18 months ago. Until more devices are added, these are > not MFD drivers. When will you add the remaining devices? > New devices will be added when enabling functions on the hikey960 board. v530 provides a clk for wifi/bt, that can be added very quick. >> +static int hi6421v530_pmic_probe(struct platform_device *pdev) >> +{ >> + struct hi6421_pmic *pmic; >> + struct resource *res; >> + void __iomem *base; >> + int ret; >> + >> + pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL); >> + if (!pmic) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + base = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(base)) >> + return PTR_ERR(base); >> + >> + pmic->regmap = devm_regmap_init_mmio_clk(&pdev->dev, NULL, base, >> + &hi6421_regmap_config); >> + if (IS_ERR(pmic->regmap)) { >> + dev_err(&pdev->dev, >> + "regmap init failed: %ld\n", PTR_ERR(pmic->regmap)); > > "Failed to initialise Regmap" Thanks, will update in the existing drivers/mfd/hi6421-pmic-core.c > >> + return PTR_ERR(pmic->regmap); >> + } >> + >> + platform_set_drvdata(pdev, pmic); >> + >> + ret = devm_mfd_add_devices(&pdev->dev, 0, hi6421v530_devs, > > Use the #defines provided instead of '0'. > >> + ARRAY_SIZE(hi6421v530_devs), NULL, 0, NULL); >> + if (ret) { >> + dev_err(&pdev->dev, "add mfd devices failed: %d\n", ret); > > "Failed to add child devices" > Thanks, will update in the existing drivers/mfd/hi6421-pmic-core.c >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static const struct of_device_id of_hi6421v530_pmic_match_tbl[] = { >> + { .compatible = "hisilicon,hi6421v530-pmic", }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, of_hi6421v530_pmic_match_tbl); > > Drop the "_tbl" part, it's implied and superfluous. > Will update. >> +static struct platform_driver hi6421v530_pmic_driver = { >> + .driver = { >> + .name = "hi6421v530_pmic", > > One space please. > Thanks, will update. I will send updates in v3 soon. -Guodong >> + .of_match_table = of_hi6421v530_pmic_match_tbl, >> + }, >> + .probe = hi6421v530_pmic_probe, >> +}; >> +module_platform_driver(hi6421v530_pmic_driver); >> + >> +MODULE_AUTHOR("Guodong Xu <guodong.xu@linaro.org>"); >> +MODULE_DESCRIPTION("Hi6421v530 PMIC driver"); >> +MODULE_LICENSE("GPL v2"); > > > -- > Lee Jones > Linaro STMicroelectronics Landing Team Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This patchset adds driver for HiSilicon Hi6421v530 PMIC. Mainline kernel already has driver support to a similar chip, Hi6421. Hi6421 and Hi6421v530 are both from the same vendor, HiSilicon, but they are at different revisions. They both use the same Memory-mapped I/O method to communicate with Main SoC. However, they differ quite a lot in their regulator designs. Eg. they have completely different LDO voltage points. In order to enable future extension of Hi6421v530 functionality, a new mfd driver and regulator driver are added in this patchset. Only header file hi6421-pmic.h is reused between them. Patch 2 is for just this purpose. Patch 3 and 4 adds mfd and regulator driver respectively. Patch 5 is dts change, it depends and can be applied on hi3660/hikey960 patchset [1]. Patch 6 enables the relevant config items. [1], http://www.spinics.net/lists/devicetree/msg178303.html Guodong Xu (4): dt-bindings: mfd: Add hi6421v530 bindings mfd: hi6421-pmic: move hi6421_regmap_config definition to header file mfd: hi6421v530: add support for HiSilicon Hi6421v530 arm64: defconfig: enable hi6421v530 MFD and regulator Wang Xiaoyin (2): regulator: hi6421v530: add driver for hi6421v530 voltage regulator arm64: dts: hikey960: add device node for pmic and regulators .../bindings/mfd/hisilicon,hi6421v530.txt | 25 ++ arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 60 ++++ arch/arm64/configs/defconfig | 2 + drivers/mfd/Kconfig | 9 + drivers/mfd/Makefile | 1 + drivers/mfd/hi6421-pmic-core.c | 7 - drivers/mfd/hi6421v530-pmic.c | 92 ++++++ drivers/regulator/Kconfig | 10 + drivers/regulator/Makefile | 1 + drivers/regulator/hi6421v530-regulator.c | 355 +++++++++++++++++++++ include/linux/mfd/hi6421-pmic.h | 6 + 11 files changed, 561 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/mfd/hisilicon,hi6421v530.txt create mode 100644 drivers/mfd/hi6421v530-pmic.c create mode 100644 drivers/regulator/hi6421v530-regulator.c -- 2.10.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html