| Message ID | 1452514817-118311-4-git-send-email-puck.chen@hisilicon.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, 11 Jan 2016, Chen Feng wrote: > Add pmic mfd driver to support hisilicon hi665x. PMIC MFD > Signed-off-by: Chen Feng <puck.chen@hisilicon.com> > Signed-off-by: Fei Wang <w.f@huawei.com> > Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com> > --- > drivers/mfd/Kconfig | 10 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/hi655x-pmic.c | 169 ++++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/hi655x-pmic.h | 56 +++++++++++++ > 4 files changed, 236 insertions(+) > create mode 100644 drivers/mfd/hi655x-pmic.c > create mode 100644 include/linux/mfd/hi655x-pmic.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 4d92df6..299d972 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -284,6 +284,16 @@ config MFD_HI6421_PMIC > menus in order to enable them. > We communicate with the Hi6421 via memory-mapped I/O. > > +config MFD_HI655X_PMIC > + tristate "HiSilicon Hi655X series PMU/Codec IC" > + depends on ARCH_HISI || (COMPILE_TEST && ARM64) Why not just COMPILE_TEST? > + depends on OF So this will not COMPILE_TEST if OF is not enabled. > + select MFD_CORE > + select REGMAP_MMIO > + select REGMAP_IRQ > + help > + Select this option to enable Hisilicon hi655x series pmic driver. > + > config HTC_EGPIO > bool "HTC EGPIO support" > depends on GPIOLIB && ARM > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index a8b76b8..6a7b0e1 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -186,6 +186,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_HI655X_PMIC) += hi655x-pmic.o > obj-$(CONFIG_MFD_DLN2) += dln2.o > obj-$(CONFIG_MFD_RT5033) += rt5033.o > obj-$(CONFIG_MFD_SKY81452) += sky81452.o > diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c > new file mode 100644 > index 0000000..aab18f7 > --- /dev/null > +++ b/drivers/mfd/hi655x-pmic.c > @@ -0,0 +1,169 @@ > +/* > + * Device driver for regulators in hi655x IC We know it's a device driver. And I hope it's not a regulator driver. > + * Copyright (c) 2016 Hisilicon. > + * > + * Chen Feng <puck.chen@hisilicon.com> > + * Fei Wang <w.f@huawei.com> Author(s): > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/io.h> > +#include <linux/interrupt.h> > +#include <linux/init.h> > +#include <linux/gpio.h> > +#include <linux/of_gpio.h> > +#include <linux/platform_device.h> > +#include <linux/of_platform.h> > +#include <linux/mfd/core.h> > +#include <linux/mfd/hi655x-pmic.h> > +#include <linux/regmap.h> Alphabetical please. > +static const struct mfd_cell hi655x_pmic_devs[] = { > + { .name = "hi655x-regulator", }, > +}; What other devices are there? > +static const struct regmap_irq hi655x_irqs[] = { > + { .reg_offset = 0, .mask = OTMP_D1R_INT }, > + { .reg_offset = 0, .mask = VSYS_2P5_R_INT }, > + { .reg_offset = 0, .mask = VSYS_UV_D3R_INT }, > + { .reg_offset = 0, .mask = VSYS_6P0_D200UR_INT }, > + { .reg_offset = 0, .mask = PWRON_D4SR_INT }, > + { .reg_offset = 0, .mask = PWRON_D20F_INT }, > + { .reg_offset = 0, .mask = PWRON_D20R_INT }, > + { .reg_offset = 0, .mask = RESERVE_INT }, > +}; > + > +static const struct regmap_irq_chip hi655x_irq_chip = { > + .name = "hi655x-pmic", > + .irqs = hi655x_irqs, > + .num_regs = 1, > + .num_irqs = ARRAY_SIZE(hi655x_irqs), > + .status_base = HI655X_IRQ_STAT_BASE, > + .mask_base = HI655X_IRQ_MASK_BASE, > +}; > + > +static unsigned int hi655x_pmic_get_version(struct hi655x_pmic *pmic) > +{ > + u32 val; > + > + regmap_read(pmic->regmap, > + HI655X_BUS_ADDR(HI655X_VER_REG), &val); > + > + return val; > +} This is a small function that you only use once. Probably better just to call regmap_read() directly from below. > +static struct regmap_config hi655x_regmap_config = { > + .reg_bits = 32, > + .reg_stride = HI655X_STRIDE, > + .val_bits = 8, > + .max_register = HI655X_BUS_ADDR(0xFFF), > +}; > + > +static void hi655x_local_irq_clear(struct regmap *map) > +{ > + int i; > + > + regmap_write(map, HI655X_ANA_IRQM_BASE, HI655X_IRQ_CLR); > + for (i = 0; i < HI655X_IRQ_ARRAY; i++) { > + regmap_write(map, HI655X_IRQ_STAT_BASE + i * HI655X_STRIDE, > + HI655X_IRQ_CLR); > + } > +} > + > +static int hi655x_pmic_probe(struct platform_device *pdev) > +{ > + int ret; > + struct hi655x_pmic *pmic; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + void __iomem *base; > + > + pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL); You need a NULL check here. > + pmic->dev = dev; > + > + pmic->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!pmic->res) { > + dev_err(dev, "platform_get_resource err\n"); This is not a good error message. Besides, we don't normally try to catch this error, as it's checked for you in devm_ioremap_resource(). > + return -ENOENT; > + } '\n' here. > + base = devm_ioremap_resource(dev, pmic->res); > + if (!base) { > + dev_err(dev, "cannot map register memory\n"); devm_ioremap_resource() already prints a message on error. No need for this. > + return -ENOMEM; > + } > + pmic->regmap = devm_regmap_init_mmio_clk(dev, NULL, base, > + &hi655x_regmap_config); You need to check for an error here. > + pmic->ver = hi655x_pmic_get_version(pmic); > + if ((pmic->ver < PMU_VER_START) || (pmic->ver > PMU_VER_END)) { > + dev_warn(dev, "it is wrong pmu version\n"); You can't issue a _warn, then return an error. Please use _err. ... and change to "PMU version %d unsupported". > + return -EINVAL; > + } > + > + hi655x_local_irq_clear(pmic->regmap); > + > + pmic->gpio = of_get_named_gpio(np, "pmic-gpios", 0); > + if (!gpio_is_valid(pmic->gpio)) { > + dev_err(dev, "cannot get the pmic-gpios\n"); > + return -ENODEV; > + } > + > + ret = devm_gpio_request_one(dev, pmic->gpio, GPIOF_IN, > + "hi655x_pmic_irq"); > + if (ret < 0) { > + dev_err(dev, "failed to request gpio %d ret = %d\n", More readable if you use the IRQ name, as you know it. "Failed to obtain 'hi655x_pmic_irq'" > + pmic->gpio, ret); > + return ret; > + } > + > + ret = regmap_add_irq_chip(pmic->regmap, gpio_to_irq(pmic->gpio), > + IRQF_TRIGGER_LOW | IRQF_NO_SUSPEND, 0, > + &hi655x_irq_chip, &pmic->irq_data); > + if (ret) { > + dev_err(dev, "add pmic irq chip error! ret %d\n", ret); > + return ret; > + } > + > + /* bind pmic to device */ "Bind PMIC to device" It would be better if you dropped the comment completely to be honest. > + platform_set_drvdata(pdev, pmic); > + > + ret = mfd_add_devices(dev, 0, hi655x_pmic_devs, Please use the #defines for the second parameter. > + ARRAY_SIZE(hi655x_pmic_devs), NULL, 0, NULL); > + if (ret) { > + dev_err(dev, "add mfd devices failed: %d\n", ret); "Failed to register device". > + regmap_del_irq_chip(pmic->irq, pmic->irq_data); > + return ret; > + } > + > + return 0; > +} > + > +static int hi655x_pmic_remove(struct platform_device *pdev) > +{ > + mfd_remove_devices(&pdev->dev); > + > + return 0; > +} > + > +static const struct of_device_id of_hi655x_pmic_match_tbl[] = { > + { .compatible = "hisilicon,hi655x-pmic", }, > + {}, > +}; > + > +static struct platform_driver hi655x_pmic_driver = { > + .driver = { > + .name = "hi655x-pmic", > + .of_match_table = of_hi655x_pmic_match_tbl, of_match_ptr() > + }, > + .probe = hi655x_pmic_probe, > + .remove = hi655x_pmic_remove, > +}; > +module_platform_driver(hi655x_pmic_driver); > + > +MODULE_AUTHOR("Chen Feng <puck.chen@hisilicon.com>"); > +MODULE_DESCRIPTION("Hisilicon hi655x pmic driver"); PMIC > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/mfd/hi655x-pmic.h b/include/linux/mfd/hi655x-pmic.h > new file mode 100644 > index 0000000..e1af0a1 > --- /dev/null > +++ b/include/linux/mfd/hi655x-pmic.h > @@ -0,0 +1,56 @@ > +/* > + * Device driver for regulators in hi655x IC > + * > + * Copyright (c) 2016 Hisilicon. > + * > + * Chen Feng <puck.chen@hisilicon.com> > + * Fei Wang <w.f@huawei.com> Author(s): > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#ifndef __HI655X_PMIC_H > +#define __HI655X_PMIC_H > + > +/* Hi655x registers are mapped to memory bus in 4 bytes stride */ > +#define HI655X_STRIDE (4) > +#define HI655X_BUS_ADDR(x) ((x) << 2) > + > +#define HI655X_BITS (8) > + > +#define HI655X_NR_IRQ (32) > + > +#define HI655X_IRQ_STAT_BASE (0x003 << 2) > +#define HI655X_IRQ_MASK_BASE (0x007 << 2) > +#define HI655X_ANA_IRQM_BASE (0x1b5 << 2) > +#define HI655X_IRQ_ARRAY (4) > +#define HI655X_IRQ_MASK (0xFF) > +#define HI655X_IRQ_CLR (0xFF) > +#define HI655X_VER_REG (0x00) > + > +#define PMU_VER_START (0x10) > +#define PMU_VER_END (0x38) > + > +#define RESERVE_INT (BIT(7)) > +#define PWRON_D20R_INT (BIT(6)) > +#define PWRON_D20F_INT (BIT(5)) > +#define PWRON_D4SR_INT (BIT(4)) > +#define VSYS_6P0_D200UR_INT (BIT(3)) > +#define VSYS_UV_D3R_INT (BIT(2)) > +#define VSYS_2P5_R_INT (BIT(1)) > +#define OTMP_D1R_INT (BIT(0)) No need for the extra ()'s > +struct hi655x_pmic { > + struct resource *res; > + struct device *dev; > + struct regmap *regmap; > + struct clk *clk; Is this used? > + int irq; > + int gpio; > + unsigned int ver; > + struct regmap_irq_chip_data *irq_data; > +}; Check to see if they are all used. > +#endif -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
On 2016/1/25 22:22, Lee Jones wrote: > On Mon, 11 Jan 2016, Chen Feng wrote: > >> Add pmic mfd driver to support hisilicon hi665x. > > PMIC MFD > ok. >> Signed-off-by: Chen Feng <puck.chen@hisilicon.com> >> Signed-off-by: Fei Wang <w.f@huawei.com> >> Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com> >> --- >> drivers/mfd/Kconfig | 10 +++ >> drivers/mfd/Makefile | 1 + >> drivers/mfd/hi655x-pmic.c | 169 ++++++++++++++++++++++++++++++++++++++++ >> include/linux/mfd/hi655x-pmic.h | 56 +++++++++++++ >> 4 files changed, 236 insertions(+) >> create mode 100644 drivers/mfd/hi655x-pmic.c >> create mode 100644 include/linux/mfd/hi655x-pmic.h >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> index 4d92df6..299d972 100644 >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -284,6 +284,16 @@ config MFD_HI6421_PMIC >> menus in order to enable them. >> We communicate with the Hi6421 via memory-mapped I/O. >> >> +config MFD_HI655X_PMIC >> + tristate "HiSilicon Hi655X series PMU/Codec IC" >> + depends on ARCH_HISI || (COMPILE_TEST && ARM64) > > Why not just COMPILE_TEST? ok, the V6 already just COMPILE_TEST. > >> + depends on OF > > So this will not COMPILE_TEST if OF is not enabled. > >> + select MFD_CORE >> + select REGMAP_MMIO >> + select REGMAP_IRQ >> + help >> + Select this option to enable Hisilicon hi655x series pmic driver. >> + >> config HTC_EGPIO >> bool "HTC EGPIO support" >> depends on GPIOLIB && ARM >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >> index a8b76b8..6a7b0e1 100644 >> --- a/drivers/mfd/Makefile >> +++ b/drivers/mfd/Makefile >> @@ -186,6 +186,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_HI655X_PMIC) += hi655x-pmic.o >> obj-$(CONFIG_MFD_DLN2) += dln2.o >> obj-$(CONFIG_MFD_RT5033) += rt5033.o >> obj-$(CONFIG_MFD_SKY81452) += sky81452.o >> diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c >> new file mode 100644 >> index 0000000..aab18f7 >> --- /dev/null >> +++ b/drivers/mfd/hi655x-pmic.c >> @@ -0,0 +1,169 @@ >> +/* >> + * Device driver for regulators in hi655x IC > > We know it's a device driver. And I hope it's not a regulator driver. > ok, I will change the name. >> + * Copyright (c) 2016 Hisilicon. >> + * >> + * Chen Feng <puck.chen@hisilicon.com> >> + * Fei Wang <w.f@huawei.com> > > Author(s): > ok. >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#include <linux/io.h> >> +#include <linux/interrupt.h> >> +#include <linux/init.h> >> +#include <linux/gpio.h> >> +#include <linux/of_gpio.h> >> +#include <linux/platform_device.h> >> +#include <linux/of_platform.h> >> +#include <linux/mfd/core.h> >> +#include <linux/mfd/hi655x-pmic.h> >> +#include <linux/regmap.h> > > Alphabetical please. > ok. >> +static const struct mfd_cell hi655x_pmic_devs[] = { >> + { .name = "hi655x-regulator", }, >> +}; > > What other devices are there? > These patches only add regulator,the RTC and Power-key will be added later. So I just list the regulator this time. >> +static const struct regmap_irq hi655x_irqs[] = { >> + { .reg_offset = 0, .mask = OTMP_D1R_INT }, >> + { .reg_offset = 0, .mask = VSYS_2P5_R_INT }, >> + { .reg_offset = 0, .mask = VSYS_UV_D3R_INT }, >> + { .reg_offset = 0, .mask = VSYS_6P0_D200UR_INT }, >> + { .reg_offset = 0, .mask = PWRON_D4SR_INT }, >> + { .reg_offset = 0, .mask = PWRON_D20F_INT }, >> + { .reg_offset = 0, .mask = PWRON_D20R_INT }, >> + { .reg_offset = 0, .mask = RESERVE_INT }, >> +}; >> + >> +static const struct regmap_irq_chip hi655x_irq_chip = { >> + .name = "hi655x-pmic", >> + .irqs = hi655x_irqs, >> + .num_regs = 1, >> + .num_irqs = ARRAY_SIZE(hi655x_irqs), >> + .status_base = HI655X_IRQ_STAT_BASE, >> + .mask_base = HI655X_IRQ_MASK_BASE, >> +}; >> + >> +static unsigned int hi655x_pmic_get_version(struct hi655x_pmic *pmic) >> +{ >> + u32 val; >> + >> + regmap_read(pmic->regmap, >> + HI655X_BUS_ADDR(HI655X_VER_REG), &val); >> + >> + return val; >> +} > > This is a small function that you only use once. > > Probably better just to call regmap_read() directly from below. > ok. >> +static struct regmap_config hi655x_regmap_config = { >> + .reg_bits = 32, >> + .reg_stride = HI655X_STRIDE, >> + .val_bits = 8, >> + .max_register = HI655X_BUS_ADDR(0xFFF), >> +}; >> + >> +static void hi655x_local_irq_clear(struct regmap *map) >> +{ >> + int i; >> + >> + regmap_write(map, HI655X_ANA_IRQM_BASE, HI655X_IRQ_CLR); >> + for (i = 0; i < HI655X_IRQ_ARRAY; i++) { >> + regmap_write(map, HI655X_IRQ_STAT_BASE + i * HI655X_STRIDE, >> + HI655X_IRQ_CLR); >> + } >> +} >> + >> +static int hi655x_pmic_probe(struct platform_device *pdev) >> +{ >> + int ret; >> + struct hi655x_pmic *pmic; >> + struct device *dev = &pdev->dev; >> + struct device_node *np = dev->of_node; >> + void __iomem *base; >> + >> + pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL); > > You need a NULL check here. > ok. >> + pmic->dev = dev; >> + >> + pmic->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!pmic->res) { >> + dev_err(dev, "platform_get_resource err\n"); > > This is not a good error message. > > Besides, we don't normally try to catch this error, as it's checked > for you in devm_ioremap_resource(). > >> + return -ENOENT; >> + } > > '\n' here. > >> + base = devm_ioremap_resource(dev, pmic->res); >> + if (!base) { >> + dev_err(dev, "cannot map register memory\n"); > > devm_ioremap_resource() already prints a message on error. No need > for this. > >> + return -ENOMEM; >> + } >> + pmic->regmap = devm_regmap_init_mmio_clk(dev, NULL, base, >> + &hi655x_regmap_config); > > You need to check for an error here. ok > >> + pmic->ver = hi655x_pmic_get_version(pmic); >> + if ((pmic->ver < PMU_VER_START) || (pmic->ver > PMU_VER_END)) { >> + dev_warn(dev, "it is wrong pmu version\n"); > > You can't issue a _warn, then return an error. Please use _err. > > ... and change to "PMU version %d unsupported". > >> + return -EINVAL; >> + } >> + >> + hi655x_local_irq_clear(pmic->regmap); >> + >> + pmic->gpio = of_get_named_gpio(np, "pmic-gpios", 0); >> + if (!gpio_is_valid(pmic->gpio)) { >> + dev_err(dev, "cannot get the pmic-gpios\n"); >> + return -ENODEV; >> + } >> + >> + ret = devm_gpio_request_one(dev, pmic->gpio, GPIOF_IN, >> + "hi655x_pmic_irq"); >> + if (ret < 0) { >> + dev_err(dev, "failed to request gpio %d ret = %d\n", > > More readable if you use the IRQ name, as you know it. > > "Failed to obtain 'hi655x_pmic_irq'" > >> + pmic->gpio, ret); >> + return ret; >> + } >> + >> + ret = regmap_add_irq_chip(pmic->regmap, gpio_to_irq(pmic->gpio), >> + IRQF_TRIGGER_LOW | IRQF_NO_SUSPEND, 0, >> + &hi655x_irq_chip, &pmic->irq_data); >> + if (ret) { >> + dev_err(dev, "add pmic irq chip error! ret %d\n", ret); >> + return ret; >> + } >> + >> + /* bind pmic to device */ > > "Bind PMIC to device" > > It would be better if you dropped the comment completely to be honest. > >> + platform_set_drvdata(pdev, pmic); >> + >> + ret = mfd_add_devices(dev, 0, hi655x_pmic_devs, > > Please use the #defines for the second parameter. > >> + ARRAY_SIZE(hi655x_pmic_devs), NULL, 0, NULL); >> + if (ret) { >> + dev_err(dev, "add mfd devices failed: %d\n", ret); > > "Failed to register device". > >> + regmap_del_irq_chip(pmic->irq, pmic->irq_data); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +static int hi655x_pmic_remove(struct platform_device *pdev) >> +{ >> + mfd_remove_devices(&pdev->dev); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id of_hi655x_pmic_match_tbl[] = { >> + { .compatible = "hisilicon,hi655x-pmic", }, >> + {}, >> +}; >> + >> +static struct platform_driver hi655x_pmic_driver = { >> + .driver = { >> + .name = "hi655x-pmic", >> + .of_match_table = of_hi655x_pmic_match_tbl, > > of_match_ptr() > >> + }, >> + .probe = hi655x_pmic_probe, >> + .remove = hi655x_pmic_remove, >> +}; >> +module_platform_driver(hi655x_pmic_driver); >> + >> +MODULE_AUTHOR("Chen Feng <puck.chen@hisilicon.com>"); >> +MODULE_DESCRIPTION("Hisilicon hi655x pmic driver"); > > PMIC > >> +MODULE_LICENSE("GPL v2"); >> diff --git a/include/linux/mfd/hi655x-pmic.h b/include/linux/mfd/hi655x-pmic.h >> new file mode 100644 >> index 0000000..e1af0a1 >> --- /dev/null >> +++ b/include/linux/mfd/hi655x-pmic.h >> @@ -0,0 +1,56 @@ >> +/* >> + * Device driver for regulators in hi655x IC >> + * >> + * Copyright (c) 2016 Hisilicon. >> + * >> + * Chen Feng <puck.chen@hisilicon.com> >> + * Fei Wang <w.f@huawei.com> > > Author(s): > >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#ifndef __HI655X_PMIC_H >> +#define __HI655X_PMIC_H >> + >> +/* Hi655x registers are mapped to memory bus in 4 bytes stride */ >> +#define HI655X_STRIDE (4) >> +#define HI655X_BUS_ADDR(x) ((x) << 2) >> + >> +#define HI655X_BITS (8) >> + >> +#define HI655X_NR_IRQ (32) >> + >> +#define HI655X_IRQ_STAT_BASE (0x003 << 2) >> +#define HI655X_IRQ_MASK_BASE (0x007 << 2) >> +#define HI655X_ANA_IRQM_BASE (0x1b5 << 2) >> +#define HI655X_IRQ_ARRAY (4) >> +#define HI655X_IRQ_MASK (0xFF) >> +#define HI655X_IRQ_CLR (0xFF) >> +#define HI655X_VER_REG (0x00) >> + >> +#define PMU_VER_START (0x10) >> +#define PMU_VER_END (0x38) >> + >> +#define RESERVE_INT (BIT(7)) >> +#define PWRON_D20R_INT (BIT(6)) >> +#define PWRON_D20F_INT (BIT(5)) >> +#define PWRON_D4SR_INT (BIT(4)) >> +#define VSYS_6P0_D200UR_INT (BIT(3)) >> +#define VSYS_UV_D3R_INT (BIT(2)) >> +#define VSYS_2P5_R_INT (BIT(1)) >> +#define OTMP_D1R_INT (BIT(0)) > > No need for the extra ()'s > >> +struct hi655x_pmic { >> + struct resource *res; >> + struct device *dev; >> + struct regmap *regmap; >> + struct clk *clk; > > Is this used? > >> + int irq; >> + int gpio; >> + unsigned int ver; >> + struct regmap_irq_chip_data *irq_data; >> +}; > > Check to see if they are all used. > >> +#endif > The other comments are all accepted, thanks for your review.
Hi Lee, Thanks for your review! There is one things need your confirm. Please help to see it below. On 2016/1/25 22:22, Lee Jones wrote: > On Mon, 11 Jan 2016, Chen Feng wrote: > >> Add pmic mfd driver to support hisilicon hi665x. [..] > >> +static const struct mfd_cell hi655x_pmic_devs[] = { >> + { .name = "hi655x-regulator", }, >> +}; > > What other devices are there? > Current the MFD PMIC driver only has regulator enable. The RTC & Power-key are not in these patch sets. They will be added later. Will this be accepted? >> +static const struct regmap_irq hi655x_irqs[] = { >> + { .reg_offset = 0, .mask = OTMP_D1R_INT },
On Thu, 28 Jan 2016, chenfeng wrote: > Hi Lee, > > Thanks for your review! > There is one things need your confirm. > Please help to see it below. > > On 2016/1/25 22:22, Lee Jones wrote: > > On Mon, 11 Jan 2016, Chen Feng wrote: > > > >> Add pmic mfd driver to support hisilicon hi665x. > [..] > > > > >> +static const struct mfd_cell hi655x_pmic_devs[] = { > >> + { .name = "hi655x-regulator", }, > >> +}; > > > > What other devices are there? > > > > Current the MFD PMIC driver only has regulator enable. > > The RTC & Power-key are not in these patch sets. > > They will be added later. > > Will this be accepted? Yes. > >> +static const struct regmap_irq hi655x_irqs[] = { > >> + { .reg_offset = 0, .mask = OTMP_D1R_INT }, > -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 4d92df6..299d972 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -284,6 +284,16 @@ config MFD_HI6421_PMIC menus in order to enable them. We communicate with the Hi6421 via memory-mapped I/O. +config MFD_HI655X_PMIC + tristate "HiSilicon Hi655X series PMU/Codec IC" + depends on ARCH_HISI || (COMPILE_TEST && ARM64) + depends on OF + select MFD_CORE + select REGMAP_MMIO + select REGMAP_IRQ + help + Select this option to enable Hisilicon hi655x series pmic driver. + config HTC_EGPIO bool "HTC EGPIO support" depends on GPIOLIB && ARM diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index a8b76b8..6a7b0e1 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -186,6 +186,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_HI655X_PMIC) += hi655x-pmic.o obj-$(CONFIG_MFD_DLN2) += dln2.o obj-$(CONFIG_MFD_RT5033) += rt5033.o obj-$(CONFIG_MFD_SKY81452) += sky81452.o diff --git a/drivers/mfd/hi655x-pmic.c b/drivers/mfd/hi655x-pmic.c new file mode 100644 index 0000000..aab18f7 --- /dev/null +++ b/drivers/mfd/hi655x-pmic.c @@ -0,0 +1,169 @@ +/* + * Device driver for regulators in hi655x IC + * + * Copyright (c) 2016 Hisilicon. + * + * Chen Feng <puck.chen@hisilicon.com> + * Fei Wang <w.f@huawei.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/io.h> +#include <linux/interrupt.h> +#include <linux/init.h> +#include <linux/gpio.h> +#include <linux/of_gpio.h> +#include <linux/platform_device.h> +#include <linux/of_platform.h> +#include <linux/mfd/core.h> +#include <linux/mfd/hi655x-pmic.h> +#include <linux/regmap.h> + +static const struct mfd_cell hi655x_pmic_devs[] = { + { .name = "hi655x-regulator", }, +}; + +static const struct regmap_irq hi655x_irqs[] = { + { .reg_offset = 0, .mask = OTMP_D1R_INT }, + { .reg_offset = 0, .mask = VSYS_2P5_R_INT }, + { .reg_offset = 0, .mask = VSYS_UV_D3R_INT }, + { .reg_offset = 0, .mask = VSYS_6P0_D200UR_INT }, + { .reg_offset = 0, .mask = PWRON_D4SR_INT }, + { .reg_offset = 0, .mask = PWRON_D20F_INT }, + { .reg_offset = 0, .mask = PWRON_D20R_INT }, + { .reg_offset = 0, .mask = RESERVE_INT }, +}; + +static const struct regmap_irq_chip hi655x_irq_chip = { + .name = "hi655x-pmic", + .irqs = hi655x_irqs, + .num_regs = 1, + .num_irqs = ARRAY_SIZE(hi655x_irqs), + .status_base = HI655X_IRQ_STAT_BASE, + .mask_base = HI655X_IRQ_MASK_BASE, +}; + +static unsigned int hi655x_pmic_get_version(struct hi655x_pmic *pmic) +{ + u32 val; + + regmap_read(pmic->regmap, + HI655X_BUS_ADDR(HI655X_VER_REG), &val); + + return val; +} + +static struct regmap_config hi655x_regmap_config = { + .reg_bits = 32, + .reg_stride = HI655X_STRIDE, + .val_bits = 8, + .max_register = HI655X_BUS_ADDR(0xFFF), +}; + +static void hi655x_local_irq_clear(struct regmap *map) +{ + int i; + + regmap_write(map, HI655X_ANA_IRQM_BASE, HI655X_IRQ_CLR); + for (i = 0; i < HI655X_IRQ_ARRAY; i++) { + regmap_write(map, HI655X_IRQ_STAT_BASE + i * HI655X_STRIDE, + HI655X_IRQ_CLR); + } +} + +static int hi655x_pmic_probe(struct platform_device *pdev) +{ + int ret; + struct hi655x_pmic *pmic; + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + void __iomem *base; + + pmic = devm_kzalloc(dev, sizeof(*pmic), GFP_KERNEL); + pmic->dev = dev; + + pmic->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!pmic->res) { + dev_err(dev, "platform_get_resource err\n"); + return -ENOENT; + } + base = devm_ioremap_resource(dev, pmic->res); + if (!base) { + dev_err(dev, "cannot map register memory\n"); + return -ENOMEM; + } + pmic->regmap = devm_regmap_init_mmio_clk(dev, NULL, base, + &hi655x_regmap_config); + + pmic->ver = hi655x_pmic_get_version(pmic); + if ((pmic->ver < PMU_VER_START) || (pmic->ver > PMU_VER_END)) { + dev_warn(dev, "it is wrong pmu version\n"); + return -EINVAL; + } + + hi655x_local_irq_clear(pmic->regmap); + + pmic->gpio = of_get_named_gpio(np, "pmic-gpios", 0); + if (!gpio_is_valid(pmic->gpio)) { + dev_err(dev, "cannot get the pmic-gpios\n"); + return -ENODEV; + } + + ret = devm_gpio_request_one(dev, pmic->gpio, GPIOF_IN, + "hi655x_pmic_irq"); + if (ret < 0) { + dev_err(dev, "failed to request gpio %d ret = %d\n", + pmic->gpio, ret); + return ret; + } + + ret = regmap_add_irq_chip(pmic->regmap, gpio_to_irq(pmic->gpio), + IRQF_TRIGGER_LOW | IRQF_NO_SUSPEND, 0, + &hi655x_irq_chip, &pmic->irq_data); + if (ret) { + dev_err(dev, "add pmic irq chip error! ret %d\n", ret); + return ret; + } + + /* bind pmic to device */ + platform_set_drvdata(pdev, pmic); + + ret = mfd_add_devices(dev, 0, hi655x_pmic_devs, + ARRAY_SIZE(hi655x_pmic_devs), NULL, 0, NULL); + if (ret) { + dev_err(dev, "add mfd devices failed: %d\n", ret); + regmap_del_irq_chip(pmic->irq, pmic->irq_data); + return ret; + } + + return 0; +} + +static int hi655x_pmic_remove(struct platform_device *pdev) +{ + mfd_remove_devices(&pdev->dev); + + return 0; +} + +static const struct of_device_id of_hi655x_pmic_match_tbl[] = { + { .compatible = "hisilicon,hi655x-pmic", }, + {}, +}; + +static struct platform_driver hi655x_pmic_driver = { + .driver = { + .name = "hi655x-pmic", + .of_match_table = of_hi655x_pmic_match_tbl, + }, + .probe = hi655x_pmic_probe, + .remove = hi655x_pmic_remove, +}; +module_platform_driver(hi655x_pmic_driver); + +MODULE_AUTHOR("Chen Feng <puck.chen@hisilicon.com>"); +MODULE_DESCRIPTION("Hisilicon hi655x pmic driver"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/mfd/hi655x-pmic.h b/include/linux/mfd/hi655x-pmic.h new file mode 100644 index 0000000..e1af0a1 --- /dev/null +++ b/include/linux/mfd/hi655x-pmic.h @@ -0,0 +1,56 @@ +/* + * Device driver for regulators in hi655x IC + * + * Copyright (c) 2016 Hisilicon. + * + * Chen Feng <puck.chen@hisilicon.com> + * Fei Wang <w.f@huawei.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#ifndef __HI655X_PMIC_H +#define __HI655X_PMIC_H + +/* Hi655x registers are mapped to memory bus in 4 bytes stride */ +#define HI655X_STRIDE (4) +#define HI655X_BUS_ADDR(x) ((x) << 2) + +#define HI655X_BITS (8) + +#define HI655X_NR_IRQ (32) + +#define HI655X_IRQ_STAT_BASE (0x003 << 2) +#define HI655X_IRQ_MASK_BASE (0x007 << 2) +#define HI655X_ANA_IRQM_BASE (0x1b5 << 2) +#define HI655X_IRQ_ARRAY (4) +#define HI655X_IRQ_MASK (0xFF) +#define HI655X_IRQ_CLR (0xFF) +#define HI655X_VER_REG (0x00) + +#define PMU_VER_START (0x10) +#define PMU_VER_END (0x38) + +#define RESERVE_INT (BIT(7)) +#define PWRON_D20R_INT (BIT(6)) +#define PWRON_D20F_INT (BIT(5)) +#define PWRON_D4SR_INT (BIT(4)) +#define VSYS_6P0_D200UR_INT (BIT(3)) +#define VSYS_UV_D3R_INT (BIT(2)) +#define VSYS_2P5_R_INT (BIT(1)) +#define OTMP_D1R_INT (BIT(0)) + +struct hi655x_pmic { + struct resource *res; + struct device *dev; + struct regmap *regmap; + struct clk *clk; + int irq; + int gpio; + unsigned int ver; + struct regmap_irq_chip_data *irq_data; +}; + +#endif