All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] mfd: pf1550: add pf1550 mfd driver
@ 2018-04-17 14:20 Abel Vesa
  2018-04-17 14:20 ` [PATCH 2/2] power: pf1550_charger: add pf1550 charger driver Abel Vesa
  2018-04-24 11:27 ` [PATCH 1/2] mfd: pf1550: add pf1550 mfd driver Enric Balletbo Serra
  0 siblings, 2 replies; 4+ messages in thread
From: Abel Vesa @ 2018-04-17 14:20 UTC (permalink / raw)
  To: Lee Jones, Sebastian Reichel, Robin Gong
  Cc: linux-kernel, linux-pm, linux-imx, Abel Vesa, Robin Gong, Abel Vesa

From: Robin Gong <b38343@freescale.com>

Add basic pf1550 mfd driver.

Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/mfd/Kconfig        |  14 +++
 drivers/mfd/Makefile       |   2 +
 drivers/mfd/pf1550.c       | 282 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/pf1550.h | 241 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 539 insertions(+)
 create mode 100644 drivers/mfd/pf1550.c
 create mode 100644 include/linux/mfd/pf1550.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5..5acd239 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -404,6 +404,20 @@ config MFD_MX25_TSADC
 	  i.MX25 processors. They consist of a conversion queue for general
 	  purpose ADC and a queue for Touchscreens.
 
+config MFD_PF1550
+	tristate "Freescale Semiconductor PF1550 PMIC Support"
+	depends on I2C=y
+	select MFD_CORE
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  Say yes here to add support for Freescale Semiconductor PF1550.
+	  This is a companion Power Management IC with regulators, ONKEY,
+	  and charger control on chip.
+	  This driver provides common support for accessing the device;
+	  additional drivers must be enabled in order to use the functionality
+	  of the device.
+
 config MFD_HI6421_PMIC
 	tristate "HiSilicon Hi6421 PMU/Codec IC"
 	depends on OF
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e9fd20d..b8ac19b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -97,6 +97,8 @@ obj-$(CONFIG_MFD_MC13XXX)	+= mc13xxx-core.o
 obj-$(CONFIG_MFD_MC13XXX_SPI)	+= mc13xxx-spi.o
 obj-$(CONFIG_MFD_MC13XXX_I2C)	+= mc13xxx-i2c.o
 
+obj-$(CONFIG_MFD_PF1550)	+= pf1550.o
+
 obj-$(CONFIG_MFD_CORE)		+= mfd-core.o
 
 obj-$(CONFIG_EZX_PCAP)		+= ezx-pcap.o
