* [PATCH 0/3] mfd: axp20x: Add support for PMIC axp202 and axp209
@ 2014-02-08 16:03 Carlo Caione
2014-02-08 16:03 ` [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC Carlo Caione
` (2 more replies)
0 siblings, 3 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-08 16:03 UTC (permalink / raw)
To: linux-arm-kernel
This set of patches add prelinary support for PMIC X-Powers axp202 and axp209.
Only two subsystem are supported at the moment: power enable key (PEK) and
regulators. Drivers for these two sub-system will be submitted in later
patch-sets.
Carlo Caione (3):
mfd: axp20x: Add mfd driver for axp20x PMIC
mfd: axp20x: Add dtsi for axp20x
mfd: axp20x: Add bindings documentation
Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++
arch/arm/boot/dts/axp20x.dtsi | 9 +
drivers/mfd/Kconfig | 12 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/axp20x.c | 251 +++++++++++++++++++++++
include/linux/mfd/axp20x.h | 178 ++++++++++++++++
6 files changed, 538 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
create mode 100644 arch/arm/boot/dts/axp20x.dtsi
create mode 100644 drivers/mfd/axp20x.c
create mode 100644 include/linux/mfd/axp20x.h
--
1.8.5.3
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-08 16:03 [PATCH 0/3] mfd: axp20x: Add support for PMIC axp202 and axp209 Carlo Caione
@ 2014-02-08 16:03 ` Carlo Caione
2014-02-10 20:02 ` Maxime Ripard
2014-02-10 21:19 ` Lee Jones
2014-02-08 16:03 ` [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x Carlo Caione
2014-02-08 16:03 ` [PATCH 3/3] mfd: axp20x: Add bindings documentation Carlo Caione
2 siblings, 2 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-08 16:03 UTC (permalink / raw)
To: linux-arm-kernel
This patch introduces the preliminary support for PMICs X-Powers AXP202
and AXP209. The core contains support only for two sub-modules (PEK
and regulators) that will be added with two different patch-sets.
Signed-off-by: Carlo Caione <carlo@caione.org>
---
drivers/mfd/Kconfig | 12 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/axp20x.c | 251 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/mfd/axp20x.h | 178 ++++++++++++++++++++++++++++++++
4 files changed, 442 insertions(+)
create mode 100644 drivers/mfd/axp20x.c
create mode 100644 include/linux/mfd/axp20x.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index dd67158..33d38c4 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -59,6 +59,18 @@ config MFD_AAT2870_CORE
additional drivers must be enabled in order to use the
functionality of the device.
+config MFD_AXP20X
+ bool "X-Powers AXP20X"
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ depends on I2C=y
+ help
+ If you say Y here you get support for the AXP20X.
+ 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_CROS_EC
tristate "ChromeOS Embedded Controller"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8a28dc9..371020e 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_PMIC_DA9052) += da9052-irq.o
obj-$(CONFIG_PMIC_DA9052) += da9052-core.o
obj-$(CONFIG_MFD_DA9052_SPI) += da9052-spi.o
obj-$(CONFIG_MFD_DA9052_I2C) += da9052-i2c.o
+obj-$(CONFIG_MFD_AXP20X) += axp20x.o
obj-$(CONFIG_MFD_LP8788) += lp8788.o lp8788-irq.o
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
new file mode 100644
index 0000000..efd0cb3
--- /dev/null
+++ b/drivers/mfd/axp20x.c
@@ -0,0 +1,251 @@
+/*
+ * Author: Carlo Caione <carlo@caione.org>
+ *
+ * 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/err.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/regulator/consumer.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/mfd/core.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+
+static const struct regmap_range axp20x_writeable_ranges[] = {
+ {
+ .range_min = AXP20X_DATA(0),
+ .range_max = AXP20X_IRQ5_STATE,
+ }, {
+ .range_min = AXP20X_DCDC_MODE,
+ .range_max = AXP20X_FG_RES,
+ },
+};
+
+static const struct regmap_range axp20x_volatile_ranges[] = {
+ {
+ .range_min = AXP20X_IRQ1_EN,
+ .range_max = AXP20X_IRQ5_STATE,
+ },
+};
+
+static const struct regmap_access_table axp20x_writeable_table = {
+ .yes_ranges = axp20x_writeable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(axp20x_writeable_ranges),
+};
+
+static const struct regmap_access_table axp20x_volatile_table = {
+ .yes_ranges = axp20x_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
+};
+
+static struct resource axp20x_pek_resources[] = {
+ {
+ .name = "PEK_DBR",
+ .start = AXP20X_IRQ_PEK_RIS_EDGE,
+ .end = AXP20X_IRQ_PEK_RIS_EDGE,
+ .flags = IORESOURCE_IRQ,
+ },
+ {
+ .name = "PEK_DBF",
+ .start = AXP20X_IRQ_PEK_FAL_EDGE,
+ .end = AXP20X_IRQ_PEK_FAL_EDGE,
+ .flags = IORESOURCE_IRQ,
+ },
+
+};
+
+static const struct regmap_config axp20x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .wr_table = &axp20x_writeable_table,
+ .volatile_table = &axp20x_volatile_table,
+ .max_register = AXP20X_FG_RES,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+#define AXP20X_IRQ(_irq, _off, _mask) \
+ [AXP20X_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
+
+static const struct regmap_irq axp20x_regmap_irqs[] = {
+ AXP20X_IRQ(ACIN_OVER_V, 0, 7),
+ AXP20X_IRQ(ACIN_PLUGIN, 0, 6),
+ AXP20X_IRQ(ACIN_REMOVAL, 0, 5),
+ AXP20X_IRQ(VBUS_OVER_V, 0, 4),
+ AXP20X_IRQ(VBUS_PLUGIN, 0, 3),
+ AXP20X_IRQ(VBUS_REMOVAL, 0, 2),
+ AXP20X_IRQ(VBUS_V_LOW, 0, 1),
+ AXP20X_IRQ(BATT_PLUGIN, 1, 7),
+ AXP20X_IRQ(BATT_REMOVAL, 1, 6),
+ AXP20X_IRQ(BATT_ENT_ACT_MODE, 1, 5),
+ AXP20X_IRQ(BATT_EXIT_ACT_MODE, 1, 4),
+ AXP20X_IRQ(CHARG, 1, 3),
+ AXP20X_IRQ(CHARG_DONE, 1, 2),
+ AXP20X_IRQ(BATT_TEMP_HIGH, 1, 1),
+ AXP20X_IRQ(BATT_TEMP_LOW, 1, 0),
+ AXP20X_IRQ(DIE_TEMP_HIGH, 2, 7),
+ AXP20X_IRQ(CHARG_I_LOW, 2, 6),
+ AXP20X_IRQ(DCDC1_V_LONG, 2, 5),
+ AXP20X_IRQ(DCDC2_V_LONG, 2, 4),
+ AXP20X_IRQ(DCDC3_V_LONG, 2, 3),
+ AXP20X_IRQ(PEK_SHORT, 2, 1),
+ AXP20X_IRQ(PEK_LONG, 2, 0),
+ AXP20X_IRQ(N_OE_PWR_ON, 3, 7),
+ AXP20X_IRQ(N_OE_PWR_OFF, 3, 6),
+ AXP20X_IRQ(VBUS_VALID, 3, 5),
+ AXP20X_IRQ(VBUS_NOT_VALID, 3, 4),
+ AXP20X_IRQ(VBUS_SESS_VALID, 3, 3),
+ AXP20X_IRQ(VBUS_SESS_END, 3, 2),
+ AXP20X_IRQ(LOW_PWR_LVL1, 3, 1),
+ AXP20X_IRQ(LOW_PWR_LVL2, 3, 0),
+ AXP20X_IRQ(TIMER, 4, 7),
+ AXP20X_IRQ(PEK_RIS_EDGE, 4, 6),
+ AXP20X_IRQ(PEK_FAL_EDGE, 4, 5),
+ AXP20X_IRQ(GPIO3_INPUT, 4, 3),
+ AXP20X_IRQ(GPIO2_INPUT, 4, 2),
+ AXP20X_IRQ(GPIO1_INPUT, 4, 1),
+ AXP20X_IRQ(GPIO0_INPUT, 4, 0),
+};
+
+static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
+ .name = "axp20x_irq_chip",
+ .status_base = AXP20X_IRQ1_STATE,
+ .ack_base = AXP20X_IRQ1_STATE,
+ .mask_base = AXP20X_IRQ1_EN,
+ .num_regs = 5,
+ .irqs = axp20x_regmap_irqs,
+ .num_irqs = ARRAY_SIZE(axp20x_regmap_irqs),
+ .mask_invert = 1,
+ .init_ack_masked = 1,
+};
+
+static struct mfd_cell axp20x_cells[] = {
+ {
+ .name = "axp20x-pek",
+ .of_compatible = "x-powers,axp20x-pek",
+ .num_resources = ARRAY_SIZE(axp20x_pek_resources),
+ .resources = axp20x_pek_resources,
+ }, {
+ .name = "axp20x-regulator",
+ },
+};
+
+const struct of_device_id axp20x_of_match[] = {
+ { .compatible = "x-powers,axp20x", .data = (void *)AXP20X },
+ { },
+};
+
+static struct axp20x_dev *axp20x_pm_power_off;
+static void axp20x_power_off(void)
+{
+ regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL, 0x80);
+}
+
+static int axp20x_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct axp20x_dev *axp20x;
+ const struct of_device_id *of_id;
+ int ret;
+
+ axp20x = devm_kzalloc(&i2c->dev, sizeof(*axp20x), GFP_KERNEL);
+ if (!axp20x)
+ return -ENOMEM;
+
+ of_id = of_match_device(axp20x_of_match, &i2c->dev);
+ if (!of_id) {
+ dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
+ return -ENODEV;
+ }
+ axp20x->variant = (int) of_id->data;
+
+ axp20x->i2c_client = i2c;
+ i2c_set_clientdata(i2c, axp20x);
+
+ axp20x->dev = &i2c->dev;
+ dev_set_drvdata(axp20x->dev, axp20x);
+
+ axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
+ if (IS_ERR(axp20x->regmap)) {
+ ret = PTR_ERR(axp20x->regmap);
+ dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
+ return ret;
+ }
+
+ axp20x->irq = i2c->irq;
+ ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
+ IRQF_ONESHOT | IRQF_SHARED, -1,
+ &axp20x_regmap_irq_chip,
+ &axp20x->regmap_irqc);
+ if (ret != 0) {
+ dev_err(&i2c->dev, "failed to add irq chip: %d\n", ret);
+ return ret;
+ }
+
+ ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
+ ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
+ if (ret != 0) {
+ dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
+ goto mfd_err;
+ }
+
+ if (!pm_power_off) {
+ axp20x_pm_power_off = axp20x;
+ pm_power_off = axp20x_power_off;
+ }
+
+ dev_info(&i2c->dev, "AXP20X driver loaded\n");
+
+ return 0;
+
+mfd_err:
+ regmap_del_irq_chip(axp20x->irq, axp20x->regmap_irqc);
+
+ return ret;
+}
+
+static int axp20x_i2c_remove(struct i2c_client *i2c)
+{
+ struct axp20x_dev *axp20x = i2c_get_clientdata(i2c);
+
+ if (axp20x == axp20x_pm_power_off) {
+ axp20x_pm_power_off = NULL;
+ pm_power_off = NULL;
+ }
+
+ mfd_remove_devices(axp20x->dev);
+ regmap_del_irq_chip(axp20x->i2c_client->irq, axp20x->regmap_irqc);
+
+ return 0;
+}
+
+static const struct i2c_device_id axp20x_i2c_id[] = {
+ { "axp20x", AXP20X },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
+
+static struct i2c_driver axp20x_i2c_driver = {
+ .driver = {
+ .name = "axp20x",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(axp20x_of_match),
+ },
+ .probe = axp20x_i2c_probe,
+ .remove = axp20x_i2c_remove,
+ .id_table = axp20x_i2c_id,
+};
+
+module_i2c_driver(axp20x_i2c_driver);
+
+MODULE_DESCRIPTION("PMIC MFD core driver for AXP20X");
+MODULE_AUTHOR("Carlo Caione <carlo@caione.org>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
new file mode 100644
index 0000000..94d99fd
--- /dev/null
+++ b/include/linux/mfd/axp20x.h
@@ -0,0 +1,178 @@
+/*
+ * Functions to access AXP20X power management chip.
+ *
+ * Copyright (C) 2013, Carlo Caione <carlo@caione.org>
+ *
+ * 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 __LINUX_MFD_AXP20X_H
+#define __LINUX_MFD_AXP20X_H
+
+#define AXP20X 0
+
+#define AXP20X_DATA(m) (0x04 + (m))
+
+/* Power supply */
+#define AXP20X_PWR_INPUT_STATUS 0x00
+#define AXP20X_PWR_OP_MODE 0x01
+#define AXP20X_USB_OTG_STATUS 0x02
+#define AXP20X_PWR_OUT_CTRL 0x12
+#define AXP20X_DCDC2_V_OUT 0x23
+#define AXP20X_DCDC2_LDO3_V_SCAL 0x25
+#define AXP20X_DCDC3_V_OUT 0x27
+#define AXP20X_LDO24_V_OUT 0x28
+#define AXP20X_LDO3_V_OUT 0x29
+#define AXP20X_VBUS_IPSOUT_MGMT 0x30
+#define AXP20X_V_OFF 0x31
+#define AXP20X_OFF_CTRL 0x32
+#define AXP20X_CHRG_CTRL1 0x33
+#define AXP20X_CHRG_CTRL2 0x34
+#define AXP20X_CHRG_BAK_CTRL 0x35
+#define AXP20X_PEK_KEY 0x36
+#define AXP20X_DCDC_FREQ 0x37
+#define AXP20X_V_LTF_CHRG 0x38
+#define AXP20X_V_HTF_CHRG 0x39
+#define AXP20X_APS_WARN_L1 0x3a
+#define AXP20X_APS_WARN_L2 0x3b
+#define AXP20X_V_LTF_DISCHRG 0x3c
+#define AXP20X_V_HTF_DISCHRG 0x3d
+
+/* Interrupt */
+#define AXP20X_IRQ1_EN 0x40
+#define AXP20X_IRQ2_EN 0x41
+#define AXP20X_IRQ3_EN 0x42
+#define AXP20X_IRQ4_EN 0x43
+#define AXP20X_IRQ5_EN 0x44
+#define AXP20X_IRQ1_STATE 0x48
+#define AXP20X_IRQ2_STATE 0x49
+#define AXP20X_IRQ3_STATE 0x4a
+#define AXP20X_IRQ4_STATE 0x4b
+#define AXP20X_IRQ5_STATE 0x4c
+
+/* ADC */
+#define AXP20X_ACIN_V_ADC_H 0x56
+#define AXP20X_ACIN_V_ADC_L 0x57
+#define AXP20X_ACIN_I_ADC_H 0x58
+#define AXP20X_ACIN_I_ADC_L 0x59
+#define AXP20X_VBUS_V_ADC_H 0x5a
+#define AXP20X_VBUS_V_ADC_L 0x5b
+#define AXP20X_VBUS_I_ADC_H 0x5c
+#define AXP20X_VBUS_I_ADC_L 0x5d
+#define AXP20X_TEMP_ADC_H 0x5e
+#define AXP20X_TEMP_ADC_L 0x5f
+#define AXP20X_TS_IN_H 0x62
+#define AXP20X_TS_IN_L 0x63
+#define AXP20X_GPIO0_V_ADC_H 0x64
+#define AXP20X_GPIO0_V_ADC_L 0x65
+#define AXP20X_GPIO1_V_ADC_H 0x66
+#define AXP20X_GPIO1_V_ADC_L 0x67
+#define AXP20X_PWR_BATT_H 0x70
+#define AXP20X_PWR_BATT_M 0x71
+#define AXP20X_PWR_BATT_L 0x72
+#define AXP20X_BATT_V_H 0x78
+#define AXP20X_BATT_V_L 0x79
+#define AXP20X_BATT_CHRG_I_H 0x7a
+#define AXP20X_BATT_CHRG_I_L 0x7b
+#define AXP20X_BATT_DISCHRG_I_H 0x7c
+#define AXP20X_BATT_DISCHRG_I_L 0x7d
+#define AXP20X_IPSOUT_V_HIGH_H 0x7e
+#define AXP20X_IPSOUT_V_HIGH_L 0x7f
+
+/* Power supply */
+#define AXP20X_DCDC_MODE 0x80
+#define AXP20X_ADC_EN1 0x82
+#define AXP20X_ADC_EN2 0x83
+#define AXP20X_ADC_RATE 0x84
+#define AXP20X_GPIO10_IN_RANGE 0x85
+#define AXP20X_GPIO1_ADC_IRQ_RIS 0x86
+#define AXP20X_GPIO1_ADC_IRQ_FAL 0x87
+#define AXP20X_TIMER_CTRL 0x8a
+#define AXP20X_VBUS_MON 0x8b
+#define AXP20X_OVER_TMP 0x8f
+
+/* GPIO */
+#define AXP20X_GPIO0_CTRL 0x90
+#define AXP20X_LDO5_V_OUT 0x91
+#define AXP20X_GPIO1_CTRL 0x92
+#define AXP20X_GPIO2_CTRL 0x93
+#define AXP20X_GPIO20_SS 0x94
+#define AXP20X_GPIO3_CTRL 0x95
+
+/* Battery */
+#define AXP20X_CHRG_CC_31_24 0xb0
+#define AXP20X_CHRG_CC_23_16 0xb1
+#define AXP20X_CHRG_CC_15_8 0xb2
+#define AXP20X_CHRG_CC_7_0 0xb3
+#define AXP20X_DISCHRG_CC_31_24 0xb4
+#define AXP20X_DISCHRG_CC_23_16 0xb5
+#define AXP20X_DISCHRG_CC_15_8 0xb6
+#define AXP20X_DISCHRG_CC_7_0 0xb7
+#define AXP20X_CC_CTRL 0xb8
+#define AXP20X_FG_RES 0xb9
+
+/* Regulators IDs */
+enum {
+ AXP20X_LDO1 = 0,
+ AXP20X_LDO2,
+ AXP20X_LDO3,
+ AXP20X_LDO4,
+ AXP20X_LDO5,
+ AXP20X_DCDC2,
+ AXP20X_DCDC3,
+ AXP20X_REG_ID_MAX,
+};
+
+/* IRQs */
+enum {
+ AXP20X_IRQ_ACIN_OVER_V = 1,
+ AXP20X_IRQ_ACIN_PLUGIN,
+ AXP20X_IRQ_ACIN_REMOVAL,
+ AXP20X_IRQ_VBUS_OVER_V,
+ AXP20X_IRQ_VBUS_PLUGIN,
+ AXP20X_IRQ_VBUS_REMOVAL,
+ AXP20X_IRQ_VBUS_V_LOW,
+ AXP20X_IRQ_BATT_PLUGIN,
+ AXP20X_IRQ_BATT_REMOVAL,
+ AXP20X_IRQ_BATT_ENT_ACT_MODE,
+ AXP20X_IRQ_BATT_EXIT_ACT_MODE,
+ AXP20X_IRQ_CHARG,
+ AXP20X_IRQ_CHARG_DONE,
+ AXP20X_IRQ_BATT_TEMP_HIGH,
+ AXP20X_IRQ_BATT_TEMP_LOW,
+ AXP20X_IRQ_DIE_TEMP_HIGH,
+ AXP20X_IRQ_CHARG_I_LOW,
+ AXP20X_IRQ_DCDC1_V_LONG,
+ AXP20X_IRQ_DCDC2_V_LONG,
+ AXP20X_IRQ_DCDC3_V_LONG,
+ AXP20X_IRQ_PEK_SHORT = 22,
+ AXP20X_IRQ_PEK_LONG,
+ AXP20X_IRQ_N_OE_PWR_ON,
+ AXP20X_IRQ_N_OE_PWR_OFF,
+ AXP20X_IRQ_VBUS_VALID,
+ AXP20X_IRQ_VBUS_NOT_VALID,
+ AXP20X_IRQ_VBUS_SESS_VALID,
+ AXP20X_IRQ_VBUS_SESS_END,
+ AXP20X_IRQ_LOW_PWR_LVL1,
+ AXP20X_IRQ_LOW_PWR_LVL2,
+ AXP20X_IRQ_TIMER,
+ AXP20X_IRQ_PEK_RIS_EDGE,
+ AXP20X_IRQ_PEK_FAL_EDGE,
+ AXP20X_IRQ_GPIO3_INPUT,
+ AXP20X_IRQ_GPIO2_INPUT,
+ AXP20X_IRQ_GPIO1_INPUT,
+ AXP20X_IRQ_GPIO0_INPUT,
+};
+
+struct axp20x_dev {
+ struct device *dev;
+ struct i2c_client *i2c_client;
+ struct regmap *regmap;
+ struct regmap_irq_chip_data *regmap_irqc;
+ int variant;
+ int irq;
+};
+
+#endif /* __LINUX_MFD_AXP20X_H */
--
1.8.5.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x
2014-02-08 16:03 [PATCH 0/3] mfd: axp20x: Add support for PMIC axp202 and axp209 Carlo Caione
2014-02-08 16:03 ` [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC Carlo Caione
@ 2014-02-08 16:03 ` Carlo Caione
2014-02-10 19:59 ` Maxime Ripard
2014-02-10 20:08 ` Lee Jones
2014-02-08 16:03 ` [PATCH 3/3] mfd: axp20x: Add bindings documentation Carlo Caione
2 siblings, 2 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-08 16:03 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds a new dtsi for supporting axp20x.
Signed-off-by: Carlo Caione <carlo@caione.org>
---
arch/arm/boot/dts/axp20x.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
create mode 100644 arch/arm/boot/dts/axp20x.dtsi
diff --git a/arch/arm/boot/dts/axp20x.dtsi b/arch/arm/boot/dts/axp20x.dtsi
new file mode 100644
index 0000000..98d4c7c
--- /dev/null
+++ b/arch/arm/boot/dts/axp20x.dtsi
@@ -0,0 +1,9 @@
+/*
+* Integrated Power Management Chip AXP209
+*/
+
+&axp {
+ compatible = "x-powers,axp20x";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+};
--
1.8.5.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/3] mfd: axp20x: Add bindings documentation
2014-02-08 16:03 [PATCH 0/3] mfd: axp20x: Add support for PMIC axp202 and axp209 Carlo Caione
2014-02-08 16:03 ` [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC Carlo Caione
2014-02-08 16:03 ` [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x Carlo Caione
@ 2014-02-08 16:03 ` Carlo Caione
2014-02-10 20:05 ` Lee Jones
` (2 more replies)
2 siblings, 3 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-08 16:03 UTC (permalink / raw)
To: linux-arm-kernel
Bindings documentation for the axp20x driver. In this file also two
sub-nodes (PEK and regulators) are documented.
Signed-off-by: Carlo Caione <carlo@caione.org>
---
Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++++++++++++++++++
1 file changed, 87 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
new file mode 100644
index 0000000..ccea6b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
@@ -0,0 +1,87 @@
+* axp20x device tree bindings
+
+The axp20x family current members :-
+axp202 (x-powers)
+axp209 (x-powers)
+
+Required properties:
+- compatible : Should be "x-powers,axp20x" (for axp202 and axp209)
+- interrupt-controller : axp20x has its own internal IRQs
+- #interrupt-cells : should be set to 1
+- interrupt-parent : The parent interrupt controller
+- interrupts : Specifies the list of interrupt lines which are handled by
+ the device in the parent controller's notation
+- reg : Specifies base physical address and size of the registers
+
+Sub-nodes
+* regulators : Contain the regulator nodes. The regulators are bound using
+ their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3,
+ ldo4, ldo5.
+ The bindings details of individual regulator device can be found in:
+ Documentation/devicetree/bindings/regulator/regulator.txt with the
+ exception of:
+
+ - dcdc-freq : defines the work frequency of DC-DC where
+ F=(1+dcdc-freq*5%)*1.5MHz
+
+* axp20x-pek : Power Enable Key
+ - compatible : should be "x-powers,axp20x-pek"
+ - interrupts : two interrupt numbers with order defined by interrupt-names
+ (one irq number for rising transition of the power key, the
+ other one for falling transition)
+ - interrupt-names : should be "PEK_DBR" and "PEK_DBF"
+
+Example:
+
+axp {
+ compatible = "x-powers,axp20x";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ axp20x-pek {
+ compatible = "x-powers,axp20x-pek";
+ interrupts = <33>, <34>;
+ interrupt-names = "PEK_DBR", "PEK_DBF";
+ };
+
+ regulators {
+ dcdc-freq = "8";
+
+ axp_dcdc2: dcdc2 {
+ regulator-min-microvolt = <700000>;
+ regulator-max-microvolt = <2275000>;
+ dcdc-workmode = <0>;
+ };
+
+ axp_dcdc3: dcdc3 {
+ regulator-min-microvolt = <700000>;
+ regulator-max-microvolt = <3500000>;
+ dcdc-workmode = <0>;
+ };
+
+ axp_ldo1: ldo1 {
+ regulator-min-microvolt = <1300000>;
+ regulator-max-microvolt = <1300000>;
+ };
+
+ axp_ldo2: ldo2 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ axp_ldo3: ldo3 {
+ regulator-min-microvolt = <700000>;
+ regulator-max-microvolt = <3500000>;
+ };
+
+ axp_ldo4: ldo4 {
+ regulator-min-microvolt = <1250000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ axp_ldo5: ldo5 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ };
+ };
+};
--
1.8.5.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x
2014-02-08 16:03 ` [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x Carlo Caione
@ 2014-02-10 19:59 ` Maxime Ripard
2014-02-10 20:09 ` [linux-sunxi] " Carlo Caione
2014-02-10 20:08 ` Lee Jones
1 sibling, 1 reply; 23+ messages in thread
From: Maxime Ripard @ 2014-02-10 19:59 UTC (permalink / raw)
To: linux-arm-kernel
Hi Carlo,
On Sat, Feb 08, 2014 at 05:03:47PM +0100, Carlo Caione wrote:
> This patch adds a new dtsi for supporting axp20x.
>
> Signed-off-by: Carlo Caione <carlo@caione.org>
> ---
> arch/arm/boot/dts/axp20x.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
> create mode 100644 arch/arm/boot/dts/axp20x.dtsi
>
> diff --git a/arch/arm/boot/dts/axp20x.dtsi b/arch/arm/boot/dts/axp20x.dtsi
> new file mode 100644
> index 0000000..98d4c7c
> --- /dev/null
> +++ b/arch/arm/boot/dts/axp20x.dtsi
> @@ -0,0 +1,9 @@
> +/*
> +* Integrated Power Management Chip AXP209
> +*/
> +
> +&axp {
> + compatible = "x-powers,axp20x";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +};
Hmmm, this refers to an undefined node, I can't see this DTSI used
anywhere, and why do you need a DTSI of its own in the first place?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140210/b62a7ee4/attachment.sig>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-08 16:03 ` [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC Carlo Caione
@ 2014-02-10 20:02 ` Maxime Ripard
2014-02-10 20:25 ` [linux-sunxi] " Carlo Caione
2014-02-10 21:19 ` Lee Jones
1 sibling, 1 reply; 23+ messages in thread
From: Maxime Ripard @ 2014-02-10 20:02 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Feb 08, 2014 at 05:03:46PM +0100, Carlo Caione wrote:
> This patch introduces the preliminary support for PMICs X-Powers AXP202
> and AXP209. The core contains support only for two sub-modules (PEK
> and regulators) that will be added with two different patch-sets.
>
> Signed-off-by: Carlo Caione <carlo@caione.org>
> ---
> drivers/mfd/Kconfig | 12 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/axp20x.c | 251 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/axp20x.h | 178 ++++++++++++++++++++++++++++++++
> 4 files changed, 442 insertions(+)
> create mode 100644 drivers/mfd/axp20x.c
> create mode 100644 include/linux/mfd/axp20x.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index dd67158..33d38c4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -59,6 +59,18 @@ config MFD_AAT2870_CORE
> additional drivers must be enabled in order to use the
> functionality of the device.
>
> +config MFD_AXP20X
> + bool "X-Powers AXP20X"
> + select MFD_CORE
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + depends on I2C=y
> + help
> + If you say Y here you get support for the AXP20X.
> + 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_CROS_EC
> tristate "ChromeOS Embedded Controller"
> select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 8a28dc9..371020e 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_PMIC_DA9052) += da9052-irq.o
> obj-$(CONFIG_PMIC_DA9052) += da9052-core.o
> obj-$(CONFIG_MFD_DA9052_SPI) += da9052-spi.o
> obj-$(CONFIG_MFD_DA9052_I2C) += da9052-i2c.o
> +obj-$(CONFIG_MFD_AXP20X) += axp20x.o
>
> obj-$(CONFIG_MFD_LP8788) += lp8788.o lp8788-irq.o
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> new file mode 100644
> index 0000000..efd0cb3
> --- /dev/null
> +++ b/drivers/mfd/axp20x.c
> @@ -0,0 +1,251 @@
> +/*
> + * Author: Carlo Caione <carlo@caione.org>
> + *
> + * 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/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/mfd/axp20x.h>
> +#include <linux/mfd/core.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +
> +static const struct regmap_range axp20x_writeable_ranges[] = {
> + {
> + .range_min = AXP20X_DATA(0),
> + .range_max = AXP20X_IRQ5_STATE,
> + }, {
> + .range_min = AXP20X_DCDC_MODE,
> + .range_max = AXP20X_FG_RES,
> + },
> +};
> +
> +static const struct regmap_range axp20x_volatile_ranges[] = {
> + {
> + .range_min = AXP20X_IRQ1_EN,
> + .range_max = AXP20X_IRQ5_STATE,
> + },
> +};
> +
> +static const struct regmap_access_table axp20x_writeable_table = {
> + .yes_ranges = axp20x_writeable_ranges,
> + .n_yes_ranges = ARRAY_SIZE(axp20x_writeable_ranges),
> +};
> +
> +static const struct regmap_access_table axp20x_volatile_table = {
> + .yes_ranges = axp20x_volatile_ranges,
> + .n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
> +};
> +
> +static struct resource axp20x_pek_resources[] = {
> + {
> + .name = "PEK_DBR",
> + .start = AXP20X_IRQ_PEK_RIS_EDGE,
> + .end = AXP20X_IRQ_PEK_RIS_EDGE,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .name = "PEK_DBF",
> + .start = AXP20X_IRQ_PEK_FAL_EDGE,
> + .end = AXP20X_IRQ_PEK_FAL_EDGE,
> + .flags = IORESOURCE_IRQ,
> + },
> +
> +};
>From your documentation, it seems like you want to declare them in the
DT too. Why do you need to declare the resources in both locations?
> +
> +static const struct regmap_config axp20x_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .wr_table = &axp20x_writeable_table,
> + .volatile_table = &axp20x_volatile_table,
> + .max_register = AXP20X_FG_RES,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +#define AXP20X_IRQ(_irq, _off, _mask) \
> + [AXP20X_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
> +
> +static const struct regmap_irq axp20x_regmap_irqs[] = {
> + AXP20X_IRQ(ACIN_OVER_V, 0, 7),
> + AXP20X_IRQ(ACIN_PLUGIN, 0, 6),
> + AXP20X_IRQ(ACIN_REMOVAL, 0, 5),
> + AXP20X_IRQ(VBUS_OVER_V, 0, 4),
> + AXP20X_IRQ(VBUS_PLUGIN, 0, 3),
> + AXP20X_IRQ(VBUS_REMOVAL, 0, 2),
> + AXP20X_IRQ(VBUS_V_LOW, 0, 1),
> + AXP20X_IRQ(BATT_PLUGIN, 1, 7),
> + AXP20X_IRQ(BATT_REMOVAL, 1, 6),
> + AXP20X_IRQ(BATT_ENT_ACT_MODE, 1, 5),
> + AXP20X_IRQ(BATT_EXIT_ACT_MODE, 1, 4),
> + AXP20X_IRQ(CHARG, 1, 3),
> + AXP20X_IRQ(CHARG_DONE, 1, 2),
> + AXP20X_IRQ(BATT_TEMP_HIGH, 1, 1),
> + AXP20X_IRQ(BATT_TEMP_LOW, 1, 0),
> + AXP20X_IRQ(DIE_TEMP_HIGH, 2, 7),
> + AXP20X_IRQ(CHARG_I_LOW, 2, 6),
> + AXP20X_IRQ(DCDC1_V_LONG, 2, 5),
> + AXP20X_IRQ(DCDC2_V_LONG, 2, 4),
> + AXP20X_IRQ(DCDC3_V_LONG, 2, 3),
> + AXP20X_IRQ(PEK_SHORT, 2, 1),
> + AXP20X_IRQ(PEK_LONG, 2, 0),
> + AXP20X_IRQ(N_OE_PWR_ON, 3, 7),
> + AXP20X_IRQ(N_OE_PWR_OFF, 3, 6),
> + AXP20X_IRQ(VBUS_VALID, 3, 5),
> + AXP20X_IRQ(VBUS_NOT_VALID, 3, 4),
> + AXP20X_IRQ(VBUS_SESS_VALID, 3, 3),
> + AXP20X_IRQ(VBUS_SESS_END, 3, 2),
> + AXP20X_IRQ(LOW_PWR_LVL1, 3, 1),
> + AXP20X_IRQ(LOW_PWR_LVL2, 3, 0),
> + AXP20X_IRQ(TIMER, 4, 7),
> + AXP20X_IRQ(PEK_RIS_EDGE, 4, 6),
> + AXP20X_IRQ(PEK_FAL_EDGE, 4, 5),
> + AXP20X_IRQ(GPIO3_INPUT, 4, 3),
> + AXP20X_IRQ(GPIO2_INPUT, 4, 2),
> + AXP20X_IRQ(GPIO1_INPUT, 4, 1),
> + AXP20X_IRQ(GPIO0_INPUT, 4, 0),
> +};
> +
> +static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
> + .name = "axp20x_irq_chip",
> + .status_base = AXP20X_IRQ1_STATE,
> + .ack_base = AXP20X_IRQ1_STATE,
> + .mask_base = AXP20X_IRQ1_EN,
> + .num_regs = 5,
> + .irqs = axp20x_regmap_irqs,
> + .num_irqs = ARRAY_SIZE(axp20x_regmap_irqs),
> + .mask_invert = 1,
> + .init_ack_masked = 1,
> +};
> +
> +static struct mfd_cell axp20x_cells[] = {
> + {
> + .name = "axp20x-pek",
> + .of_compatible = "x-powers,axp20x-pek",
> + .num_resources = ARRAY_SIZE(axp20x_pek_resources),
> + .resources = axp20x_pek_resources,
> + }, {
> + .name = "axp20x-regulator",
> + },
> +};
> +
> +const struct of_device_id axp20x_of_match[] = {
> + { .compatible = "x-powers,axp20x", .data = (void *)AXP20X },
> + { },
> +};
> +
> +static struct axp20x_dev *axp20x_pm_power_off;
> +static void axp20x_power_off(void)
> +{
> + regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL, 0x80);
> +}
> +
> +static int axp20x_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + struct axp20x_dev *axp20x;
> + const struct of_device_id *of_id;
> + int ret;
> +
> + axp20x = devm_kzalloc(&i2c->dev, sizeof(*axp20x), GFP_KERNEL);
> + if (!axp20x)
> + return -ENOMEM;
> +
> + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> + if (!of_id) {
> + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> + return -ENODEV;
> + }
> + axp20x->variant = (int) of_id->data;
> +
> + axp20x->i2c_client = i2c;
> + i2c_set_clientdata(i2c, axp20x);
> +
> + axp20x->dev = &i2c->dev;
> + dev_set_drvdata(axp20x->dev, axp20x);
> +
> + axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
> + if (IS_ERR(axp20x->regmap)) {
> + ret = PTR_ERR(axp20x->regmap);
> + dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
> + return ret;
> + }
> +
> + axp20x->irq = i2c->irq;
> + ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
> + IRQF_ONESHOT | IRQF_SHARED, -1,
> + &axp20x_regmap_irq_chip,
> + &axp20x->regmap_irqc);
> + if (ret != 0) {
> + dev_err(&i2c->dev, "failed to add irq chip: %d\n", ret);
> + return ret;
> + }
> +
> + ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
> + ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
> + if (ret != 0) {
> + dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
> + goto mfd_err;
> + }
> +
> + if (!pm_power_off) {
> + axp20x_pm_power_off = axp20x;
> + pm_power_off = axp20x_power_off;
> + }
> +
> + dev_info(&i2c->dev, "AXP20X driver loaded\n");
> +
> + return 0;
> +
> +mfd_err:
> + regmap_del_irq_chip(axp20x->irq, axp20x->regmap_irqc);
> +
> + return ret;
> +}
> +
> +static int axp20x_i2c_remove(struct i2c_client *i2c)
> +{
> + struct axp20x_dev *axp20x = i2c_get_clientdata(i2c);
> +
> + if (axp20x == axp20x_pm_power_off) {
> + axp20x_pm_power_off = NULL;
> + pm_power_off = NULL;
> + }
> +
> + mfd_remove_devices(axp20x->dev);
> + regmap_del_irq_chip(axp20x->i2c_client->irq, axp20x->regmap_irqc);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id axp20x_i2c_id[] = {
> + { "axp20x", AXP20X },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
> +
> +static struct i2c_driver axp20x_i2c_driver = {
> + .driver = {
> + .name = "axp20x",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(axp20x_of_match),
> + },
> + .probe = axp20x_i2c_probe,
> + .remove = axp20x_i2c_remove,
> + .id_table = axp20x_i2c_id,
> +};
> +
> +module_i2c_driver(axp20x_i2c_driver);
> +
> +MODULE_DESCRIPTION("PMIC MFD core driver for AXP20X");
> +MODULE_AUTHOR("Carlo Caione <carlo@caione.org>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> new file mode 100644
> index 0000000..94d99fd
> --- /dev/null
> +++ b/include/linux/mfd/axp20x.h
> @@ -0,0 +1,178 @@
> +/*
> + * Functions to access AXP20X power management chip.
> + *
> + * Copyright (C) 2013, Carlo Caione <carlo@caione.org>
> + *
> + * 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 __LINUX_MFD_AXP20X_H
> +#define __LINUX_MFD_AXP20X_H
> +
> +#define AXP20X 0
> +
> +#define AXP20X_DATA(m) (0x04 + (m))
> +
> +/* Power supply */
> +#define AXP20X_PWR_INPUT_STATUS 0x00
> +#define AXP20X_PWR_OP_MODE 0x01
> +#define AXP20X_USB_OTG_STATUS 0x02
> +#define AXP20X_PWR_OUT_CTRL 0x12
> +#define AXP20X_DCDC2_V_OUT 0x23
> +#define AXP20X_DCDC2_LDO3_V_SCAL 0x25
> +#define AXP20X_DCDC3_V_OUT 0x27
> +#define AXP20X_LDO24_V_OUT 0x28
> +#define AXP20X_LDO3_V_OUT 0x29
> +#define AXP20X_VBUS_IPSOUT_MGMT 0x30
> +#define AXP20X_V_OFF 0x31
> +#define AXP20X_OFF_CTRL 0x32
> +#define AXP20X_CHRG_CTRL1 0x33
> +#define AXP20X_CHRG_CTRL2 0x34
> +#define AXP20X_CHRG_BAK_CTRL 0x35
> +#define AXP20X_PEK_KEY 0x36
> +#define AXP20X_DCDC_FREQ 0x37
> +#define AXP20X_V_LTF_CHRG 0x38
> +#define AXP20X_V_HTF_CHRG 0x39
> +#define AXP20X_APS_WARN_L1 0x3a
> +#define AXP20X_APS_WARN_L2 0x3b
> +#define AXP20X_V_LTF_DISCHRG 0x3c
> +#define AXP20X_V_HTF_DISCHRG 0x3d
> +
> +/* Interrupt */
> +#define AXP20X_IRQ1_EN 0x40
> +#define AXP20X_IRQ2_EN 0x41
> +#define AXP20X_IRQ3_EN 0x42
> +#define AXP20X_IRQ4_EN 0x43
> +#define AXP20X_IRQ5_EN 0x44
> +#define AXP20X_IRQ1_STATE 0x48
> +#define AXP20X_IRQ2_STATE 0x49
> +#define AXP20X_IRQ3_STATE 0x4a
> +#define AXP20X_IRQ4_STATE 0x4b
> +#define AXP20X_IRQ5_STATE 0x4c
> +
> +/* ADC */
> +#define AXP20X_ACIN_V_ADC_H 0x56
> +#define AXP20X_ACIN_V_ADC_L 0x57
> +#define AXP20X_ACIN_I_ADC_H 0x58
> +#define AXP20X_ACIN_I_ADC_L 0x59
> +#define AXP20X_VBUS_V_ADC_H 0x5a
> +#define AXP20X_VBUS_V_ADC_L 0x5b
> +#define AXP20X_VBUS_I_ADC_H 0x5c
> +#define AXP20X_VBUS_I_ADC_L 0x5d
> +#define AXP20X_TEMP_ADC_H 0x5e
> +#define AXP20X_TEMP_ADC_L 0x5f
> +#define AXP20X_TS_IN_H 0x62
> +#define AXP20X_TS_IN_L 0x63
> +#define AXP20X_GPIO0_V_ADC_H 0x64
> +#define AXP20X_GPIO0_V_ADC_L 0x65
> +#define AXP20X_GPIO1_V_ADC_H 0x66
> +#define AXP20X_GPIO1_V_ADC_L 0x67
> +#define AXP20X_PWR_BATT_H 0x70
> +#define AXP20X_PWR_BATT_M 0x71
> +#define AXP20X_PWR_BATT_L 0x72
> +#define AXP20X_BATT_V_H 0x78
> +#define AXP20X_BATT_V_L 0x79
> +#define AXP20X_BATT_CHRG_I_H 0x7a
> +#define AXP20X_BATT_CHRG_I_L 0x7b
> +#define AXP20X_BATT_DISCHRG_I_H 0x7c
> +#define AXP20X_BATT_DISCHRG_I_L 0x7d
> +#define AXP20X_IPSOUT_V_HIGH_H 0x7e
> +#define AXP20X_IPSOUT_V_HIGH_L 0x7f
> +
> +/* Power supply */
> +#define AXP20X_DCDC_MODE 0x80
> +#define AXP20X_ADC_EN1 0x82
> +#define AXP20X_ADC_EN2 0x83
> +#define AXP20X_ADC_RATE 0x84
> +#define AXP20X_GPIO10_IN_RANGE 0x85
> +#define AXP20X_GPIO1_ADC_IRQ_RIS 0x86
> +#define AXP20X_GPIO1_ADC_IRQ_FAL 0x87
> +#define AXP20X_TIMER_CTRL 0x8a
> +#define AXP20X_VBUS_MON 0x8b
> +#define AXP20X_OVER_TMP 0x8f
> +
> +/* GPIO */
> +#define AXP20X_GPIO0_CTRL 0x90
> +#define AXP20X_LDO5_V_OUT 0x91
> +#define AXP20X_GPIO1_CTRL 0x92
> +#define AXP20X_GPIO2_CTRL 0x93
> +#define AXP20X_GPIO20_SS 0x94
> +#define AXP20X_GPIO3_CTRL 0x95
> +
> +/* Battery */
> +#define AXP20X_CHRG_CC_31_24 0xb0
> +#define AXP20X_CHRG_CC_23_16 0xb1
> +#define AXP20X_CHRG_CC_15_8 0xb2
> +#define AXP20X_CHRG_CC_7_0 0xb3
> +#define AXP20X_DISCHRG_CC_31_24 0xb4
> +#define AXP20X_DISCHRG_CC_23_16 0xb5
> +#define AXP20X_DISCHRG_CC_15_8 0xb6
> +#define AXP20X_DISCHRG_CC_7_0 0xb7
> +#define AXP20X_CC_CTRL 0xb8
> +#define AXP20X_FG_RES 0xb9
> +
> +/* Regulators IDs */
> +enum {
> + AXP20X_LDO1 = 0,
> + AXP20X_LDO2,
> + AXP20X_LDO3,
> + AXP20X_LDO4,
> + AXP20X_LDO5,
> + AXP20X_DCDC2,
> + AXP20X_DCDC3,
> + AXP20X_REG_ID_MAX,
> +};
> +
> +/* IRQs */
> +enum {
> + AXP20X_IRQ_ACIN_OVER_V = 1,
> + AXP20X_IRQ_ACIN_PLUGIN,
> + AXP20X_IRQ_ACIN_REMOVAL,
> + AXP20X_IRQ_VBUS_OVER_V,
> + AXP20X_IRQ_VBUS_PLUGIN,
> + AXP20X_IRQ_VBUS_REMOVAL,
> + AXP20X_IRQ_VBUS_V_LOW,
> + AXP20X_IRQ_BATT_PLUGIN,
> + AXP20X_IRQ_BATT_REMOVAL,
> + AXP20X_IRQ_BATT_ENT_ACT_MODE,
> + AXP20X_IRQ_BATT_EXIT_ACT_MODE,
> + AXP20X_IRQ_CHARG,
> + AXP20X_IRQ_CHARG_DONE,
> + AXP20X_IRQ_BATT_TEMP_HIGH,
> + AXP20X_IRQ_BATT_TEMP_LOW,
> + AXP20X_IRQ_DIE_TEMP_HIGH,
> + AXP20X_IRQ_CHARG_I_LOW,
> + AXP20X_IRQ_DCDC1_V_LONG,
> + AXP20X_IRQ_DCDC2_V_LONG,
> + AXP20X_IRQ_DCDC3_V_LONG,
> + AXP20X_IRQ_PEK_SHORT = 22,
> + AXP20X_IRQ_PEK_LONG,
> + AXP20X_IRQ_N_OE_PWR_ON,
> + AXP20X_IRQ_N_OE_PWR_OFF,
> + AXP20X_IRQ_VBUS_VALID,
> + AXP20X_IRQ_VBUS_NOT_VALID,
> + AXP20X_IRQ_VBUS_SESS_VALID,
> + AXP20X_IRQ_VBUS_SESS_END,
> + AXP20X_IRQ_LOW_PWR_LVL1,
> + AXP20X_IRQ_LOW_PWR_LVL2,
> + AXP20X_IRQ_TIMER,
> + AXP20X_IRQ_PEK_RIS_EDGE,
> + AXP20X_IRQ_PEK_FAL_EDGE,
> + AXP20X_IRQ_GPIO3_INPUT,
> + AXP20X_IRQ_GPIO2_INPUT,
> + AXP20X_IRQ_GPIO1_INPUT,
> + AXP20X_IRQ_GPIO0_INPUT,
> +};
> +
> +struct axp20x_dev {
> + struct device *dev;
> + struct i2c_client *i2c_client;
> + struct regmap *regmap;
> + struct regmap_irq_chip_data *regmap_irqc;
> + int variant;
> + int irq;
> +};
> +
> +#endif /* __LINUX_MFD_AXP20X_H */
> --
> 1.8.5.3
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140210/29cdfb48/attachment-0001.sig>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/3] mfd: axp20x: Add bindings documentation
2014-02-08 16:03 ` [PATCH 3/3] mfd: axp20x: Add bindings documentation Carlo Caione
@ 2014-02-10 20:05 ` Lee Jones
2014-02-10 20:12 ` Maxime Ripard
2014-02-12 17:12 ` Mark Brown
2 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2014-02-10 20:05 UTC (permalink / raw)
To: linux-arm-kernel
> Bindings documentation for the axp20x driver. In this file also two
> sub-nodes (PEK and regulators) are documented.
>
> Signed-off-by: Carlo Caione <carlo@caione.org>
> ---
> Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++++++++++++++++++
You need to CC the Device Tree guys.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x
2014-02-08 16:03 ` [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x Carlo Caione
2014-02-10 19:59 ` Maxime Ripard
@ 2014-02-10 20:08 ` Lee Jones
2014-02-10 20:10 ` Carlo Caione
1 sibling, 1 reply; 23+ messages in thread
From: Lee Jones @ 2014-02-10 20:08 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, 08 Feb 2014, Carlo Caione wrote:
> This patch adds a new dtsi for supporting axp20x.
>
> Signed-off-by: Carlo Caione <carlo@caione.org>
> ---
> arch/arm/boot/dts/axp20x.dtsi | 9 +++++++++
> 1 file changed, 9 insertions(+)
> create mode 100644 arch/arm/boot/dts/axp20x.dtsi
>
> diff --git a/arch/arm/boot/dts/axp20x.dtsi b/arch/arm/boot/dts/axp20x.dtsi
> new file mode 100644
> index 0000000..98d4c7c
> --- /dev/null
> +++ b/arch/arm/boot/dts/axp20x.dtsi
> @@ -0,0 +1,9 @@
> +/*
> +* Integrated Power Management Chip AXP209
> +*/
> +
> +&axp {
> + compatible = "x-powers,axp20x";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +};
I's suggest not doing it this way. Having a .dtsi file for every
device is not the way Device Tree has been designed. This snippet
should be copied directly into device's .dts(i) files.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x
2014-02-10 19:59 ` Maxime Ripard
@ 2014-02-10 20:09 ` Carlo Caione
0 siblings, 0 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-10 20:09 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 8:59 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Carlo,
>
> On Sat, Feb 08, 2014 at 05:03:47PM +0100, Carlo Caione wrote:
>> This patch adds a new dtsi for supporting axp20x.
>>
>> Signed-off-by: Carlo Caione <carlo@caione.org>
>> ---
>> arch/arm/boot/dts/axp20x.dtsi | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>> create mode 100644 arch/arm/boot/dts/axp20x.dtsi
>>
>> diff --git a/arch/arm/boot/dts/axp20x.dtsi b/arch/arm/boot/dts/axp20x.dtsi
>> new file mode 100644
>> index 0000000..98d4c7c
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/axp20x.dtsi
>> @@ -0,0 +1,9 @@
>> +/*
>> +* Integrated Power Management Chip AXP209
>> +*/
>> +
>> +&axp {
>> + compatible = "x-powers,axp20x";
>> + interrupt-controller;
>> + #interrupt-cells = <1>;
>> +};
>
> Hmmm, this refers to an undefined node, I can't see this DTSI used
> anywhere, and why do you need a DTSI of its own in the first place?
Since axp is an independent MFD this could be integrated in the DTS of
the SoC using it (multiple SoC could be using the same axp), that's
why I left it in a DTSI.
i.e. it will be integrated in sun7i-a20-cubieboard2.dts once the
problem with the NMI controller has been solved (and we have a NMI
controller to link the axp to).
Thanks,
--
Carlo Caione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x
2014-02-10 20:08 ` Lee Jones
@ 2014-02-10 20:10 ` Carlo Caione
0 siblings, 0 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-10 20:10 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 9:08 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Sat, 08 Feb 2014, Carlo Caione wrote:
>
>> This patch adds a new dtsi for supporting axp20x.
>>
>> Signed-off-by: Carlo Caione <carlo@caione.org>
>> ---
>> arch/arm/boot/dts/axp20x.dtsi | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>> create mode 100644 arch/arm/boot/dts/axp20x.dtsi
>>
>> diff --git a/arch/arm/boot/dts/axp20x.dtsi b/arch/arm/boot/dts/axp20x.dtsi
>> new file mode 100644
>> index 0000000..98d4c7c
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/axp20x.dtsi
>> @@ -0,0 +1,9 @@
>> +/*
>> +* Integrated Power Management Chip AXP209
>> +*/
>> +
>> +&axp {
>> + compatible = "x-powers,axp20x";
>> + interrupt-controller;
>> + #interrupt-cells = <1>;
>> +};
>
> I's suggest not doing it this way. Having a .dtsi file for every
> device is not the way Device Tree has been designed. This snippet
> should be copied directly into device's .dts(i) files.
Well, ok then :) I'll fix it in v2.
Thanks,
--
Carlo Caione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/3] mfd: axp20x: Add bindings documentation
2014-02-08 16:03 ` [PATCH 3/3] mfd: axp20x: Add bindings documentation Carlo Caione
2014-02-10 20:05 ` Lee Jones
@ 2014-02-10 20:12 ` Maxime Ripard
2014-02-10 20:37 ` [linux-sunxi] " Carlo Caione
2014-02-12 17:12 ` Mark Brown
2 siblings, 1 reply; 23+ messages in thread
From: Maxime Ripard @ 2014-02-10 20:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi Carlo,
On Sat, Feb 08, 2014 at 05:03:48PM +0100, Carlo Caione wrote:
> Bindings documentation for the axp20x driver. In this file also two
> sub-nodes (PEK and regulators) are documented.
>
> Signed-off-by: Carlo Caione <carlo@caione.org>
> ---
> Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++++++++++++++++++
> 1 file changed, 87 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> new file mode 100644
> index 0000000..ccea6b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> @@ -0,0 +1,87 @@
> +* axp20x device tree bindings
> +
> +The axp20x family current members :-
> +axp202 (x-powers)
> +axp209 (x-powers)
> +
> +Required properties:
> +- compatible : Should be "x-powers,axp20x" (for axp202 and axp209)
"Generic" compatibles are usually a bad thing, for several reasons,
mostly because there's no way to actually differentiate the two
without keeping adding DT properties (and hence, being unable to
actually fix something or add a quirk for one single of these devices
without having to modify the DT too.)
Please use the "real" compatibles.
> +- interrupt-controller : axp20x has its own internal IRQs
> +- #interrupt-cells : should be set to 1
> +- interrupt-parent : The parent interrupt controller
Is this really required? It was not in your DTSI.
> +- interrupts : Specifies the list of interrupt lines which are handled by
> + the device in the parent controller's notation
Hmmm, I'm not sure what you mean here.
> +- reg : Specifies base physical address and size of the registers
Base physical address? Isn't it a I2C device?
> +
> +Sub-nodes
> +* regulators : Contain the regulator nodes. The regulators are bound using
> + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3,
> + ldo4, ldo5.
> + The bindings details of individual regulator device can be found in:
> + Documentation/devicetree/bindings/regulator/regulator.txt with the
> + exception of:
I'm guessing this is where you differentiate between AXP202 and
AXP209?
> +
> + - dcdc-freq : defines the work frequency of DC-DC where
> + F=(1+dcdc-freq*5%)*1.5MHz
I'd very much prefer this DCDC frequency to be in Hz, or a similar
unit, and let the driver do the conversion.
> +
> +* axp20x-pek : Power Enable Key
> + - compatible : should be "x-powers,axp20x-pek"
> + - interrupts : two interrupt numbers with order defined by interrupt-names
> + (one irq number for rising transition of the power key, the
> + other one for falling transition)
> + - interrupt-names : should be "PEK_DBR" and "PEK_DBF"
Is this actually needed since you declare the resources in your driver?
> +
> +Example:
> +
> +axp {
> + compatible = "x-powers,axp20x";
> + interrupt-controller;
> + #interrupt-cells = <1>;
No reg property ?
> +
> + axp20x-pek {
> + compatible = "x-powers,axp20x-pek";
> + interrupts = <33>, <34>;
> + interrupt-names = "PEK_DBR", "PEK_DBF";
> + };
> +
> + regulators {
> + dcdc-freq = "8";
> +
> + axp_dcdc2: dcdc2 {
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <2275000>;
> + dcdc-workmode = <0>;
And what is this dcdc-workmode property about?
> + };
> +
> + axp_dcdc3: dcdc3 {
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <3500000>;
> + dcdc-workmode = <0>;
> + };
> +
> + axp_ldo1: ldo1 {
> + regulator-min-microvolt = <1300000>;
> + regulator-max-microvolt = <1300000>;
> + };
> +
> + axp_ldo2: ldo2 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + axp_ldo3: ldo3 {
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <3500000>;
> + };
> +
> + axp_ldo4: ldo4 {
> + regulator-min-microvolt = <1250000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + axp_ldo5: ldo5 {
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + };
> + };
> +};
> --
> 1.8.5.3
>
Thanks for working on this!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140210/03fc1689/attachment.sig>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-10 20:02 ` Maxime Ripard
@ 2014-02-10 20:25 ` Carlo Caione
0 siblings, 0 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-10 20:25 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 9:02 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
>> +static struct resource axp20x_pek_resources[] = {
>> + {
>> + .name = "PEK_DBR",
>> + .start = AXP20X_IRQ_PEK_RIS_EDGE,
>> + .end = AXP20X_IRQ_PEK_RIS_EDGE,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .name = "PEK_DBF",
>> + .start = AXP20X_IRQ_PEK_FAL_EDGE,
>> + .end = AXP20X_IRQ_PEK_FAL_EDGE,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> +
>> +};
>
> From your documentation, it seems like you want to declare them in the
> DT too. Why do you need to declare the resources in both locations?
It's not really a need, I thought it was a bit "clearer" also to have
them in DT.
But I agree it is redundant. I'll fix in v2.
Thank you,
--
Carlo Caione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 3/3] mfd: axp20x: Add bindings documentation
2014-02-10 20:12 ` Maxime Ripard
@ 2014-02-10 20:37 ` Carlo Caione
2014-02-10 22:01 ` Maxime Ripard
0 siblings, 1 reply; 23+ messages in thread
From: Carlo Caione @ 2014-02-10 20:37 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 9:12 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Carlo,
>
> On Sat, Feb 08, 2014 at 05:03:48PM +0100, Carlo Caione wrote:
>> Bindings documentation for the axp20x driver. In this file also two
>> sub-nodes (PEK and regulators) are documented.
>>
>> Signed-off-by: Carlo Caione <carlo@caione.org>
>> ---
>> Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++++++++++++++++++
>> 1 file changed, 87 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> new file mode 100644
>> index 0000000..ccea6b8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
>> @@ -0,0 +1,87 @@
>> +* axp20x device tree bindings
>> +
>> +The axp20x family current members :-
>> +axp202 (x-powers)
>> +axp209 (x-powers)
>> +
>> +Required properties:
>> +- compatible : Should be "x-powers,axp20x" (for axp202 and axp209)
>
> "Generic" compatibles are usually a bad thing, for several reasons,
> mostly because there's no way to actually differentiate the two
> without keeping adding DT properties (and hence, being unable to
> actually fix something or add a quirk for one single of these devices
> without having to modify the DT too.)
>
> Please use the "real" compatibles.
Ok
>> +- interrupt-controller : axp20x has its own internal IRQs
>> +- #interrupt-cells : should be set to 1
>> +- interrupt-parent : The parent interrupt controller
>
> Is this really required? It was not in your DTSI.
It wasn't in my DTSI since it was supposed to be used by DTS importing
this DTSI.
I'll fix it in v2.
>> +- interrupts : Specifies the list of interrupt lines which are handled by
>> + the device in the parent controller's notation
>
> Hmmm, I'm not sure what you mean here.
It means that the notation used to indicate the interrupt for this
device has to follow the notation of the parent controller (i.e. in
terms of number of interrupt cells).
I'll rewrite the statement in v2.
>> +- reg : Specifies base physical address and size of the registers
>
> Base physical address? Isn't it a I2C device?
Right. cut&paste error.
>> +
>> +Sub-nodes
>> +* regulators : Contain the regulator nodes. The regulators are bound using
>> + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3,
>> + ldo4, ldo5.
>> + The bindings details of individual regulator device can be found in:
>> + Documentation/devicetree/bindings/regulator/regulator.txt with the
>> + exception of:
>
> I'm guessing this is where you differentiate between AXP202 and
> AXP209?
Not really. AXP202 and AXP209 have the same regulators (with the same
constrains)
>> +
>> + - dcdc-freq : defines the work frequency of DC-DC where
>> + F=(1+dcdc-freq*5%)*1.5MHz
>
> I'd very much prefer this DCDC frequency to be in Hz, or a similar
> unit, and let the driver do the conversion.
Ok. Fix in v2.
>> +
>> +* axp20x-pek : Power Enable Key
>> + - compatible : should be "x-powers,axp20x-pek"
>> + - interrupts : two interrupt numbers with order defined by interrupt-names
>> + (one irq number for rising transition of the power key, the
>> + other one for falling transition)
>> + - interrupt-names : should be "PEK_DBR" and "PEK_DBF"
>
> Is this actually needed since you declare the resources in your driver?
Not needed.
>> +
>> +Example:
>> +
>> +axp {
>> + compatible = "x-powers,axp20x";
>> + interrupt-controller;
>> + #interrupt-cells = <1>;
>
> No reg property ?
Also the reg property was supposed to be used in the DTS. I'll fix it.
>> +
>> + axp20x-pek {
>> + compatible = "x-powers,axp20x-pek";
>> + interrupts = <33>, <34>;
>> + interrupt-names = "PEK_DBR", "PEK_DBF";
>> + };
>> +
>> + regulators {
>> + dcdc-freq = "8";
>> +
>> + axp_dcdc2: dcdc2 {
>> + regulator-min-microvolt = <700000>;
>> + regulator-max-microvolt = <2275000>;
>> + dcdc-workmode = <0>;
>
> And what is this dcdc-workmode property about?
Oh. I missed that, sorry.
>> + };
>> +
>> + axp_dcdc3: dcdc3 {
>> + regulator-min-microvolt = <700000>;
>> + regulator-max-microvolt = <3500000>;
>> + dcdc-workmode = <0>;
>> + };
>> +
>> + axp_ldo1: ldo1 {
>> + regulator-min-microvolt = <1300000>;
>> + regulator-max-microvolt = <1300000>;
>> + };
>> +
>> + axp_ldo2: ldo2 {
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <3300000>;
>> + };
>> +
>> + axp_ldo3: ldo3 {
>> + regulator-min-microvolt = <700000>;
>> + regulator-max-microvolt = <3500000>;
>> + };
>> +
>> + axp_ldo4: ldo4 {
>> + regulator-min-microvolt = <1250000>;
>> + regulator-max-microvolt = <3300000>;
>> + };
>> +
>> + axp_ldo5: ldo5 {
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <3300000>;
>> + };
>> + };
>> +};
>> --
>> 1.8.5.3
>>
>
> Thanks for working on this!
Thank you,
--
Carlo Caione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-08 16:03 ` [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC Carlo Caione
2014-02-10 20:02 ` Maxime Ripard
@ 2014-02-10 21:19 ` Lee Jones
2014-02-10 22:34 ` [linux-sunxi] " Carlo Caione
1 sibling, 1 reply; 23+ messages in thread
From: Lee Jones @ 2014-02-10 21:19 UTC (permalink / raw)
To: linux-arm-kernel
> This patch introduces the preliminary support for PMICs X-Powers AXP202
> and AXP209. The core contains support only for two sub-modules (PEK
> and regulators) that will be added with two different patch-sets.
>
> Signed-off-by: Carlo Caione <carlo@caione.org>
> ---
> drivers/mfd/Kconfig | 12 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/axp20x.c | 251 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/axp20x.h | 178 ++++++++++++++++++++++++++++++++
> 4 files changed, 442 insertions(+)
> create mode 100644 drivers/mfd/axp20x.c
> create mode 100644 include/linux/mfd/axp20x.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index dd67158..33d38c4 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -59,6 +59,18 @@ config MFD_AAT2870_CORE
> additional drivers must be enabled in order to use the
> functionality of the device.
<snip>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> new file mode 100644
> index 0000000..efd0cb3
> --- /dev/null
> +++ b/drivers/mfd/axp20x.c
> @@ -0,0 +1,251 @@
> +/*
A nice descriptive title here would be good.
> + * Author: Carlo Caione <carlo@caione.org>
> + *
> + * 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/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/mfd/axp20x.h>
> +#include <linux/mfd/core.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +
> +static const struct regmap_range axp20x_writeable_ranges[] = {
> + {
> + .range_min = AXP20X_DATA(0),
> + .range_max = AXP20X_IRQ5_STATE,
> + }, {
> + .range_min = AXP20X_DCDC_MODE,
> + .range_max = AXP20X_FG_RES,
> + },
> +};
> +
> +static const struct regmap_range axp20x_volatile_ranges[] = {
> + {
> + .range_min = AXP20X_IRQ1_EN,
> + .range_max = AXP20X_IRQ5_STATE,
> + },
> +};
> +
> +static const struct regmap_access_table axp20x_writeable_table = {
> + .yes_ranges = axp20x_writeable_ranges,
> + .n_yes_ranges = ARRAY_SIZE(axp20x_writeable_ranges),
> +};
> +
> +static const struct regmap_access_table axp20x_volatile_table = {
> + .yes_ranges = axp20x_volatile_ranges,
> + .n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
> +};
> +
> +static struct resource axp20x_pek_resources[] = {
> + {
> + .name = "PEK_DBR",
> + .start = AXP20X_IRQ_PEK_RIS_EDGE,
> + .end = AXP20X_IRQ_PEK_RIS_EDGE,
> + .flags = IORESOURCE_IRQ,
> + },
> + {
> + .name = "PEK_DBF",
> + .start = AXP20X_IRQ_PEK_FAL_EDGE,
> + .end = AXP20X_IRQ_PEK_FAL_EDGE,
> + .flags = IORESOURCE_IRQ,
> + },
> +
Superfluous new line.
> +};
> +
> +static const struct regmap_config axp20x_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .wr_table = &axp20x_writeable_table,
> + .volatile_table = &axp20x_volatile_table,
> + .max_register = AXP20X_FG_RES,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +#define AXP20X_IRQ(_irq, _off, _mask) \
> + [AXP20X_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
> +
> +static const struct regmap_irq axp20x_regmap_irqs[] = {
> + AXP20X_IRQ(ACIN_OVER_V, 0, 7),
> + AXP20X_IRQ(ACIN_PLUGIN, 0, 6),
> + AXP20X_IRQ(ACIN_REMOVAL, 0, 5),
> + AXP20X_IRQ(VBUS_OVER_V, 0, 4),
> + AXP20X_IRQ(VBUS_PLUGIN, 0, 3),
> + AXP20X_IRQ(VBUS_REMOVAL, 0, 2),
> + AXP20X_IRQ(VBUS_V_LOW, 0, 1),
> + AXP20X_IRQ(BATT_PLUGIN, 1, 7),
> + AXP20X_IRQ(BATT_REMOVAL, 1, 6),
> + AXP20X_IRQ(BATT_ENT_ACT_MODE, 1, 5),
> + AXP20X_IRQ(BATT_EXIT_ACT_MODE, 1, 4),
> + AXP20X_IRQ(CHARG, 1, 3),
> + AXP20X_IRQ(CHARG_DONE, 1, 2),
> + AXP20X_IRQ(BATT_TEMP_HIGH, 1, 1),
> + AXP20X_IRQ(BATT_TEMP_LOW, 1, 0),
> + AXP20X_IRQ(DIE_TEMP_HIGH, 2, 7),
> + AXP20X_IRQ(CHARG_I_LOW, 2, 6),
> + AXP20X_IRQ(DCDC1_V_LONG, 2, 5),
> + AXP20X_IRQ(DCDC2_V_LONG, 2, 4),
> + AXP20X_IRQ(DCDC3_V_LONG, 2, 3),
> + AXP20X_IRQ(PEK_SHORT, 2, 1),
> + AXP20X_IRQ(PEK_LONG, 2, 0),
> + AXP20X_IRQ(N_OE_PWR_ON, 3, 7),
> + AXP20X_IRQ(N_OE_PWR_OFF, 3, 6),
> + AXP20X_IRQ(VBUS_VALID, 3, 5),
> + AXP20X_IRQ(VBUS_NOT_VALID, 3, 4),
> + AXP20X_IRQ(VBUS_SESS_VALID, 3, 3),
> + AXP20X_IRQ(VBUS_SESS_END, 3, 2),
> + AXP20X_IRQ(LOW_PWR_LVL1, 3, 1),
> + AXP20X_IRQ(LOW_PWR_LVL2, 3, 0),
> + AXP20X_IRQ(TIMER, 4, 7),
> + AXP20X_IRQ(PEK_RIS_EDGE, 4, 6),
> + AXP20X_IRQ(PEK_FAL_EDGE, 4, 5),
> + AXP20X_IRQ(GPIO3_INPUT, 4, 3),
> + AXP20X_IRQ(GPIO2_INPUT, 4, 2),
> + AXP20X_IRQ(GPIO1_INPUT, 4, 1),
> + AXP20X_IRQ(GPIO0_INPUT, 4, 0),
> +};
Where are these handled i.e. where is the irq_handler located?
> +static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
> + .name = "axp20x_irq_chip",
> + .status_base = AXP20X_IRQ1_STATE,
> + .ack_base = AXP20X_IRQ1_STATE,
> + .mask_base = AXP20X_IRQ1_EN,
> + .num_regs = 5,
> + .irqs = axp20x_regmap_irqs,
> + .num_irqs = ARRAY_SIZE(axp20x_regmap_irqs),
> + .mask_invert = 1,
This is a bool; true or false please.
> + .init_ack_masked = 1,
Same here.
> +};
> +
> +static struct mfd_cell axp20x_cells[] = {
> + {
> + .name = "axp20x-pek",
> + .of_compatible = "x-powers,axp20x-pek",
> + .num_resources = ARRAY_SIZE(axp20x_pek_resources),
> + .resources = axp20x_pek_resources,
I already saw the comments about these.
> + }, {
> + .name = "axp20x-regulator",
> + },
> +};
> +
> +const struct of_device_id axp20x_of_match[] = {
> + { .compatible = "x-powers,axp20x", .data = (void *)AXP20X },
There's no need to add device IDs if you only support one device.
> + { },
> +};
> +
> +static struct axp20x_dev *axp20x_pm_power_off;
This looks pretty unconventional. What's the point of it?
> +static void axp20x_power_off(void)
> +{
> + regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL, 0x80);
What does the 0x80 do exactly? Think about #defining.
> +}
> +
> +static int axp20x_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + struct axp20x_dev *axp20x;
> + const struct of_device_id *of_id;
> + int ret;
> +
> + axp20x = devm_kzalloc(&i2c->dev, sizeof(*axp20x), GFP_KERNEL);
> + if (!axp20x)
> + return -ENOMEM;
> +
> + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> + if (!of_id) {
> + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> + return -ENODEV;
> + }
> + axp20x->variant = (int) of_id->data;
Lots of code here surrounding added device support, but only one
device is supported. Why so?
> + axp20x->i2c_client = i2c;
> + i2c_set_clientdata(i2c, axp20x);
> +
> + axp20x->dev = &i2c->dev;
> + dev_set_drvdata(axp20x->dev, axp20x);
Do you make use of all this saving of the device container?
If so, where?
Also:
i2c_set_clientdata(i2c)
and:
dev_set_drvdata(i2c->dev);
... do exactly the same thing i.e. set i2c->dev->p->device_data.
> + axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
> + if (IS_ERR(axp20x->regmap)) {
> + ret = PTR_ERR(axp20x->regmap);
> + dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
> + return ret;
> + }
> +
> + axp20x->irq = i2c->irq;
> + ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
> + IRQF_ONESHOT | IRQF_SHARED, -1,
> + &axp20x_regmap_irq_chip,
> + &axp20x->regmap_irqc);
> + if (ret != 0) {
It's more succinct to say:
if (!ret)
> + dev_err(&i2c->dev, "failed to add irq chip: %d\n", ret);
> + return ret;
> + }
> +
> + ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
> + ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
> + if (ret != 0) {
> + dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
> + goto mfd_err;
> + }
> +
> + if (!pm_power_off) {
> + axp20x_pm_power_off = axp20x;
> + pm_power_off = axp20x_power_off;
> + }
Can you describe to me what you're using the pm_power_off call-back
for please?
> + dev_info(&i2c->dev, "AXP20X driver loaded\n");
> +
> + return 0;
> +
> +mfd_err:
> + regmap_del_irq_chip(axp20x->irq, axp20x->regmap_irqc);
> +
> + return ret;
> +}
> +
> +static int axp20x_i2c_remove(struct i2c_client *i2c)
> +{
> + struct axp20x_dev *axp20x = i2c_get_clientdata(i2c);
> +
> + if (axp20x == axp20x_pm_power_off) {
> + axp20x_pm_power_off = NULL;
> + pm_power_off = NULL;
> + }
> +
> + mfd_remove_devices(axp20x->dev);
> + regmap_del_irq_chip(axp20x->i2c_client->irq, axp20x->regmap_irqc);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id axp20x_i2c_id[] = {
> + { "axp20x", AXP20X },
It doesn't look like you're using this ID either?
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
> +
> +static struct i2c_driver axp20x_i2c_driver = {
> + .driver = {
> + .name = "axp20x",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(axp20x_of_match),
> + },
> + .probe = axp20x_i2c_probe,
> + .remove = axp20x_i2c_remove,
> + .id_table = axp20x_i2c_id,
> +};
> +
> +module_i2c_driver(axp20x_i2c_driver);
> +
> +MODULE_DESCRIPTION("PMIC MFD core driver for AXP20X");
> +MODULE_AUTHOR("Carlo Caione <carlo@caione.org>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> new file mode 100644
> index 0000000..94d99fd
> --- /dev/null
> +++ b/include/linux/mfd/axp20x.h
> @@ -0,0 +1,178 @@
> +/*
> + * Functions to access AXP20X power management chip.
> + *
> + * Copyright (C) 2013, Carlo Caione <carlo@caione.org>
> + *
> + * 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 __LINUX_MFD_AXP20X_H
> +#define __LINUX_MFD_AXP20X_H
> +
> +#define AXP20X 0
> +
> +#define AXP20X_DATA(m) (0x04 + (m))
This is not a great name for a macro. 'data' and 'value' seldom make
for good names for variables/macros. Please replace it with something
more descriptive.
In fact, on closer inspection it appears as though you only use this
once while defining writable registers. What happened to registers
0x00 to 0x03, and why have the top registers below been omitted from
the range(s)?
> +
> +/* Power supply */
> +#define AXP20X_PWR_INPUT_STATUS 0x00
> +#define AXP20X_PWR_OP_MODE 0x01
> +#define AXP20X_USB_OTG_STATUS 0x02
> +#define AXP20X_PWR_OUT_CTRL 0x12
> +#define AXP20X_DCDC2_V_OUT 0x23
> +#define AXP20X_DCDC2_LDO3_V_SCAL 0x25
> +#define AXP20X_DCDC3_V_OUT 0x27
> +#define AXP20X_LDO24_V_OUT 0x28
> +#define AXP20X_LDO3_V_OUT 0x29
> +#define AXP20X_VBUS_IPSOUT_MGMT 0x30
> +#define AXP20X_V_OFF 0x31
> +#define AXP20X_OFF_CTRL 0x32
> +#define AXP20X_CHRG_CTRL1 0x33
> +#define AXP20X_CHRG_CTRL2 0x34
> +#define AXP20X_CHRG_BAK_CTRL 0x35
> +#define AXP20X_PEK_KEY 0x36
> +#define AXP20X_DCDC_FREQ 0x37
> +#define AXP20X_V_LTF_CHRG 0x38
> +#define AXP20X_V_HTF_CHRG 0x39
> +#define AXP20X_APS_WARN_L1 0x3a
> +#define AXP20X_APS_WARN_L2 0x3b
> +#define AXP20X_V_LTF_DISCHRG 0x3c
> +#define AXP20X_V_HTF_DISCHRG 0x3d
> +
> +/* Interrupt */
> +#define AXP20X_IRQ1_EN 0x40
> +#define AXP20X_IRQ2_EN 0x41
> +#define AXP20X_IRQ3_EN 0x42
> +#define AXP20X_IRQ4_EN 0x43
> +#define AXP20X_IRQ5_EN 0x44
> +#define AXP20X_IRQ1_STATE 0x48
> +#define AXP20X_IRQ2_STATE 0x49
> +#define AXP20X_IRQ3_STATE 0x4a
> +#define AXP20X_IRQ4_STATE 0x4b
> +#define AXP20X_IRQ5_STATE 0x4c
> +
> +/* ADC */
> +#define AXP20X_ACIN_V_ADC_H 0x56
> +#define AXP20X_ACIN_V_ADC_L 0x57
> +#define AXP20X_ACIN_I_ADC_H 0x58
> +#define AXP20X_ACIN_I_ADC_L 0x59
> +#define AXP20X_VBUS_V_ADC_H 0x5a
> +#define AXP20X_VBUS_V_ADC_L 0x5b
> +#define AXP20X_VBUS_I_ADC_H 0x5c
> +#define AXP20X_VBUS_I_ADC_L 0x5d
> +#define AXP20X_TEMP_ADC_H 0x5e
> +#define AXP20X_TEMP_ADC_L 0x5f
> +#define AXP20X_TS_IN_H 0x62
> +#define AXP20X_TS_IN_L 0x63
> +#define AXP20X_GPIO0_V_ADC_H 0x64
> +#define AXP20X_GPIO0_V_ADC_L 0x65
> +#define AXP20X_GPIO1_V_ADC_H 0x66
> +#define AXP20X_GPIO1_V_ADC_L 0x67
> +#define AXP20X_PWR_BATT_H 0x70
> +#define AXP20X_PWR_BATT_M 0x71
> +#define AXP20X_PWR_BATT_L 0x72
> +#define AXP20X_BATT_V_H 0x78
> +#define AXP20X_BATT_V_L 0x79
> +#define AXP20X_BATT_CHRG_I_H 0x7a
> +#define AXP20X_BATT_CHRG_I_L 0x7b
> +#define AXP20X_BATT_DISCHRG_I_H 0x7c
> +#define AXP20X_BATT_DISCHRG_I_L 0x7d
> +#define AXP20X_IPSOUT_V_HIGH_H 0x7e
> +#define AXP20X_IPSOUT_V_HIGH_L 0x7f
> +
> +/* Power supply */
> +#define AXP20X_DCDC_MODE 0x80
> +#define AXP20X_ADC_EN1 0x82
> +#define AXP20X_ADC_EN2 0x83
> +#define AXP20X_ADC_RATE 0x84
> +#define AXP20X_GPIO10_IN_RANGE 0x85
> +#define AXP20X_GPIO1_ADC_IRQ_RIS 0x86
> +#define AXP20X_GPIO1_ADC_IRQ_FAL 0x87
> +#define AXP20X_TIMER_CTRL 0x8a
> +#define AXP20X_VBUS_MON 0x8b
> +#define AXP20X_OVER_TMP 0x8f
> +
> +/* GPIO */
> +#define AXP20X_GPIO0_CTRL 0x90
> +#define AXP20X_LDO5_V_OUT 0x91
> +#define AXP20X_GPIO1_CTRL 0x92
> +#define AXP20X_GPIO2_CTRL 0x93
> +#define AXP20X_GPIO20_SS 0x94
> +#define AXP20X_GPIO3_CTRL 0x95
> +
> +/* Battery */
> +#define AXP20X_CHRG_CC_31_24 0xb0
> +#define AXP20X_CHRG_CC_23_16 0xb1
> +#define AXP20X_CHRG_CC_15_8 0xb2
> +#define AXP20X_CHRG_CC_7_0 0xb3
> +#define AXP20X_DISCHRG_CC_31_24 0xb4
> +#define AXP20X_DISCHRG_CC_23_16 0xb5
> +#define AXP20X_DISCHRG_CC_15_8 0xb6
> +#define AXP20X_DISCHRG_CC_7_0 0xb7
> +#define AXP20X_CC_CTRL 0xb8
> +#define AXP20X_FG_RES 0xb9
> +
> +/* Regulators IDs */
> +enum {
> + AXP20X_LDO1 = 0,
> + AXP20X_LDO2,
> + AXP20X_LDO3,
> + AXP20X_LDO4,
> + AXP20X_LDO5,
> + AXP20X_DCDC2,
> + AXP20X_DCDC3,
> + AXP20X_REG_ID_MAX,
> +};
> +
> +/* IRQs */
> +enum {
> + AXP20X_IRQ_ACIN_OVER_V = 1,
> + AXP20X_IRQ_ACIN_PLUGIN,
> + AXP20X_IRQ_ACIN_REMOVAL,
> + AXP20X_IRQ_VBUS_OVER_V,
> + AXP20X_IRQ_VBUS_PLUGIN,
> + AXP20X_IRQ_VBUS_REMOVAL,
> + AXP20X_IRQ_VBUS_V_LOW,
> + AXP20X_IRQ_BATT_PLUGIN,
> + AXP20X_IRQ_BATT_REMOVAL,
> + AXP20X_IRQ_BATT_ENT_ACT_MODE,
> + AXP20X_IRQ_BATT_EXIT_ACT_MODE,
> + AXP20X_IRQ_CHARG,
> + AXP20X_IRQ_CHARG_DONE,
> + AXP20X_IRQ_BATT_TEMP_HIGH,
> + AXP20X_IRQ_BATT_TEMP_LOW,
> + AXP20X_IRQ_DIE_TEMP_HIGH,
> + AXP20X_IRQ_CHARG_I_LOW,
> + AXP20X_IRQ_DCDC1_V_LONG,
> + AXP20X_IRQ_DCDC2_V_LONG,
> + AXP20X_IRQ_DCDC3_V_LONG,
> + AXP20X_IRQ_PEK_SHORT = 22,
> + AXP20X_IRQ_PEK_LONG,
> + AXP20X_IRQ_N_OE_PWR_ON,
> + AXP20X_IRQ_N_OE_PWR_OFF,
> + AXP20X_IRQ_VBUS_VALID,
> + AXP20X_IRQ_VBUS_NOT_VALID,
> + AXP20X_IRQ_VBUS_SESS_VALID,
> + AXP20X_IRQ_VBUS_SESS_END,
> + AXP20X_IRQ_LOW_PWR_LVL1,
> + AXP20X_IRQ_LOW_PWR_LVL2,
> + AXP20X_IRQ_TIMER,
> + AXP20X_IRQ_PEK_RIS_EDGE,
> + AXP20X_IRQ_PEK_FAL_EDGE,
> + AXP20X_IRQ_GPIO3_INPUT,
> + AXP20X_IRQ_GPIO2_INPUT,
> + AXP20X_IRQ_GPIO1_INPUT,
> + AXP20X_IRQ_GPIO0_INPUT,
> +};
> +
> +struct axp20x_dev {
> + struct device *dev;
> + struct i2c_client *i2c_client;
> + struct regmap *regmap;
> + struct regmap_irq_chip_data *regmap_irqc;
> + int variant;
> + int irq;
> +};
Is this used anywhere else except in the MFD driver?
If not, consider moving it into the *.c file.
> +#endif /* __LINUX_MFD_AXP20X_H */
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 3/3] mfd: axp20x: Add bindings documentation
2014-02-10 20:37 ` [linux-sunxi] " Carlo Caione
@ 2014-02-10 22:01 ` Maxime Ripard
2014-02-10 22:38 ` Carlo Caione
0 siblings, 1 reply; 23+ messages in thread
From: Maxime Ripard @ 2014-02-10 22:01 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 09:37:37PM +0100, Carlo Caione wrote:
> >> +Sub-nodes
> >> +* regulators : Contain the regulator nodes. The regulators are bound using
> >> + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3,
> >> + ldo4, ldo5.
> >> + The bindings details of individual regulator device can be found in:
> >> + Documentation/devicetree/bindings/regulator/regulator.txt with the
> >> + exception of:
> >
> > I'm guessing this is where you differentiate between AXP202 and
> > AXP209?
>
> Not really. AXP202 and AXP209 have the same regulators (with the same
> constrains)
Ok. What's the difference between the two then?
Thanks,
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140210/7fcfc338/attachment.sig>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-10 21:19 ` Lee Jones
@ 2014-02-10 22:34 ` Carlo Caione
2014-02-11 6:58 ` Carlo Caione
2014-02-11 9:15 ` Lee Jones
0 siblings, 2 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-10 22:34 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 10:19 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> This patch introduces the preliminary support for PMICs X-Powers AXP202
>> and AXP209. The core contains support only for two sub-modules (PEK
>> and regulators) that will be added with two different patch-sets.
>>
>> Signed-off-by: Carlo Caione <carlo@caione.org>
>> ---
>> drivers/mfd/Kconfig | 12 +++
>> drivers/mfd/Makefile | 1 +
>> drivers/mfd/axp20x.c | 251 +++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/mfd/axp20x.h | 178 ++++++++++++++++++++++++++++++++
>> 4 files changed, 442 insertions(+)
>> create mode 100644 drivers/mfd/axp20x.c
>> create mode 100644 include/linux/mfd/axp20x.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index dd67158..33d38c4 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -59,6 +59,18 @@ config MFD_AAT2870_CORE
>> additional drivers must be enabled in order to use the
>> functionality of the device.
>
> <snip>
>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> new file mode 100644
>> index 0000000..efd0cb3
>> --- /dev/null
>> +++ b/drivers/mfd/axp20x.c
>> @@ -0,0 +1,251 @@
>> +/*
>
> A nice descriptive title here would be good.
Agree.
>> + * Author: Carlo Caione <carlo@caione.org>
>> + *
>> + * 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/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/mfd/axp20x.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_irq.h>
>> +
>> +static const struct regmap_range axp20x_writeable_ranges[] = {
>> + {
>> + .range_min = AXP20X_DATA(0),
>> + .range_max = AXP20X_IRQ5_STATE,
>> + }, {
>> + .range_min = AXP20X_DCDC_MODE,
>> + .range_max = AXP20X_FG_RES,
>> + },
>> +};
>> +
>> +static const struct regmap_range axp20x_volatile_ranges[] = {
>> + {
>> + .range_min = AXP20X_IRQ1_EN,
>> + .range_max = AXP20X_IRQ5_STATE,
>> + },
>> +};
>> +
>> +static const struct regmap_access_table axp20x_writeable_table = {
>> + .yes_ranges = axp20x_writeable_ranges,
>> + .n_yes_ranges = ARRAY_SIZE(axp20x_writeable_ranges),
>> +};
>> +
>> +static const struct regmap_access_table axp20x_volatile_table = {
>> + .yes_ranges = axp20x_volatile_ranges,
>> + .n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
>> +};
>> +
>> +static struct resource axp20x_pek_resources[] = {
>> + {
>> + .name = "PEK_DBR",
>> + .start = AXP20X_IRQ_PEK_RIS_EDGE,
>> + .end = AXP20X_IRQ_PEK_RIS_EDGE,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .name = "PEK_DBF",
>> + .start = AXP20X_IRQ_PEK_FAL_EDGE,
>> + .end = AXP20X_IRQ_PEK_FAL_EDGE,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> +
>
> Superfluous new line.
>
>> +};
>> +
>> +static const struct regmap_config axp20x_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .wr_table = &axp20x_writeable_table,
>> + .volatile_table = &axp20x_volatile_table,
>> + .max_register = AXP20X_FG_RES,
>> + .cache_type = REGCACHE_RBTREE,
>> +};
>> +
>> +#define AXP20X_IRQ(_irq, _off, _mask) \
>> + [AXP20X_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
>> +
>> +static const struct regmap_irq axp20x_regmap_irqs[] = {
>> + AXP20X_IRQ(ACIN_OVER_V, 0, 7),
>> + AXP20X_IRQ(ACIN_PLUGIN, 0, 6),
>> + AXP20X_IRQ(ACIN_REMOVAL, 0, 5),
>> + AXP20X_IRQ(VBUS_OVER_V, 0, 4),
>> + AXP20X_IRQ(VBUS_PLUGIN, 0, 3),
>> + AXP20X_IRQ(VBUS_REMOVAL, 0, 2),
>> + AXP20X_IRQ(VBUS_V_LOW, 0, 1),
>> + AXP20X_IRQ(BATT_PLUGIN, 1, 7),
>> + AXP20X_IRQ(BATT_REMOVAL, 1, 6),
>> + AXP20X_IRQ(BATT_ENT_ACT_MODE, 1, 5),
>> + AXP20X_IRQ(BATT_EXIT_ACT_MODE, 1, 4),
>> + AXP20X_IRQ(CHARG, 1, 3),
>> + AXP20X_IRQ(CHARG_DONE, 1, 2),
>> + AXP20X_IRQ(BATT_TEMP_HIGH, 1, 1),
>> + AXP20X_IRQ(BATT_TEMP_LOW, 1, 0),
>> + AXP20X_IRQ(DIE_TEMP_HIGH, 2, 7),
>> + AXP20X_IRQ(CHARG_I_LOW, 2, 6),
>> + AXP20X_IRQ(DCDC1_V_LONG, 2, 5),
>> + AXP20X_IRQ(DCDC2_V_LONG, 2, 4),
>> + AXP20X_IRQ(DCDC3_V_LONG, 2, 3),
>> + AXP20X_IRQ(PEK_SHORT, 2, 1),
>> + AXP20X_IRQ(PEK_LONG, 2, 0),
>> + AXP20X_IRQ(N_OE_PWR_ON, 3, 7),
>> + AXP20X_IRQ(N_OE_PWR_OFF, 3, 6),
>> + AXP20X_IRQ(VBUS_VALID, 3, 5),
>> + AXP20X_IRQ(VBUS_NOT_VALID, 3, 4),
>> + AXP20X_IRQ(VBUS_SESS_VALID, 3, 3),
>> + AXP20X_IRQ(VBUS_SESS_END, 3, 2),
>> + AXP20X_IRQ(LOW_PWR_LVL1, 3, 1),
>> + AXP20X_IRQ(LOW_PWR_LVL2, 3, 0),
>> + AXP20X_IRQ(TIMER, 4, 7),
>> + AXP20X_IRQ(PEK_RIS_EDGE, 4, 6),
>> + AXP20X_IRQ(PEK_FAL_EDGE, 4, 5),
>> + AXP20X_IRQ(GPIO3_INPUT, 4, 3),
>> + AXP20X_IRQ(GPIO2_INPUT, 4, 2),
>> + AXP20X_IRQ(GPIO1_INPUT, 4, 1),
>> + AXP20X_IRQ(GPIO0_INPUT, 4, 0),
>> +};
>
> Where are these handled i.e. where is the irq_handler located?
Each one is used by a different driver for each subsystem of the MFD,
so each driver will have a specific irq_handler. I need the full list
here to register with regmap_add_irq_chip() the generic
regmap_irq_thread.
>> +static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
>> + .name = "axp20x_irq_chip",
>> + .status_base = AXP20X_IRQ1_STATE,
>> + .ack_base = AXP20X_IRQ1_STATE,
>> + .mask_base = AXP20X_IRQ1_EN,
>> + .num_regs = 5,
>> + .irqs = axp20x_regmap_irqs,
>> + .num_irqs = ARRAY_SIZE(axp20x_regmap_irqs),
>> + .mask_invert = 1,
>
> This is a bool; true or false please.
>
>> + .init_ack_masked = 1,
>
> Same here.
I'll fix both in v2.
>> +};
>> +
>> +static struct mfd_cell axp20x_cells[] = {
>> + {
>> + .name = "axp20x-pek",
>> + .of_compatible = "x-powers,axp20x-pek",
>> + .num_resources = ARRAY_SIZE(axp20x_pek_resources),
>> + .resources = axp20x_pek_resources,
>
> I already saw the comments about these.
>
>> + }, {
>> + .name = "axp20x-regulator",
>> + },
>> +};
>> +
>> +const struct of_device_id axp20x_of_match[] = {
>> + { .compatible = "x-powers,axp20x", .data = (void *)AXP20X },
>
> There's no need to add device IDs if you only support one device.
Ok. But what if in the future we want to add a new device?
>> + { },
>> +};
>> +
>> +static struct axp20x_dev *axp20x_pm_power_off;
>
> This looks pretty unconventional. What's the point of it?
On a single board we can have multiple AXPs so I track which one is in
charge of powering off the board (and to get the correct device in the
axp20x_power_off())
>> +static void axp20x_power_off(void)
>> +{
>> + regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL, 0x80);
>
> What does the 0x80 do exactly? Think about #defining.
It's only used here, but I'll #define it.
>> +}
>> +
>> +static int axp20x_i2c_probe(struct i2c_client *i2c,
>> + const struct i2c_device_id *id)
>> +{
>> + struct axp20x_dev *axp20x;
>> + const struct of_device_id *of_id;
>> + int ret;
>> +
>> + axp20x = devm_kzalloc(&i2c->dev, sizeof(*axp20x), GFP_KERNEL);
>> + if (!axp20x)
>> + return -ENOMEM;
>> +
>> + of_id = of_match_device(axp20x_of_match, &i2c->dev);
>> + if (!of_id) {
>> + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
>> + return -ENODEV;
>> + }
>> + axp20x->variant = (int) of_id->data;
>
> Lots of code here surrounding added device support, but only one
> device is supported. Why so?
Because at the moment I support only axp202 and axp209 but I wanted
something future-proof
>> + axp20x->i2c_client = i2c;
>> + i2c_set_clientdata(i2c, axp20x);
>> +
>> + axp20x->dev = &i2c->dev;
>> + dev_set_drvdata(axp20x->dev, axp20x);
>
> Do you make use of all this saving of the device container?
>
> If so, where?
In the drivers for subsystems (input, regulators, gpio, etc..)
> Also:
> i2c_set_clientdata(i2c)
>
> and:
> dev_set_drvdata(i2c->dev);
>
> ... do exactly the same thing i.e. set i2c->dev->p->device_data.
Right.
>> + axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
>> + if (IS_ERR(axp20x->regmap)) {
>> + ret = PTR_ERR(axp20x->regmap);
>> + dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + axp20x->irq = i2c->irq;
>> + ret = regmap_add_irq_chip(axp20x->regmap, axp20x->irq,
>> + IRQF_ONESHOT | IRQF_SHARED, -1,
>> + &axp20x_regmap_irq_chip,
>> + &axp20x->regmap_irqc);
>> + if (ret != 0) {
>
> It's more succinct to say:
> if (!ret)
Agree
>> + dev_err(&i2c->dev, "failed to add irq chip: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
>> + ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
>> + if (ret != 0) {
>> + dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
>> + goto mfd_err;
>> + }
>> +
>> + if (!pm_power_off) {
>> + axp20x_pm_power_off = axp20x;
>> + pm_power_off = axp20x_power_off;
>> + }
>
> Can you describe to me what you're using the pm_power_off call-back
> for please?
It's meant to poweroff the board
>> + dev_info(&i2c->dev, "AXP20X driver loaded\n");
>> +
>> + return 0;
>> +
>> +mfd_err:
>> + regmap_del_irq_chip(axp20x->irq, axp20x->regmap_irqc);
>> +
>> + return ret;
>> +}
>> +
>> +static int axp20x_i2c_remove(struct i2c_client *i2c)
>> +{
>> + struct axp20x_dev *axp20x = i2c_get_clientdata(i2c);
>> +
>> + if (axp20x == axp20x_pm_power_off) {
>> + axp20x_pm_power_off = NULL;
>> + pm_power_off = NULL;
>> + }
>> +
>> + mfd_remove_devices(axp20x->dev);
>> + regmap_del_irq_chip(axp20x->i2c_client->irq, axp20x->regmap_irqc);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id axp20x_i2c_id[] = {
>> + { "axp20x", AXP20X },
>
> It doesn't look like you're using this ID either?
No, I am not. I'll fix it in v2.
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
>> +
>> +static struct i2c_driver axp20x_i2c_driver = {
>> + .driver = {
>> + .name = "axp20x",
>> + .owner = THIS_MODULE,
>> + .of_match_table = of_match_ptr(axp20x_of_match),
>> + },
>> + .probe = axp20x_i2c_probe,
>> + .remove = axp20x_i2c_remove,
>> + .id_table = axp20x_i2c_id,
>> +};
>> +
>> +module_i2c_driver(axp20x_i2c_driver);
>> +
>> +MODULE_DESCRIPTION("PMIC MFD core driver for AXP20X");
>> +MODULE_AUTHOR("Carlo Caione <carlo@caione.org>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
>> new file mode 100644
>> index 0000000..94d99fd
>> --- /dev/null
>> +++ b/include/linux/mfd/axp20x.h
>> @@ -0,0 +1,178 @@
>> +/*
>> + * Functions to access AXP20X power management chip.
>> + *
>> + * Copyright (C) 2013, Carlo Caione <carlo@caione.org>
>> + *
>> + * 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 __LINUX_MFD_AXP20X_H
>> +#define __LINUX_MFD_AXP20X_H
>> +
>> +#define AXP20X 0
>> +
>> +#define AXP20X_DATA(m) (0x04 + (m))
>
> This is not a great name for a macro. 'data' and 'value' seldom make
> for good names for variables/macros. Please replace it with something
> more descriptive.
Agree. I'll change it.
> In fact, on closer inspection it appears as though you only use this
> once while defining writable registers. What happened to registers
> 0x00 to 0x03, and why have the top registers below been omitted from
> the range(s)?
All the registers defined here can be used by the drivers of the
different subsystems including this header file, thus all the
registers are reported here though they are not directly used by the
core driver.
About the range you are right, I'll fix it in v2. Thanks for noticing.
>> +
>> +/* Power supply */
>> +#define AXP20X_PWR_INPUT_STATUS 0x00
>> +#define AXP20X_PWR_OP_MODE 0x01
>> +#define AXP20X_USB_OTG_STATUS 0x02
>> +#define AXP20X_PWR_OUT_CTRL 0x12
>> +#define AXP20X_DCDC2_V_OUT 0x23
>> +#define AXP20X_DCDC2_LDO3_V_SCAL 0x25
>> +#define AXP20X_DCDC3_V_OUT 0x27
>> +#define AXP20X_LDO24_V_OUT 0x28
>> +#define AXP20X_LDO3_V_OUT 0x29
>> +#define AXP20X_VBUS_IPSOUT_MGMT 0x30
>> +#define AXP20X_V_OFF 0x31
>> +#define AXP20X_OFF_CTRL 0x32
>> +#define AXP20X_CHRG_CTRL1 0x33
>> +#define AXP20X_CHRG_CTRL2 0x34
>> +#define AXP20X_CHRG_BAK_CTRL 0x35
>> +#define AXP20X_PEK_KEY 0x36
>> +#define AXP20X_DCDC_FREQ 0x37
>> +#define AXP20X_V_LTF_CHRG 0x38
>> +#define AXP20X_V_HTF_CHRG 0x39
>> +#define AXP20X_APS_WARN_L1 0x3a
>> +#define AXP20X_APS_WARN_L2 0x3b
>> +#define AXP20X_V_LTF_DISCHRG 0x3c
>> +#define AXP20X_V_HTF_DISCHRG 0x3d
>> +
>> +/* Interrupt */
>> +#define AXP20X_IRQ1_EN 0x40
>> +#define AXP20X_IRQ2_EN 0x41
>> +#define AXP20X_IRQ3_EN 0x42
>> +#define AXP20X_IRQ4_EN 0x43
>> +#define AXP20X_IRQ5_EN 0x44
>> +#define AXP20X_IRQ1_STATE 0x48
>> +#define AXP20X_IRQ2_STATE 0x49
>> +#define AXP20X_IRQ3_STATE 0x4a
>> +#define AXP20X_IRQ4_STATE 0x4b
>> +#define AXP20X_IRQ5_STATE 0x4c
>> +
>> +/* ADC */
>> +#define AXP20X_ACIN_V_ADC_H 0x56
>> +#define AXP20X_ACIN_V_ADC_L 0x57
>> +#define AXP20X_ACIN_I_ADC_H 0x58
>> +#define AXP20X_ACIN_I_ADC_L 0x59
>> +#define AXP20X_VBUS_V_ADC_H 0x5a
>> +#define AXP20X_VBUS_V_ADC_L 0x5b
>> +#define AXP20X_VBUS_I_ADC_H 0x5c
>> +#define AXP20X_VBUS_I_ADC_L 0x5d
>> +#define AXP20X_TEMP_ADC_H 0x5e
>> +#define AXP20X_TEMP_ADC_L 0x5f
>> +#define AXP20X_TS_IN_H 0x62
>> +#define AXP20X_TS_IN_L 0x63
>> +#define AXP20X_GPIO0_V_ADC_H 0x64
>> +#define AXP20X_GPIO0_V_ADC_L 0x65
>> +#define AXP20X_GPIO1_V_ADC_H 0x66
>> +#define AXP20X_GPIO1_V_ADC_L 0x67
>> +#define AXP20X_PWR_BATT_H 0x70
>> +#define AXP20X_PWR_BATT_M 0x71
>> +#define AXP20X_PWR_BATT_L 0x72
>> +#define AXP20X_BATT_V_H 0x78
>> +#define AXP20X_BATT_V_L 0x79
>> +#define AXP20X_BATT_CHRG_I_H 0x7a
>> +#define AXP20X_BATT_CHRG_I_L 0x7b
>> +#define AXP20X_BATT_DISCHRG_I_H 0x7c
>> +#define AXP20X_BATT_DISCHRG_I_L 0x7d
>> +#define AXP20X_IPSOUT_V_HIGH_H 0x7e
>> +#define AXP20X_IPSOUT_V_HIGH_L 0x7f
>> +
>> +/* Power supply */
>> +#define AXP20X_DCDC_MODE 0x80
>> +#define AXP20X_ADC_EN1 0x82
>> +#define AXP20X_ADC_EN2 0x83
>> +#define AXP20X_ADC_RATE 0x84
>> +#define AXP20X_GPIO10_IN_RANGE 0x85
>> +#define AXP20X_GPIO1_ADC_IRQ_RIS 0x86
>> +#define AXP20X_GPIO1_ADC_IRQ_FAL 0x87
>> +#define AXP20X_TIMER_CTRL 0x8a
>> +#define AXP20X_VBUS_MON 0x8b
>> +#define AXP20X_OVER_TMP 0x8f
>> +
>> +/* GPIO */
>> +#define AXP20X_GPIO0_CTRL 0x90
>> +#define AXP20X_LDO5_V_OUT 0x91
>> +#define AXP20X_GPIO1_CTRL 0x92
>> +#define AXP20X_GPIO2_CTRL 0x93
>> +#define AXP20X_GPIO20_SS 0x94
>> +#define AXP20X_GPIO3_CTRL 0x95
>> +
>> +/* Battery */
>> +#define AXP20X_CHRG_CC_31_24 0xb0
>> +#define AXP20X_CHRG_CC_23_16 0xb1
>> +#define AXP20X_CHRG_CC_15_8 0xb2
>> +#define AXP20X_CHRG_CC_7_0 0xb3
>> +#define AXP20X_DISCHRG_CC_31_24 0xb4
>> +#define AXP20X_DISCHRG_CC_23_16 0xb5
>> +#define AXP20X_DISCHRG_CC_15_8 0xb6
>> +#define AXP20X_DISCHRG_CC_7_0 0xb7
>> +#define AXP20X_CC_CTRL 0xb8
>> +#define AXP20X_FG_RES 0xb9
>> +
>> +/* Regulators IDs */
>> +enum {
>> + AXP20X_LDO1 = 0,
>> + AXP20X_LDO2,
>> + AXP20X_LDO3,
>> + AXP20X_LDO4,
>> + AXP20X_LDO5,
>> + AXP20X_DCDC2,
>> + AXP20X_DCDC3,
>> + AXP20X_REG_ID_MAX,
>> +};
>> +
>> +/* IRQs */
>> +enum {
>> + AXP20X_IRQ_ACIN_OVER_V = 1,
>> + AXP20X_IRQ_ACIN_PLUGIN,
>> + AXP20X_IRQ_ACIN_REMOVAL,
>> + AXP20X_IRQ_VBUS_OVER_V,
>> + AXP20X_IRQ_VBUS_PLUGIN,
>> + AXP20X_IRQ_VBUS_REMOVAL,
>> + AXP20X_IRQ_VBUS_V_LOW,
>> + AXP20X_IRQ_BATT_PLUGIN,
>> + AXP20X_IRQ_BATT_REMOVAL,
>> + AXP20X_IRQ_BATT_ENT_ACT_MODE,
>> + AXP20X_IRQ_BATT_EXIT_ACT_MODE,
>> + AXP20X_IRQ_CHARG,
>> + AXP20X_IRQ_CHARG_DONE,
>> + AXP20X_IRQ_BATT_TEMP_HIGH,
>> + AXP20X_IRQ_BATT_TEMP_LOW,
>> + AXP20X_IRQ_DIE_TEMP_HIGH,
>> + AXP20X_IRQ_CHARG_I_LOW,
>> + AXP20X_IRQ_DCDC1_V_LONG,
>> + AXP20X_IRQ_DCDC2_V_LONG,
>> + AXP20X_IRQ_DCDC3_V_LONG,
>> + AXP20X_IRQ_PEK_SHORT = 22,
>> + AXP20X_IRQ_PEK_LONG,
>> + AXP20X_IRQ_N_OE_PWR_ON,
>> + AXP20X_IRQ_N_OE_PWR_OFF,
>> + AXP20X_IRQ_VBUS_VALID,
>> + AXP20X_IRQ_VBUS_NOT_VALID,
>> + AXP20X_IRQ_VBUS_SESS_VALID,
>> + AXP20X_IRQ_VBUS_SESS_END,
>> + AXP20X_IRQ_LOW_PWR_LVL1,
>> + AXP20X_IRQ_LOW_PWR_LVL2,
>> + AXP20X_IRQ_TIMER,
>> + AXP20X_IRQ_PEK_RIS_EDGE,
>> + AXP20X_IRQ_PEK_FAL_EDGE,
>> + AXP20X_IRQ_GPIO3_INPUT,
>> + AXP20X_IRQ_GPIO2_INPUT,
>> + AXP20X_IRQ_GPIO1_INPUT,
>> + AXP20X_IRQ_GPIO0_INPUT,
>> +};
>> +
>> +struct axp20x_dev {
>> + struct device *dev;
>> + struct i2c_client *i2c_client;
>> + struct regmap *regmap;
>> + struct regmap_irq_chip_data *regmap_irqc;
>> + int variant;
>> + int irq;
>> +};
>
> Is this used anywhere else except in the MFD driver?
>
> If not, consider moving it into the *.c file.
Yes, it is used by drivers of subsystems.
Thank you,
--
Carlo Caione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 3/3] mfd: axp20x: Add bindings documentation
2014-02-10 22:01 ` Maxime Ripard
@ 2014-02-10 22:38 ` Carlo Caione
2014-02-12 17:16 ` Mark Brown
0 siblings, 1 reply; 23+ messages in thread
From: Carlo Caione @ 2014-02-10 22:38 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 11:01 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Feb 10, 2014 at 09:37:37PM +0100, Carlo Caione wrote:
>> >> +Sub-nodes
>> >> +* regulators : Contain the regulator nodes. The regulators are bound using
>> >> + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3,
>> >> + ldo4, ldo5.
>> >> + The bindings details of individual regulator device can be found in:
>> >> + Documentation/devicetree/bindings/regulator/regulator.txt with the
>> >> + exception of:
>> >
>> > I'm guessing this is where you differentiate between AXP202 and
>> > AXP209?
>>
>> Not really. AXP202 and AXP209 have the same regulators (with the same
>> constrains)
>
> Ok. What's the difference between the two then?
Honestly I have no idea. For what I have seen (core, PEK and
regulators) AXP202 and AXP209 are identical.
--
Carlo Caione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-10 22:34 ` [linux-sunxi] " Carlo Caione
@ 2014-02-11 6:58 ` Carlo Caione
2014-02-11 9:15 ` Lee Jones
1 sibling, 0 replies; 23+ messages in thread
From: Carlo Caione @ 2014-02-11 6:58 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 11:34 PM, Carlo Caione <carlo@caione.org> wrote:
>>> +static struct axp20x_dev *axp20x_pm_power_off;
>>
>> This looks pretty unconventional. What's the point of it?
>
> On a single board we can have multiple AXPs so I track which one is in
> charge of powering off the board (and to get the correct device in the
> axp20x_power_off())
<snip>
>>> +
>>> + if (!pm_power_off) {
>>> + axp20x_pm_power_off = axp20x;
>>> + pm_power_off = axp20x_power_off;
>>> + }
>>
>> Can you describe to me what you're using the pm_power_off call-back
>> for please?
>
> It's meant to poweroff the board
Actually what is missing here is also a way to determine which device
is in charge of powering off when multiple AXPs are registered.
I'll fix it in v2.
Thanks,
--
Carlo Caione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-10 22:34 ` [linux-sunxi] " Carlo Caione
2014-02-11 6:58 ` Carlo Caione
@ 2014-02-11 9:15 ` Lee Jones
2014-02-11 9:33 ` Carlo Caione
1 sibling, 1 reply; 23+ messages in thread
From: Lee Jones @ 2014-02-11 9:15 UTC (permalink / raw)
To: linux-arm-kernel
> >> +static const struct regmap_irq axp20x_regmap_irqs[] = {
> >> + AXP20X_IRQ(ACIN_OVER_V, 0, 7),
> >> + AXP20X_IRQ(ACIN_PLUGIN, 0, 6),
[...]
> >> + AXP20X_IRQ(GPIO1_INPUT, 4, 1),
> >> + AXP20X_IRQ(GPIO0_INPUT, 4, 0),
> >> +};
> >
> > Where are these handled i.e. where is the irq_handler located?
>
> Each one is used by a different driver for each subsystem of the MFD,
> so each driver will have a specific irq_handler. I need the full list
> here to register with regmap_add_irq_chip() the generic
> regmap_irq_thread.
Okay, this is all I needed. I must confess that I haven't _used_
regmap_add_irq_chip() before and was a little confused as to how it
worked exactly. We used to handle hierarchical IRQs ourselves, but
this is better I think.
<snip>
> >> +const struct of_device_id axp20x_of_match[] = {
> >> + { .compatible = "x-powers,axp20x", .data = (void *)AXP20X },
> >
> > There's no need to add device IDs if you only support one device.
>
> Ok. But what if in the future we want to add a new device?
Then we add support for device identification. Until then, it's just
meaningless cruft.
> >> + { },
> >> +};
> >> +
> >> +static struct axp20x_dev *axp20x_pm_power_off;
> >
> > This looks pretty unconventional. What's the point of it?
>
> On a single board we can have multiple AXPs so I track which one is in
> charge of powering off the board (and to get the correct device in the
> axp20x_power_off())
Is it this device's responsibility to shut down the _entire_ board? Or
does the call below only turn off _this_ device?
> >> +static void axp20x_power_off(void)
> >> +{
> >> + regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL, 0x80);
<snip>
> >> + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> >> + if (!of_id) {
> >> + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> >> + return -ENODEV;
> >> + }
> >> + axp20x->variant = (int) of_id->data;
> >
> > Lots of code here surrounding added device support, but only one
> > device is supported. Why so?
>
> Because at the moment I support only axp202 and axp209 but I wanted
> something future-proof
Nothing is future-proof. :)
You only need to add this functionality when it's going to be
utilised.
> >> + axp20x->i2c_client = i2c;
> >> + i2c_set_clientdata(i2c, axp20x);
> >> +
> >> + axp20x->dev = &i2c->dev;
> >> + dev_set_drvdata(axp20x->dev, axp20x);
> >
> > Do you make use of all this saving of the device container?
> >
> > If so, where?
>
> In the drivers for subsystems (input, regulators, gpio, etc..)
Can you link me to the patches please? Any reason why they're not in
this set? By submitting them together you give the Maintainers a good
over-view on how the system works together.
> > Also:
> > i2c_set_clientdata(i2c)
> >
> > and:
> > dev_set_drvdata(i2c->dev);
> >
> > ... do exactly the same thing i.e. set i2c->dev->p->device_data.
>
> Right.
Right. So why are you doing them both?
<snip>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-11 9:15 ` Lee Jones
@ 2014-02-11 9:33 ` Carlo Caione
2014-02-11 10:08 ` Lee Jones
0 siblings, 1 reply; 23+ messages in thread
From: Carlo Caione @ 2014-02-11 9:33 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Feb 11, 2014 at 10:15 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> +const struct of_device_id axp20x_of_match[] = {
>> >> + { .compatible = "x-powers,axp20x", .data = (void *)AXP20X },
>> >
>> > There's no need to add device IDs if you only support one device.
>>
>> Ok. But what if in the future we want to add a new device?
>
> Then we add support for device identification. Until then, it's just
> meaningless cruft.
Ok, I'll get rid of it in v2.
>> >> + { },
>> >> +};
>> >> +
>> >> +static struct axp20x_dev *axp20x_pm_power_off;
>> >
>> > This looks pretty unconventional. What's the point of it?
>>
>> On a single board we can have multiple AXPs so I track which one is in
>> charge of powering off the board (and to get the correct device in the
>> axp20x_power_off())
>
> Is it this device's responsibility to shut down the _entire_ board? Or
> does the call below only turn off _this_ device?
AXP shutdowns the entire board
>> >> +static void axp20x_power_off(void)
>> >> +{
>> >> + regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL, 0x80);
>
> <snip>
>
>> >> + of_id = of_match_device(axp20x_of_match, &i2c->dev);
>> >> + if (!of_id) {
>> >> + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
>> >> + return -ENODEV;
>> >> + }
>> >> + axp20x->variant = (int) of_id->data;
>> >
>> > Lots of code here surrounding added device support, but only one
>> > device is supported. Why so?
>>
>> Because at the moment I support only axp202 and axp209 but I wanted
>> something future-proof
>
> Nothing is future-proof. :)
>
> You only need to add this functionality when it's going to be
> utilised.
I agree. Fix in v2.
>> >> + axp20x->i2c_client = i2c;
>> >> + i2c_set_clientdata(i2c, axp20x);
>> >> +
>> >> + axp20x->dev = &i2c->dev;
>> >> + dev_set_drvdata(axp20x->dev, axp20x);
>> >
>> > Do you make use of all this saving of the device container?
>> >
>> > If so, where?
>>
>> In the drivers for subsystems (input, regulators, gpio, etc..)
>
> Can you link me to the patches please? Any reason why they're not in
> this set? By submitting them together you give the Maintainers a good
> over-view on how the system works together.
I wasn't sure it was a good idea to submit all the drivers altogether,
so my idea was to submit one driver at a time.
Do you think is it better to submit all the subsystems in one patch-set?
>> > Also:
>> > i2c_set_clientdata(i2c)
>> >
>> > and:
>> > dev_set_drvdata(i2c->dev);
>> >
>> > ... do exactly the same thing i.e. set i2c->dev->p->device_data.
>>
>> Right.
>
> Right. So why are you doing them both?
Right == I'll fix it :)
Thank you,
--
Carlo Caione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC
2014-02-11 9:33 ` Carlo Caione
@ 2014-02-11 10:08 ` Lee Jones
0 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2014-02-11 10:08 UTC (permalink / raw)
To: linux-arm-kernel
> >> >> + axp20x->i2c_client = i2c;
> >> >> + i2c_set_clientdata(i2c, axp20x);
> >> >> +
> >> >> + axp20x->dev = &i2c->dev;
> >> >> + dev_set_drvdata(axp20x->dev, axp20x);
> >> >
> >> > Do you make use of all this saving of the device container?
> >> >
> >> > If so, where?
> >>
> >> In the drivers for subsystems (input, regulators, gpio, etc..)
> >
> > Can you link me to the patches please? Any reason why they're not in
> > this set? By submitting them together you give the Maintainers a good
> > over-view on how the system works together.
>
> I wasn't sure it was a good idea to submit all the drivers altogether,
> so my idea was to submit one driver at a time.
> Do you think is it better to submit all the subsystems in one patch-set?
Normally.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/3] mfd: axp20x: Add bindings documentation
2014-02-08 16:03 ` [PATCH 3/3] mfd: axp20x: Add bindings documentation Carlo Caione
2014-02-10 20:05 ` Lee Jones
2014-02-10 20:12 ` Maxime Ripard
@ 2014-02-12 17:12 ` Mark Brown
2 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2014-02-12 17:12 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Feb 08, 2014 at 05:03:48PM +0100, Carlo Caione wrote:
> Bindings documentation for the axp20x driver. In this file also two
> sub-nodes (PEK and regulators) are documented.
You probably want to CC bindings to the maintianers for the relevant
subsystes too.
> + - dcdc-freq : defines the work frequency of DC-DC where
> + F=(1+dcdc-freq*5%)*1.5MHz
It sees a bit nicer to either have this be the frequency and work back
to the register setting or name it after the register field that gets
updated as a result. I'm also not sure what *5% means - I'm guessing
it's *0.05 but it'd be clearer to just write that.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140212/a4448312/attachment.sig>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [linux-sunxi] Re: [PATCH 3/3] mfd: axp20x: Add bindings documentation
2014-02-10 22:38 ` Carlo Caione
@ 2014-02-12 17:16 ` Mark Brown
0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2014-02-12 17:16 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Feb 10, 2014 at 11:38:47PM +0100, Carlo Caione wrote:
> On Mon, Feb 10, 2014 at 11:01 PM, Maxime Ripard
> > Ok. What's the difference between the two then?
> Honestly I have no idea. For what I have seen (core, PEK and
> regulators) AXP202 and AXP209 are identical.
I'd guess something in the electrical or mechanical specs (eg, maximum
current draws or packaging which are often related anyway), it's quite
common to get multiple PMICs that are silicon identical but differently
packaged or binned.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140212/95bb7c52/attachment.sig>
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2014-02-12 17:16 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-08 16:03 [PATCH 0/3] mfd: axp20x: Add support for PMIC axp202 and axp209 Carlo Caione
2014-02-08 16:03 ` [PATCH 1/3] mfd: axp20x: Add mfd driver for axp20x PMIC Carlo Caione
2014-02-10 20:02 ` Maxime Ripard
2014-02-10 20:25 ` [linux-sunxi] " Carlo Caione
2014-02-10 21:19 ` Lee Jones
2014-02-10 22:34 ` [linux-sunxi] " Carlo Caione
2014-02-11 6:58 ` Carlo Caione
2014-02-11 9:15 ` Lee Jones
2014-02-11 9:33 ` Carlo Caione
2014-02-11 10:08 ` Lee Jones
2014-02-08 16:03 ` [PATCH 2/3] mfd: axp20x: Add dtsi for axp20x Carlo Caione
2014-02-10 19:59 ` Maxime Ripard
2014-02-10 20:09 ` [linux-sunxi] " Carlo Caione
2014-02-10 20:08 ` Lee Jones
2014-02-10 20:10 ` Carlo Caione
2014-02-08 16:03 ` [PATCH 3/3] mfd: axp20x: Add bindings documentation Carlo Caione
2014-02-10 20:05 ` Lee Jones
2014-02-10 20:12 ` Maxime Ripard
2014-02-10 20:37 ` [linux-sunxi] " Carlo Caione
2014-02-10 22:01 ` Maxime Ripard
2014-02-10 22:38 ` Carlo Caione
2014-02-12 17:16 ` Mark Brown
2014-02-12 17:12 ` Mark Brown
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.