diff --git a/drivers/mfd/pf1550.c b/drivers/mfd/pf1550.c
new file mode 100644
index 0000000..70817ef
--- /dev/null
+++ b/drivers/mfd/pf1550.c
@@ -0,0 +1,282 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * pf1550.c - mfd core driver for the PF1550
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Robin Gong <yibin.gong@freescale.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.
+ *
+ * This program is distributed in the hope that 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.
+ *
+ * This driver is based on max77693.c
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/pf1550.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+static const struct mfd_cell pf1550_devs[] = {
+	{
+		.name = "pf1550-regulator",
+		.of_compatible = "fsl,pf1550-regulator",
+	},
+	{
+		.name = "pf1550-onkey",
+		.of_compatible = "fsl,pf1550-onkey",
+	},
+	{
+		.name = "pf1550-charger",
+		.of_compatible = "fsl,pf1550-charger",
+	},
+};
+
+static const struct regmap_config pf1550_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = PF1550_PMIC_REG_END,
+};
+
+static const struct regmap_irq pf1550_regulator_irqs[] = {
+	{ .reg_offset = 0, .mask = PMIC_IRQ_SW1_LS,		},
+	{ .reg_offset = 0, .mask = PMIC_IRQ_SW2_LS,		},
+	{ .reg_offset = 0, .mask = PMIC_IRQ_SW3_LS,		},
+
+	{ .reg_offset = 3, .mask = PMIC_IRQ_SW1_HS,		},
+	{ .reg_offset = 3, .mask = PMIC_IRQ_SW2_HS,		},
+	{ .reg_offset = 3, .mask = PMIC_IRQ_SW3_HS,		},
+
+	{ .reg_offset = 16, .mask = PMIC_IRQ_LDO1_FAULT,	},
+	{ .reg_offset = 16, .mask = PMIC_IRQ_LDO2_FAULT,	},
+	{ .reg_offset = 16, .mask = PMIC_IRQ_LDO3_FAULT,	},
+
+	{ .reg_offset = 22, .mask = PMIC_IRQ_TEMP_110,	},
+	{ .reg_offset = 22, .mask = PMIC_IRQ_TEMP_125,	},
+};
+
+static const struct regmap_irq_chip pf1550_regulator_irq_chip = {
+	.name			= "pf1550-regulator",
+	.status_base		= PF1550_PMIC_REG_SW_INT_STAT0,
+	.mask_base		= PF1550_PMIC_REG_SW_INT_MASK0,
+	.mask_invert		= false,
+	.num_regs		= 4,
+	.irqs			= pf1550_regulator_irqs,
+	.num_irqs		= ARRAY_SIZE(pf1550_regulator_irqs),
+};
+
+static const struct regmap_irq pf1550_onkey_irqs[] = {
+	{ .reg_offset = 0, .mask = ONKEY_IRQ_PUSHI,		},
+	{ .reg_offset = 0, .mask = ONKEY_IRQ_1SI,		},
+	{ .reg_offset = 0, .mask = ONKEY_IRQ_2SI,		},
+	{ .reg_offset = 0, .mask = ONKEY_IRQ_3SI,		},
+	{ .reg_offset = 0, .mask = ONKEY_IRQ_4SI,		},
+	{ .reg_offset = 0, .mask = ONKEY_IRQ_8SI,		},
+};
+
+static const struct regmap_irq_chip pf1550_onkey_irq_chip = {
+	.name			= "pf1550-onkey",
+	.status_base		= PF1550_PMIC_REG_ONKEY_INT_STAT0,
+	.mask_base		= PF1550_PMIC_REG_ONKEY_INT_MASK0,
+	.mask_invert		= false,
+	.num_regs		= 1,
+	.irqs			= pf1550_onkey_irqs,
+	.num_irqs		= ARRAY_SIZE(pf1550_onkey_irqs),
+};
+
+static const struct regmap_irq pf1550_charger_irqs[] = {
+	{ .reg_offset = 0, .mask = CHARG_IRQ_BAT2SOCI,		},
+	{ .reg_offset = 0, .mask = CHARG_IRQ_BATI,		},
+	{ .reg_offset = 0, .mask = CHARG_IRQ_CHGI,		},
+	{ .reg_offset = 0, .mask = CHARG_IRQ_VBUSI,		},
+	{ .reg_offset = 0, .mask = CHARG_IRQ_DPMI,		},
+	{ .reg_offset = 0, .mask = CHARG_IRQ_THMI,		},
+};
+
+static const struct regmap_irq_chip pf1550_charger_irq_chip = {
+	.name			= "pf1550-charger",
+	.status_base		= PF1550_CHARG_REG_CHG_INT,
+	.mask_base		= PF1550_CHARG_REG_CHG_INT_MASK,
+	.mask_invert		= false,
+	.num_regs		= 1,
+	.irqs			= pf1550_charger_irqs,
+	.num_irqs		= ARRAY_SIZE(pf1550_charger_irqs),
+};
+
+static int pf1550_i2c_probe(struct i2c_client *i2c,
+			    const struct i2c_device_id *id)
+{
+	struct pf1550_dev *pf1550;
+	unsigned int reg_data = 0;
+	int ret = 0;
+
+	pf1550 = devm_kzalloc(&i2c->dev,
+			sizeof(struct pf1550_dev), GFP_KERNEL);
+	if (!pf1550)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, pf1550);
+	pf1550->dev = &i2c->dev;
+	pf1550->i2c = i2c;
+	pf1550->irq = i2c->irq;
+
+	pf1550->regmap = devm_regmap_init_i2c(i2c, &pf1550_regmap_config);
+	if (IS_ERR(pf1550->regmap)) {
+		ret = PTR_ERR(pf1550->regmap);
+		dev_err(pf1550->dev, "failed to allocate register map: %d\n",
+				ret);
+		return ret;
+	}
+
+	ret = regmap_read(pf1550->regmap, PF1550_PMIC_REG_DEVICE_ID, &reg_data);
+	if (ret < 0 || reg_data != 0x7c) {
+		dev_err(pf1550->dev, "device not found!\n");
+		return ret;
+	}
+
+	pf1550->type = PF1550;
+	dev_info(pf1550->dev, "pf1550 found.\n");
+
+	ret = regmap_add_irq_chip(pf1550->regmap, pf1550->irq,
+				IRQF_ONESHOT | IRQF_SHARED |
+				IRQF_TRIGGER_FALLING, 0,
+				&pf1550_regulator_irq_chip,
+				&pf1550->irq_data_regulator);
+	if (ret) {
+		dev_err(pf1550->dev, "failed to add irq1 chip: %d\n", ret);
+		goto err_regulator_irq;
+	}
+
+	ret = regmap_add_irq_chip(pf1550->regmap, pf1550->irq,
+				IRQF_ONESHOT | IRQF_SHARED |
+				IRQF_TRIGGER_FALLING, 0,
+				&pf1550_onkey_irq_chip,
+				&pf1550->irq_data_onkey);
+	if (ret) {
+		dev_err(pf1550->dev, "failed to add irq3 chip: %d\n", ret);
+		goto err_onkey_irq;
+	}
+
+	ret = regmap_add_irq_chip(pf1550->regmap, pf1550->irq,
+				IRQF_ONESHOT | IRQF_SHARED |
+				IRQF_TRIGGER_FALLING, 0,
+				&pf1550_charger_irq_chip,
+				&pf1550->irq_data_charger);
+	if (ret) {
+		dev_err(pf1550->dev, "failed to add irq4 chip: %d\n", ret);
+		goto err_charger_irq;
+	}
+
+	ret = mfd_add_devices(pf1550->dev, -1, pf1550_devs,
+			      ARRAY_SIZE(pf1550_devs), NULL, 0, NULL);
+	if (ret < 0)
+		goto err_mfd;
+
+	return ret;
+
+err_mfd:
+	mfd_remove_devices(pf1550->dev);
+err_charger_irq:
+	regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_charger);
+err_onkey_irq:
+	regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_regulator);
+err_regulator_irq:
+	return ret;
+}
+
+static int pf1550_i2c_remove(struct i2c_client *i2c)
+{
+	struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
+
+	mfd_remove_devices(pf1550->dev);
+
+	regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_regulator);
+	regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_onkey);
+	regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_charger);
+
+	return 0;
+}
+
+static const struct i2c_device_id pf1550_i2c_id[] = {
+	{ "pf1550", PF1550 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pf1550_i2c_id);
+
+static int pf1550_suspend(struct device *dev)
+{
+	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+	struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
+
+	if (device_may_wakeup(dev)) {
+		enable_irq_wake(pf1550->irq);
+		disable_irq(pf1550->irq);
+	}
+
+	return 0;
+}
+
+static int pf1550_resume(struct device *dev)
+{
+	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+	struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
+
+	if (device_may_wakeup(dev)) {
+		disable_irq_wake(pf1550->irq);
+		enable_irq(pf1550->irq);
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops pf1550_pm = {
+	.suspend = pf1550_suspend,
+	.resume = pf1550_resume,
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id pf1550_dt_match[] = {
+	{ .compatible = "fsl,pf1550" },
+	{},
+};
+#endif
+
+static struct i2c_driver pf1550_i2c_driver = {
+	.driver = {
+		   .name = "pf1550",
+		   .owner = THIS_MODULE,
+		   .pm = &pf1550_pm,
+		   .of_match_table = of_match_ptr(pf1550_dt_match),
+	},
+	.probe = pf1550_i2c_probe,
+	.remove = pf1550_i2c_remove,
+	.id_table = pf1550_i2c_id,
+};
+
+static int __init pf1550_i2c_init(void)
+{
+	return i2c_add_driver(&pf1550_i2c_driver);
+}
+/* init early so consumer devices can complete system boot */
+subsys_initcall(pf1550_i2c_init);
+
+static void __exit pf1550_i2c_exit(void)
+{
+	i2c_del_driver(&pf1550_i2c_driver);
+}
+module_exit(pf1550_i2c_exit);
+
+MODULE_DESCRIPTION("Freescale PF1550 multi-function core driver");
+MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
+MODULE_LICENSE("GPL");
+
diff --git a/include/linux/mfd/pf1550.h b/include/linux/mfd/pf1550.h
new file mode 100644
index 0000000..9721929
--- /dev/null
+++ b/include/linux/mfd/pf1550.h
@@ -0,0 +1,241 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * pf1550.h - mfd head file for PF1550
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Robin Gong <yibin.gong@freescale.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.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#ifndef __LINUX_MFD_PF1550_H
+#define __LINUX_MFD_PF1550_H
+
+#include <linux/i2c.h>
+
+enum chips { PF1550 = 1, };
+
+enum pf1550_pmic_reg {
+	/* PMIC regulator part */
+	PF1550_PMIC_REG_DEVICE_ID		= 0x00,
+	PF1550_PMIC_REG_OTP_FLAVOR		= 0x01,
+	PF1550_PMIC_REG_SILICON_REV		= 0x02,
+
+	PF1550_PMIC_REG_INT_CATEGORY		= 0x06,
+	PF1550_PMIC_REG_SW_INT_STAT0		= 0x08,
+	PF1550_PMIC_REG_SW_INT_MASK0		= 0x09,
+	PF1550_PMIC_REG_SW_INT_SENSE0		= 0x0A,
+	PF1550_PMIC_REG_SW_INT_STAT1		= 0x0B,
+	PF1550_PMIC_REG_SW_INT_MASK1		= 0x0C,
+	PF1550_PMIC_REG_SW_INT_SENSE1		= 0x0D,
+	PF1550_PMIC_REG_SW_INT_STAT2		= 0x0E,
+	PF1550_PMIC_REG_SW_INT_MASK2		= 0x0F,
+	PF1550_PMIC_REG_SW_INT_SENSE2		= 0x10,
+	PF1550_PMIC_REG_LDO_INT_STAT0		= 0x18,
+	PF1550_PMIC_REG_LDO_INT_MASK0		= 0x19,
+	PF1550_PMIC_REG_LDO_INT_SENSE0		= 0x1A,
+	PF1550_PMIC_REG_TEMP_INT_STAT0		= 0x20,
+	PF1550_PMIC_REG_TEMP_INT_MASK0		= 0x21,
+	PF1550_PMIC_REG_TEMP_INT_SENSE0		= 0x22,
+	PF1550_PMIC_REG_ONKEY_INT_STAT0		= 0x24,
+	PF1550_PMIC_REG_ONKEY_INT_MASK0		= 0x25,
+	PF1550_PMIC_REG_ONKEY_INT_SENSE0	= 0x26,
+	PF1550_PMIC_REG_MISC_INT_STAT0		= 0x28,
+	PF1550_PMIC_REG_MISC_INT_MASK0		= 0x29,
+	PF1550_PMIC_REG_MISC_INT_SENSE0		= 0x2A,
+
+	PF1550_PMIC_REG_COINCELL_CONTROL	= 0x30,
+
+	PF1550_PMIC_REG_SW1_VOLT		= 0x32,
+	PF1550_PMIC_REG_SW1_STBY_VOLT		= 0x33,
+	PF1550_PMIC_REG_SW1_SLP_VOLT		= 0x34,
+	PF1550_PMIC_REG_SW1_CTRL		= 0x35,
+	PF1550_PMIC_REG_SW1_CTRL1		= 0x36,
+	PF1550_PMIC_REG_SW2_VOLT		= 0x38,
+	PF1550_PMIC_REG_SW2_STBY_VOLT		= 0x39,
+	PF1550_PMIC_REG_SW2_SLP_VOLT		= 0x3A,
+	PF1550_PMIC_REG_SW2_CTRL		= 0x3B,
+	PF1550_PMIC_REG_SW2_CTRL1		= 0x3C,
+	PF1550_PMIC_REG_SW3_VOLT		= 0x3E,
+	PF1550_PMIC_REG_SW3_STBY_VOLT		= 0x3F,
+	PF1550_PMIC_REG_SW3_SLP_VOLT		= 0x40,
+	PF1550_PMIC_REG_SW3_CTRL		= 0x41,
+	PF1550_PMIC_REG_SW3_CTRL1		= 0x42,
+	PF1550_PMIC_REG_VSNVS_CTRL		= 0x48,
+	PF1550_PMIC_REG_VREFDDR_CTRL		= 0x4A,
+	PF1550_PMIC_REG_LDO1_VOLT		= 0x4C,
+	PF1550_PMIC_REG_LDO1_CTRL		= 0x4D,
+	PF1550_PMIC_REG_LDO2_VOLT		= 0x4F,
+	PF1550_PMIC_REG_LDO2_CTRL		= 0x50,
+	PF1550_PMIC_REG_LDO3_VOLT		= 0x52,
+	PF1550_PMIC_REG_LDO3_CTRL		= 0x53,
+	PF1550_PMIC_REG_PWRCTRL0		= 0x58,
+	PF1550_PMIC_REG_PWRCTRL1		= 0x59,
+	PF1550_PMIC_REG_PWRCTRL2		= 0x5A,
+	PF1550_PMIC_REG_PWRCTRL3		= 0x5B,
+	PF1550_PMIC_REG_SW1_PWRDN_SEQ		= 0x5F,
+	PF1550_PMIC_REG_SW2_PWRDN_SEQ		= 0x60,
+	PF1550_PMIC_REG_SW3_PWRDN_SEQ		= 0x61,
+	PF1550_PMIC_REG_LDO1_PWRDN_SEQ		= 0x62,
+	PF1550_PMIC_REG_LDO2_PWRDN_SEQ		= 0x63,
+	PF1550_PMIC_REG_LDO3_PWRDN_SEQ		= 0x64,
+	PF1550_PMIC_REG_VREFDDR_PWRDN_SEQ	= 0x65,
+
+	PF1550_PMIC_REG_STATE_INFO		= 0x67,
+	PF1550_PMIC_REG_I2C_ADDR		= 0x68,
+	PF1550_PMIC_REG_IO_DRV0			= 0x69,
+	PF1550_PMIC_REG_IO_DRV1			= 0x6A,
+	PF1550_PMIC_REG_RC_16MHZ		= 0x6B,
+	PF1550_PMIC_REG_KEY			= 0x6F,
+
+	/* charger part */
+	PF1550_CHARG_REG_CHG_INT		= 0x80,
+	PF1550_CHARG_REG_CHG_INT_MASK		= 0x82,
+	PF1550_CHARG_REG_CHG_INT_OK		= 0x84,
+	PF1550_CHARG_REG_VBUS_SNS		= 0x86,
+	PF1550_CHARG_REG_CHG_SNS		= 0x87,
+	PF1550_CHARG_REG_BATT_SNS		= 0x88,
+	PF1550_CHARG_REG_CHG_OPER		= 0x89,
+	PF1550_CHARG_REG_CHG_TMR		= 0x8A,
+	PF1550_CHARG_REG_CHG_EOC_CNFG		= 0x8D,
+	PF1550_CHARG_REG_CHG_CURR_CNFG		= 0x8E,
+	PF1550_CHARG_REG_BATT_REG		= 0x8F,
+	PF1550_CHARG_REG_BATFET_CNFG		= 0x91,
+	PF1550_CHARG_REG_THM_REG_CNFG		= 0x92,
+	PF1550_CHARG_REG_VBUS_INLIM_CNFG	= 0x94,
+	PF1550_CHARG_REG_VBUS_LIN_DPM		= 0x95,
+	PF1550_CHARG_REG_USB_PHY_LDO_CNFG	= 0x96,
+	PF1550_CHARG_REG_DBNC_DELAY_TIME	= 0x98,
+	PF1550_CHARG_REG_CHG_INT_CNFG		= 0x99,
+	PF1550_CHARG_REG_THM_ADJ_SETTING	= 0x9A,
+	PF1550_CHARG_REG_VBUS2SYS_CNFG		= 0x9B,
+	PF1550_CHARG_REG_LED_PWM		= 0x9C,
+	PF1550_CHARG_REG_FAULT_BATFET_CNFG	= 0x9D,
+	PF1550_CHARG_REG_LED_CNFG		= 0x9E,
+	PF1550_CHARG_REG_CHGR_KEY2		= 0x9F,
+
+	PF1550_PMIC_REG_END			= 0xff,
+};
+
+#define PF1550_CHG_PRECHARGE		0
+#define PF1550_CHG_CONSTANT_CURRENT	1
+#define PF1550_CHG_CONSTANT_VOL		2
+#define PF1550_CHG_EOC			3
+#define PF1550_CHG_DONE			4
+#define PF1550_CHG_TIMER_FAULT		6
+#define PF1550_CHG_SUSPEND		7
+#define PF1550_CHG_OFF_INV		8
+#define PF1550_CHG_BAT_OVER		9
+#define PF1550_CHG_OFF_TEMP		10
+#define PF1550_CHG_LINEAR_ONLY		12
+#define PF1550_CHG_SNS_MASK		0xf
+
+#define PF1550_BAT_NO_VBUS		0
+#define PF1550_BAT_LOW_THAN_PRECHARG	1
+#define PF1550_BAT_CHARG_FAIL		2
+#define PF1550_BAT_HIGH_THAN_PRECHARG	4
+#define PF1550_BAT_OVER_VOL		5
+#define	PF1550_BAT_NO_DETECT		6
+#define PF1550_BAT_SNS_MASK		0x7
+
+#define PF1550_VBUS_UVLO		BIT(2)
+#define PF1550_VBUS_IN2SYS		BIT(3)
+#define PF1550_VBUS_OVLO		BIT(4)
+#define PF1550_VBUS_VALID		BIT(5)
+
+#define PF1550_CHARG_REG_BATT_REG_CHGCV_MASK		0x3f
+#define PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT		6
+#define PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK		0x3
+#define PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT	2
+#define PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK	0x3
+
+#define PMIC_IRQ_SW1_LS		BIT(0)
+#define PMIC_IRQ_SW2_LS		BIT(1)
+#define PMIC_IRQ_SW3_LS		BIT(2)
+#define PMIC_IRQ_SW1_HS		BIT(0)
+#define PMIC_IRQ_SW2_HS		BIT(1)
+#define PMIC_IRQ_SW3_HS		BIT(2)
+#define PMIC_IRQ_LDO1_FAULT	BIT(0)
+#define PMIC_IRQ_LDO2_FAULT	BIT(1)
+#define PMIC_IRQ_LDO3_FAULT	BIT(2)
+#define PMIC_IRQ_TEMP_110	BIT(0)
+#define PMIC_IRQ_TEMP_125	BIT(1)
+
+#define ONKEY_IRQ_PUSHI		BIT(0)
+#define ONKEY_IRQ_1SI		BIT(1)
+#define ONKEY_IRQ_2SI		BIT(2)
+#define ONKEY_IRQ_3SI		BIT(3)
+#define ONKEY_IRQ_4SI		BIT(4)
+#define ONKEY_IRQ_8SI		BIT(5)
+
+#define CHARG_IRQ_BAT2SOCI	BIT(1)
+#define CHARG_IRQ_BATI		BIT(2)
+#define CHARG_IRQ_CHGI		BIT(3)
+#define CHARG_IRQ_VBUSI		BIT(5)
+#define CHARG_IRQ_DPMI		BIT(6)
+#define CHARG_IRQ_THMI		BIT(7)
+
+enum pf1550_irq {
+	PF1550_PMIC_IRQ_SW1_LS,
+	PF1550_PMIC_IRQ_SW2_LS,
+	PF1550_PMIC_IRQ_SW3_LS,
+	PF1550_PMIC_IRQ_SW1_HS,
+	PF1550_PMIC_IRQ_SW2_HS,
+	PF1550_PMIC_IRQ_SW3_HS,
+	PF1550_PMIC_IRQ_LDO1_FAULT,
+	PF1550_PMIC_IRQ_LDO2_FAULT,
+	PF1550_PMIC_IRQ_LDO3_FAULT,
+	PF1550_PMIC_IRQ_TEMP_110,
+	PF1550_PMIC_IRQ_TEMP_125,
+
+	PF1550_ONKEY_IRQ_PUSHI,
+	PF1550_ONKEY_IRQ_1SI,
+	PF1550_ONKEY_IRQ_2SI,
+	PF1550_ONKEY_IRQ_3SI,
+	PF1550_ONKEY_IRQ_4SI,
+	PF1550_ONKEY_IRQ_8SI,
+
+	PF1550_CHARG_IRQ_BAT2SOCI,
+	PF1550_CHARG_IRQ_BATI,
+	PF1550_CHARG_IRQ_CHGI,
+	PF1550_CHARG_IRQ_VBUSI,
+	PF1550_CHARG_IRQ_DPMI,
+	PF1550_CHARG_IRQ_THMI,
+};
+
+enum pf1550_regulators {
+	PF1550_SW1,
+	PF1550_SW2,
+	PF1550_SW3,
+	PF1550_VREFDDR,
+	PF1550_LDO1,
+	PF1550_LDO2,
+	PF1550_LDO3,
+};
+
+struct pf1550_irq_info {
+	unsigned int irq;
+	const char *name;
+	unsigned int virq;
+};
+
+struct pf1550_dev {
+	struct device *dev;
+	struct i2c_client *i2c;
+	int type;
+	struct regmap *regmap;
+	struct regmap_irq_chip_data *irq_data_regulator;
+	struct regmap_irq_chip_data *irq_data_onkey;
+	struct regmap_irq_chip_data *irq_data_charger;
+	int irq;
+};
+
+#endif /* __LINUX_MFD_PF1550_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] power: pf1550_charger: add pf1550 charger driver
  2018-04-17 14:20 [PATCH 1/2] mfd: pf1550: add pf1550 mfd driver Abel Vesa
@ 2018-04-17 14:20 ` Abel Vesa
  2018-04-24 22:02   ` Enric Balletbo Serra
  2018-04-24 11:27 ` [PATCH 1/2] mfd: pf1550: add pf1550 mfd driver Enric Balletbo Serra
  1 sibling, 1 reply; 4+ messages in thread
From: Abel Vesa @ 2018-04-17 14:20 UTC (permalink / raw)
  To: Lee Jones, Sebastian Reichel, Robin Gong
  Cc: linux-kernel, linux-pm, linux-imx, Abel Vesa, Robin Gong, Abel Vesa

From: Robin Gong <b38343@freescale.com>

Add basic pf1550 charger driver.

Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/power/supply/Kconfig          |   6 +
 drivers/power/supply/Makefile         |   1 +
 drivers/power/supply/pf1550_charger.c | 660 ++++++++++++++++++++++++++++++++++
 3 files changed, 667 insertions(+)
 create mode 100644 drivers/power/supply/pf1550_charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 428b426..2ebd38c 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -380,6 +380,12 @@ config CHARGER_PCF50633
 	help
 	 Say Y to include support for NXP PCF50633 Main Battery Charger.
 
+config CHARGER_PF1550
+	tristate "Freescale PF1550 battery charger driver"
+	depends on MFD_PF1550
+	help
+	 Say Y to enable support for the Freescale PF1550 battery charger.
+
 config BATTERY_JZ4740
 	tristate "Ingenic JZ4740 battery"
 	depends on MACH_JZ4740
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index e83aa84..0b2e424 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_BATTERY_S3C_ADC)	+= s3c_adc_battery.o
 obj-$(CONFIG_BATTERY_TWL4030_MADC)	+= twl4030_madc_battery.o
 obj-$(CONFIG_CHARGER_88PM860X)	+= 88pm860x_charger.o
 obj-$(CONFIG_CHARGER_PCF50633)	+= pcf50633-charger.o
+obj-$(CONFIG_CHARGER_PF1550)	+= pf1550_charger.o
 obj-$(CONFIG_BATTERY_JZ4740)	+= jz4740-battery.o
 obj-$(CONFIG_BATTERY_RX51)	+= rx51_battery.o
 obj-$(CONFIG_AB8500_BM)		+= ab8500_bmdata.o ab8500_charger.o ab8500_fg.o ab8500_btemp.o abx500_chargalg.o pm2301_charger.o
diff --git a/drivers/power/supply/pf1550_charger.c b/drivers/power/supply/pf1550_charger.c
new file mode 100644
index 0000000..c5e0c55
--- /dev/null
+++ b/drivers/power/supply/pf1550_charger.c
@@ -0,0 +1,660 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * pf1550_charger.c - regulator driver for the PF1550
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Robin Gong <yibin.gong@freescale.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.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/mfd/pf1550.h>
+
+#define PF1550_CHARGER_NAME		"pf1550-charger"
+#define PF1550_DEFAULT_CONSTANT_VOLT	4200000
+#define PF1550_DEFAULT_MIN_SYSTEM_VOLT	3500000
+#define PF1550_DEFAULT_THERMAL_TEMP	75
+
+static const char *pf1550_charger_model		= "PF1550";
+static const char *pf1550_charger_manufacturer	= "Freescale";
+
+struct pf1550_charger {
+	struct device *dev;
+	struct pf1550_dev *pf1550;
+	struct power_supply *charger;
+	struct power_supply_desc psy_desc;
+	int irq;
+	struct delayed_work irq_work;
+	struct mutex mutex;
+
+	u32 constant_volt;
+	u32 min_system_volt;
+	u32 thermal_regulation_temp;
+};
+
+static struct pf1550_irq_info pf1550_charger_irqs[] = {
+	{ PF1550_CHARG_IRQ_BAT2SOCI,		"BAT2SOC" },
+	{ PF1550_CHARG_IRQ_BATI,		"BAT" },
+	{ PF1550_CHARG_IRQ_CHGI,		"CHG" },
+	{ PF1550_CHARG_IRQ_VBUSI,		"VBUS" },
+	{ PF1550_CHARG_IRQ_THMI,		"THM" },
+};
+
+static int pf1550_get_charger_state(struct regmap *regmap, int *val)
+{
+	int ret;
+	unsigned int data;
+
+	ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
+	if (ret < 0)
+		return ret;
+
+	data &= PF1550_CHG_SNS_MASK;
+
+	switch (data) {
+	case PF1550_CHG_PRECHARGE:
+	case PF1550_CHG_CONSTANT_CURRENT:
+	case PF1550_CHG_CONSTANT_VOL:
+	case PF1550_CHG_EOC:
+		*val = POWER_SUPPLY_STATUS_CHARGING;
+		break;
+	case PF1550_CHG_DONE:
+		*val = POWER_SUPPLY_STATUS_FULL;
+		break;
+	case PF1550_CHG_TIMER_FAULT:
+	case PF1550_CHG_SUSPEND:
+		*val = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		break;
+	case PF1550_CHG_OFF_INV:
+	case PF1550_CHG_OFF_TEMP:
+	case PF1550_CHG_LINEAR_ONLY:
+		*val = POWER_SUPPLY_STATUS_DISCHARGING;
+		break;
+	default:
+		*val = POWER_SUPPLY_STATUS_UNKNOWN;
+	}
+
+	return 0;
+}
+
+static int pf1550_get_charge_type(struct regmap *regmap, int *val)
+{
+	int ret;
+	unsigned int data;
+
+	ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
+	if (ret < 0)
+		return ret;
+
+	data &= PF1550_CHG_SNS_MASK;
+
+	switch (data) {
+	case PF1550_CHG_SNS_MASK:
+		*val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+		break;
+	case PF1550_CHG_CONSTANT_CURRENT:
+	case PF1550_CHG_CONSTANT_VOL:
+	case PF1550_CHG_EOC:
+		*val = POWER_SUPPLY_CHARGE_TYPE_FAST;
+		break;
+	case PF1550_CHG_DONE:
+	case PF1550_CHG_TIMER_FAULT:
+	case PF1550_CHG_SUSPEND:
+	case PF1550_CHG_OFF_INV:
+	case PF1550_CHG_BAT_OVER:
+	case PF1550_CHG_OFF_TEMP:
+	case PF1550_CHG_LINEAR_ONLY:
+		*val = POWER_SUPPLY_CHARGE_TYPE_NONE;
+		break;
+	default:
+		*val = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+	}
+
+	return 0;
+}
+
+/*
+ * Supported health statuses:
+ *  - POWER_SUPPLY_HEALTH_DEAD
+ *  - POWER_SUPPLY_HEALTH_GOOD
+ *  - POWER_SUPPLY_HEALTH_OVERVOLTAGE
+ *  - POWER_SUPPLY_HEALTH_UNKNOWN
+ */
+static int pf1550_get_battery_health(struct regmap *regmap, int *val)
+{
+	int ret;
+	unsigned int data;
+
+	ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
+	if (ret < 0)
+		return ret;
+
+	data &= PF1550_BAT_SNS_MASK;
+
+	switch (data) {
+	case PF1550_BAT_NO_DETECT:
+		*val = POWER_SUPPLY_HEALTH_DEAD;
+		break;
+	case PF1550_BAT_NO_VBUS:
+	case PF1550_BAT_LOW_THAN_PRECHARG:
+	case PF1550_BAT_CHARG_FAIL:
+	case PF1550_BAT_HIGH_THAN_PRECHARG:
+		*val = POWER_SUPPLY_HEALTH_GOOD;
+		break;
+	case PF1550_BAT_OVER_VOL:
+		*val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+		break;
+	default:
+		*val = POWER_SUPPLY_HEALTH_UNKNOWN;
+		break;
+	}
+
+	return 0;
+}
+
+static int pf1550_get_present(struct regmap *regmap, int *val)
+{
+	unsigned int data;
+	int ret;
+
+	ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
+	if (ret < 0)
+		return ret;
+
+	data &= PF1550_BAT_SNS_MASK;
+	*val = (data == PF1550_BAT_NO_DETECT) ? 0 : 1;
+
+	return 0;
+}
+
+static int pf1550_get_online(struct regmap *regmap, int *val)
+{
+	unsigned int data;
+	int ret;
+
+	ret = regmap_read(regmap, PF1550_CHARG_REG_VBUS_SNS, &data);
+	if (ret < 0)
+		return ret;
+
+	*val = (data & PF1550_VBUS_VALID) ? 1 : 0;
+
+	return 0;
+}
+
+static void pf1550_chg_bat_isr(struct pf1550_charger *chg)
+{
+	unsigned int data;
+
+	if (regmap_read(chg->pf1550->regmap,
+			PF1550_CHARG_REG_BATT_SNS, &data)) {
+		dev_err(chg->dev, "Read BATT_SNS error.\n");
+		return;
+	}
+
+	switch (data & PF1550_BAT_SNS_MASK) {
+	case PF1550_BAT_NO_VBUS:
+		dev_dbg(chg->dev, "No valid VBUS input.\n");
+		break;
+	case PF1550_BAT_LOW_THAN_PRECHARG:
+		dev_dbg(chg->dev, "VBAT < VPRECHG.LB.\n");
+		break;
+	case PF1550_BAT_CHARG_FAIL:
+		dev_dbg(chg->dev, "Battery charging failed.\n");
+		break;
+	case PF1550_BAT_HIGH_THAN_PRECHARG:
+		dev_dbg(chg->dev, "VBAT > VPRECHG.LB.\n");
+		break;
+	case PF1550_BAT_OVER_VOL:
+		dev_dbg(chg->dev, "VBAT > VBATOV.\n");
+		break;
+	case PF1550_BAT_NO_DETECT:
+		dev_dbg(chg->dev, "Battery not detected.\n");
+		break;
+	default:
+		dev_err(chg->dev, "Unknown value read:%x\n",
+			data & PF1550_CHG_SNS_MASK);
+	}
+}
+
+static void pf1550_chg_chg_isr(struct pf1550_charger *chg)
+{
+	unsigned int data;
+
+	if (regmap_read(chg->pf1550->regmap,
+			PF1550_CHARG_REG_CHG_SNS, &data)) {
+		dev_err(chg->dev, "Read CHG_SNS error.\n");
+		return;
+	}
+
+	switch (data & PF1550_CHG_SNS_MASK) {
+	case PF1550_CHG_PRECHARGE:
+		dev_dbg(chg->dev, "In pre-charger mode.\n");
+		break;
+	case PF1550_CHG_CONSTANT_CURRENT:
+		dev_dbg(chg->dev, "In fast-charge constant current mode.\n");
+		break;
+	case PF1550_CHG_CONSTANT_VOL:
+		dev_dbg(chg->dev, "In fast-charge constant voltage mode.\n");
+		break;
+	case PF1550_CHG_EOC:
+		dev_dbg(chg->dev, "In EOC mode.\n");
+		break;
+	case PF1550_CHG_DONE:
+		dev_dbg(chg->dev, "In DONE mode.\n");
+		break;
+	case PF1550_CHG_TIMER_FAULT:
+		dev_info(chg->dev, "In timer fault mode.\n");
+		break;
+	case PF1550_CHG_SUSPEND:
+		dev_info(chg->dev, "In thermistor suspend mode.\n");
+		break;
+	case PF1550_CHG_OFF_INV:
+		dev_info(chg->dev, "Input invalid, charger off.\n");
+		break;
+	case PF1550_CHG_BAT_OVER:
+		dev_info(chg->dev, "Battery over-voltage.\n");
+		break;
+	case PF1550_CHG_OFF_TEMP:
+		dev_info(chg->dev, "Temp high, charger off.\n");
+		break;
+	case PF1550_CHG_LINEAR_ONLY:
+		dev_dbg(chg->dev, "In Linear mode, not charging.\n");
+		break;
+	default:
+		dev_err(chg->dev, "Unknown value read:%x\n",
+			data & PF1550_CHG_SNS_MASK);
+	}
+}
+
+static void pf1550_chg_vbus_isr(struct pf1550_charger *chg)
+{
+	enum power_supply_type old_type;
+	unsigned int data;
+
+	if (regmap_read(chg->pf1550->regmap,
+			PF1550_CHARG_REG_VBUS_SNS, &data)) {
+		dev_err(chg->dev, "Read VBUS_SNS error.\n");
+		return;
+	}
+
+	old_type = chg->psy_desc.type;
+
+	if (data & PF1550_VBUS_UVLO) {
+		chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+		dev_dbg(chg->dev, "VBUS deattached.\n");
+	}
+	if (data & PF1550_VBUS_IN2SYS)
+		dev_dbg(chg->dev, "VBUS_IN2SYS_SNS.\n");
+	if (data & PF1550_VBUS_OVLO)
+		dev_dbg(chg->dev, "VBUS_OVLO_SNS.\n");
+	if (data & PF1550_VBUS_VALID) {
+		chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
+		dev_dbg(chg->dev, "VBUS attached.\n");
+	}
+
+	if (old_type != chg->psy_desc.type)
+		power_supply_changed(chg->charger);
+}
+
+static irqreturn_t pf1550_charger_irq_handler(int irq, void *data)
+{
+	struct pf1550_charger *chg = data;
+
+	chg->irq = irq;
+	schedule_delayed_work(&chg->irq_work,  msecs_to_jiffies(10));
+
+	return IRQ_HANDLED;
+}
+
+static void pf1550_charger_irq_work(struct work_struct *work)
+{
+	struct pf1550_charger *chg = container_of(to_delayed_work(work),
+						  struct pf1550_charger,
+						  irq_work);
+	int i, irq_type = -1;
+	unsigned int status;
+
+	if (!chg->charger)
+		return;
+
+	mutex_lock(&chg->mutex);
+
+	for (i = 0; i < ARRAY_SIZE(pf1550_charger_irqs); i++)
+		if (chg->irq == pf1550_charger_irqs[i].virq)
+			irq_type = pf1550_charger_irqs[i].irq;
+
+	switch (irq_type) {
+	case PF1550_CHARG_IRQ_BAT2SOCI:
+		dev_info(chg->dev, "BAT to SYS Overcurrent interrupt.\n");
+		break;
+	case PF1550_CHARG_IRQ_BATI:
+		pf1550_chg_bat_isr(chg);
+		break;
+	case PF1550_CHARG_IRQ_CHGI:
+		pf1550_chg_chg_isr(chg);
+		break;
+	case PF1550_CHARG_IRQ_VBUSI:
+		pf1550_chg_vbus_isr(chg);
+		break;
+	case PF1550_CHARG_IRQ_THMI:
+		dev_info(chg->dev, "Thermal interrupt.\n");
+		break;
+	default:
+		dev_err(chg->dev, "unknown interrupt occurred.\n");
+	}
+
+	if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT, &status))
+		dev_err(chg->dev, "Read CHG_INT error.\n");
+	if (regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT, status))
+		dev_err(chg->dev, "clear CHG_INT error.\n");
+
+	mutex_unlock(&chg->mutex);
+}
+
+static enum power_supply_property pf1550_charger_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_MANUFACTURER,
+};
+
+static int pf1550_charger_get_property(struct power_supply *psy,
+			    enum power_supply_property psp,
+			    union power_supply_propval *val)
+{
+	struct pf1550_charger *chg = power_supply_get_drvdata(psy);
+	struct regmap *regmap = chg->pf1550->regmap;
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		ret = pf1550_get_charger_state(regmap, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		ret = pf1550_get_charge_type(regmap, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		ret = pf1550_get_battery_health(regmap, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		ret = pf1550_get_present(regmap, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = pf1550_get_online(regmap, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = pf1550_charger_model;
+		break;
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		val->strval = pf1550_charger_manufacturer;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int pf1550_set_constant_volt(struct pf1550_charger *chg,
+		unsigned int uvolt)
+{
+	unsigned int data;
+
+	if (uvolt >= 3500000 && uvolt <= 4440000)
+		data = 8 + (uvolt - 3500000) / 20000;
+	else {
+		dev_err(chg->dev, "Wrong value for constant voltage\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(chg->dev, "Charging constant voltage: %u (0x%x)\n", uvolt,
+			data);
+
+	return regmap_update_bits(chg->pf1550->regmap,
+			PF1550_CHARG_REG_BATT_REG,
+			PF1550_CHARG_REG_BATT_REG_CHGCV_MASK, data);
+}
+
+static int pf1550_set_min_system_volt(struct pf1550_charger *chg,
+		unsigned int uvolt)
+{
+	unsigned int data;
+
+	switch (uvolt) {
+	case 3500000:
+		data = 0x0;
+		break;
+	case 3700000:
+		data = 0x1;
+		break;
+	case 4300000:
+		data = 0x2;
+		break;
+	default:
+		dev_err(chg->dev, "Wrong value for minimum system voltage\n");
+		return -EINVAL;
+	}
+
+	data <<= PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT;
+
+	dev_dbg(chg->dev, "Minimum system regulation voltage: %u (0x%x)\n",
+			uvolt, data);
+
+	return regmap_update_bits(chg->pf1550->regmap,
+			PF1550_CHARG_REG_BATT_REG,
+			PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK, data);
+}
+
+static int pf1550_set_thermal_regulation_temp(struct pf1550_charger *chg,
+		unsigned int cels)
+{
+	unsigned int data;
+
+	switch (cels) {
+	case 60:
+		data = 0x0;
+		break;
+	case 75:
+		data = 0x1;
+		break;
+	case 90:
+		data = 0x2;
+		break;
+	case 105:
+		data = 0x3;
+		break;
+	default:
+		dev_err(chg->dev, "Wrong value for thermal temperature\n");
+		return -EINVAL;
+	}
+
+	data <<= PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT;
+
+	dev_dbg(chg->dev, "Thermal regulation loop temperature: %u (0x%x)\n",
+			cels, data);
+
+	return regmap_update_bits(chg->pf1550->regmap,
+			PF1550_CHARG_REG_THM_REG_CNFG,
+			PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK, data);
+}
+
+/*
+ * Sets charger registers to proper and safe default values.
+ */
+static int pf1550_reg_init(struct pf1550_charger *chg)
+{
+	int ret;
+	unsigned int data;
+
+	/* Unmask charger interrupt, mask DPMI and reserved bit */
+	ret =  regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT_MASK,
+				0x51);
+	if (ret) {
+		dev_err(chg->dev, "Error unmask charger interrupt: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS,
+				&data);
+	if (ret) {
+		dev_err(chg->dev, "Read charg vbus_sns error: %d\n", ret);
+		return ret;
+	}
+
+	if (data & PF1550_VBUS_VALID)
+		chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
+
+	ret = pf1550_set_constant_volt(chg, chg->constant_volt);
+	if (ret)
+		return ret;
+
+	ret = pf1550_set_min_system_volt(chg, chg->min_system_volt);
+	if (ret)
+		return ret;
+
+	ret = pf1550_set_thermal_regulation_temp(chg,
+			chg->thermal_regulation_temp);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int pf1550_dt_init(struct device *dev, struct pf1550_charger *chg)
+{
+	struct device_node *np = dev->of_node;
+
+	if (!np) {
+		dev_err(dev, "no charger OF node\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(np, "fsl,constant-microvolt",
+			&chg->constant_volt))
+		chg->constant_volt = PF1550_DEFAULT_CONSTANT_VOLT;
+
+	if (of_property_read_u32(np, "fsl,min-system-microvolt",
+			&chg->min_system_volt))
+		chg->min_system_volt = PF1550_DEFAULT_MIN_SYSTEM_VOLT;
+
+	if (of_property_read_u32(np, "fsl,thermal-regulation",
+			&chg->thermal_regulation_temp))
+		chg->thermal_regulation_temp = PF1550_DEFAULT_THERMAL_TEMP;
+
+	return 0;
+}
+
+static int pf1550_charger_probe(struct platform_device *pdev)
+{
+	struct pf1550_charger *chg;
+	struct power_supply_config psy_cfg = {};
+	struct pf1550_dev *pf1550 = dev_get_drvdata(pdev->dev.parent);
+	int i, ret;
+
+	chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
+	if (!chg)
+		return -ENOMEM;
+
+	chg->dev = &pdev->dev;
+	chg->pf1550 = pf1550;
+
+	platform_set_drvdata(pdev, chg);
+
+	ret = pf1550_dt_init(&pdev->dev, chg);
+	if (ret)
+		return ret;
+
+	mutex_init(&chg->mutex);
+
+	INIT_DELAYED_WORK(&chg->irq_work, pf1550_charger_irq_work);
+
+	for (i = 0; i < ARRAY_SIZE(pf1550_charger_irqs); i++) {
+		struct pf1550_irq_info *charger_irq =
+						&pf1550_charger_irqs[i];
+		unsigned int virq = 0;
+
+		virq = regmap_irq_get_virq(pf1550->irq_data_charger,
+					charger_irq->irq);
+		if (!virq)
+			return -EINVAL;
+
+		charger_irq->virq = virq;
+
+		ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
+					pf1550_charger_irq_handler,
+					IRQF_NO_SUSPEND,
+					charger_irq->name, chg);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"failed: irq request (IRQ: %d, error :%d)\n",
+				charger_irq->irq, ret);
+			return ret;
+		}
+	}
+
+	psy_cfg.drv_data = chg;
+
+	chg->psy_desc.name = PF1550_CHARGER_NAME;
+	chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	chg->psy_desc.get_property = pf1550_charger_get_property;
+	chg->psy_desc.properties = pf1550_charger_props;
+	chg->psy_desc.num_properties = ARRAY_SIZE(pf1550_charger_props);
+
+	chg->charger = power_supply_register(&pdev->dev, &chg->psy_desc,
+						&psy_cfg);
+	if (IS_ERR(chg->charger)) {
+		dev_err(&pdev->dev, "failed: power supply register\n");
+		ret = PTR_ERR(chg->charger);
+		return ret;
+	}
+
+	ret = pf1550_reg_init(chg);
+
+	return ret;
+}
+
+static int pf1550_charger_remove(struct platform_device *pdev)
+{
+	struct pf1550_charger *chg = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&chg->irq_work);
+	power_supply_unregister(chg->charger);
+
+	return 0;
+}
+
+static const struct platform_device_id pf1550_charger_id[] = {
+	{ "pf1550-charger", 0, },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, pf1550_charger_id);
+
+static struct platform_driver pf1550_charger_driver = {
+	.driver = {
+		.name	= "pf1550-charger",
+	},
+	.probe		= pf1550_charger_probe,
+	.remove		= pf1550_charger_remove,
+	.id_table	= pf1550_charger_id,
+};
+module_platform_driver(pf1550_charger_driver);
+
+MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
+MODULE_DESCRIPTION("PF1550 charger driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] mfd: pf1550: add pf1550 mfd driver
  2018-04-17 14:20 [PATCH 1/2] mfd: pf1550: add pf1550 mfd driver Abel Vesa
  2018-04-17 14:20 ` [PATCH 2/2] power: pf1550_charger: add pf1550 charger driver Abel Vesa
@ 2018-04-24 11:27 ` Enric Balletbo Serra
  1 sibling, 0 replies; 4+ messages in thread
From: Enric Balletbo Serra @ 2018-04-24 11:27 UTC (permalink / raw)
  To: Abel Vesa
  Cc: Lee Jones, Sebastian Reichel, Robin Gong, linux-kernel,
	Linux PM list, linux-imx, Abel Vesa, Robin Gong

Hi Abel,

Thanks for the patch. I am not the maintainer nor an official reviewer
but I submitted some patches in the mfd system and below is a review
based on some comments I received when I posted a driver. So hope will
be helpful for you.

2018-04-17 16:20 GMT+02:00 Abel Vesa <abel.vesa@nxp.com>:
> From: Robin Gong <b38343@freescale.com>
>
> Add basic pf1550 mfd driver.
>

For new drivers a better explanation of what the chip is and what
supports is appreciable ;)

> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>  drivers/mfd/Kconfig        |  14 +++
>  drivers/mfd/Makefile       |   2 +
>  drivers/mfd/pf1550.c       | 282 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/pf1550.h | 241 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 539 insertions(+)
>  create mode 100644 drivers/mfd/pf1550.c
>  create mode 100644 include/linux/mfd/pf1550.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index b860eb5..5acd239 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -404,6 +404,20 @@ config MFD_MX25_TSADC
>           i.MX25 processors. They consist of a conversion queue for general
>           purpose ADC and a queue for Touchscreens.
>
> +config MFD_PF1550
> +       tristate "Freescale Semiconductor PF1550 PMIC Support"
> +       depends on I2C=y
> +       select MFD_CORE
> +       select REGMAP_I2C
> +       select REGMAP_IRQ
> +       help
> +         Say yes here to add support for Freescale Semiconductor PF1550.
> +         This is a companion Power Management IC with regulators, ONKEY,
> +         and charger control on chip.
> +         This driver provides common support for accessing the device;
> +         additional drivers must be enabled in order to use the functionality
> +         of the device.
> +
>  config MFD_HI6421_PMIC
>         tristate "HiSilicon Hi6421 PMU/Codec IC"
>         depends on OF
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e9fd20d..b8ac19b 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -97,6 +97,8 @@ obj-$(CONFIG_MFD_MC13XXX)     += mc13xxx-core.o
>  obj-$(CONFIG_MFD_MC13XXX_SPI)  += mc13xxx-spi.o
>  obj-$(CONFIG_MFD_MC13XXX_I2C)  += mc13xxx-i2c.o
>
> +obj-$(CONFIG_MFD_PF1550)       += pf1550.o
> +
>  obj-$(CONFIG_MFD_CORE)         += mfd-core.o
>
>  obj-$(CONFIG_EZX_PCAP)         += ezx-pcap.o
> diff --git a/drivers/mfd/pf1550.c b/drivers/mfd/pf1550.c
> new file mode 100644
> index 0000000..70817ef
> --- /dev/null
> +++ b/drivers/mfd/pf1550.c
> @@ -0,0 +1,282 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * pf1550.c - mfd core driver for the PF1550
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Robin Gong <yibin.gong@freescale.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.
> + *
> + * This program is distributed in the hope that 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.
> + *

I think that the general consensus is to remove the GPL notice once
you have added the SPDX License Identifier. So this notice is not
required.

> + * This driver is based on max77693.c
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/pf1550.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +static const struct mfd_cell pf1550_devs[] = {
> +       {
> +               .name = "pf1550-regulator",
> +               .of_compatible = "fsl,pf1550-regulator",
> +       },

Is the pf1550-regulator submitted to upstream? I did not find it, I
think that is better introduce this compatible when the driver and the
documentation is in place.

> +       {
> +               .name = "pf1550-onkey",
> +               .of_compatible = "fsl,pf1550-onkey",
> +       },

Same here.

> +       {
> +               .name = "pf1550-charger",
> +               .of_compatible = "fsl,pf1550-charger",
> +       },
> +};
> +

Ok, I see that you submitted the driver in this series, the
documentation is also missing.

> +static const struct regmap_config pf1550_regmap_config = {
> +       .reg_bits = 8,
> +       .val_bits = 8,
> +       .max_register = PF1550_PMIC_REG_END,
> +};
> +
> +static const struct regmap_irq pf1550_regulator_irqs[] = {
> +       { .reg_offset = 0, .mask = PMIC_IRQ_SW1_LS,             },
> +       { .reg_offset = 0, .mask = PMIC_IRQ_SW2_LS,             },
> +       { .reg_offset = 0, .mask = PMIC_IRQ_SW3_LS,             },
> +
> +       { .reg_offset = 3, .mask = PMIC_IRQ_SW1_HS,             },
> +       { .reg_offset = 3, .mask = PMIC_IRQ_SW2_HS,             },
> +       { .reg_offset = 3, .mask = PMIC_IRQ_SW3_HS,             },
> +
> +       { .reg_offset = 16, .mask = PMIC_IRQ_LDO1_FAULT,        },
> +       { .reg_offset = 16, .mask = PMIC_IRQ_LDO2_FAULT,        },
> +       { .reg_offset = 16, .mask = PMIC_IRQ_LDO3_FAULT,        },
> +
> +       { .reg_offset = 22, .mask = PMIC_IRQ_TEMP_110,  },
> +       { .reg_offset = 22, .mask = PMIC_IRQ_TEMP_125,  },
> +};
> +
> +static const struct regmap_irq_chip pf1550_regulator_irq_chip = {
> +       .name                   = "pf1550-regulator",
> +       .status_base            = PF1550_PMIC_REG_SW_INT_STAT0,
> +       .mask_base              = PF1550_PMIC_REG_SW_INT_MASK0,
> +       .mask_invert            = false,
> +       .num_regs               = 4,
> +       .irqs                   = pf1550_regulator_irqs,
> +       .num_irqs               = ARRAY_SIZE(pf1550_regulator_irqs),
> +};
> +
> +static const struct regmap_irq pf1550_onkey_irqs[] = {
> +       { .reg_offset = 0, .mask = ONKEY_IRQ_PUSHI,             },
> +       { .reg_offset = 0, .mask = ONKEY_IRQ_1SI,               },
> +       { .reg_offset = 0, .mask = ONKEY_IRQ_2SI,               },
> +       { .reg_offset = 0, .mask = ONKEY_IRQ_3SI,               },
> +       { .reg_offset = 0, .mask = ONKEY_IRQ_4SI,               },
> +       { .reg_offset = 0, .mask = ONKEY_IRQ_8SI,               },
> +};
> +
> +static const struct regmap_irq_chip pf1550_onkey_irq_chip = {
> +       .name                   = "pf1550-onkey",
> +       .status_base            = PF1550_PMIC_REG_ONKEY_INT_STAT0,
> +       .mask_base              = PF1550_PMIC_REG_ONKEY_INT_MASK0,
> +       .mask_invert            = false,
> +       .num_regs               = 1,
> +       .irqs                   = pf1550_onkey_irqs,
> +       .num_irqs               = ARRAY_SIZE(pf1550_onkey_irqs),
> +};
> +
> +static const struct regmap_irq pf1550_charger_irqs[] = {
> +       { .reg_offset = 0, .mask = CHARG_IRQ_BAT2SOCI,          },
> +       { .reg_offset = 0, .mask = CHARG_IRQ_BATI,              },
> +       { .reg_offset = 0, .mask = CHARG_IRQ_CHGI,              },
> +       { .reg_offset = 0, .mask = CHARG_IRQ_VBUSI,             },
> +       { .reg_offset = 0, .mask = CHARG_IRQ_DPMI,              },
> +       { .reg_offset = 0, .mask = CHARG_IRQ_THMI,              },
> +};
> +

nit: In general, the latest field and the latest element do not should
have the comma.

> +static const struct regmap_irq_chip pf1550_charger_irq_chip = {
> +       .name                   = "pf1550-charger",
> +       .status_base            = PF1550_CHARG_REG_CHG_INT,
> +       .mask_base              = PF1550_CHARG_REG_CHG_INT_MASK,
> +       .mask_invert            = false,
> +       .num_regs               = 1,
> +       .irqs                   = pf1550_charger_irqs,
> +       .num_irqs               = ARRAY_SIZE(pf1550_charger_irqs),
> +};
> +
> +static int pf1550_i2c_probe(struct i2c_client *i2c,
> +                           const struct i2c_device_id *id)

nit: Here and in other places alignment should match open parenthesis

> +{
> +       struct pf1550_dev *pf1550;
> +       unsigned int reg_data = 0;
> +       int ret = 0;
> +
> +       pf1550 = devm_kzalloc(&i2c->dev,
> +                       sizeof(struct pf1550_dev), GFP_KERNEL);

Alignment should match open parenthesis again.

IMHO is a good idea running checkpatch with --strict, you will catch
these things.

> +       if (!pf1550)
> +               return -ENOMEM;
> +
> +       i2c_set_clientdata(i2c, pf1550);
> +       pf1550->dev = &i2c->dev;
> +       pf1550->i2c = i2c;
> +       pf1550->irq = i2c->irq;
> +
> +       pf1550->regmap = devm_regmap_init_i2c(i2c, &pf1550_regmap_config);
> +       if (IS_ERR(pf1550->regmap)) {
> +               ret = PTR_ERR(pf1550->regmap);
> +               dev_err(pf1550->dev, "failed to allocate register map: %d\n",
> +                               ret);
> +               return ret;
> +       }
> +
> +       ret = regmap_read(pf1550->regmap, PF1550_PMIC_REG_DEVICE_ID, &reg_data);
> +       if (ret < 0 || reg_data != 0x7c) {
> +               dev_err(pf1550->dev, "device not found!\n");
> +               return ret;
> +       }
> +
> +       pf1550->type = PF1550;
> +       dev_info(pf1550->dev, "pf1550 found.\n");
> +
> +       ret = regmap_add_irq_chip(pf1550->regmap, pf1550->irq,
> +                               IRQF_ONESHOT | IRQF_SHARED |
> +                               IRQF_TRIGGER_FALLING, 0,
> +                               &pf1550_regulator_irq_chip,
> +                               &pf1550->irq_data_regulator);
> +       if (ret) {
> +               dev_err(pf1550->dev, "failed to add irq1 chip: %d\n", ret);
> +               goto err_regulator_irq;
> +       }
> +
> +       ret = regmap_add_irq_chip(pf1550->regmap, pf1550->irq,
> +                               IRQF_ONESHOT | IRQF_SHARED |
> +                               IRQF_TRIGGER_FALLING, 0,
> +                               &pf1550_onkey_irq_chip,
> +                               &pf1550->irq_data_onkey);

Use the device managed version devm_regmap_add_irq_chip

> +       if (ret) {
> +               dev_err(pf1550->dev, "failed to add irq3 chip: %d\n", ret);
> +               goto err_onkey_irq;
> +       }
> +
> +       ret = regmap_add_irq_chip(pf1550->regmap, pf1550->irq,
> +                               IRQF_ONESHOT | IRQF_SHARED |
> +                               IRQF_TRIGGER_FALLING, 0,
> +                               &pf1550_charger_irq_chip,
> +                               &pf1550->irq_data_charger);

Use the device managed version devm_regmap_add_irq_chip

> +       if (ret) {
> +               dev_err(pf1550->dev, "failed to add irq4 chip: %d\n", ret);
> +               goto err_charger_irq;
> +       }
> +
> +       ret = mfd_add_devices(pf1550->dev, -1, pf1550_devs,
> +                             ARRAY_SIZE(pf1550_devs), NULL, 0, NULL);

You can use the managed version devm_mfd_add_devices.

Also, -1 has his own define which is PLATFORM_DEVID_NONE, use that instead.


> +       if (ret < 0)
> +               goto err_mfd;
> +
> +       return ret;
> +
> +err_mfd:
> +       mfd_remove_devices(pf1550->dev);
> +err_charger_irq:
> +       regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_charger);
> +err_onkey_irq:
> +       regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_regulator);
> +err_regulator_irq:
> +       return ret;

All this error handling can be simplified using the device managed versions.

> +}
> +
> +static int pf1550_i2c_remove(struct i2c_client *i2c)
> +{
> +       struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
> +
> +       mfd_remove_devices(pf1550->dev);
> +
> +       regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_regulator);
> +       regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_onkey);
> +       regmap_del_irq_chip(pf1550->irq, pf1550->irq_data_charger);
> +
> +       return 0;
> +}
> +

The full function can be remove after move to device managed version
of these functions.


> +static const struct i2c_device_id pf1550_i2c_id[] = {
> +       { "pf1550", PF1550 },
> +       { }

Adding sentinel is a good practice

{ /* sentinel */ }

> +};
> +MODULE_DEVICE_TABLE(i2c, pf1550_i2c_id);
> +
> +static int pf1550_suspend(struct device *dev)
> +{
> +       struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> +       struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
> +
> +       if (device_may_wakeup(dev)) {
> +               enable_irq_wake(pf1550->irq);
> +               disable_irq(pf1550->irq);
> +       }
> +
> +       return 0;
> +}
> +
> +static int pf1550_resume(struct device *dev)
> +{
> +       struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
> +       struct pf1550_dev *pf1550 = i2c_get_clientdata(i2c);
> +
> +       if (device_may_wakeup(dev)) {
> +               disable_irq_wake(pf1550->irq);
> +               enable_irq(pf1550->irq);
> +       }
> +
> +       return 0;
> +}
> +
> +static const struct dev_pm_ops pf1550_pm = {
> +       .suspend = pf1550_suspend,
> +       .resume = pf1550_resume,
> +};
> +

Replace this with SIMPLE_DEV_PM_OPS helper macro.

> +#ifdef CONFIG_OF
> +static const struct of_device_id pf1550_dt_match[] = {
> +       { .compatible = "fsl,pf1550" },

"fsl,pf1550" appears un-documented. You need to add the documentation
binding for this.

> +       {},

Add sentinel and remove the comma.

{ /* sentinel */ }

> +};

Guess you are missing the module device table here.

MODULE_DEVICE_TABLE(of, pf1550_dt_match);

> +#endif
> +
> +static struct i2c_driver pf1550_i2c_driver = {
> +       .driver = {
> +                  .name = "pf1550",
> +                  .owner = THIS_MODULE,

An i2c driver does not need the owner, the core does it for you, so
you can remove this field.

> +                  .pm = &pf1550_pm,
> +                  .of_match_table = of_match_ptr(pf1550_dt_match),
> +       },
> +       .probe = pf1550_i2c_probe,
> +       .remove = pf1550_i2c_remove,
> +       .id_table = pf1550_i2c_id,
> +};
> +
> +static int __init pf1550_i2c_init(void)
> +{
> +       return i2c_add_driver(&pf1550_i2c_driver);
> +}
> +/* init early so consumer devices can complete system boot */
> +subsys_initcall(pf1550_i2c_init);
> +
> +static void __exit pf1550_i2c_exit(void)
> +{
> +       i2c_del_driver(&pf1550_i2c_driver);
> +}
> +module_exit(pf1550_i2c_exit);
> +

If I am not wrong subsys_initcall is guaranteed to be executed before
the procedure declared as module_init, but this only makes sense when
the driver is built-in. Your driver can be built as a module and I
think that should be possible as module, so I am wondering if you
really need to use the subsys_initcall here or if you can simply use
the helper macro for registering a modular I2C driver and remove the
__init and __exit functions.

module_i2c_driver(pf1550_i2c_driver);

> +MODULE_DESCRIPTION("Freescale PF1550 multi-function core driver");
> +MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
> +MODULE_LICENSE("GPL");
> +
> diff --git a/include/linux/mfd/pf1550.h b/include/linux/mfd/pf1550.h
> new file mode 100644
> index 0000000..9721929
> --- /dev/null
> +++ b/include/linux/mfd/pf1550.h
> @@ -0,0 +1,241 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * pf1550.h - mfd head file for PF1550
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Robin Gong <yibin.gong@freescale.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.
> + *
> + * This program is distributed in the hope that 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.

Remove the GPL notice as you have already added the SPDX License Identifier.

> + */
> +
> +#ifndef __LINUX_MFD_PF1550_H
> +#define __LINUX_MFD_PF1550_H
> +
> +#include <linux/i2c.h>
> +
> +enum chips { PF1550 = 1, };
> +
> +enum pf1550_pmic_reg {
> +       /* PMIC regulator part */
> +       PF1550_PMIC_REG_DEVICE_ID               = 0x00,
> +       PF1550_PMIC_REG_OTP_FLAVOR              = 0x01,
> +       PF1550_PMIC_REG_SILICON_REV             = 0x02,
> +
> +       PF1550_PMIC_REG_INT_CATEGORY            = 0x06,
> +       PF1550_PMIC_REG_SW_INT_STAT0            = 0x08,
> +       PF1550_PMIC_REG_SW_INT_MASK0            = 0x09,
> +       PF1550_PMIC_REG_SW_INT_SENSE0           = 0x0A,
> +       PF1550_PMIC_REG_SW_INT_STAT1            = 0x0B,
> +       PF1550_PMIC_REG_SW_INT_MASK1            = 0x0C,
> +       PF1550_PMIC_REG_SW_INT_SENSE1           = 0x0D,
> +       PF1550_PMIC_REG_SW_INT_STAT2            = 0x0E,
> +       PF1550_PMIC_REG_SW_INT_MASK2            = 0x0F,
> +       PF1550_PMIC_REG_SW_INT_SENSE2           = 0x10,
> +       PF1550_PMIC_REG_LDO_INT_STAT0           = 0x18,
> +       PF1550_PMIC_REG_LDO_INT_MASK0           = 0x19,
> +       PF1550_PMIC_REG_LDO_INT_SENSE0          = 0x1A,
> +       PF1550_PMIC_REG_TEMP_INT_STAT0          = 0x20,
> +       PF1550_PMIC_REG_TEMP_INT_MASK0          = 0x21,
> +       PF1550_PMIC_REG_TEMP_INT_SENSE0         = 0x22,
> +       PF1550_PMIC_REG_ONKEY_INT_STAT0         = 0x24,
> +       PF1550_PMIC_REG_ONKEY_INT_MASK0         = 0x25,
> +       PF1550_PMIC_REG_ONKEY_INT_SENSE0        = 0x26,
> +       PF1550_PMIC_REG_MISC_INT_STAT0          = 0x28,
> +       PF1550_PMIC_REG_MISC_INT_MASK0          = 0x29,
> +       PF1550_PMIC_REG_MISC_INT_SENSE0         = 0x2A,
> +
> +       PF1550_PMIC_REG_COINCELL_CONTROL        = 0x30,
> +
> +       PF1550_PMIC_REG_SW1_VOLT                = 0x32,
> +       PF1550_PMIC_REG_SW1_STBY_VOLT           = 0x33,
> +       PF1550_PMIC_REG_SW1_SLP_VOLT            = 0x34,
> +       PF1550_PMIC_REG_SW1_CTRL                = 0x35,
> +       PF1550_PMIC_REG_SW1_CTRL1               = 0x36,
> +       PF1550_PMIC_REG_SW2_VOLT                = 0x38,
> +       PF1550_PMIC_REG_SW2_STBY_VOLT           = 0x39,
> +       PF1550_PMIC_REG_SW2_SLP_VOLT            = 0x3A,
> +       PF1550_PMIC_REG_SW2_CTRL                = 0x3B,
> +       PF1550_PMIC_REG_SW2_CTRL1               = 0x3C,
> +       PF1550_PMIC_REG_SW3_VOLT                = 0x3E,
> +       PF1550_PMIC_REG_SW3_STBY_VOLT           = 0x3F,
> +       PF1550_PMIC_REG_SW3_SLP_VOLT            = 0x40,
> +       PF1550_PMIC_REG_SW3_CTRL                = 0x41,
> +       PF1550_PMIC_REG_SW3_CTRL1               = 0x42,
> +       PF1550_PMIC_REG_VSNVS_CTRL              = 0x48,
> +       PF1550_PMIC_REG_VREFDDR_CTRL            = 0x4A,
> +       PF1550_PMIC_REG_LDO1_VOLT               = 0x4C,
> +       PF1550_PMIC_REG_LDO1_CTRL               = 0x4D,
> +       PF1550_PMIC_REG_LDO2_VOLT               = 0x4F,
> +       PF1550_PMIC_REG_LDO2_CTRL               = 0x50,
> +       PF1550_PMIC_REG_LDO3_VOLT               = 0x52,
> +       PF1550_PMIC_REG_LDO3_CTRL               = 0x53,
> +       PF1550_PMIC_REG_PWRCTRL0                = 0x58,
> +       PF1550_PMIC_REG_PWRCTRL1                = 0x59,
> +       PF1550_PMIC_REG_PWRCTRL2                = 0x5A,
> +       PF1550_PMIC_REG_PWRCTRL3                = 0x5B,
> +       PF1550_PMIC_REG_SW1_PWRDN_SEQ           = 0x5F,
> +       PF1550_PMIC_REG_SW2_PWRDN_SEQ           = 0x60,
> +       PF1550_PMIC_REG_SW3_PWRDN_SEQ           = 0x61,
> +       PF1550_PMIC_REG_LDO1_PWRDN_SEQ          = 0x62,
> +       PF1550_PMIC_REG_LDO2_PWRDN_SEQ          = 0x63,
> +       PF1550_PMIC_REG_LDO3_PWRDN_SEQ          = 0x64,
> +       PF1550_PMIC_REG_VREFDDR_PWRDN_SEQ       = 0x65,
> +
> +       PF1550_PMIC_REG_STATE_INFO              = 0x67,
> +       PF1550_PMIC_REG_I2C_ADDR                = 0x68,
> +       PF1550_PMIC_REG_IO_DRV0                 = 0x69,
> +       PF1550_PMIC_REG_IO_DRV1                 = 0x6A,
> +       PF1550_PMIC_REG_RC_16MHZ                = 0x6B,
> +       PF1550_PMIC_REG_KEY                     = 0x6F,
> +
> +       /* charger part */
> +       PF1550_CHARG_REG_CHG_INT                = 0x80,
> +       PF1550_CHARG_REG_CHG_INT_MASK           = 0x82,
> +       PF1550_CHARG_REG_CHG_INT_OK             = 0x84,
> +       PF1550_CHARG_REG_VBUS_SNS               = 0x86,
> +       PF1550_CHARG_REG_CHG_SNS                = 0x87,
> +       PF1550_CHARG_REG_BATT_SNS               = 0x88,
> +       PF1550_CHARG_REG_CHG_OPER               = 0x89,
> +       PF1550_CHARG_REG_CHG_TMR                = 0x8A,
> +       PF1550_CHARG_REG_CHG_EOC_CNFG           = 0x8D,
> +       PF1550_CHARG_REG_CHG_CURR_CNFG          = 0x8E,
> +       PF1550_CHARG_REG_BATT_REG               = 0x8F,
> +       PF1550_CHARG_REG_BATFET_CNFG            = 0x91,
> +       PF1550_CHARG_REG_THM_REG_CNFG           = 0x92,
> +       PF1550_CHARG_REG_VBUS_INLIM_CNFG        = 0x94,
> +       PF1550_CHARG_REG_VBUS_LIN_DPM           = 0x95,
> +       PF1550_CHARG_REG_USB_PHY_LDO_CNFG       = 0x96,
> +       PF1550_CHARG_REG_DBNC_DELAY_TIME        = 0x98,
> +       PF1550_CHARG_REG_CHG_INT_CNFG           = 0x99,
> +       PF1550_CHARG_REG_THM_ADJ_SETTING        = 0x9A,
> +       PF1550_CHARG_REG_VBUS2SYS_CNFG          = 0x9B,
> +       PF1550_CHARG_REG_LED_PWM                = 0x9C,
> +       PF1550_CHARG_REG_FAULT_BATFET_CNFG      = 0x9D,
> +       PF1550_CHARG_REG_LED_CNFG               = 0x9E,
> +       PF1550_CHARG_REG_CHGR_KEY2              = 0x9F,
> +
> +       PF1550_PMIC_REG_END                     = 0xff,
> +};
> +
> +#define PF1550_CHG_PRECHARGE           0
> +#define PF1550_CHG_CONSTANT_CURRENT    1
> +#define PF1550_CHG_CONSTANT_VOL                2
> +#define PF1550_CHG_EOC                 3
> +#define PF1550_CHG_DONE                        4
> +#define PF1550_CHG_TIMER_FAULT         6
> +#define PF1550_CHG_SUSPEND             7
> +#define PF1550_CHG_OFF_INV             8
> +#define PF1550_CHG_BAT_OVER            9
> +#define PF1550_CHG_OFF_TEMP            10
> +#define PF1550_CHG_LINEAR_ONLY         12
> +#define PF1550_CHG_SNS_MASK            0xf
> +
> +#define PF1550_BAT_NO_VBUS             0
> +#define PF1550_BAT_LOW_THAN_PRECHARG   1
> +#define PF1550_BAT_CHARG_FAIL          2
> +#define PF1550_BAT_HIGH_THAN_PRECHARG  4
> +#define PF1550_BAT_OVER_VOL            5
> +#define        PF1550_BAT_NO_DETECT            6
> +#define PF1550_BAT_SNS_MASK            0x7
> +
> +#define PF1550_VBUS_UVLO               BIT(2)
> +#define PF1550_VBUS_IN2SYS             BIT(3)
> +#define PF1550_VBUS_OVLO               BIT(4)
> +#define PF1550_VBUS_VALID              BIT(5)
> +
> +#define PF1550_CHARG_REG_BATT_REG_CHGCV_MASK           0x3f
> +#define PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT                6
> +#define PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK         0x3
> +#define PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT    2
> +#define PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK     0x3
> +
> +#define PMIC_IRQ_SW1_LS                BIT(0)
> +#define PMIC_IRQ_SW2_LS                BIT(1)
> +#define PMIC_IRQ_SW3_LS                BIT(2)
> +#define PMIC_IRQ_SW1_HS                BIT(0)
> +#define PMIC_IRQ_SW2_HS                BIT(1)
> +#define PMIC_IRQ_SW3_HS                BIT(2)
> +#define PMIC_IRQ_LDO1_FAULT    BIT(0)
> +#define PMIC_IRQ_LDO2_FAULT    BIT(1)
> +#define PMIC_IRQ_LDO3_FAULT    BIT(2)
> +#define PMIC_IRQ_TEMP_110      BIT(0)
> +#define PMIC_IRQ_TEMP_125      BIT(1)
> +
> +#define ONKEY_IRQ_PUSHI                BIT(0)
> +#define ONKEY_IRQ_1SI          BIT(1)
> +#define ONKEY_IRQ_2SI          BIT(2)
> +#define ONKEY_IRQ_3SI          BIT(3)
> +#define ONKEY_IRQ_4SI          BIT(4)
> +#define ONKEY_IRQ_8SI          BIT(5)
> +
> +#define CHARG_IRQ_BAT2SOCI     BIT(1)
> +#define CHARG_IRQ_BATI         BIT(2)
> +#define CHARG_IRQ_CHGI         BIT(3)
> +#define CHARG_IRQ_VBUSI                BIT(5)
> +#define CHARG_IRQ_DPMI         BIT(6)
> +#define CHARG_IRQ_THMI         BIT(7)
> +
> +enum pf1550_irq {
> +       PF1550_PMIC_IRQ_SW1_LS,
> +       PF1550_PMIC_IRQ_SW2_LS,
> +       PF1550_PMIC_IRQ_SW3_LS,
> +       PF1550_PMIC_IRQ_SW1_HS,
> +       PF1550_PMIC_IRQ_SW2_HS,
> +       PF1550_PMIC_IRQ_SW3_HS,
> +       PF1550_PMIC_IRQ_LDO1_FAULT,
> +       PF1550_PMIC_IRQ_LDO2_FAULT,
> +       PF1550_PMIC_IRQ_LDO3_FAULT,
> +       PF1550_PMIC_IRQ_TEMP_110,
> +       PF1550_PMIC_IRQ_TEMP_125,
> +
> +       PF1550_ONKEY_IRQ_PUSHI,
> +       PF1550_ONKEY_IRQ_1SI,
> +       PF1550_ONKEY_IRQ_2SI,
> +       PF1550_ONKEY_IRQ_3SI,
> +       PF1550_ONKEY_IRQ_4SI,
> +       PF1550_ONKEY_IRQ_8SI,
> +
> +       PF1550_CHARG_IRQ_BAT2SOCI,
> +       PF1550_CHARG_IRQ_BATI,
> +       PF1550_CHARG_IRQ_CHGI,
> +       PF1550_CHARG_IRQ_VBUSI,
> +       PF1550_CHARG_IRQ_DPMI,
> +       PF1550_CHARG_IRQ_THMI,
> +};
> +
> +enum pf1550_regulators {
> +       PF1550_SW1,
> +       PF1550_SW2,
> +       PF1550_SW3,
> +       PF1550_VREFDDR,
> +       PF1550_LDO1,
> +       PF1550_LDO2,
> +       PF1550_LDO3,
> +};
> +
> +struct pf1550_irq_info {
> +       unsigned int irq;
> +       const char *name;
> +       unsigned int virq;
> +};
> +
> +struct pf1550_dev {
> +       struct device *dev;
> +       struct i2c_client *i2c;
> +       int type;
> +       struct regmap *regmap;
> +       struct regmap_irq_chip_data *irq_data_regulator;
> +       struct regmap_irq_chip_data *irq_data_onkey;
> +       struct regmap_irq_chip_data *irq_data_charger;
> +       int irq;
> +};
> +
> +#endif /* __LINUX_MFD_PF1550_H */
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] power: pf1550_charger: add pf1550 charger driver
  2018-04-17 14:20 ` [PATCH 2/2] power: pf1550_charger: add pf1550 charger driver Abel Vesa
@ 2018-04-24 22:02   ` Enric Balletbo Serra
  0 siblings, 0 replies; 4+ messages in thread
From: Enric Balletbo Serra @ 2018-04-24 22:02 UTC (permalink / raw)
  To: Abel Vesa
  Cc: Lee Jones, Sebastian Reichel, Robin Gong, linux-kernel,
	Linux PM list, linux-imx, Abel Vesa

Hi Abel,

Some comments below, hope will be helpful for you.

2018-04-17 16:20 GMT+02:00 Abel Vesa <abel.vesa@nxp.com>:
> From: Robin Gong <b38343@freescale.com>
>
> Add basic pf1550 charger driver.
>
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
> ---
>  drivers/power/supply/Kconfig          |   6 +
>  drivers/power/supply/Makefile         |   1 +
>  drivers/power/supply/pf1550_charger.c | 660 ++++++++++++++++++++++++++++++++++
>  3 files changed, 667 insertions(+)
>  create mode 100644 drivers/power/supply/pf1550_charger.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 428b426..2ebd38c 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -380,6 +380,12 @@ config CHARGER_PCF50633
>         help
>          Say Y to include support for NXP PCF50633 Main Battery Charger.
>
> +config CHARGER_PF1550
> +       tristate "Freescale PF1550 battery charger driver"
> +       depends on MFD_PF1550
> +       help
> +        Say Y to enable support for the Freescale PF1550 battery charger.
> +
>  config BATTERY_JZ4740
>         tristate "Ingenic JZ4740 battery"
>         depends on MACH_JZ4740
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index e83aa84..0b2e424 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_BATTERY_S3C_ADC) += s3c_adc_battery.o
>  obj-$(CONFIG_BATTERY_TWL4030_MADC)     += twl4030_madc_battery.o
>  obj-$(CONFIG_CHARGER_88PM860X) += 88pm860x_charger.o
>  obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
> +obj-$(CONFIG_CHARGER_PF1550)   += pf1550_charger.o
>  obj-$(CONFIG_BATTERY_JZ4740)   += jz4740-battery.o
>  obj-$(CONFIG_BATTERY_RX51)     += rx51_battery.o
>  obj-$(CONFIG_AB8500_BM)                += ab8500_bmdata.o ab8500_charger.o ab8500_fg.o ab8500_btemp.o abx500_chargalg.o pm2301_charger.o
> diff --git a/drivers/power/supply/pf1550_charger.c b/drivers/power/supply/pf1550_charger.c
> new file mode 100644
> index 0000000..c5e0c55
> --- /dev/null
> +++ b/drivers/power/supply/pf1550_charger.c
> @@ -0,0 +1,660 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * pf1550_charger.c - regulator driver for the PF1550
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Robin Gong <yibin.gong@freescale.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.
> + *
> + * This program is distributed in the hope that 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.

I think that the general consensus is to remove the GPL notice if you
provide the SPDX license identifier.

> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/pf1550.h>
> +
> +#define PF1550_CHARGER_NAME            "pf1550-charger"
> +#define PF1550_DEFAULT_CONSTANT_VOLT   4200000
> +#define PF1550_DEFAULT_MIN_SYSTEM_VOLT 3500000
> +#define PF1550_DEFAULT_THERMAL_TEMP    75
> +
> +static const char *pf1550_charger_model                = "PF1550";
> +static const char *pf1550_charger_manufacturer = "Freescale";
> +
> +struct pf1550_charger {
> +       struct device *dev;
> +       struct pf1550_dev *pf1550;
> +       struct power_supply *charger;
> +       struct power_supply_desc psy_desc;
> +       int irq;
> +       struct delayed_work irq_work;
> +       struct mutex mutex;
> +
> +       u32 constant_volt;
> +       u32 min_system_volt;
> +       u32 thermal_regulation_temp;
> +};
> +
> +static struct pf1550_irq_info pf1550_charger_irqs[] = {
> +       { PF1550_CHARG_IRQ_BAT2SOCI,            "BAT2SOC" },
> +       { PF1550_CHARG_IRQ_BATI,                "BAT" },
> +       { PF1550_CHARG_IRQ_CHGI,                "CHG" },
> +       { PF1550_CHARG_IRQ_VBUSI,               "VBUS" },
> +       { PF1550_CHARG_IRQ_THMI,                "THM" },
> +};
> +
> +static int pf1550_get_charger_state(struct regmap *regmap, int *val)
> +{
> +       int ret;
> +       unsigned int data;
> +
> +       ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
> +       if (ret < 0)
> +               return ret;
> +
> +       data &= PF1550_CHG_SNS_MASK;
> +
> +       switch (data) {
> +       case PF1550_CHG_PRECHARGE:
> +       case PF1550_CHG_CONSTANT_CURRENT:
> +       case PF1550_CHG_CONSTANT_VOL:
> +       case PF1550_CHG_EOC:
> +               *val = POWER_SUPPLY_STATUS_CHARGING;
> +               break;
> +       case PF1550_CHG_DONE:
> +               *val = POWER_SUPPLY_STATUS_FULL;
> +               break;
> +       case PF1550_CHG_TIMER_FAULT:
> +       case PF1550_CHG_SUSPEND:
> +               *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +               break;
> +       case PF1550_CHG_OFF_INV:
> +       case PF1550_CHG_OFF_TEMP:
> +       case PF1550_CHG_LINEAR_ONLY:
> +               *val = POWER_SUPPLY_STATUS_DISCHARGING;
> +               break;
> +       default:
> +               *val = POWER_SUPPLY_STATUS_UNKNOWN;
> +       }
> +
> +       return 0;
> +}
> +
> +static int pf1550_get_charge_type(struct regmap *regmap, int *val)
> +{
> +       int ret;
> +       unsigned int data;
> +
> +       ret = regmap_read(regmap, PF1550_CHARG_REG_CHG_SNS, &data);
> +       if (ret < 0)
> +               return ret;
> +
> +       data &= PF1550_CHG_SNS_MASK;
> +
> +       switch (data) {
> +       case PF1550_CHG_SNS_MASK:
> +               *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> +               break;
> +       case PF1550_CHG_CONSTANT_CURRENT:
> +       case PF1550_CHG_CONSTANT_VOL:
> +       case PF1550_CHG_EOC:
> +               *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
> +               break;
> +       case PF1550_CHG_DONE:
> +       case PF1550_CHG_TIMER_FAULT:
> +       case PF1550_CHG_SUSPEND:
> +       case PF1550_CHG_OFF_INV:
> +       case PF1550_CHG_BAT_OVER:
> +       case PF1550_CHG_OFF_TEMP:
> +       case PF1550_CHG_LINEAR_ONLY:
> +               *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
> +               break;
> +       default:
> +               *val = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
> +       }
> +
> +       return 0;
> +}
> +
> +/*
> + * Supported health statuses:
> + *  - POWER_SUPPLY_HEALTH_DEAD
> + *  - POWER_SUPPLY_HEALTH_GOOD
> + *  - POWER_SUPPLY_HEALTH_OVERVOLTAGE
> + *  - POWER_SUPPLY_HEALTH_UNKNOWN
> + */
> +static int pf1550_get_battery_health(struct regmap *regmap, int *val)
> +{
> +       int ret;
> +       unsigned int data;
> +
> +       ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
> +       if (ret < 0)
> +               return ret;
> +
> +       data &= PF1550_BAT_SNS_MASK;
> +
> +       switch (data) {
> +       case PF1550_BAT_NO_DETECT:
> +               *val = POWER_SUPPLY_HEALTH_DEAD;
> +               break;
> +       case PF1550_BAT_NO_VBUS:
> +       case PF1550_BAT_LOW_THAN_PRECHARG:
> +       case PF1550_BAT_CHARG_FAIL:
> +       case PF1550_BAT_HIGH_THAN_PRECHARG:
> +               *val = POWER_SUPPLY_HEALTH_GOOD;
> +               break;
> +       case PF1550_BAT_OVER_VOL:
> +               *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +               break;
> +       default:
> +               *val = POWER_SUPPLY_HEALTH_UNKNOWN;
> +               break;
> +       }
> +
> +       return 0;
> +}
> +
> +static int pf1550_get_present(struct regmap *regmap, int *val)
> +{
> +       unsigned int data;
> +       int ret;
> +
> +       ret = regmap_read(regmap, PF1550_CHARG_REG_BATT_SNS, &data);
> +       if (ret < 0)
> +               return ret;
> +
> +       data &= PF1550_BAT_SNS_MASK;
> +       *val = (data == PF1550_BAT_NO_DETECT) ? 0 : 1;
> +
> +       return 0;
> +}
> +
> +static int pf1550_get_online(struct regmap *regmap, int *val)
> +{
> +       unsigned int data;
> +       int ret;
> +
> +       ret = regmap_read(regmap, PF1550_CHARG_REG_VBUS_SNS, &data);
> +       if (ret < 0)
> +               return ret;
> +
> +       *val = (data & PF1550_VBUS_VALID) ? 1 : 0;
> +
> +       return 0;
> +}
> +
> +static void pf1550_chg_bat_isr(struct pf1550_charger *chg)
> +{
> +       unsigned int data;
> +
> +       if (regmap_read(chg->pf1550->regmap,
> +                       PF1550_CHARG_REG_BATT_SNS, &data)) {
> +               dev_err(chg->dev, "Read BATT_SNS error.\n");
> +               return;
> +       }
> +
> +       switch (data & PF1550_BAT_SNS_MASK) {
> +       case PF1550_BAT_NO_VBUS:
> +               dev_dbg(chg->dev, "No valid VBUS input.\n");
> +               break;
> +       case PF1550_BAT_LOW_THAN_PRECHARG:
> +               dev_dbg(chg->dev, "VBAT < VPRECHG.LB.\n");
> +               break;
> +       case PF1550_BAT_CHARG_FAIL:
> +               dev_dbg(chg->dev, "Battery charging failed.\n");
> +               break;
> +       case PF1550_BAT_HIGH_THAN_PRECHARG:
> +               dev_dbg(chg->dev, "VBAT > VPRECHG.LB.\n");
> +               break;
> +       case PF1550_BAT_OVER_VOL:
> +               dev_dbg(chg->dev, "VBAT > VBATOV.\n");
> +               break;
> +       case PF1550_BAT_NO_DETECT:
> +               dev_dbg(chg->dev, "Battery not detected.\n");
> +               break;
> +       default:
> +               dev_err(chg->dev, "Unknown value read:%x\n",
> +                       data & PF1550_CHG_SNS_MASK);
> +       }
> +}
> +

nit: This function only prints a debug message or an error and you do
nothing with the info,  maybe you can consider adding this kind of
info in debugfs instead?

> +static void pf1550_chg_chg_isr(struct pf1550_charger *chg)
> +{
> +       unsigned int data;
> +
> +       if (regmap_read(chg->pf1550->regmap,
> +                       PF1550_CHARG_REG_CHG_SNS, &data)) {
> +               dev_err(chg->dev, "Read CHG_SNS error.\n");
> +               return;
> +       }
> +
> +       switch (data & PF1550_CHG_SNS_MASK) {
> +       case PF1550_CHG_PRECHARGE:
> +               dev_dbg(chg->dev, "In pre-charger mode.\n");
> +               break;
> +       case PF1550_CHG_CONSTANT_CURRENT:
> +               dev_dbg(chg->dev, "In fast-charge constant current mode.\n");
> +               break;
> +       case PF1550_CHG_CONSTANT_VOL:
> +               dev_dbg(chg->dev, "In fast-charge constant voltage mode.\n");
> +               break;
> +       case PF1550_CHG_EOC:
> +               dev_dbg(chg->dev, "In EOC mode.\n");
> +               break;
> +       case PF1550_CHG_DONE:
> +               dev_dbg(chg->dev, "In DONE mode.\n");
> +               break;
> +       case PF1550_CHG_TIMER_FAULT:
> +               dev_info(chg->dev, "In timer fault mode.\n");
> +               break;
> +       case PF1550_CHG_SUSPEND:
> +               dev_info(chg->dev, "In thermistor suspend mode.\n");
> +               break;
> +       case PF1550_CHG_OFF_INV:
> +               dev_info(chg->dev, "Input invalid, charger off.\n");
> +               break;
> +       case PF1550_CHG_BAT_OVER:
> +               dev_info(chg->dev, "Battery over-voltage.\n");
> +               break;
> +       case PF1550_CHG_OFF_TEMP:
> +               dev_info(chg->dev, "Temp high, charger off.\n");
> +               break;
> +       case PF1550_CHG_LINEAR_ONLY:
> +               dev_dbg(chg->dev, "In Linear mode, not charging.\n");
> +               break;
> +       default:
> +               dev_err(chg->dev, "Unknown value read:%x\n",
> +                       data & PF1550_CHG_SNS_MASK);
> +       }
> +}
> +

nit: debugfs?

> +static void pf1550_chg_vbus_isr(struct pf1550_charger *chg)
> +{
> +       enum power_supply_type old_type;
> +       unsigned int data;
> +
> +       if (regmap_read(chg->pf1550->regmap,
> +                       PF1550_CHARG_REG_VBUS_SNS, &data)) {
> +               dev_err(chg->dev, "Read VBUS_SNS error.\n");
> +               return;
> +       }
> +
> +       old_type = chg->psy_desc.type;
> +
> +       if (data & PF1550_VBUS_UVLO) {
> +               chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> +               dev_dbg(chg->dev, "VBUS deattached.\n");
> +       }
> +       if (data & PF1550_VBUS_IN2SYS)
> +               dev_dbg(chg->dev, "VBUS_IN2SYS_SNS.\n");
> +       if (data & PF1550_VBUS_OVLO)
> +               dev_dbg(chg->dev, "VBUS_OVLO_SNS.\n");
> +       if (data & PF1550_VBUS_VALID) {
> +               chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
> +               dev_dbg(chg->dev, "VBUS attached.\n");
> +       }
> +
> +       if (old_type != chg->psy_desc.type)
> +               power_supply_changed(chg->charger);
> +}
> +
> +static irqreturn_t pf1550_charger_irq_handler(int irq, void *data)
> +{
> +       struct pf1550_charger *chg = data;
> +
> +       chg->irq = irq;
> +       schedule_delayed_work(&chg->irq_work,  msecs_to_jiffies(10));
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static void pf1550_charger_irq_work(struct work_struct *work)
> +{
> +       struct pf1550_charger *chg = container_of(to_delayed_work(work),
> +                                                 struct pf1550_charger,
> +                                                 irq_work);
> +       int i, irq_type = -1;
> +       unsigned int status;
> +
> +       if (!chg->charger)
> +               return;
> +
> +       mutex_lock(&chg->mutex);
> +
> +       for (i = 0; i < ARRAY_SIZE(pf1550_charger_irqs); i++)
> +               if (chg->irq == pf1550_charger_irqs[i].virq)
> +                       irq_type = pf1550_charger_irqs[i].irq;
> +
> +       switch (irq_type) {
> +       case PF1550_CHARG_IRQ_BAT2SOCI:
> +               dev_info(chg->dev, "BAT to SYS Overcurrent interrupt.\n");
> +               break;
> +       case PF1550_CHARG_IRQ_BATI:
> +               pf1550_chg_bat_isr(chg);
> +               break;
> +       case PF1550_CHARG_IRQ_CHGI:
> +               pf1550_chg_chg_isr(chg);
> +               break;
> +       case PF1550_CHARG_IRQ_VBUSI:
> +               pf1550_chg_vbus_isr(chg);
> +               break;
> +       case PF1550_CHARG_IRQ_THMI:
> +               dev_info(chg->dev, "Thermal interrupt.\n");
> +               break;
> +       default:
> +               dev_err(chg->dev, "unknown interrupt occurred.\n");
> +       }
> +
> +       if (regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT, &status))
> +               dev_err(chg->dev, "Read CHG_INT error.\n");
> +       if (regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT, status))
> +               dev_err(chg->dev, "clear CHG_INT error.\n");
> +
> +       mutex_unlock(&chg->mutex);
> +}
> +
> +static enum power_supply_property pf1550_charger_props[] = {
> +       POWER_SUPPLY_PROP_STATUS,
> +       POWER_SUPPLY_PROP_CHARGE_TYPE,
> +       POWER_SUPPLY_PROP_HEALTH,
> +       POWER_SUPPLY_PROP_PRESENT,
> +       POWER_SUPPLY_PROP_ONLINE,
> +       POWER_SUPPLY_PROP_MODEL_NAME,
> +       POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> +static int pf1550_charger_get_property(struct power_supply *psy,
> +                           enum power_supply_property psp,
> +                           union power_supply_propval *val)
> +{
> +       struct pf1550_charger *chg = power_supply_get_drvdata(psy);
> +       struct regmap *regmap = chg->pf1550->regmap;
> +       int ret = 0;
> +
> +       switch (psp) {
> +       case POWER_SUPPLY_PROP_STATUS:
> +               ret = pf1550_get_charger_state(regmap, &val->intval);
> +               break;
> +       case POWER_SUPPLY_PROP_CHARGE_TYPE:
> +               ret = pf1550_get_charge_type(regmap, &val->intval);
> +               break;
> +       case POWER_SUPPLY_PROP_HEALTH:
> +               ret = pf1550_get_battery_health(regmap, &val->intval);
> +               break;
> +       case POWER_SUPPLY_PROP_PRESENT:
> +               ret = pf1550_get_present(regmap, &val->intval);
> +               break;
> +       case POWER_SUPPLY_PROP_ONLINE:
> +               ret = pf1550_get_online(regmap, &val->intval);
> +               break;
> +       case POWER_SUPPLY_PROP_MODEL_NAME:
> +               val->strval = pf1550_charger_model;
> +               break;
> +       case POWER_SUPPLY_PROP_MANUFACTURER:
> +               val->strval = pf1550_charger_manufacturer;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return ret;
> +}
> +
> +static int pf1550_set_constant_volt(struct pf1550_charger *chg,
> +               unsigned int uvolt)
> +{
> +       unsigned int data;
> +
> +       if (uvolt >= 3500000 && uvolt <= 4440000)
> +               data = 8 + (uvolt - 3500000) / 20000;

You should use braces {} for this statement.

> +       else {
> +               dev_err(chg->dev, "Wrong value for constant voltage\n");
> +               return -EINVAL;
> +       }
> +
> +       dev_dbg(chg->dev, "Charging constant voltage: %u (0x%x)\n", uvolt,
> +                       data);
> +
> +       return regmap_update_bits(chg->pf1550->regmap,
> +                       PF1550_CHARG_REG_BATT_REG,
> +                       PF1550_CHARG_REG_BATT_REG_CHGCV_MASK, data);
> +}
> +
> +static int pf1550_set_min_system_volt(struct pf1550_charger *chg,
> +               unsigned int uvolt)
> +{
> +       unsigned int data;
> +
> +       switch (uvolt) {
> +       case 3500000:
> +               data = 0x0;
> +               break;
> +       case 3700000:
> +               data = 0x1;
> +               break;
> +       case 4300000:
> +               data = 0x2;
> +               break;
> +       default:
> +               dev_err(chg->dev, "Wrong value for minimum system voltage\n");
> +               return -EINVAL;
> +       }
> +
> +       data <<= PF1550_CHARG_REG_BATT_REG_VMINSYS_SHIFT;
> +
> +       dev_dbg(chg->dev, "Minimum system regulation voltage: %u (0x%x)\n",
> +                       uvolt, data);
> +
> +       return regmap_update_bits(chg->pf1550->regmap,
> +                       PF1550_CHARG_REG_BATT_REG,
> +                       PF1550_CHARG_REG_BATT_REG_VMINSYS_MASK, data);
> +}
> +
> +static int pf1550_set_thermal_regulation_temp(struct pf1550_charger *chg,
> +               unsigned int cels)
> +{
> +       unsigned int data;
> +
> +       switch (cels) {
> +       case 60:
> +               data = 0x0;
> +               break;
> +       case 75:
> +               data = 0x1;
> +               break;
> +       case 90:
> +               data = 0x2;
> +               break;
> +       case 105:
> +               data = 0x3;
> +               break;
> +       default:
> +               dev_err(chg->dev, "Wrong value for thermal temperature\n");
> +               return -EINVAL;
> +       }
> +
> +       data <<= PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_SHIFT;
> +
> +       dev_dbg(chg->dev, "Thermal regulation loop temperature: %u (0x%x)\n",
> +                       cels, data);
> +
> +       return regmap_update_bits(chg->pf1550->regmap,
> +                       PF1550_CHARG_REG_THM_REG_CNFG,
> +                       PF1550_CHARG_REG_THM_REG_CNFG_REGTEMP_MASK, data);
> +}
> +
> +/*
> + * Sets charger registers to proper and safe default values.
> + */
> +static int pf1550_reg_init(struct pf1550_charger *chg)
> +{
> +       int ret;
> +       unsigned int data;
> +
> +       /* Unmask charger interrupt, mask DPMI and reserved bit */
> +       ret =  regmap_write(chg->pf1550->regmap, PF1550_CHARG_REG_CHG_INT_MASK,
> +                               0x51);

Better do not use magic numbers.

> +       if (ret) {
> +               dev_err(chg->dev, "Error unmask charger interrupt: %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = regmap_read(chg->pf1550->regmap, PF1550_CHARG_REG_VBUS_SNS,
> +                               &data);
> +       if (ret) {
> +               dev_err(chg->dev, "Read charg vbus_sns error: %d\n", ret);
> +               return ret;
> +       }
> +
> +       if (data & PF1550_VBUS_VALID)
> +               chg->psy_desc.type = POWER_SUPPLY_TYPE_MAINS;
> +
> +       ret = pf1550_set_constant_volt(chg, chg->constant_volt);
> +       if (ret)
> +               return ret;
> +
> +       ret = pf1550_set_min_system_volt(chg, chg->min_system_volt);
> +       if (ret)
> +               return ret;
> +
> +       ret = pf1550_set_thermal_regulation_temp(chg,
> +                       chg->thermal_regulation_temp);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static int pf1550_dt_init(struct device *dev, struct pf1550_charger *chg)
> +{
> +       struct device_node *np = dev->of_node;
> +
> +       if (!np) {
> +               dev_err(dev, "no charger OF node\n");
> +               return -EINVAL;
> +       }
> +
> +       if (of_property_read_u32(np, "fsl,constant-microvolt",
> +                       &chg->constant_volt))
> +               chg->constant_volt = PF1550_DEFAULT_CONSTANT_VOLT;
> +
> +       if (of_property_read_u32(np, "fsl,min-system-microvolt",
> +                       &chg->min_system_volt))
> +               chg->min_system_volt = PF1550_DEFAULT_MIN_SYSTEM_VOLT;
> +
> +       if (of_property_read_u32(np, "fsl,thermal-regulation",
> +                       &chg->thermal_regulation_temp))
> +               chg->thermal_regulation_temp = PF1550_DEFAULT_THERMAL_TEMP;
> +

All these properties should be documented with a binding.

> +       return 0;
> +}
> +
> +static int pf1550_charger_probe(struct platform_device *pdev)
> +{
> +       struct pf1550_charger *chg;
> +       struct power_supply_config psy_cfg = {};
> +       struct pf1550_dev *pf1550 = dev_get_drvdata(pdev->dev.parent);
> +       int i, ret;
> +
> +       chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
> +       if (!chg)
> +               return -ENOMEM;
> +
> +       chg->dev = &pdev->dev;
> +       chg->pf1550 = pf1550;
> +
> +       platform_set_drvdata(pdev, chg);
> +
> +       ret = pf1550_dt_init(&pdev->dev, chg);
> +       if (ret)
> +               return ret;
> +
> +       mutex_init(&chg->mutex);
> +
> +       INIT_DELAYED_WORK(&chg->irq_work, pf1550_charger_irq_work);
> +
> +       for (i = 0; i < ARRAY_SIZE(pf1550_charger_irqs); i++) {
> +               struct pf1550_irq_info *charger_irq =
> +                                               &pf1550_charger_irqs[i];
> +               unsigned int virq = 0;
> +
> +               virq = regmap_irq_get_virq(pf1550->irq_data_charger,
> +                                       charger_irq->irq);
> +               if (!virq)
> +                       return -EINVAL;
> +
> +               charger_irq->virq = virq;
> +
> +               ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
> +                                       pf1550_charger_irq_handler,
> +                                       IRQF_NO_SUSPEND,
> +                                       charger_irq->name, chg);
> +               if (ret) {
> +                       dev_err(&pdev->dev,
> +                               "failed: irq request (IRQ: %d, error :%d)\n",
> +                               charger_irq->irq, ret);
> +                       return ret;
> +               }
> +       }
> +
> +       psy_cfg.drv_data = chg;
> +
> +       chg->psy_desc.name = PF1550_CHARGER_NAME;
> +       chg->psy_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> +       chg->psy_desc.get_property = pf1550_charger_get_property;
> +       chg->psy_desc.properties = pf1550_charger_props;
> +       chg->psy_desc.num_properties = ARRAY_SIZE(pf1550_charger_props);
> +
> +       chg->charger = power_supply_register(&pdev->dev, &chg->psy_desc,
> +                                               &psy_cfg);

Use the devm_ variant.

> +       if (IS_ERR(chg->charger)) {
> +               dev_err(&pdev->dev, "failed: power supply register\n");
> +               ret = PTR_ERR(chg->charger);
> +               return ret;
> +       }
> +
> +       ret = pf1550_reg_init(chg);
> +
> +       return ret;
> +}
> +
> +static int pf1550_charger_remove(struct platform_device *pdev)
> +{
> +       struct pf1550_charger *chg = platform_get_drvdata(pdev);
> +
> +       cancel_delayed_work_sync(&chg->irq_work);
> +       power_supply_unregister(chg->charger);

You can drop power_supply_unregister after using the devm_
registration function.

> +
> +       return 0;
> +}
> +
> +static const struct platform_device_id pf1550_charger_id[] = {
> +       { "pf1550-charger", 0, },

Remove the comma after the 0

> +       { }
> +};

{ /*sentinel */ }

> +MODULE_DEVICE_TABLE(platform, pf1550_charger_id);
> +
> +static struct platform_driver pf1550_charger_driver = {
> +       .driver = {
> +               .name   = "pf1550-charger",
> +       },
> +       .probe          = pf1550_charger_probe,
> +       .remove         = pf1550_charger_remove,
> +       .id_table       = pf1550_charger_id,
> +};
> +module_platform_driver(pf1550_charger_driver);
> +
> +MODULE_AUTHOR("Robin Gong <yibin.gong@freescale.com>");
> +MODULE_DESCRIPTION("PF1550 charger driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-04-24 22:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-17 14:20 [PATCH 1/2] mfd: pf1550: add pf1550 mfd driver Abel Vesa
2018-04-17 14:20 ` [PATCH 2/2] power: pf1550_charger: add pf1550 charger driver Abel Vesa
2018-04-24 22:02   ` Enric Balletbo Serra
2018-04-24 11:27 ` [PATCH 1/2] mfd: pf1550: add pf1550 mfd driver Enric Balletbo Serra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.