All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Initial support for XPowers AXP288 PMIC
@ 2014-09-09 13:02 ` Jacob Pan
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jacob Pan

XPowers AXP288 is a customized PMIC found on some Intel Baytrail-CR platforms.
It comes with sub-functions such as USB charging, fuel gauge, ADC, and many LDO
and BUCK channels.

By extending the existing AXP20x driver, this patchset adds basic support
for AXP288 PMIC with GPADC as one MFD cell device driver. It also adds hooks
for ACPI opregion handler driver which can be used to handle ACPI requests.

Currently, the PMIC driver in this patchset does not support platform data
enumeration. But when ACPI _DSD and unified device properties become available,
cell devices with platform data will be added.

This patch does not use intel_soc_pmic core for i2c and regmap handling in that
axp288 shares similar programming interface with other Xpower PMICs supported in
axp20x.c. Therefore, extending axp20x.c to include axp288 makes more sense.

Changes v2:
	- use format -M for 1/4
	- minor tweak based on Maxime's review

Jacob Pan (4):
  mfd/axp20x: rename files to support more devices
  mfd/axp2xx: extend axp20x to support axp288 pmic
  regulator/axp20x: use axp2xx consolidated header
  iio/adc/axp288: add support for axp288 gpadc

 drivers/iio/adc/Kconfig                  |   8 +
 drivers/iio/adc/Makefile                 |   1 +
 drivers/iio/adc/axp288_gpadc.c           | 250 ++++++++++++++++
 drivers/mfd/Kconfig                      |   7 +-
 drivers/mfd/Makefile                     |   2 +-
 drivers/mfd/axp20x.c                     | 258 ----------------
 drivers/mfd/axp2xx.c                     | 496 +++++++++++++++++++++++++++++++
 drivers/regulator/axp20x-regulator.c     |   6 +-
 include/linux/mfd/{axp20x.h => axp2xx.h} |  57 +++-
 9 files changed, 819 insertions(+), 266 deletions(-)
 create mode 100644 drivers/iio/adc/axp288_gpadc.c
 delete mode 100644 drivers/mfd/axp20x.c
 create mode 100644 drivers/mfd/axp2xx.c
 rename include/linux/mfd/{axp20x.h => axp2xx.h} (79%)

-- 
1.9.1


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

* [PATCH v2 0/4] Initial support for XPowers AXP288 PMIC
@ 2014-09-09 13:02 ` Jacob Pan
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jaco

XPowers AXP288 is a customized PMIC found on some Intel Baytrail-CR platforms.
It comes with sub-functions such as USB charging, fuel gauge, ADC, and many LDO
and BUCK channels.

By extending the existing AXP20x driver, this patchset adds basic support
for AXP288 PMIC with GPADC as one MFD cell device driver. It also adds hooks
for ACPI opregion handler driver which can be used to handle ACPI requests.

Currently, the PMIC driver in this patchset does not support platform data
enumeration. But when ACPI _DSD and unified device properties become available,
cell devices with platform data will be added.

This patch does not use intel_soc_pmic core for i2c and regmap handling in that
axp288 shares similar programming interface with other Xpower PMICs supported in
axp20x.c. Therefore, extending axp20x.c to include axp288 makes more sense.

Changes v2:
	- use format -M for 1/4
	- minor tweak based on Maxime's review

Jacob Pan (4):
  mfd/axp20x: rename files to support more devices
  mfd/axp2xx: extend axp20x to support axp288 pmic
  regulator/axp20x: use axp2xx consolidated header
  iio/adc/axp288: add support for axp288 gpadc

 drivers/iio/adc/Kconfig                  |   8 +
 drivers/iio/adc/Makefile                 |   1 +
 drivers/iio/adc/axp288_gpadc.c           | 250 ++++++++++++++++
 drivers/mfd/Kconfig                      |   7 +-
 drivers/mfd/Makefile                     |   2 +-
 drivers/mfd/axp20x.c                     | 258 ----------------
 drivers/mfd/axp2xx.c                     | 496 +++++++++++++++++++++++++++++++
 drivers/regulator/axp20x-regulator.c     |   6 +-
 include/linux/mfd/{axp20x.h => axp2xx.h} |  57 +++-
 9 files changed, 819 insertions(+), 266 deletions(-)
 create mode 100644 drivers/iio/adc/axp288_gpadc.c
 delete mode 100644 drivers/mfd/axp20x.c
 create mode 100644 drivers/mfd/axp2xx.c
 rename include/linux/mfd/{axp20x.h => axp2xx.h} (79%)

-- 
1.9.1

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

* [PATCH v2 1/4] mfd/axp20x: rename files to support more devices
  2014-09-09 13:02 ` Jacob Pan
@ 2014-09-09 13:02   ` Jacob Pan
  -1 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jacob Pan

More XPowers PMIC devices can be supported by extending this driver, so
rename it to axp2xx to cover axp288 variant.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/mfd/Kconfig                      | 7 ++++---
 drivers/mfd/Makefile                     | 2 +-
 drivers/mfd/{axp20x.c => axp2xx.c}       | 2 +-
 include/linux/mfd/{axp20x.h => axp2xx.h} | 0
 4 files changed, 6 insertions(+), 5 deletions(-)
 rename drivers/mfd/{axp20x.c => axp2xx.c} (99%)
 rename include/linux/mfd/{axp20x.h => axp2xx.h} (100%)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..42a70a3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -67,14 +67,15 @@ config MFD_BCM590XX
 	help
 	  Support for the BCM590xx PMUs from Broadcom
 
-config MFD_AXP20X
-	bool "X-Powers AXP20X"
+config MFD_AXP2XX
+	bool "X-Powers AXP2XX"
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
 	depends on I2C=y
 	help
-	  If you say Y here you get support for the X-Powers AXP202 and AXP209.
+	  If you say Y here you get support for the X-Powers AXP202, AXP209 and
+	  AXP288 power management IC (PMIC).
 	  This driver include only the core APIs. You have to select individual
 	  components like regulators or the PEK (Power Enable Key) under the
 	  corresponding menus.
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..55d76b3 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -103,7 +103,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_AXP2XX)	+= axp2xx.o
 
 obj-$(CONFIG_MFD_LP3943)	+= lp3943.o
 obj-$(CONFIG_MFD_LP8788)	+= lp8788.o lp8788-irq.o
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp2xx.c
similarity index 99%
rename from drivers/mfd/axp20x.c
rename to drivers/mfd/axp2xx.c
index dee6539..c534443 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp2xx.c
@@ -21,7 +21,7 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/regulator/consumer.h>
-#include <linux/mfd/axp20x.h>
+#include <linux/mfd/axp2xx.h>
 #include <linux/mfd/core.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp2xx.h
similarity index 100%
rename from include/linux/mfd/axp20x.h
rename to include/linux/mfd/axp2xx.h
-- 
1.9.1


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

* [PATCH v2 1/4] mfd/axp20x: rename files to support more devices
@ 2014-09-09 13:02   ` Jacob Pan
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jaco

More XPowers PMIC devices can be supported by extending this driver, so
rename it to axp2xx to cover axp288 variant.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/mfd/Kconfig                      | 7 ++++---
 drivers/mfd/Makefile                     | 2 +-
 drivers/mfd/{axp20x.c => axp2xx.c}       | 2 +-
 include/linux/mfd/{axp20x.h => axp2xx.h} | 0
 4 files changed, 6 insertions(+), 5 deletions(-)
 rename drivers/mfd/{axp20x.c => axp2xx.c} (99%)
 rename include/linux/mfd/{axp20x.h => axp2xx.h} (100%)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..42a70a3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -67,14 +67,15 @@ config MFD_BCM590XX
 	help
 	  Support for the BCM590xx PMUs from Broadcom
 
-config MFD_AXP20X
-	bool "X-Powers AXP20X"
+config MFD_AXP2XX
+	bool "X-Powers AXP2XX"
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
 	depends on I2C=y
 	help
-	  If you say Y here you get support for the X-Powers AXP202 and AXP209.
+	  If you say Y here you get support for the X-Powers AXP202, AXP209 and
+	  AXP288 power management IC (PMIC).
 	  This driver include only the core APIs. You have to select individual
 	  components like regulators or the PEK (Power Enable Key) under the
 	  corresponding menus.
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..55d76b3 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -103,7 +103,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_AXP2XX)	+= axp2xx.o
 
 obj-$(CONFIG_MFD_LP3943)	+= lp3943.o
 obj-$(CONFIG_MFD_LP8788)	+= lp8788.o lp8788-irq.o
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp2xx.c
similarity index 99%
rename from drivers/mfd/axp20x.c
rename to drivers/mfd/axp2xx.c
index dee6539..c534443 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp2xx.c
@@ -21,7 +21,7 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/regulator/consumer.h>
-#include <linux/mfd/axp20x.h>
+#include <linux/mfd/axp2xx.h>
 #include <linux/mfd/core.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp2xx.h
similarity index 100%
rename from include/linux/mfd/axp20x.h
rename to include/linux/mfd/axp2xx.h
-- 
1.9.1

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

* [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic
@ 2014-09-09 13:02   ` Jacob Pan
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jacob Pan

XPower AXP288 is a customized PMIC for Intel Baytrail-CR platforms. Similar
to AXP202/209, AXP288 comes with USB charger, more LDO and BUCK channels, and
AD converter. It also provides extended status and interrupt reporting
capabilities than the devices supported in axp20x.c.

In addition to feature extension, this patch also adds ACPI binding for
enumeration and hooks for ACPI custom operational region handlers.

Files and common data structures have been renamed from axp20x to axp2xx
in order to suit the extended scope of devices.

This consolidated driver should support more Xpower PMICs in both device
tree and ACPI based platforms.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/mfd/axp2xx.c       | 426 +++++++++++++++++++++++++++++++++++----------
 include/linux/mfd/axp2xx.h |  57 +++++-
 2 files changed, 388 insertions(+), 95 deletions(-)

diff --git a/drivers/mfd/axp2xx.c b/drivers/mfd/axp2xx.c
index c534443..4830bbe 100644
--- a/drivers/mfd/axp2xx.c
+++ b/drivers/mfd/axp2xx.c
@@ -1,9 +1,9 @@
 /*
- * axp20x.c - MFD core driver for the X-Powers AXP202 and AXP209
+ * axp2xx.c - MFD core driver for the X-Powers AXP202, AXP209, and AXP288
  *
- * AXP20x comprises an adaptive USB-Compatible PWM charger, 2 BUCK DC-DC
- * converters, 5 LDOs, multiple 12-bit ADCs of voltage, current and temperature
- * as well as 4 configurable GPIOs.
+ * AXP2xx typically comprises an adaptive USB-Compatible PWM charger, BUCK DC-DC
+ * converters, LDOs, multiple 12-bit ADCs of voltage, current and temperature
+ * as well as configurable GPIOs.
  *
  * Author: Carlo Caione <carlo@caione.org>
  *
@@ -25,9 +25,14 @@
 #include <linux/mfd/core.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
+#include <linux/acpi.h>
 
 #define AXP20X_OFF	0x80
 
+static struct mfd_cell *axp2xx_cells;
+static int axp2xx_nr_cells;
+static struct regmap_config *regmap_cfg;
+
 static const struct regmap_range axp20x_writeable_ranges[] = {
 	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
 	regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
@@ -47,6 +52,25 @@ static const struct regmap_access_table axp20x_volatile_table = {
 	.n_yes_ranges	= ARRAY_SIZE(axp20x_volatile_ranges),
 };
 
+static const struct regmap_range axp288_writeable_ranges[] = {
+	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ6_STATE),
+	regmap_reg_range(AXP20X_DCDC_MODE, AXP288_FG_TUNE5),
+};
+
+static const struct regmap_range axp288_volatile_ranges[] = {
+	regmap_reg_range(AXP20X_IRQ1_EN,  AXP20X_IPSOUT_V_HIGH_L),
+};
+
+static const struct regmap_access_table axp288_writeable_table = {
+	.yes_ranges	= axp288_writeable_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(axp288_writeable_ranges),
+};
+
+static const struct regmap_access_table axp288_volatile_table = {
+	.yes_ranges	= axp288_volatile_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(axp288_volatile_ranges),
+};
+
 static struct resource axp20x_pek_resources[] = {
 	{
 		.name	= "PEK_DBR",
@@ -61,7 +85,40 @@ static struct resource axp20x_pek_resources[] = {
 	},
 };
 
-static const struct regmap_config axp20x_regmap_config = {
+static struct resource axp288_battery_resources[] = {
+	{
+		.start = AXP288_IRQ_QWBTU,
+		.end   = AXP288_IRQ_QWBTU,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_WBTU,
+		.end   = AXP288_IRQ_WBTU,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_QWBTO,
+		.end   = AXP288_IRQ_QWBTO,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_WBTO,
+		.end   = AXP288_IRQ_WBTO,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_WL2,
+		.end   = AXP288_IRQ_WL2,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_WL1,
+		.end   = AXP288_IRQ_WL1,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct regmap_config axp20x_regmap_config = {
 	.reg_bits	= 8,
 	.val_bits	= 8,
 	.wr_table	= &axp20x_writeable_table,
@@ -70,47 +127,96 @@ static const struct regmap_config axp20x_regmap_config = {
 	.cache_type	= REGCACHE_RBTREE,
 };
 
-#define AXP20X_IRQ(_irq, _off, _mask) \
-	[AXP20X_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
+static struct regmap_config axp288_regmap_config = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.wr_table	= &axp288_writeable_table,
+	.volatile_table	= &axp288_volatile_table,
+	.max_register	= AXP288_FG_TUNE5,
+	.cache_type	= REGCACHE_RBTREE,
+};
+
+#define INIT_REGMAP_IRQ(_variant, _irq, _off, _mask)			\
+	[_variant##_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),
+	INIT_REGMAP_IRQ(AXP20X, ACIN_OVER_V,		0, 7),
+	INIT_REGMAP_IRQ(AXP20X, ACIN_PLUGIN,		0, 6),
+	INIT_REGMAP_IRQ(AXP20X, ACIN_REMOVAL,	        0, 5),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_OVER_V,		0, 4),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_PLUGIN,		0, 3),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_REMOVAL,	        0, 2),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_V_LOW,		0, 1),
+	INIT_REGMAP_IRQ(AXP20X, BATT_PLUGIN,		1, 7),
+	INIT_REGMAP_IRQ(AXP20X, BATT_REMOVAL,	        1, 6),
+	INIT_REGMAP_IRQ(AXP20X, BATT_ENT_ACT_MODE,	1, 5),
+	INIT_REGMAP_IRQ(AXP20X, BATT_EXIT_ACT_MODE,	1, 4),
+	INIT_REGMAP_IRQ(AXP20X, CHARG,		        1, 3),
+	INIT_REGMAP_IRQ(AXP20X, CHARG_DONE,		1, 2),
+	INIT_REGMAP_IRQ(AXP20X, BATT_TEMP_HIGH,	        1, 1),
+	INIT_REGMAP_IRQ(AXP20X, BATT_TEMP_LOW,	        1, 0),
+	INIT_REGMAP_IRQ(AXP20X, DIE_TEMP_HIGH,	        2, 7),
+	INIT_REGMAP_IRQ(AXP20X, CHARG_I_LOW,		2, 6),
+	INIT_REGMAP_IRQ(AXP20X, DCDC1_V_LONG,	        2, 5),
+	INIT_REGMAP_IRQ(AXP20X, DCDC2_V_LONG,	        2, 4),
+	INIT_REGMAP_IRQ(AXP20X, DCDC3_V_LONG,	        2, 3),
+	INIT_REGMAP_IRQ(AXP20X, PEK_SHORT,		2, 1),
+	INIT_REGMAP_IRQ(AXP20X, PEK_LONG,		2, 0),
+	INIT_REGMAP_IRQ(AXP20X, N_OE_PWR_ON,		3, 7),
+	INIT_REGMAP_IRQ(AXP20X, N_OE_PWR_OFF,	        3, 6),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_VALID,		3, 5),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_NOT_VALID,	        3, 4),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_SESS_VALID,	3, 3),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_SESS_END,	        3, 2),
+	INIT_REGMAP_IRQ(AXP20X, LOW_PWR_LVL1,	        3, 1),
+	INIT_REGMAP_IRQ(AXP20X, LOW_PWR_LVL2,	        3, 0),
+	INIT_REGMAP_IRQ(AXP20X, TIMER,		        4, 7),
+	INIT_REGMAP_IRQ(AXP20X, PEK_RIS_EDGE,	        4, 6),
+	INIT_REGMAP_IRQ(AXP20X, PEK_FAL_EDGE,	        4, 5),
+	INIT_REGMAP_IRQ(AXP20X, GPIO3_INPUT,		4, 3),
+	INIT_REGMAP_IRQ(AXP20X, GPIO2_INPUT,		4, 2),
+	INIT_REGMAP_IRQ(AXP20X, GPIO1_INPUT,		4, 1),
+	INIT_REGMAP_IRQ(AXP20X, GPIO0_INPUT,		4, 0),
+};
+
+/* some IRQs are compatible with axp20x models */
+static const struct regmap_irq axp288_regmap_irqs[] = {
+	INIT_REGMAP_IRQ(AXP20X, VBUS_REMOVAL,           0, 2),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_PLUGIN,            0, 3),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_OVER_V,            0, 4),
+
+	INIT_REGMAP_IRQ(AXP20X, CHARG_DONE,             1, 2),
+	INIT_REGMAP_IRQ(AXP20X, CHARG,                  1, 3),
+	INIT_REGMAP_IRQ(AXP288, SAFE_QUIT,              1, 4),
+	INIT_REGMAP_IRQ(AXP288, SAFE_ENTER,             1, 5),
+	INIT_REGMAP_IRQ(AXP20X, BATT_REMOVAL,           1, 6),
+	INIT_REGMAP_IRQ(AXP20X, BATT_PLUGIN,            1, 7),
+
+	INIT_REGMAP_IRQ(AXP288, QWBTU,                  2, 0),
+	INIT_REGMAP_IRQ(AXP288, WBTU,                   2, 1),
+	INIT_REGMAP_IRQ(AXP288, QWBTO,                  2, 2),
+	INIT_REGMAP_IRQ(AXP288, WBTU,                   2, 3),
+	INIT_REGMAP_IRQ(AXP288, QCBTU,                  2, 4),
+	INIT_REGMAP_IRQ(AXP288, CBTU,                   2, 5),
+	INIT_REGMAP_IRQ(AXP288, QCBTO,                  2, 6),
+	INIT_REGMAP_IRQ(AXP288, CBTO,                   2, 7),
+
+	INIT_REGMAP_IRQ(AXP288, WL2,                    3, 0),
+	INIT_REGMAP_IRQ(AXP288, WL1,                    3, 1),
+	INIT_REGMAP_IRQ(AXP288, GPADC,                  3, 2),
+	INIT_REGMAP_IRQ(AXP288, OT,                     3, 7),
+
+	INIT_REGMAP_IRQ(AXP288, GPIO0,                  4, 0),
+	INIT_REGMAP_IRQ(AXP288, GPIO1,                  4, 1),
+	INIT_REGMAP_IRQ(AXP288, POKO,                   4, 2),
+	INIT_REGMAP_IRQ(AXP288, POKL,                   4, 3),
+	INIT_REGMAP_IRQ(AXP288, POKS,                   4, 4),
+	INIT_REGMAP_IRQ(AXP288, POKN,                   4, 5),
+	INIT_REGMAP_IRQ(AXP288, POKP,                   4, 6),
+	INIT_REGMAP_IRQ(AXP20X, TIMER,                  4, 7),
+
+	INIT_REGMAP_IRQ(AXP288, MV_CHNG,                5, 0),
+	INIT_REGMAP_IRQ(AXP288, BC_USB_CHNG,            5, 1),
 };
 
 static const struct of_device_id axp20x_of_match[] = {
@@ -123,19 +229,26 @@ MODULE_DEVICE_TABLE(of, axp20x_of_match);
 /*
  * This is useless for OF-enabled devices, but it is needed by I2C subsystem
  */
-static const struct i2c_device_id axp20x_i2c_id[] = {
+static const struct i2c_device_id axp2xx_i2c_id[] = {
 	{ },
 };
-MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
+MODULE_DEVICE_TABLE(i2c, axp2xx_i2c_id);
 
-static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
-	.name			= "axp20x_irq_chip",
+static struct acpi_device_id axp2xx_acpi_match[] = {
+	{
+		.id = "INT33F4",
+		.driver_data = (kernel_ulong_t)AXP288_ID,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, axp2xx_acpi_match);
+
+/* common irq chip attributes only */
+static struct regmap_irq_chip axp2xx_regmap_irq_chip = {
+	.name			= "axp2xx_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		= true,
 	.init_ack_masked	= true,
 };
@@ -161,98 +274,223 @@ static struct mfd_cell axp20x_cells[] = {
 	},
 };
 
-static struct axp20x_dev *axp20x_pm_power_off;
-static void axp20x_power_off(void)
+static struct resource axp288_adc_resources[] = {
+	{
+		.name	= "GPADC",
+		.start = AXP288_IRQ_GPADC,
+		.end   = AXP288_IRQ_GPADC,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct resource axp288_charger_resources[] = {
+	{
+		.start = AXP288_IRQ_OV,
+		.end   = AXP288_IRQ_OV,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_DONE,
+		.end   = AXP288_IRQ_DONE,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_CHARGING,
+		.end   = AXP288_IRQ_CHARGING,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_SAFE_QUIT,
+		.end   = AXP288_IRQ_SAFE_QUIT,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_SAFE_ENTER,
+		.end   = AXP288_IRQ_SAFE_ENTER,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_QCBTU,
+		.end   = AXP288_IRQ_QCBTU,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_CBTU,
+		.end   = AXP288_IRQ_CBTU,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_QCBTO,
+		.end   = AXP288_IRQ_QCBTO,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_CBTO,
+		.end   = AXP288_IRQ_CBTO,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct mfd_cell axp288_cells[] = {
+	{
+		.name = "axp288_adc",
+		.num_resources = ARRAY_SIZE(axp288_adc_resources),
+		.resources = axp288_adc_resources,
+	},
+	{
+		.name = "axp288_charger",
+		.num_resources = ARRAY_SIZE(axp288_charger_resources),
+		.resources = axp288_charger_resources,
+	},
+	{
+		.name = "axp288_battery",
+		.num_resources = ARRAY_SIZE(axp288_battery_resources),
+		.resources = axp288_battery_resources,
+	},
+	{
+		.name = "axp288_acpi_opregion",
+	},
+};
+
+static struct axp2xx_dev *axp2xx_pm_power_off;
+static void axp2xx_power_off(void)
 {
-	regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL,
+	if (axp2xx_pm_power_off->variant == AXP288_ID)
+		return;
+	regmap_write(axp2xx_pm_power_off->regmap, AXP20X_OFF_CTRL,
 		     AXP20X_OFF);
 }
 
-static int axp20x_i2c_probe(struct i2c_client *i2c,
-			 const struct i2c_device_id *id)
+static int axp2xx_match_device(struct axp2xx_dev *axp2xx, struct device *dev)
 {
-	struct axp20x_dev *axp20x;
+	const struct acpi_device_id *acpi_id;
 	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(axp2xx_of_match, dev);
+	if (of_id) {
+		axp2xx->variant = (long) of_id->data;
+		goto found_match;
+	}
 
-	of_id = of_match_device(axp20x_of_match, &i2c->dev);
-	if (!of_id) {
-		dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
+	acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!acpi_id || !acpi_id->driver_data) {
+		dev_err(dev, "Unable to setup AXP2XX ACPI data\n");
+		return -ENODEV;
+	}
+	axp2xx->variant = (long) acpi_id->driver_data;
+
+found_match:
+	switch (axp2xx->variant) {
+	case AXP202_ID:
+	case AXP209_ID:
+		dev_dbg(dev, "AXP2xx variant AXP202/209 found\n");
+		axp2xx_nr_cells = ARRAY_SIZE(axp20x_cells);
+		axp2xx_cells = axp20x_cells;
+		regmap_cfg = &axp20x_regmap_config;
+		axp2xx_regmap_irq_chip.num_regs	= 5;
+		axp2xx_regmap_irq_chip.irqs = axp20x_regmap_irqs;
+		axp2xx_regmap_irq_chip.num_irqs	=
+			ARRAY_SIZE(axp20x_regmap_irqs);
+		break;
+	case AXP288_ID:
+		dev_dbg(dev, "AXP2xx variant AXP288 found\n");
+		axp2xx_cells = axp288_cells;
+		axp2xx_nr_cells = ARRAY_SIZE(axp288_cells);
+		axp2xx_regmap_irq_chip.irqs = axp288_regmap_irqs;
+		axp2xx_regmap_irq_chip.num_irqs	=
+			ARRAY_SIZE(axp288_regmap_irqs);
+		axp2xx_regmap_irq_chip.num_regs	= 6;
+		regmap_cfg = &axp288_regmap_config;
+		break;
+	default:
+		dev_err(dev, "unsupported AXP2XX ID %lu\n", axp2xx->variant);
 		return -ENODEV;
 	}
-	axp20x->variant = (long) of_id->data;
 
-	axp20x->i2c_client = i2c;
-	axp20x->dev = &i2c->dev;
-	dev_set_drvdata(axp20x->dev, axp20x);
+	return 0;
+}
+
+static int axp2xx_i2c_probe(struct i2c_client *i2c,
+			const struct i2c_device_id *id)
+{
+	struct axp2xx_dev *axp2xx;
+	int ret;
+
+	axp2xx = devm_kzalloc(&i2c->dev, sizeof(*axp2xx), GFP_KERNEL);
+	if (!axp2xx)
+		return -ENOMEM;
+
+	ret = axp2xx_match_device(axp2xx, &i2c->dev);
+	if (ret)
+		return ret;
+	axp2xx->i2c_client = i2c;
+	axp2xx->dev = &i2c->dev;
+	dev_set_drvdata(axp2xx->dev, axp2xx);
 
-	axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
-	if (IS_ERR(axp20x->regmap)) {
-		ret = PTR_ERR(axp20x->regmap);
+	axp2xx->regmap = devm_regmap_init_i2c(i2c, regmap_cfg);
+	if (IS_ERR(axp2xx->regmap)) {
+		ret = PTR_ERR(axp2xx->regmap);
 		dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
 		return ret;
 	}
 
-	ret = regmap_add_irq_chip(axp20x->regmap, i2c->irq,
+	ret = regmap_add_irq_chip(axp2xx->regmap, i2c->irq,
 				  IRQF_ONESHOT | IRQF_SHARED, -1,
-				  &axp20x_regmap_irq_chip,
-				  &axp20x->regmap_irqc);
+				  &axp2xx_regmap_irq_chip,
+				  &axp2xx->regmap_irqc);
 	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);
+	ret = mfd_add_devices(axp2xx->dev, -1, axp2xx_cells,
+			axp2xx_nr_cells, NULL, 0, NULL);
 
 	if (ret) {
 		dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
-		regmap_del_irq_chip(i2c->irq, axp20x->regmap_irqc);
+		regmap_del_irq_chip(i2c->irq, axp2xx->regmap_irqc);
 		return ret;
 	}
 
 	if (!pm_power_off) {
-		axp20x_pm_power_off = axp20x;
-		pm_power_off = axp20x_power_off;
+		axp2xx_pm_power_off = axp2xx;
+		pm_power_off = axp2xx_power_off;
 	}
 
-	dev_info(&i2c->dev, "AXP20X driver loaded\n");
+	dev_info(&i2c->dev, "AXP2XX driver loaded\n");
 
 	return 0;
 }
 
-static int axp20x_i2c_remove(struct i2c_client *i2c)
+static int axp2xx_i2c_remove(struct i2c_client *i2c)
 {
-	struct axp20x_dev *axp20x = i2c_get_clientdata(i2c);
+	struct axp2xx_dev *axp2xx = i2c_get_clientdata(i2c);
 
-	if (axp20x == axp20x_pm_power_off) {
-		axp20x_pm_power_off = NULL;
+	if (axp2xx == axp2xx_pm_power_off) {
+		axp2xx_pm_power_off = NULL;
 		pm_power_off = NULL;
 	}
 
-	mfd_remove_devices(axp20x->dev);
-	regmap_del_irq_chip(axp20x->i2c_client->irq, axp20x->regmap_irqc);
+	mfd_remove_devices(axp2xx->dev);
+	regmap_del_irq_chip(axp2xx->i2c_client->irq, axp2xx->regmap_irqc);
 
 	return 0;
 }
 
-static struct i2c_driver axp20x_i2c_driver = {
+static struct i2c_driver axp2xx_i2c_driver = {
 	.driver = {
-		.name	= "axp20x",
+		.name	= "axp2xx",
 		.owner	= THIS_MODULE,
-		.of_match_table	= of_match_ptr(axp20x_of_match),
+		.of_match_table	= of_match_ptr(axp2xx_of_match),
+		.acpi_match_table = ACPI_PTR(axp2xx_acpi_match),
 	},
-	.probe		= axp20x_i2c_probe,
-	.remove		= axp20x_i2c_remove,
-	.id_table	= axp20x_i2c_id,
+	.probe		= axp2xx_i2c_probe,
+	.remove		= axp2xx_i2c_remove,
+	.id_table	= axp2xx_i2c_id,
 };
 
-module_i2c_driver(axp20x_i2c_driver);
+module_i2c_driver(axp2xx_i2c_driver);
 
-MODULE_DESCRIPTION("PMIC MFD core driver for AXP20X");
+MODULE_DESCRIPTION("PMIC MFD core driver for AXP2XX");
 MODULE_AUTHOR("Carlo Caione <carlo@caione.org>");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/axp2xx.h b/include/linux/mfd/axp2xx.h
index d0e31a2..d73cad7 100644
--- a/include/linux/mfd/axp2xx.h
+++ b/include/linux/mfd/axp2xx.h
@@ -14,6 +14,8 @@
 enum {
 	AXP202_ID = 0,
 	AXP209_ID,
+	AXP288_ID,
+	NR_AXP2XX_VARIANTS,
 };
 
 #define AXP20X_DATACACHE(m)		(0x04 + (m))
@@ -49,11 +51,13 @@ enum {
 #define AXP20X_IRQ3_EN			0x42
 #define AXP20X_IRQ4_EN			0x43
 #define AXP20X_IRQ5_EN			0x44
+#define AXP20X_IRQ6_EN			0x45
 #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
+#define AXP20X_IRQ6_STATE		0x4d
 
 /* ADC */
 #define AXP20X_ACIN_V_ADC_H		0x56
@@ -116,6 +120,15 @@ enum {
 #define AXP20X_CC_CTRL			0xb8
 #define AXP20X_FG_RES			0xb9
 
+/* AXP288 specific registers */
+#define AXP288_PMIC_ADC_H               0x56
+#define AXP288_PMIC_ADC_L               0x57
+#define AXP288_ADC_TS_PIN_CTRL          0x84
+
+#define AXP288_PMIC_ADC_EN              0x84
+#define AXP288_FG_TUNE5			0xed
+
+
 /* Regulators IDs */
 enum {
 	AXP20X_LDO1 = 0,
@@ -169,7 +182,49 @@ enum {
 	AXP20X_IRQ_GPIO0_INPUT,
 };
 
-struct axp20x_dev {
+enum axp288_irqs {
+	AXP288_IRQ_VBUS_FALL     = 2,
+	AXP288_IRQ_VBUS_RISE,
+	AXP288_IRQ_OV,
+	AXP288_IRQ_FALLING_ALT,
+	AXP288_IRQ_RISING_ALT,
+	AXP288_IRQ_OV_ALT,
+	AXP288_IRQ_DONE          = 10,
+	AXP288_IRQ_CHARGING,
+	AXP288_IRQ_SAFE_QUIT,
+	AXP288_IRQ_SAFE_ENTER,
+	AXP288_IRQ_ABSENT,
+	AXP288_IRQ_APPEND,
+	AXP288_IRQ_QWBTU,
+	AXP288_IRQ_WBTU,
+	AXP288_IRQ_QWBTO,
+	AXP288_IRQ_WBTO,
+	AXP288_IRQ_QCBTU,
+	AXP288_IRQ_CBTU,
+	AXP288_IRQ_QCBTO,
+	AXP288_IRQ_CBTO,
+	AXP288_IRQ_WL2,
+	AXP288_IRQ_WL1,
+	AXP288_IRQ_GPADC,
+	AXP288_IRQ_OT            = 31,
+	AXP288_IRQ_GPIO0,
+	AXP288_IRQ_GPIO1,
+	AXP288_IRQ_POKO,
+	AXP288_IRQ_POKL,
+	AXP288_IRQ_POKS,
+	AXP288_IRQ_POKN,
+	AXP288_IRQ_POKP,
+	AXP288_IRQ_TIMER,
+	AXP288_IRQ_MV_CHNG,
+	AXP288_IRQ_BC_USB_CHNG,
+};
+
+#define AXP288_TS_ADC_H		0x58
+#define AXP288_TS_ADC_L		0x59
+#define AXP288_GP_ADC_H		0x5a
+#define AXP288_GP_ADC_L		0x5b
+
+struct axp2xx_dev {
 	struct device			*dev;
 	struct i2c_client		*i2c_client;
 	struct regmap			*regmap;
-- 
1.9.1


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

* [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic
@ 2014-09-09 13:02   ` Jacob Pan
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jaco

XPower AXP288 is a customized PMIC for Intel Baytrail-CR platforms. Similar
to AXP202/209, AXP288 comes with USB charger, more LDO and BUCK channels, and
AD converter. It also provides extended status and interrupt reporting
capabilities than the devices supported in axp20x.c.

In addition to feature extension, this patch also adds ACPI binding for
enumeration and hooks for ACPI custom operational region handlers.

Files and common data structures have been renamed from axp20x to axp2xx
in order to suit the extended scope of devices.

This consolidated driver should support more Xpower PMICs in both device
tree and ACPI based platforms.

Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/mfd/axp2xx.c       | 426 +++++++++++++++++++++++++++++++++++----------
 include/linux/mfd/axp2xx.h |  57 +++++-
 2 files changed, 388 insertions(+), 95 deletions(-)

diff --git a/drivers/mfd/axp2xx.c b/drivers/mfd/axp2xx.c
index c534443..4830bbe 100644
--- a/drivers/mfd/axp2xx.c
+++ b/drivers/mfd/axp2xx.c
@@ -1,9 +1,9 @@
 /*
- * axp20x.c - MFD core driver for the X-Powers AXP202 and AXP209
+ * axp2xx.c - MFD core driver for the X-Powers AXP202, AXP209, and AXP288
  *
- * AXP20x comprises an adaptive USB-Compatible PWM charger, 2 BUCK DC-DC
- * converters, 5 LDOs, multiple 12-bit ADCs of voltage, current and temperature
- * as well as 4 configurable GPIOs.
+ * AXP2xx typically comprises an adaptive USB-Compatible PWM charger, BUCK DC-DC
+ * converters, LDOs, multiple 12-bit ADCs of voltage, current and temperature
+ * as well as configurable GPIOs.
  *
  * Author: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
  *
@@ -25,9 +25,14 @@
 #include <linux/mfd/core.h>
 #include <linux/of_device.h>
 #include <linux/of_irq.h>
+#include <linux/acpi.h>
 
 #define AXP20X_OFF	0x80
 
+static struct mfd_cell *axp2xx_cells;
+static int axp2xx_nr_cells;
+static struct regmap_config *regmap_cfg;
+
 static const struct regmap_range axp20x_writeable_ranges[] = {
 	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
 	regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
@@ -47,6 +52,25 @@ static const struct regmap_access_table axp20x_volatile_table = {
 	.n_yes_ranges	= ARRAY_SIZE(axp20x_volatile_ranges),
 };
 
+static const struct regmap_range axp288_writeable_ranges[] = {
+	regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ6_STATE),
+	regmap_reg_range(AXP20X_DCDC_MODE, AXP288_FG_TUNE5),
+};
+
+static const struct regmap_range axp288_volatile_ranges[] = {
+	regmap_reg_range(AXP20X_IRQ1_EN,  AXP20X_IPSOUT_V_HIGH_L),
+};
+
+static const struct regmap_access_table axp288_writeable_table = {
+	.yes_ranges	= axp288_writeable_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(axp288_writeable_ranges),
+};
+
+static const struct regmap_access_table axp288_volatile_table = {
+	.yes_ranges	= axp288_volatile_ranges,
+	.n_yes_ranges	= ARRAY_SIZE(axp288_volatile_ranges),
+};
+
 static struct resource axp20x_pek_resources[] = {
 	{
 		.name	= "PEK_DBR",
@@ -61,7 +85,40 @@ static struct resource axp20x_pek_resources[] = {
 	},
 };
 
-static const struct regmap_config axp20x_regmap_config = {
+static struct resource axp288_battery_resources[] = {
+	{
+		.start = AXP288_IRQ_QWBTU,
+		.end   = AXP288_IRQ_QWBTU,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_WBTU,
+		.end   = AXP288_IRQ_WBTU,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_QWBTO,
+		.end   = AXP288_IRQ_QWBTO,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_WBTO,
+		.end   = AXP288_IRQ_WBTO,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_WL2,
+		.end   = AXP288_IRQ_WL2,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_WL1,
+		.end   = AXP288_IRQ_WL1,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct regmap_config axp20x_regmap_config = {
 	.reg_bits	= 8,
 	.val_bits	= 8,
 	.wr_table	= &axp20x_writeable_table,
@@ -70,47 +127,96 @@ static const struct regmap_config axp20x_regmap_config = {
 	.cache_type	= REGCACHE_RBTREE,
 };
 
-#define AXP20X_IRQ(_irq, _off, _mask) \
-	[AXP20X_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
+static struct regmap_config axp288_regmap_config = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.wr_table	= &axp288_writeable_table,
+	.volatile_table	= &axp288_volatile_table,
+	.max_register	= AXP288_FG_TUNE5,
+	.cache_type	= REGCACHE_RBTREE,
+};
+
+#define INIT_REGMAP_IRQ(_variant, _irq, _off, _mask)			\
+	[_variant##_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),
+	INIT_REGMAP_IRQ(AXP20X, ACIN_OVER_V,		0, 7),
+	INIT_REGMAP_IRQ(AXP20X, ACIN_PLUGIN,		0, 6),
+	INIT_REGMAP_IRQ(AXP20X, ACIN_REMOVAL,	        0, 5),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_OVER_V,		0, 4),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_PLUGIN,		0, 3),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_REMOVAL,	        0, 2),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_V_LOW,		0, 1),
+	INIT_REGMAP_IRQ(AXP20X, BATT_PLUGIN,		1, 7),
+	INIT_REGMAP_IRQ(AXP20X, BATT_REMOVAL,	        1, 6),
+	INIT_REGMAP_IRQ(AXP20X, BATT_ENT_ACT_MODE,	1, 5),
+	INIT_REGMAP_IRQ(AXP20X, BATT_EXIT_ACT_MODE,	1, 4),
+	INIT_REGMAP_IRQ(AXP20X, CHARG,		        1, 3),
+	INIT_REGMAP_IRQ(AXP20X, CHARG_DONE,		1, 2),
+	INIT_REGMAP_IRQ(AXP20X, BATT_TEMP_HIGH,	        1, 1),
+	INIT_REGMAP_IRQ(AXP20X, BATT_TEMP_LOW,	        1, 0),
+	INIT_REGMAP_IRQ(AXP20X, DIE_TEMP_HIGH,	        2, 7),
+	INIT_REGMAP_IRQ(AXP20X, CHARG_I_LOW,		2, 6),
+	INIT_REGMAP_IRQ(AXP20X, DCDC1_V_LONG,	        2, 5),
+	INIT_REGMAP_IRQ(AXP20X, DCDC2_V_LONG,	        2, 4),
+	INIT_REGMAP_IRQ(AXP20X, DCDC3_V_LONG,	        2, 3),
+	INIT_REGMAP_IRQ(AXP20X, PEK_SHORT,		2, 1),
+	INIT_REGMAP_IRQ(AXP20X, PEK_LONG,		2, 0),
+	INIT_REGMAP_IRQ(AXP20X, N_OE_PWR_ON,		3, 7),
+	INIT_REGMAP_IRQ(AXP20X, N_OE_PWR_OFF,	        3, 6),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_VALID,		3, 5),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_NOT_VALID,	        3, 4),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_SESS_VALID,	3, 3),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_SESS_END,	        3, 2),
+	INIT_REGMAP_IRQ(AXP20X, LOW_PWR_LVL1,	        3, 1),
+	INIT_REGMAP_IRQ(AXP20X, LOW_PWR_LVL2,	        3, 0),
+	INIT_REGMAP_IRQ(AXP20X, TIMER,		        4, 7),
+	INIT_REGMAP_IRQ(AXP20X, PEK_RIS_EDGE,	        4, 6),
+	INIT_REGMAP_IRQ(AXP20X, PEK_FAL_EDGE,	        4, 5),
+	INIT_REGMAP_IRQ(AXP20X, GPIO3_INPUT,		4, 3),
+	INIT_REGMAP_IRQ(AXP20X, GPIO2_INPUT,		4, 2),
+	INIT_REGMAP_IRQ(AXP20X, GPIO1_INPUT,		4, 1),
+	INIT_REGMAP_IRQ(AXP20X, GPIO0_INPUT,		4, 0),
+};
+
+/* some IRQs are compatible with axp20x models */
+static const struct regmap_irq axp288_regmap_irqs[] = {
+	INIT_REGMAP_IRQ(AXP20X, VBUS_REMOVAL,           0, 2),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_PLUGIN,            0, 3),
+	INIT_REGMAP_IRQ(AXP20X, VBUS_OVER_V,            0, 4),
+
+	INIT_REGMAP_IRQ(AXP20X, CHARG_DONE,             1, 2),
+	INIT_REGMAP_IRQ(AXP20X, CHARG,                  1, 3),
+	INIT_REGMAP_IRQ(AXP288, SAFE_QUIT,              1, 4),
+	INIT_REGMAP_IRQ(AXP288, SAFE_ENTER,             1, 5),
+	INIT_REGMAP_IRQ(AXP20X, BATT_REMOVAL,           1, 6),
+	INIT_REGMAP_IRQ(AXP20X, BATT_PLUGIN,            1, 7),
+
+	INIT_REGMAP_IRQ(AXP288, QWBTU,                  2, 0),
+	INIT_REGMAP_IRQ(AXP288, WBTU,                   2, 1),
+	INIT_REGMAP_IRQ(AXP288, QWBTO,                  2, 2),
+	INIT_REGMAP_IRQ(AXP288, WBTU,                   2, 3),
+	INIT_REGMAP_IRQ(AXP288, QCBTU,                  2, 4),
+	INIT_REGMAP_IRQ(AXP288, CBTU,                   2, 5),
+	INIT_REGMAP_IRQ(AXP288, QCBTO,                  2, 6),
+	INIT_REGMAP_IRQ(AXP288, CBTO,                   2, 7),
+
+	INIT_REGMAP_IRQ(AXP288, WL2,                    3, 0),
+	INIT_REGMAP_IRQ(AXP288, WL1,                    3, 1),
+	INIT_REGMAP_IRQ(AXP288, GPADC,                  3, 2),
+	INIT_REGMAP_IRQ(AXP288, OT,                     3, 7),
+
+	INIT_REGMAP_IRQ(AXP288, GPIO0,                  4, 0),
+	INIT_REGMAP_IRQ(AXP288, GPIO1,                  4, 1),
+	INIT_REGMAP_IRQ(AXP288, POKO,                   4, 2),
+	INIT_REGMAP_IRQ(AXP288, POKL,                   4, 3),
+	INIT_REGMAP_IRQ(AXP288, POKS,                   4, 4),
+	INIT_REGMAP_IRQ(AXP288, POKN,                   4, 5),
+	INIT_REGMAP_IRQ(AXP288, POKP,                   4, 6),
+	INIT_REGMAP_IRQ(AXP20X, TIMER,                  4, 7),
+
+	INIT_REGMAP_IRQ(AXP288, MV_CHNG,                5, 0),
+	INIT_REGMAP_IRQ(AXP288, BC_USB_CHNG,            5, 1),
 };
 
 static const struct of_device_id axp20x_of_match[] = {
@@ -123,19 +229,26 @@ MODULE_DEVICE_TABLE(of, axp20x_of_match);
 /*
  * This is useless for OF-enabled devices, but it is needed by I2C subsystem
  */
-static const struct i2c_device_id axp20x_i2c_id[] = {
+static const struct i2c_device_id axp2xx_i2c_id[] = {
 	{ },
 };
-MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
+MODULE_DEVICE_TABLE(i2c, axp2xx_i2c_id);
 
-static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
-	.name			= "axp20x_irq_chip",
+static struct acpi_device_id axp2xx_acpi_match[] = {
+	{
+		.id = "INT33F4",
+		.driver_data = (kernel_ulong_t)AXP288_ID,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, axp2xx_acpi_match);
+
+/* common irq chip attributes only */
+static struct regmap_irq_chip axp2xx_regmap_irq_chip = {
+	.name			= "axp2xx_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		= true,
 	.init_ack_masked	= true,
 };
@@ -161,98 +274,223 @@ static struct mfd_cell axp20x_cells[] = {
 	},
 };
 
-static struct axp20x_dev *axp20x_pm_power_off;
-static void axp20x_power_off(void)
+static struct resource axp288_adc_resources[] = {
+	{
+		.name	= "GPADC",
+		.start = AXP288_IRQ_GPADC,
+		.end   = AXP288_IRQ_GPADC,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct resource axp288_charger_resources[] = {
+	{
+		.start = AXP288_IRQ_OV,
+		.end   = AXP288_IRQ_OV,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_DONE,
+		.end   = AXP288_IRQ_DONE,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_CHARGING,
+		.end   = AXP288_IRQ_CHARGING,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_SAFE_QUIT,
+		.end   = AXP288_IRQ_SAFE_QUIT,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_SAFE_ENTER,
+		.end   = AXP288_IRQ_SAFE_ENTER,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_QCBTU,
+		.end   = AXP288_IRQ_QCBTU,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_CBTU,
+		.end   = AXP288_IRQ_CBTU,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_QCBTO,
+		.end   = AXP288_IRQ_QCBTO,
+		.flags = IORESOURCE_IRQ,
+	},
+	{
+		.start = AXP288_IRQ_CBTO,
+		.end   = AXP288_IRQ_CBTO,
+		.flags = IORESOURCE_IRQ,
+	},
+};
+
+static struct mfd_cell axp288_cells[] = {
+	{
+		.name = "axp288_adc",
+		.num_resources = ARRAY_SIZE(axp288_adc_resources),
+		.resources = axp288_adc_resources,
+	},
+	{
+		.name = "axp288_charger",
+		.num_resources = ARRAY_SIZE(axp288_charger_resources),
+		.resources = axp288_charger_resources,
+	},
+	{
+		.name = "axp288_battery",
+		.num_resources = ARRAY_SIZE(axp288_battery_resources),
+		.resources = axp288_battery_resources,
+	},
+	{
+		.name = "axp288_acpi_opregion",
+	},
+};
+
+static struct axp2xx_dev *axp2xx_pm_power_off;
+static void axp2xx_power_off(void)
 {
-	regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL,
+	if (axp2xx_pm_power_off->variant == AXP288_ID)
+		return;
+	regmap_write(axp2xx_pm_power_off->regmap, AXP20X_OFF_CTRL,
 		     AXP20X_OFF);
 }
 
-static int axp20x_i2c_probe(struct i2c_client *i2c,
-			 const struct i2c_device_id *id)
+static int axp2xx_match_device(struct axp2xx_dev *axp2xx, struct device *dev)
 {
-	struct axp20x_dev *axp20x;
+	const struct acpi_device_id *acpi_id;
 	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(axp2xx_of_match, dev);
+	if (of_id) {
+		axp2xx->variant = (long) of_id->data;
+		goto found_match;
+	}
 
-	of_id = of_match_device(axp20x_of_match, &i2c->dev);
-	if (!of_id) {
-		dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
+	acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!acpi_id || !acpi_id->driver_data) {
+		dev_err(dev, "Unable to setup AXP2XX ACPI data\n");
+		return -ENODEV;
+	}
+	axp2xx->variant = (long) acpi_id->driver_data;
+
+found_match:
+	switch (axp2xx->variant) {
+	case AXP202_ID:
+	case AXP209_ID:
+		dev_dbg(dev, "AXP2xx variant AXP202/209 found\n");
+		axp2xx_nr_cells = ARRAY_SIZE(axp20x_cells);
+		axp2xx_cells = axp20x_cells;
+		regmap_cfg = &axp20x_regmap_config;
+		axp2xx_regmap_irq_chip.num_regs	= 5;
+		axp2xx_regmap_irq_chip.irqs = axp20x_regmap_irqs;
+		axp2xx_regmap_irq_chip.num_irqs	=
+			ARRAY_SIZE(axp20x_regmap_irqs);
+		break;
+	case AXP288_ID:
+		dev_dbg(dev, "AXP2xx variant AXP288 found\n");
+		axp2xx_cells = axp288_cells;
+		axp2xx_nr_cells = ARRAY_SIZE(axp288_cells);
+		axp2xx_regmap_irq_chip.irqs = axp288_regmap_irqs;
+		axp2xx_regmap_irq_chip.num_irqs	=
+			ARRAY_SIZE(axp288_regmap_irqs);
+		axp2xx_regmap_irq_chip.num_regs	= 6;
+		regmap_cfg = &axp288_regmap_config;
+		break;
+	default:
+		dev_err(dev, "unsupported AXP2XX ID %lu\n", axp2xx->variant);
 		return -ENODEV;
 	}
-	axp20x->variant = (long) of_id->data;
 
-	axp20x->i2c_client = i2c;
-	axp20x->dev = &i2c->dev;
-	dev_set_drvdata(axp20x->dev, axp20x);
+	return 0;
+}
+
+static int axp2xx_i2c_probe(struct i2c_client *i2c,
+			const struct i2c_device_id *id)
+{
+	struct axp2xx_dev *axp2xx;
+	int ret;
+
+	axp2xx = devm_kzalloc(&i2c->dev, sizeof(*axp2xx), GFP_KERNEL);
+	if (!axp2xx)
+		return -ENOMEM;
+
+	ret = axp2xx_match_device(axp2xx, &i2c->dev);
+	if (ret)
+		return ret;
+	axp2xx->i2c_client = i2c;
+	axp2xx->dev = &i2c->dev;
+	dev_set_drvdata(axp2xx->dev, axp2xx);
 
-	axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
-	if (IS_ERR(axp20x->regmap)) {
-		ret = PTR_ERR(axp20x->regmap);
+	axp2xx->regmap = devm_regmap_init_i2c(i2c, regmap_cfg);
+	if (IS_ERR(axp2xx->regmap)) {
+		ret = PTR_ERR(axp2xx->regmap);
 		dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
 		return ret;
 	}
 
-	ret = regmap_add_irq_chip(axp20x->regmap, i2c->irq,
+	ret = regmap_add_irq_chip(axp2xx->regmap, i2c->irq,
 				  IRQF_ONESHOT | IRQF_SHARED, -1,
-				  &axp20x_regmap_irq_chip,
-				  &axp20x->regmap_irqc);
+				  &axp2xx_regmap_irq_chip,
+				  &axp2xx->regmap_irqc);
 	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);
+	ret = mfd_add_devices(axp2xx->dev, -1, axp2xx_cells,
+			axp2xx_nr_cells, NULL, 0, NULL);
 
 	if (ret) {
 		dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
-		regmap_del_irq_chip(i2c->irq, axp20x->regmap_irqc);
+		regmap_del_irq_chip(i2c->irq, axp2xx->regmap_irqc);
 		return ret;
 	}
 
 	if (!pm_power_off) {
-		axp20x_pm_power_off = axp20x;
-		pm_power_off = axp20x_power_off;
+		axp2xx_pm_power_off = axp2xx;
+		pm_power_off = axp2xx_power_off;
 	}
 
-	dev_info(&i2c->dev, "AXP20X driver loaded\n");
+	dev_info(&i2c->dev, "AXP2XX driver loaded\n");
 
 	return 0;
 }
 
-static int axp20x_i2c_remove(struct i2c_client *i2c)
+static int axp2xx_i2c_remove(struct i2c_client *i2c)
 {
-	struct axp20x_dev *axp20x = i2c_get_clientdata(i2c);
+	struct axp2xx_dev *axp2xx = i2c_get_clientdata(i2c);
 
-	if (axp20x == axp20x_pm_power_off) {
-		axp20x_pm_power_off = NULL;
+	if (axp2xx == axp2xx_pm_power_off) {
+		axp2xx_pm_power_off = NULL;
 		pm_power_off = NULL;
 	}
 
-	mfd_remove_devices(axp20x->dev);
-	regmap_del_irq_chip(axp20x->i2c_client->irq, axp20x->regmap_irqc);
+	mfd_remove_devices(axp2xx->dev);
+	regmap_del_irq_chip(axp2xx->i2c_client->irq, axp2xx->regmap_irqc);
 
 	return 0;
 }
 
-static struct i2c_driver axp20x_i2c_driver = {
+static struct i2c_driver axp2xx_i2c_driver = {
 	.driver = {
-		.name	= "axp20x",
+		.name	= "axp2xx",
 		.owner	= THIS_MODULE,
-		.of_match_table	= of_match_ptr(axp20x_of_match),
+		.of_match_table	= of_match_ptr(axp2xx_of_match),
+		.acpi_match_table = ACPI_PTR(axp2xx_acpi_match),
 	},
-	.probe		= axp20x_i2c_probe,
-	.remove		= axp20x_i2c_remove,
-	.id_table	= axp20x_i2c_id,
+	.probe		= axp2xx_i2c_probe,
+	.remove		= axp2xx_i2c_remove,
+	.id_table	= axp2xx_i2c_id,
 };
 
-module_i2c_driver(axp20x_i2c_driver);
+module_i2c_driver(axp2xx_i2c_driver);
 
-MODULE_DESCRIPTION("PMIC MFD core driver for AXP20X");
+MODULE_DESCRIPTION("PMIC MFD core driver for AXP2XX");
 MODULE_AUTHOR("Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/axp2xx.h b/include/linux/mfd/axp2xx.h
index d0e31a2..d73cad7 100644
--- a/include/linux/mfd/axp2xx.h
+++ b/include/linux/mfd/axp2xx.h
@@ -14,6 +14,8 @@
 enum {
 	AXP202_ID = 0,
 	AXP209_ID,
+	AXP288_ID,
+	NR_AXP2XX_VARIANTS,
 };
 
 #define AXP20X_DATACACHE(m)		(0x04 + (m))
@@ -49,11 +51,13 @@ enum {
 #define AXP20X_IRQ3_EN			0x42
 #define AXP20X_IRQ4_EN			0x43
 #define AXP20X_IRQ5_EN			0x44
+#define AXP20X_IRQ6_EN			0x45
 #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
+#define AXP20X_IRQ6_STATE		0x4d
 
 /* ADC */
 #define AXP20X_ACIN_V_ADC_H		0x56
@@ -116,6 +120,15 @@ enum {
 #define AXP20X_CC_CTRL			0xb8
 #define AXP20X_FG_RES			0xb9
 
+/* AXP288 specific registers */
+#define AXP288_PMIC_ADC_H               0x56
+#define AXP288_PMIC_ADC_L               0x57
+#define AXP288_ADC_TS_PIN_CTRL          0x84
+
+#define AXP288_PMIC_ADC_EN              0x84
+#define AXP288_FG_TUNE5			0xed
+
+
 /* Regulators IDs */
 enum {
 	AXP20X_LDO1 = 0,
@@ -169,7 +182,49 @@ enum {
 	AXP20X_IRQ_GPIO0_INPUT,
 };
 
-struct axp20x_dev {
+enum axp288_irqs {
+	AXP288_IRQ_VBUS_FALL     = 2,
+	AXP288_IRQ_VBUS_RISE,
+	AXP288_IRQ_OV,
+	AXP288_IRQ_FALLING_ALT,
+	AXP288_IRQ_RISING_ALT,
+	AXP288_IRQ_OV_ALT,
+	AXP288_IRQ_DONE          = 10,
+	AXP288_IRQ_CHARGING,
+	AXP288_IRQ_SAFE_QUIT,
+	AXP288_IRQ_SAFE_ENTER,
+	AXP288_IRQ_ABSENT,
+	AXP288_IRQ_APPEND,
+	AXP288_IRQ_QWBTU,
+	AXP288_IRQ_WBTU,
+	AXP288_IRQ_QWBTO,
+	AXP288_IRQ_WBTO,
+	AXP288_IRQ_QCBTU,
+	AXP288_IRQ_CBTU,
+	AXP288_IRQ_QCBTO,
+	AXP288_IRQ_CBTO,
+	AXP288_IRQ_WL2,
+	AXP288_IRQ_WL1,
+	AXP288_IRQ_GPADC,
+	AXP288_IRQ_OT            = 31,
+	AXP288_IRQ_GPIO0,
+	AXP288_IRQ_GPIO1,
+	AXP288_IRQ_POKO,
+	AXP288_IRQ_POKL,
+	AXP288_IRQ_POKS,
+	AXP288_IRQ_POKN,
+	AXP288_IRQ_POKP,
+	AXP288_IRQ_TIMER,
+	AXP288_IRQ_MV_CHNG,
+	AXP288_IRQ_BC_USB_CHNG,
+};
+
+#define AXP288_TS_ADC_H		0x58
+#define AXP288_TS_ADC_L		0x59
+#define AXP288_GP_ADC_H		0x5a
+#define AXP288_GP_ADC_L		0x5b
+
+struct axp2xx_dev {
 	struct device			*dev;
 	struct i2c_client		*i2c_client;
 	struct regmap			*regmap;
-- 
1.9.1

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

* [PATCH v2 3/4] regulator/axp20x: use axp2xx consolidated header
  2014-09-09 13:02 ` Jacob Pan
@ 2014-09-09 13:02   ` Jacob Pan
  -1 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jacob Pan

AXP20x driver has been extended to support axp288 variant. Header file
and common data structures has also been renamed to suit the new
scope of devices supported.

This patch makes use of the renamed header and structure.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/regulator/axp20x-regulator.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 004aadb..c9b6803 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -20,7 +20,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
-#include <linux/mfd/axp20x.h>
+#include <linux/mfd/axp2xx.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
 
@@ -161,7 +161,7 @@ static struct of_regulator_match axp20x_matches[] = {
 
 static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
 {
-	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+	struct axp2xx_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 
 	if (dcdcfreq < 750) {
 		dcdcfreq = 750;
@@ -232,7 +232,7 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
 static int axp20x_regulator_probe(struct platform_device *pdev)
 {
 	struct regulator_dev *rdev;
-	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+	struct axp2xx_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 	struct regulator_config config = { };
 	struct regulator_init_data *init_data;
 	int ret, i;
-- 
1.9.1


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

* [PATCH v2 3/4] regulator/axp20x: use axp2xx consolidated header
@ 2014-09-09 13:02   ` Jacob Pan
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jaco

AXP20x driver has been extended to support axp288 variant. Header file
and common data structures has also been renamed to suit the new
scope of devices supported.

This patch makes use of the renamed header and structure.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/regulator/axp20x-regulator.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 004aadb..c9b6803 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -20,7 +20,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
-#include <linux/mfd/axp20x.h>
+#include <linux/mfd/axp2xx.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
 
@@ -161,7 +161,7 @@ static struct of_regulator_match axp20x_matches[] = {
 
 static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
 {
-	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+	struct axp2xx_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 
 	if (dcdcfreq < 750) {
 		dcdcfreq = 750;
@@ -232,7 +232,7 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
 static int axp20x_regulator_probe(struct platform_device *pdev)
 {
 	struct regulator_dev *rdev;
-	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+	struct axp2xx_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 	struct regulator_config config = { };
 	struct regulator_init_data *init_data;
 	int ret, i;
-- 
1.9.1

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

* [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc
  2014-09-09 13:02 ` Jacob Pan
@ 2014-09-09 13:02   ` Jacob Pan
  -1 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jacob Pan

Platform driver for XPowers AXP288 ADC, which is a customized PMIC for Intel
Baytrail-CR platforms. GPADC device enumerates as one of the PMIC MFD cell
devices. It uses IIO infrastructure to communicate with userspace and
consumer drivers.

Usages of ADC channels include battery charging and thermal sensors.

Based on initial work by:
Ramakrishna Pallala <ramakrishna.pallala@intel.com>

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/iio/adc/Kconfig        |   8 ++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/axp288_gpadc.c | 250 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 259 insertions(+)
 create mode 100644 drivers/iio/adc/axp288_gpadc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 11b048a..f5c61c0 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -279,4 +279,12 @@ config XILINX_XADC
 	  The driver can also be build as a module. If so, the module will be called
 	  xilinx-xadc.
 
+config AXP288_GPADC
+	tristate "X-Power AXP288 GPADC driver"
+	depends on MFD_AXP2XX
+	help
+	  Say yes here to have support for X-Power power management IC (PMIC) ADC
+	  device. Depending on platform configuration, this general purpose ADC can
+	  be used for sampling sensors such as thermal resisters.
+
 endmenu
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ad81b51..8bf0104 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o
 obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
 xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
 obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
+obj-$(CONFIG_AXP288_GPADC) += axp288_gpadc.o
diff --git a/drivers/iio/adc/axp288_gpadc.c b/drivers/iio/adc/axp288_gpadc.c
new file mode 100644
index 0000000..7ca6bbf
--- /dev/null
+++ b/drivers/iio/adc/axp288_gpadc.c
@@ -0,0 +1,250 @@
+/*
+ * axp288_gpadc.c - Xpower AXP288 PMIC GPADC Driver
+ *
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the GNU
+ * General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/axp2xx.h>
+#include <linux/platform_device.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>
+#include <linux/iio/driver.h>
+#define ADC_EN_MASK			0xF1
+#define ADC_TS_PIN_GPADC                0xF2
+#define ADC_TS_PIN_ON                   0xF3
+
+struct gpadc_info {
+	unsigned int irq;
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+#define ADC_CHANNEL(_type, _channel, _address)	        \
+	{								\
+		.indexed = 1,						\
+		.type = _type,						\
+		.channel = _channel,					\
+		.address = _address,					\
+		.datasheet_name = "CH"#_channel,			\
+		.scan_index = _channel,					\
+		.scan_type = {						\
+			.sign = 'u',					\
+			.realbits = 12,					\
+			.storagebits = 32,				\
+			.endianness = IIO_LE,				\
+		},							\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	}
+
+static const struct iio_chan_spec const axp288_adc_channels[] = {
+	ADC_CHANNEL(IIO_TEMP, 0, AXP288_TS_ADC_H),
+	ADC_CHANNEL(IIO_TEMP, 1, AXP288_PMIC_ADC_H),
+	ADC_CHANNEL(IIO_TEMP, 2, AXP288_GP_ADC_H),
+	ADC_CHANNEL(IIO_CURRENT, 3, AXP20X_BATT_CHRG_I_H),
+	ADC_CHANNEL(IIO_CURRENT, 4, AXP20X_BATT_DISCHRG_I_H),
+	ADC_CHANNEL(IIO_VOLTAGE, 5, AXP20X_BATT_V_H),
+};
+
+#define ADC_MAP(_adc_channel_label,				\
+		     _consumer_dev_name,			\
+		     _consumer_channel)				\
+	{							\
+		.adc_channel_label = _adc_channel_label,	\
+		.consumer_dev_name = _consumer_dev_name,	\
+		.consumer_channel = _consumer_channel,		\
+	}
+
+/* for consumer drivers */
+static struct iio_map axp288_iio_default_maps[] = {
+	ADC_MAP("TS_PIN", "axp288-batt", "axp288-batt-temp"),
+	ADC_MAP("PMIC_TEMP", "axp288-pmic", "axp288-pmic-temp"),
+	ADC_MAP("GPADC", "axp288-gpadc", "axp288-system-temp"),
+	ADC_MAP("BATT_CHG_I", "axp288-chrg", "axp288-chrg-curr"),
+	ADC_MAP("BATT_DISCHRG_I", "axp288-chrg", "axp288-chrg-d-curr"),
+	ADC_MAP("BATT_V", "axp288-batt", "axp288-batt-volt"),
+	{},
+};
+
+static int axp288_adc_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val, int *val2, long m)
+{
+	int ret;
+	struct gpadc_info *info = iio_priv(indio_dev);
+	unsigned int th, tl;
+
+	mutex_lock(&indio_dev->mlock);
+
+	/* special case for GPADC sample */
+	if (chan->address == AXP288_GP_ADC_H)
+		regmap_write(info->regmap, AXP288_ADC_TS_PIN_CTRL,
+			ADC_TS_PIN_GPADC);
+
+	ret = regmap_read(info->regmap, chan->address, &th);
+	if (ret) {
+		dev_err(&indio_dev->dev, "sample raw data high failed\n");
+		goto exit_done;
+	}
+
+	ret = regmap_read(info->regmap, chan->address + 1, &tl);
+	if (ret) {
+		dev_err(&indio_dev->dev, "sample raw data low failed\n");
+		goto exit_done;
+	}
+
+	*val = (th << 4) + ((tl >> 4) & 0x0F);
+	ret = IIO_VAL_INT;
+
+exit_done:
+	if (chan->address == AXP288_GP_ADC_H)
+		regmap_write(info->regmap, AXP288_ADC_TS_PIN_CTRL,
+			ADC_TS_PIN_ON);
+
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+
+static int axp288_gpadc_enable(struct regmap *regmap, bool enable)
+{
+	unsigned int pin_on, adc_en;
+
+	if (enable) {
+		pin_on = ADC_TS_PIN_ON;
+		adc_en = ADC_EN_MASK;
+	} else {
+		pin_on = ~ADC_TS_PIN_ON;
+		adc_en = ~ADC_EN_MASK;
+	}
+	if (regmap_write(regmap, AXP288_ADC_TS_PIN_CTRL, pin_on))
+		return -EIO;
+
+	return regmap_write(regmap, AXP20X_ADC_EN1, ~ADC_EN_MASK);
+}
+
+static const struct iio_info axp288_iio_info = {
+	.read_raw = &axp288_adc_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int axp288_gpadc_probe(struct platform_device *pdev)
+{
+	int err;
+	struct gpadc_info *info;
+	struct iio_dev *indio_dev;
+	struct axp2xx_dev *axp2xx = dev_get_drvdata(pdev->dev.parent);
+	int irq;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	info = iio_priv(indio_dev);
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no irq resource?\n");
+		return irq;
+	}
+	info->irq = irq;
+	platform_set_drvdata(pdev, indio_dev);
+	info->regmap = axp2xx->regmap;
+	axp288_gpadc_enable(axp2xx->regmap, true);
+
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->name = pdev->name;
+	indio_dev->channels = axp288_adc_channels;
+	indio_dev->num_channels = ARRAY_SIZE(axp288_adc_channels);
+	indio_dev->info = &axp288_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	/* REVISIT: override default map with platform data */
+	err = iio_map_array_register(indio_dev, axp288_iio_default_maps);
+	if (err)
+		goto err_disable_dev;
+
+	err = iio_device_register(indio_dev);
+	if (err < 0) {
+		dev_err(&pdev->dev, "unable to register iio device\n");
+		goto err_array_unregister;
+	}
+	return 0;
+
+err_array_unregister:
+	iio_map_array_unregister(indio_dev);
+err_disable_dev:
+	axp288_gpadc_enable(axp2xx->regmap, false);
+	return err;
+}
+
+static int axp288_gpadc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+
+	iio_device_unregister(indio_dev);
+	iio_map_array_unregister(indio_dev);
+
+	return 0;
+}
+
+#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
+static int axp288_gpadc_suspend(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct gpadc_info *info = iio_priv(indio_dev);
+
+	mutex_lock(&indio_dev->mlock);
+	ret = axp288_gpadc_enable(info->regmap, false);
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+
+static int axp288_gpadc_resume(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct gpadc_info *info = iio_priv(indio_dev);
+
+	mutex_lock(&indio_dev->mlock);
+	ret = axp288_gpadc_enable(info->regmap, true);
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+#endif
+
+static UNIVERSAL_DEV_PM_OPS(axp288_gpadc_pm_ops, axp288_gpadc_suspend,
+			axp288_gpadc_resume, NULL);
+
+static struct platform_driver axp288_gpadc_driver = {
+	.probe = axp288_gpadc_probe,
+	.remove = axp288_gpadc_remove,
+	.driver = {
+		.name = "axp288_adc",
+		.owner = THIS_MODULE,
+		.pm = &axp288_gpadc_pm_ops,
+	},
+};
+
+module_platform_driver(axp288_gpadc_driver);
+
+MODULE_AUTHOR("Jacob Pan <jacob.jun.pan@linux.intel.com>");
+MODULE_DESCRIPTION("Dollar Cove Xpower AXP288 General Purpose ADC Driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1


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

* [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc
@ 2014-09-09 13:02   ` Jacob Pan
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-09 13:02 UTC (permalink / raw)
  To: IIO, LKML, DEVICE TREE, Lee Jones
  Cc: Carlo Caione, Srinivas Pandruvada, Aaron Lu, Alan Cox,
	Jean Delvare, Samuel Ortiz, Liam Girdwood, Mark Brown,
	Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala, Jaco

Platform driver for XPowers AXP288 ADC, which is a customized PMIC for Intel
Baytrail-CR platforms. GPADC device enumerates as one of the PMIC MFD cell
devices. It uses IIO infrastructure to communicate with userspace and
consumer drivers.

Usages of ADC channels include battery charging and thermal sensors.

Based on initial work by:
Ramakrishna Pallala <ramakrishna.pallala@intel.com>

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/iio/adc/Kconfig        |   8 ++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/axp288_gpadc.c | 250 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 259 insertions(+)
 create mode 100644 drivers/iio/adc/axp288_gpadc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 11b048a..f5c61c0 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -279,4 +279,12 @@ config XILINX_XADC
 	  The driver can also be build as a module. If so, the module will be called
 	  xilinx-xadc.
 
+config AXP288_GPADC
+	tristate "X-Power AXP288 GPADC driver"
+	depends on MFD_AXP2XX
+	help
+	  Say yes here to have support for X-Power power management IC (PMIC) ADC
+	  device. Depending on platform configuration, this general purpose ADC can
+	  be used for sampling sensors such as thermal resisters.
+
 endmenu
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ad81b51..8bf0104 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o
 obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
 xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
 obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
+obj-$(CONFIG_AXP288_GPADC) += axp288_gpadc.o
diff --git a/drivers/iio/adc/axp288_gpadc.c b/drivers/iio/adc/axp288_gpadc.c
new file mode 100644
index 0000000..7ca6bbf
--- /dev/null
+++ b/drivers/iio/adc/axp288_gpadc.c
@@ -0,0 +1,250 @@
+/*
+ * axp288_gpadc.c - Xpower AXP288 PMIC GPADC Driver
+ *
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the GNU
+ * General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/axp2xx.h>
+#include <linux/platform_device.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/machine.h>
+#include <linux/iio/driver.h>
+#define ADC_EN_MASK			0xF1
+#define ADC_TS_PIN_GPADC                0xF2
+#define ADC_TS_PIN_ON                   0xF3
+
+struct gpadc_info {
+	unsigned int irq;
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+#define ADC_CHANNEL(_type, _channel, _address)	        \
+	{								\
+		.indexed = 1,						\
+		.type = _type,						\
+		.channel = _channel,					\
+		.address = _address,					\
+		.datasheet_name = "CH"#_channel,			\
+		.scan_index = _channel,					\
+		.scan_type = {						\
+			.sign = 'u',					\
+			.realbits = 12,					\
+			.storagebits = 32,				\
+			.endianness = IIO_LE,				\
+		},							\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	}
+
+static const struct iio_chan_spec const axp288_adc_channels[] = {
+	ADC_CHANNEL(IIO_TEMP, 0, AXP288_TS_ADC_H),
+	ADC_CHANNEL(IIO_TEMP, 1, AXP288_PMIC_ADC_H),
+	ADC_CHANNEL(IIO_TEMP, 2, AXP288_GP_ADC_H),
+	ADC_CHANNEL(IIO_CURRENT, 3, AXP20X_BATT_CHRG_I_H),
+	ADC_CHANNEL(IIO_CURRENT, 4, AXP20X_BATT_DISCHRG_I_H),
+	ADC_CHANNEL(IIO_VOLTAGE, 5, AXP20X_BATT_V_H),
+};
+
+#define ADC_MAP(_adc_channel_label,				\
+		     _consumer_dev_name,			\
+		     _consumer_channel)				\
+	{							\
+		.adc_channel_label = _adc_channel_label,	\
+		.consumer_dev_name = _consumer_dev_name,	\
+		.consumer_channel = _consumer_channel,		\
+	}
+
+/* for consumer drivers */
+static struct iio_map axp288_iio_default_maps[] = {
+	ADC_MAP("TS_PIN", "axp288-batt", "axp288-batt-temp"),
+	ADC_MAP("PMIC_TEMP", "axp288-pmic", "axp288-pmic-temp"),
+	ADC_MAP("GPADC", "axp288-gpadc", "axp288-system-temp"),
+	ADC_MAP("BATT_CHG_I", "axp288-chrg", "axp288-chrg-curr"),
+	ADC_MAP("BATT_DISCHRG_I", "axp288-chrg", "axp288-chrg-d-curr"),
+	ADC_MAP("BATT_V", "axp288-batt", "axp288-batt-volt"),
+	{},
+};
+
+static int axp288_adc_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val, int *val2, long m)
+{
+	int ret;
+	struct gpadc_info *info = iio_priv(indio_dev);
+	unsigned int th, tl;
+
+	mutex_lock(&indio_dev->mlock);
+
+	/* special case for GPADC sample */
+	if (chan->address == AXP288_GP_ADC_H)
+		regmap_write(info->regmap, AXP288_ADC_TS_PIN_CTRL,
+			ADC_TS_PIN_GPADC);
+
+	ret = regmap_read(info->regmap, chan->address, &th);
+	if (ret) {
+		dev_err(&indio_dev->dev, "sample raw data high failed\n");
+		goto exit_done;
+	}
+
+	ret = regmap_read(info->regmap, chan->address + 1, &tl);
+	if (ret) {
+		dev_err(&indio_dev->dev, "sample raw data low failed\n");
+		goto exit_done;
+	}
+
+	*val = (th << 4) + ((tl >> 4) & 0x0F);
+	ret = IIO_VAL_INT;
+
+exit_done:
+	if (chan->address == AXP288_GP_ADC_H)
+		regmap_write(info->regmap, AXP288_ADC_TS_PIN_CTRL,
+			ADC_TS_PIN_ON);
+
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+
+static int axp288_gpadc_enable(struct regmap *regmap, bool enable)
+{
+	unsigned int pin_on, adc_en;
+
+	if (enable) {
+		pin_on = ADC_TS_PIN_ON;
+		adc_en = ADC_EN_MASK;
+	} else {
+		pin_on = ~ADC_TS_PIN_ON;
+		adc_en = ~ADC_EN_MASK;
+	}
+	if (regmap_write(regmap, AXP288_ADC_TS_PIN_CTRL, pin_on))
+		return -EIO;
+
+	return regmap_write(regmap, AXP20X_ADC_EN1, ~ADC_EN_MASK);
+}
+
+static const struct iio_info axp288_iio_info = {
+	.read_raw = &axp288_adc_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int axp288_gpadc_probe(struct platform_device *pdev)
+{
+	int err;
+	struct gpadc_info *info;
+	struct iio_dev *indio_dev;
+	struct axp2xx_dev *axp2xx = dev_get_drvdata(pdev->dev.parent);
+	int irq;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	info = iio_priv(indio_dev);
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "no irq resource?\n");
+		return irq;
+	}
+	info->irq = irq;
+	platform_set_drvdata(pdev, indio_dev);
+	info->regmap = axp2xx->regmap;
+	axp288_gpadc_enable(axp2xx->regmap, true);
+
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->name = pdev->name;
+	indio_dev->channels = axp288_adc_channels;
+	indio_dev->num_channels = ARRAY_SIZE(axp288_adc_channels);
+	indio_dev->info = &axp288_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	/* REVISIT: override default map with platform data */
+	err = iio_map_array_register(indio_dev, axp288_iio_default_maps);
+	if (err)
+		goto err_disable_dev;
+
+	err = iio_device_register(indio_dev);
+	if (err < 0) {
+		dev_err(&pdev->dev, "unable to register iio device\n");
+		goto err_array_unregister;
+	}
+	return 0;
+
+err_array_unregister:
+	iio_map_array_unregister(indio_dev);
+err_disable_dev:
+	axp288_gpadc_enable(axp2xx->regmap, false);
+	return err;
+}
+
+static int axp288_gpadc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+
+	iio_device_unregister(indio_dev);
+	iio_map_array_unregister(indio_dev);
+
+	return 0;
+}
+
+#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
+static int axp288_gpadc_suspend(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct gpadc_info *info = iio_priv(indio_dev);
+
+	mutex_lock(&indio_dev->mlock);
+	ret = axp288_gpadc_enable(info->regmap, false);
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+
+static int axp288_gpadc_resume(struct device *dev)
+{
+	int ret;
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct gpadc_info *info = iio_priv(indio_dev);
+
+	mutex_lock(&indio_dev->mlock);
+	ret = axp288_gpadc_enable(info->regmap, true);
+	mutex_unlock(&indio_dev->mlock);
+
+	return ret;
+}
+#endif
+
+static UNIVERSAL_DEV_PM_OPS(axp288_gpadc_pm_ops, axp288_gpadc_suspend,
+			axp288_gpadc_resume, NULL);
+
+static struct platform_driver axp288_gpadc_driver = {
+	.probe = axp288_gpadc_probe,
+	.remove = axp288_gpadc_remove,
+	.driver = {
+		.name = "axp288_adc",
+		.owner = THIS_MODULE,
+		.pm = &axp288_gpadc_pm_ops,
+	},
+};
+
+module_platform_driver(axp288_gpadc_driver);
+
+MODULE_AUTHOR("Jacob Pan <jacob.jun.pan@linux.intel.com>");
+MODULE_DESCRIPTION("Dollar Cove Xpower AXP288 General Purpose ADC Driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1

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

* Re: [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc
  2014-09-09 13:02   ` Jacob Pan
  (?)
@ 2014-09-09 13:46   ` Peter Meerwald
  2014-09-11 12:05     ` Jacob Pan
  2014-09-19 16:43     ` Jacob Pan
  -1 siblings, 2 replies; 24+ messages in thread
From: Peter Meerwald @ 2014-09-09 13:46 UTC (permalink / raw)
  To: Jacob Pan
  Cc: IIO, Srinivas Pandruvada, Lars-Peter Clausen, Hartmut Knaack,
	Ramakrishna Pallala

Hello,

> Platform driver for XPowers AXP288 ADC, which is a customized PMIC for Intel

XPower's

> Baytrail-CR platforms. GPADC device enumerates as one of the PMIC MFD cell
> devices. It uses IIO infrastructure to communicate with userspace and
> consumer drivers.
> 
> Usages of ADC channels include battery charging and thermal sensors.
> 
> Based on initial work by:
> Ramakrishna Pallala <ramakrishna.pallala@intel.com>

some trivial comments inline

> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
>  drivers/iio/adc/Kconfig        |   8 ++
>  drivers/iio/adc/Makefile       |   1 +
>  drivers/iio/adc/axp288_gpadc.c | 250 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 259 insertions(+)
>  create mode 100644 drivers/iio/adc/axp288_gpadc.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 11b048a..f5c61c0 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -279,4 +279,12 @@ config XILINX_XADC
>  	  The driver can also be build as a module. If so, the module will be called
>  	  xilinx-xadc.
>  
> +config AXP288_GPADC

alphabetic order please

> +	tristate "X-Power AXP288 GPADC driver"

XPower

> +	depends on MFD_AXP2XX
> +	help
> +	  Say yes here to have support for X-Power power management IC (PMIC) ADC
> +	  device. Depending on platform configuration, this general purpose ADC can
> +	  be used for sampling sensors such as thermal resisters.

resistors

> +
>  endmenu
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ad81b51..8bf0104 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o
>  obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
>  xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
>  obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
> +obj-$(CONFIG_AXP288_GPADC) += axp288_gpadc.o
> diff --git a/drivers/iio/adc/axp288_gpadc.c b/drivers/iio/adc/axp288_gpadc.c
> new file mode 100644
> index 0000000..7ca6bbf
> --- /dev/null
> +++ b/drivers/iio/adc/axp288_gpadc.c
> @@ -0,0 +1,250 @@
> +/*
> + * axp288_gpadc.c - Xpower AXP288 PMIC GPADC Driver
> + *
> + * Copyright (C) 2014 Intel Corporation
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the GNU
> + * General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/axp2xx.h>
> +#include <linux/platform_device.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/machine.h>
> +#include <linux/iio/driver.h>

newline

> +#define ADC_EN_MASK			0xF1
> +#define ADC_TS_PIN_GPADC                0xF2
> +#define ADC_TS_PIN_ON                   0xF3

use a likely unique prefix for all #defines, functions, e.g. AXP288_ADC_

> +
> +struct gpadc_info {
> +	unsigned int irq;

irq is 'int' in _probe()

> +	struct device *dev;
> +	struct regmap *regmap;
> +};
> +
> +#define ADC_CHANNEL(_type, _channel, _address)	        \
> +	{								\
> +		.indexed = 1,						\
> +		.type = _type,						\
> +		.channel = _channel,					\
> +		.address = _address,					\
> +		.datasheet_name = "CH"#_channel,			\
> +		.scan_index = _channel,					\

driver doesn't offer buffered interface, so no scan_index, scan_type 
needed

> +		.scan_type = {						\
> +			.sign = 'u',					\
> +			.realbits = 12,					\
> +			.storagebits = 32,				\
> +			.endianness = IIO_LE,				\
> +		},							\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	}
> +
> +static const struct iio_chan_spec const axp288_adc_channels[] = {
> +	ADC_CHANNEL(IIO_TEMP, 0, AXP288_TS_ADC_H),
> +	ADC_CHANNEL(IIO_TEMP, 1, AXP288_PMIC_ADC_H),
> +	ADC_CHANNEL(IIO_TEMP, 2, AXP288_GP_ADC_H),

what unit are these values in?
IIO wants milli degrees Celsius

> +	ADC_CHANNEL(IIO_CURRENT, 3, AXP20X_BATT_CHRG_I_H),
> +	ADC_CHANNEL(IIO_CURRENT, 4, AXP20X_BATT_DISCHRG_I_H),

there are no current channels documented in 
Documentation/ABI/testing/sysfs-bus-iio yet, need to describe

what unit?

> +	ADC_CHANNEL(IIO_VOLTAGE, 5, AXP20X_BATT_V_H),
> +};
> +
> +#define ADC_MAP(_adc_channel_label,				\
> +		     _consumer_dev_name,			\
> +		     _consumer_channel)				\
> +	{							\
> +		.adc_channel_label = _adc_channel_label,	\
> +		.consumer_dev_name = _consumer_dev_name,	\
> +		.consumer_channel = _consumer_channel,		\
> +	}
> +
> +/* for consumer drivers */

const probably

> +static struct iio_map axp288_iio_default_maps[] = {
> +	ADC_MAP("TS_PIN", "axp288-batt", "axp288-batt-temp"),
> +	ADC_MAP("PMIC_TEMP", "axp288-pmic", "axp288-pmic-temp"),
> +	ADC_MAP("GPADC", "axp288-gpadc", "axp288-system-temp"),
> +	ADC_MAP("BATT_CHG_I", "axp288-chrg", "axp288-chrg-curr"),
> +	ADC_MAP("BATT_DISCHRG_I", "axp288-chrg", "axp288-chrg-d-curr"),
> +	ADC_MAP("BATT_V", "axp288-batt", "axp288-batt-volt"),
> +	{},
> +};
> +
> +static int axp288_adc_read_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val, int *val2, long m)
> +{
> +	int ret;
> +	struct gpadc_info *info = iio_priv(indio_dev);
> +	unsigned int th, tl;
> +
> +	mutex_lock(&indio_dev->mlock);
> +
> +	/* special case for GPADC sample */
> +	if (chan->address == AXP288_GP_ADC_H)
> +		regmap_write(info->regmap, AXP288_ADC_TS_PIN_CTRL,
> +			ADC_TS_PIN_GPADC);
> +
> +	ret = regmap_read(info->regmap, chan->address, &th);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "sample raw data high failed\n");
> +		goto exit_done;
> +	}
> +
> +	ret = regmap_read(info->regmap, chan->address + 1, &tl);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "sample raw data low failed\n");
> +		goto exit_done;
> +	}
> +
> +	*val = (th << 4) + ((tl >> 4) & 0x0F);
> +	ret = IIO_VAL_INT;
> +
> +exit_done:
> +	if (chan->address == AXP288_GP_ADC_H)
> +		regmap_write(info->regmap, AXP288_ADC_TS_PIN_CTRL,
> +			ADC_TS_PIN_ON);
> +
> +	mutex_unlock(&indio_dev->mlock);
> +
> +	return ret;
> +}
> +
> +static int axp288_gpadc_enable(struct regmap *regmap, bool enable)
> +{
> +	unsigned int pin_on, adc_en;
> +
> +	if (enable) {
> +		pin_on = ADC_TS_PIN_ON;
> +		adc_en = ADC_EN_MASK;
> +	} else {
> +		pin_on = ~ADC_TS_PIN_ON;
> +		adc_en = ~ADC_EN_MASK;
> +	}
> +	if (regmap_write(regmap, AXP288_ADC_TS_PIN_CTRL, pin_on))
> +		return -EIO;
> +
> +	return regmap_write(regmap, AXP20X_ADC_EN1, ~ADC_EN_MASK);
> +}
> +
> +static const struct iio_info axp288_iio_info = {
> +	.read_raw = &axp288_adc_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int axp288_gpadc_probe(struct platform_device *pdev)
> +{
> +	int err;
> +	struct gpadc_info *info;
> +	struct iio_dev *indio_dev;
> +	struct axp2xx_dev *axp2xx = dev_get_drvdata(pdev->dev.parent);
> +	int irq;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	info = iio_priv(indio_dev);
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "no irq resource?\n");
> +		return irq;
> +	}
> +	info->irq = irq;
> +	platform_set_drvdata(pdev, indio_dev);
> +	info->regmap = axp2xx->regmap;
> +	axp288_gpadc_enable(axp2xx->regmap, true);
> +
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->name = pdev->name;
> +	indio_dev->channels = axp288_adc_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(axp288_adc_channels);
> +	indio_dev->info = &axp288_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	/* REVISIT: override default map with platform data */
> +	err = iio_map_array_register(indio_dev, axp288_iio_default_maps);
> +	if (err)
> +		goto err_disable_dev;
> +
> +	err = iio_device_register(indio_dev);
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "unable to register iio device\n");
> +		goto err_array_unregister;
> +	}
> +	return 0;
> +
> +err_array_unregister:
> +	iio_map_array_unregister(indio_dev);
> +err_disable_dev:
> +	axp288_gpadc_enable(axp2xx->regmap, false);
> +	return err;
> +}
> +
> +static int axp288_gpadc_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_map_array_unregister(indio_dev);

axp288_gpadc_enable(axp2xx->regmap, false);

> +
> +	return 0;
> +}
> +
> +#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
> +static int axp288_gpadc_suspend(struct device *dev)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct gpadc_info *info = iio_priv(indio_dev);
> +
> +	mutex_lock(&indio_dev->mlock);
> +	ret = axp288_gpadc_enable(info->regmap, false);
> +	mutex_unlock(&indio_dev->mlock);
> +
> +	return ret;
> +}
> +
> +static int axp288_gpadc_resume(struct device *dev)
> +{
> +	int ret;
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct gpadc_info *info = iio_priv(indio_dev);
> +
> +	mutex_lock(&indio_dev->mlock);
> +	ret = axp288_gpadc_enable(info->regmap, true);
> +	mutex_unlock(&indio_dev->mlock);
> +
> +	return ret;
> +}
> +#endif
> +
> +static UNIVERSAL_DEV_PM_OPS(axp288_gpadc_pm_ops, axp288_gpadc_suspend,
> +			axp288_gpadc_resume, NULL);
> +
> +static struct platform_driver axp288_gpadc_driver = {
> +	.probe = axp288_gpadc_probe,
> +	.remove = axp288_gpadc_remove,
> +	.driver = {
> +		.name = "axp288_adc",
> +		.owner = THIS_MODULE,
> +		.pm = &axp288_gpadc_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(axp288_gpadc_driver);
> +
> +MODULE_AUTHOR("Jacob Pan <jacob.jun.pan@linux.intel.com>");
> +MODULE_DESCRIPTION("Dollar Cove Xpower AXP288 General Purpose ADC Driver");
> +MODULE_LICENSE("GPL");
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

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

* Re: [PATCH v2 1/4] mfd/axp20x: rename files to support more devices
  2014-09-09 13:02   ` Jacob Pan
@ 2014-09-10  8:12     ` Lee Jones
  -1 siblings, 0 replies; 24+ messages in thread
From: Lee Jones @ 2014-09-10  8:12 UTC (permalink / raw)
  To: Jacob Pan
  Cc: IIO, LKML, DEVICE TREE, Carlo Caione, Srinivas Pandruvada,
	Aaron Lu, Alan Cox, Jean Delvare, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala

On Tue, 09 Sep 2014, Jacob Pan wrote:

> More XPowers PMIC devices can be supported by extending this driver, so
> rename it to axp2xx to cover axp288 variant.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
>  drivers/mfd/Kconfig                      | 7 ++++---
>  drivers/mfd/Makefile                     | 2 +-
>  drivers/mfd/{axp20x.c => axp2xx.c}       | 2 +-
>  include/linux/mfd/{axp20x.h => axp2xx.h} | 0
>  4 files changed, 6 insertions(+), 5 deletions(-)
>  rename drivers/mfd/{axp20x.c => axp2xx.c} (99%)
>  rename include/linux/mfd/{axp20x.h => axp2xx.h} (100%)

Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index de5abf2..42a70a3 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -67,14 +67,15 @@ config MFD_BCM590XX
>  	help
>  	  Support for the BCM590xx PMUs from Broadcom
>  
> -config MFD_AXP20X
> -	bool "X-Powers AXP20X"
> +config MFD_AXP2XX
> +	bool "X-Powers AXP2XX"
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
>  	depends on I2C=y
>  	help
> -	  If you say Y here you get support for the X-Powers AXP202 and AXP209.
> +	  If you say Y here you get support for the X-Powers AXP202, AXP209 and
> +	  AXP288 power management IC (PMIC).
>  	  This driver include only the core APIs. You have to select individual
>  	  components like regulators or the PEK (Power Enable Key) under the
>  	  corresponding menus.
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f001487..55d76b3 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -103,7 +103,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_AXP2XX)	+= axp2xx.o
>  
>  obj-$(CONFIG_MFD_LP3943)	+= lp3943.o
>  obj-$(CONFIG_MFD_LP8788)	+= lp8788.o lp8788-irq.o
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp2xx.c
> similarity index 99%
> rename from drivers/mfd/axp20x.c
> rename to drivers/mfd/axp2xx.c
> index dee6539..c534443 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp2xx.c
> @@ -21,7 +21,7 @@
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/regulator/consumer.h>
> -#include <linux/mfd/axp20x.h>
> +#include <linux/mfd/axp2xx.h>
>  #include <linux/mfd/core.h>
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp2xx.h
> similarity index 100%
> rename from include/linux/mfd/axp20x.h
> rename to include/linux/mfd/axp2xx.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] 24+ messages in thread

* Re: [PATCH v2 1/4] mfd/axp20x: rename files to support more devices
@ 2014-09-10  8:12     ` Lee Jones
  0 siblings, 0 replies; 24+ messages in thread
From: Lee Jones @ 2014-09-10  8:12 UTC (permalink / raw)
  To: Jacob Pan
  Cc: IIO, LKML, DEVICE TREE, Carlo Caione, Srinivas Pandruvada,
	Aaron Lu, Alan Cox, Jean Delvare, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci

On Tue, 09 Sep 2014, Jacob Pan wrote:

> More XPowers PMIC devices can be supported by extending this driver, so
> rename it to axp2xx to cover axp288 variant.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
>  drivers/mfd/Kconfig                      | 7 ++++---
>  drivers/mfd/Makefile                     | 2 +-
>  drivers/mfd/{axp20x.c => axp2xx.c}       | 2 +-
>  include/linux/mfd/{axp20x.h => axp2xx.h} | 0
>  4 files changed, 6 insertions(+), 5 deletions(-)
>  rename drivers/mfd/{axp20x.c => axp2xx.c} (99%)
>  rename include/linux/mfd/{axp20x.h => axp2xx.h} (100%)

Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index de5abf2..42a70a3 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -67,14 +67,15 @@ config MFD_BCM590XX
>  	help
>  	  Support for the BCM590xx PMUs from Broadcom
>  
> -config MFD_AXP20X
> -	bool "X-Powers AXP20X"
> +config MFD_AXP2XX
> +	bool "X-Powers AXP2XX"
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
>  	depends on I2C=y
>  	help
> -	  If you say Y here you get support for the X-Powers AXP202 and AXP209.
> +	  If you say Y here you get support for the X-Powers AXP202, AXP209 and
> +	  AXP288 power management IC (PMIC).
>  	  This driver include only the core APIs. You have to select individual
>  	  components like regulators or the PEK (Power Enable Key) under the
>  	  corresponding menus.
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f001487..55d76b3 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -103,7 +103,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_AXP2XX)	+= axp2xx.o
>  
>  obj-$(CONFIG_MFD_LP3943)	+= lp3943.o
>  obj-$(CONFIG_MFD_LP8788)	+= lp8788.o lp8788-irq.o
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp2xx.c
> similarity index 99%
> rename from drivers/mfd/axp20x.c
> rename to drivers/mfd/axp2xx.c
> index dee6539..c534443 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp2xx.c
> @@ -21,7 +21,7 @@
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/regulator/consumer.h>
> -#include <linux/mfd/axp20x.h>
> +#include <linux/mfd/axp2xx.h>
>  #include <linux/mfd/core.h>
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp2xx.h
> similarity index 100%
> rename from include/linux/mfd/axp20x.h
> rename to include/linux/mfd/axp2xx.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] 24+ messages in thread

* Re: [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic
@ 2014-09-10  8:25     ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2014-09-10  8:25 UTC (permalink / raw)
  To: Jacob Pan
  Cc: IIO, LKML, DEVICE TREE, Lee Jones, Carlo Caione,
	Srinivas Pandruvada, Aaron Lu, Alan Cox, Jean Delvare,
	Samuel Ortiz, Liam Girdwood, Mark Brown, Grant Likely,
	Greg Kroah-Hartman, Rob Herring, Lars-Peter Clausen,
	Hartmut Knaack, Fugang Duan, Arnd Bergmann, Zubair Lutfullah,
	Sebastian Reichel, Johannes Thumshirn, Philippe Reynes,
	Angelo Compagnucci, Doug Anderson, Ramakrishna Pallala

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]

On Tue, Sep 09, 2014 at 06:02:53AM -0700, Jacob Pan wrote:
> XPower AXP288 is a customized PMIC for Intel Baytrail-CR platforms. Similar
> to AXP202/209, AXP288 comes with USB charger, more LDO and BUCK channels, and
> AD converter. It also provides extended status and interrupt reporting
> capabilities than the devices supported in axp20x.c.
> 
> In addition to feature extension, this patch also adds ACPI binding for
> enumeration and hooks for ACPI custom operational region handlers.
> 
> Files and common data structures have been renamed from axp20x to axp2xx
> in order to suit the extended scope of devices.

I still feel like this renaming should be in the first patch that is
already advertised at doing it.

This is quite hard to tell what you are actually doing in this patch,
besides the renaming that is.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic
@ 2014-09-10  8:25     ` Maxime Ripard
  0 siblings, 0 replies; 24+ messages in thread
From: Maxime Ripard @ 2014-09-10  8:25 UTC (permalink / raw)
  To: Jacob Pan
  Cc: IIO, LKML, DEVICE TREE, Lee Jones, Carlo Caione,
	Srinivas Pandruvada, Aaron Lu, Alan Cox, Jean Delvare,
	Samuel Ortiz, Liam Girdwood, Mark Brown, Grant Likely,
	Greg Kroah-Hartman, Rob Herring, Lars-Peter Clausen,
	Hartmut Knaack, Fugang Duan, Arnd Bergmann, Zubair Lutfullah,
	Sebastian Reichel, Johannes Thumshirn, Philippe Reynes

[-- Attachment #1: Type: text/plain, Size: 962 bytes --]

On Tue, Sep 09, 2014 at 06:02:53AM -0700, Jacob Pan wrote:
> XPower AXP288 is a customized PMIC for Intel Baytrail-CR platforms. Similar
> to AXP202/209, AXP288 comes with USB charger, more LDO and BUCK channels, and
> AD converter. It also provides extended status and interrupt reporting
> capabilities than the devices supported in axp20x.c.
> 
> In addition to feature extension, this patch also adds ACPI binding for
> enumeration and hooks for ACPI custom operational region handlers.
> 
> Files and common data structures have been renamed from axp20x to axp2xx
> in order to suit the extended scope of devices.

I still feel like this renaming should be in the first patch that is
already advertised at doing it.

This is quite hard to tell what you are actually doing in this patch,
besides the renaming that is.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic
@ 2014-09-10  9:13     ` Lee Jones
  0 siblings, 0 replies; 24+ messages in thread
From: Lee Jones @ 2014-09-10  9:13 UTC (permalink / raw)
  To: Jacob Pan
  Cc: IIO, LKML, DEVICE TREE, Carlo Caione, Srinivas Pandruvada,
	Aaron Lu, Alan Cox, Jean Delvare, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala

On Tue, 09 Sep 2014, Jacob Pan wrote:

> XPower AXP288 is a customized PMIC for Intel Baytrail-CR platforms. Similar
> to AXP202/209, AXP288 comes with USB charger, more LDO and BUCK channels, and
> AD converter. It also provides extended status and interrupt reporting
> capabilities than the devices supported in axp20x.c.
> 
> In addition to feature extension, this patch also adds ACPI binding for
> enumeration and hooks for ACPI custom operational region handlers.
> 
> Files and common data structures have been renamed from axp20x to axp2xx
> in order to suit the extended scope of devices.
> 
> This consolidated driver should support more Xpower PMICs in both device
> tree and ACPI based platforms.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
>  drivers/mfd/axp2xx.c       | 426 +++++++++++++++++++++++++++++++++++----------
>  include/linux/mfd/axp2xx.h |  57 +++++-
>  2 files changed, 388 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/mfd/axp2xx.c b/drivers/mfd/axp2xx.c
> index c534443..4830bbe 100644
> --- a/drivers/mfd/axp2xx.c
> +++ b/drivers/mfd/axp2xx.c
> @@ -1,9 +1,9 @@
>  /*
> - * axp20x.c - MFD core driver for the X-Powers AXP202 and AXP209
> + * axp2xx.c - MFD core driver for the X-Powers AXP202, AXP209, and AXP288

Any renaming should be part of the renaming patch.

Please remove the supported devices from the description.

> - * AXP20x comprises an adaptive USB-Compatible PWM charger, 2 BUCK DC-DC
> - * converters, 5 LDOs, multiple 12-bit ADCs of voltage, current and temperature
> - * as well as 4 configurable GPIOs.
> + * AXP2xx typically comprises an adaptive USB-Compatible PWM charger, BUCK DC-DC
> + * converters, LDOs, multiple 12-bit ADCs of voltage, current and temperature
> + * as well as configurable GPIOs.
>   *
>   * Author: Carlo Caione <carlo@caione.org>
>   *
> @@ -25,9 +25,14 @@
>  #include <linux/mfd/core.h>
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
> +#include <linux/acpi.h>
>  
>  #define AXP20X_OFF	0x80
>  
> +static struct mfd_cell *axp2xx_cells;
> +static int axp2xx_nr_cells;
> +static struct regmap_config *regmap_cfg;

Only use globals if you 'really' have to.

[...]

> +static int axp2xx_match_device(struct axp2xx_dev *axp2xx, struct device *dev)
>  {
> -	struct axp20x_dev *axp20x;
> +	const struct acpi_device_id *acpi_id;
>  	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(axp2xx_of_match, dev);
> +	if (of_id) {
> +		axp2xx->variant = (long) of_id->data;
> +		goto found_match;

Place the ACPI stuff in an else instead of using goto.

> +	}
>  
> -	of_id = of_match_device(axp20x_of_match, &i2c->dev);
> -	if (!of_id) {
> -		dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> +	acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +	if (!acpi_id || !acpi_id->driver_data) {
> +		dev_err(dev, "Unable to setup AXP2XX ACPI data\n");
> +		return -ENODEV;
> +	}
> +	axp2xx->variant = (long) acpi_id->driver_data;

I think adding ACPI support should be in its own patch.

[...]

> +static int axp2xx_i2c_probe(struct i2c_client *i2c,
> +			const struct i2c_device_id *id)
> +{
> +	struct axp2xx_dev *axp2xx;
> +	int ret;
> +
> +	axp2xx = devm_kzalloc(&i2c->dev, sizeof(*axp2xx), GFP_KERNEL);
> +	if (!axp2xx)
> +		return -ENOMEM;
> +
> +	ret = axp2xx_match_device(axp2xx, &i2c->dev);
> +	if (ret)
> +		return ret;

'\n'.

> +	axp2xx->i2c_client = i2c;
> +	axp2xx->dev = &i2c->dev;
> +	dev_set_drvdata(axp2xx->dev, axp2xx);

I'll do a more thorough review once all of the patches are split up
and grouped nicely.

-- 
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] 24+ messages in thread

* Re: [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic
@ 2014-09-10  9:13     ` Lee Jones
  0 siblings, 0 replies; 24+ messages in thread
From: Lee Jones @ 2014-09-10  9:13 UTC (permalink / raw)
  To: Jacob Pan
  Cc: IIO, LKML, DEVICE TREE, Carlo Caione, Srinivas Pandruvada,
	Aaron Lu, Alan Cox, Jean Delvare, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci

On Tue, 09 Sep 2014, Jacob Pan wrote:

> XPower AXP288 is a customized PMIC for Intel Baytrail-CR platforms. Similar
> to AXP202/209, AXP288 comes with USB charger, more LDO and BUCK channels, and
> AD converter. It also provides extended status and interrupt reporting
> capabilities than the devices supported in axp20x.c.
> 
> In addition to feature extension, this patch also adds ACPI binding for
> enumeration and hooks for ACPI custom operational region handlers.
> 
> Files and common data structures have been renamed from axp20x to axp2xx
> in order to suit the extended scope of devices.
> 
> This consolidated driver should support more Xpower PMICs in both device
> tree and ACPI based platforms.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  drivers/mfd/axp2xx.c       | 426 +++++++++++++++++++++++++++++++++++----------
>  include/linux/mfd/axp2xx.h |  57 +++++-
>  2 files changed, 388 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/mfd/axp2xx.c b/drivers/mfd/axp2xx.c
> index c534443..4830bbe 100644
> --- a/drivers/mfd/axp2xx.c
> +++ b/drivers/mfd/axp2xx.c
> @@ -1,9 +1,9 @@
>  /*
> - * axp20x.c - MFD core driver for the X-Powers AXP202 and AXP209
> + * axp2xx.c - MFD core driver for the X-Powers AXP202, AXP209, and AXP288

Any renaming should be part of the renaming patch.

Please remove the supported devices from the description.

> - * AXP20x comprises an adaptive USB-Compatible PWM charger, 2 BUCK DC-DC
> - * converters, 5 LDOs, multiple 12-bit ADCs of voltage, current and temperature
> - * as well as 4 configurable GPIOs.
> + * AXP2xx typically comprises an adaptive USB-Compatible PWM charger, BUCK DC-DC
> + * converters, LDOs, multiple 12-bit ADCs of voltage, current and temperature
> + * as well as configurable GPIOs.
>   *
>   * Author: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
>   *
> @@ -25,9 +25,14 @@
>  #include <linux/mfd/core.h>
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
> +#include <linux/acpi.h>
>  
>  #define AXP20X_OFF	0x80
>  
> +static struct mfd_cell *axp2xx_cells;
> +static int axp2xx_nr_cells;
> +static struct regmap_config *regmap_cfg;

Only use globals if you 'really' have to.

[...]

> +static int axp2xx_match_device(struct axp2xx_dev *axp2xx, struct device *dev)
>  {
> -	struct axp20x_dev *axp20x;
> +	const struct acpi_device_id *acpi_id;
>  	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(axp2xx_of_match, dev);
> +	if (of_id) {
> +		axp2xx->variant = (long) of_id->data;
> +		goto found_match;

Place the ACPI stuff in an else instead of using goto.

> +	}
>  
> -	of_id = of_match_device(axp20x_of_match, &i2c->dev);
> -	if (!of_id) {
> -		dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> +	acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
> +	if (!acpi_id || !acpi_id->driver_data) {
> +		dev_err(dev, "Unable to setup AXP2XX ACPI data\n");
> +		return -ENODEV;
> +	}
> +	axp2xx->variant = (long) acpi_id->driver_data;

I think adding ACPI support should be in its own patch.

[...]

> +static int axp2xx_i2c_probe(struct i2c_client *i2c,
> +			const struct i2c_device_id *id)
> +{
> +	struct axp2xx_dev *axp2xx;
> +	int ret;
> +
> +	axp2xx = devm_kzalloc(&i2c->dev, sizeof(*axp2xx), GFP_KERNEL);
> +	if (!axp2xx)
> +		return -ENOMEM;
> +
> +	ret = axp2xx_match_device(axp2xx, &i2c->dev);
> +	if (ret)
> +		return ret;

'\n'.

> +	axp2xx->i2c_client = i2c;
> +	axp2xx->dev = &i2c->dev;
> +	dev_set_drvdata(axp2xx->dev, axp2xx);

I'll do a more thorough review once all of the patches are split up
and grouped nicely.

-- 
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] 24+ messages in thread

* Re: [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic
  2014-09-10  9:13     ` Lee Jones
@ 2014-09-10 20:11       ` Jacob Pan
  -1 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-10 20:11 UTC (permalink / raw)
  To: Lee Jones
  Cc: IIO, LKML, DEVICE TREE, Carlo Caione, Srinivas Pandruvada,
	Aaron Lu, Alan Cox, Jean Delvare, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci, Doug Anderson,
	Ramakrishna Pallala

On Wed, 10 Sep 2014 10:13:54 +0100
Lee Jones <lee.jones@linaro.org> wrote:

> I think adding ACPI support should be in its own patch.
Agree with the rest reviews comments. On this one, I agree I should
take out the non-essential part of the ACPI code. i.e. mfd cell
device for acpi opregion handler driver. Let it be part of opregion
handler driver submission later on.

However, the enumeration part of the ACPI code is essential to support
this device since it is intended to be enumerated under ACPI only, a
customized PMIC for Intel.

Thanks,

Jacob

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

* Re: [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic
@ 2014-09-10 20:11       ` Jacob Pan
  0 siblings, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-10 20:11 UTC (permalink / raw)
  To: Lee Jones
  Cc: IIO, LKML, DEVICE TREE, Carlo Caione, Srinivas Pandruvada,
	Aaron Lu, Alan Cox, Jean Delvare, Samuel Ortiz, Liam Girdwood,
	Mark Brown, Grant Likely, Greg Kroah-Hartman, Rob Herring,
	Lars-Peter Clausen, Hartmut Knaack, Fugang Duan, Arnd Bergmann,
	Zubair Lutfullah, Sebastian Reichel, Johannes Thumshirn,
	Philippe Reynes, Angelo Compagnucci

On Wed, 10 Sep 2014 10:13:54 +0100
Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> I think adding ACPI support should be in its own patch.
Agree with the rest reviews comments. On this one, I agree I should
take out the non-essential part of the ACPI code. i.e. mfd cell
device for acpi opregion handler driver. Let it be part of opregion
handler driver submission later on.

However, the enumeration part of the ACPI code is essential to support
this device since it is intended to be enumerated under ACPI only, a
customized PMIC for Intel.

Thanks,

Jacob

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

* Re: [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc
  2014-09-09 13:46   ` Peter Meerwald
@ 2014-09-11 12:05     ` Jacob Pan
  2014-09-19 16:43     ` Jacob Pan
  1 sibling, 0 replies; 24+ messages in thread
From: Jacob Pan @ 2014-09-11 12:05 UTC (permalink / raw)
  To: Peter Meerwald
  Cc: IIO, Srinivas Pandruvada, Lars-Peter Clausen, Hartmut Knaack,
	Ramakrishna Pallala

On Tue, 9 Sep 2014 15:46:31 +0200 (CEST)
Peter Meerwald <pmeerw@pmeerw.net> wrote:

> Hello,
> 
> > Platform driver for XPowers AXP288 ADC, which is a customized PMIC
> > for Intel
> 
> XPower's
> 
> > Baytrail-CR platforms. GPADC device enumerates as one of the PMIC
> > MFD cell devices. It uses IIO infrastructure to communicate with
> > userspace and consumer drivers.
> > 
> > Usages of ADC channels include battery charging and thermal sensors.
> > 
> > Based on initial work by:
> > Ramakrishna Pallala <ramakrishna.pallala@intel.com>
> 
> some trivial comments inline
> 
> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > ---
> >  drivers/iio/adc/Kconfig        |   8 ++
> >  drivers/iio/adc/Makefile       |   1 +
> >  drivers/iio/adc/axp288_gpadc.c | 250
> > +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 259
> > insertions(+) create mode 100644 drivers/iio/adc/axp288_gpadc.c
> > 
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 11b048a..f5c61c0 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -279,4 +279,12 @@ config XILINX_XADC
> >  	  The driver can also be build as a module. If so, the
> > module will be called xilinx-xadc.
> >  
> > +config AXP288_GPADC
> 
> alphabetic order please
> 
will do.
> > +	tristate "X-Power AXP288 GPADC driver"
> 
> XPower
> 
Acutally, their datasheet uses "X-Powers".
> > +	depends on MFD_AXP2XX
> > +	help
> > +	  Say yes here to have support for X-Power power
> > management IC (PMIC) ADC
> > +	  device. Depending on platform configuration, this
> > general purpose ADC can
> > +	  be used for sampling sensors such as thermal resisters.
> 
> resistors
> 
> > +
> >  endmenu
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index ad81b51..8bf0104 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -30,3 +30,4 @@ obj-$(CONFIG_VF610_ADC) += vf610_adc.o
> >  obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
> >  xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
> >  obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
> > +obj-$(CONFIG_AXP288_GPADC) += axp288_gpadc.o
> > diff --git a/drivers/iio/adc/axp288_gpadc.c
> > b/drivers/iio/adc/axp288_gpadc.c new file mode 100644
> > index 0000000..7ca6bbf
> > --- /dev/null
> > +++ b/drivers/iio/adc/axp288_gpadc.c
> > @@ -0,0 +1,250 @@
> > +/*
> > + * axp288_gpadc.c - Xpower AXP288 PMIC GPADC Driver
> > + *
> > + * Copyright (C) 2014 Intel Corporation
> > + *
> > + *
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + * This program is free software; you can redistribute it and/or
> > modify
> > + * it under the terms of the GNU General Public License as
> > published by
> > + * the Free Software Foundation; version 2 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See
> > the GNU
> > + * General Public License for more details.
> > + *
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/mfd/axp2xx.h>
> > +#include <linux/platform_device.h>
> > +
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/machine.h>
> > +#include <linux/iio/driver.h>
> 
> newline
> 
> > +#define ADC_EN_MASK			0xF1
> > +#define ADC_TS_PIN_GPADC                0xF2
> > +#define ADC_TS_PIN_ON                   0xF3
> 
> use a likely unique prefix for all #defines, functions, e.g.
> AXP288_ADC_
> 
ok.
> > +
> > +struct gpadc_info {
> > +	unsigned int irq;
> 
> irq is 'int' in _probe()
> 
right, will fix
> > +	struct device *dev;
> > +	struct regmap *regmap;
> > +};
> > +
> > +#define ADC_CHANNEL(_type, _channel, _address)	        \
> > +
> > {								\
> > +		.indexed =
> > 1,						\
> > +		.type =
> > _type,						\
> > +		.channel =
> > _channel,					\
> > +		.address =
> > _address,					\
> > +		.datasheet_name =
> > "CH"#_channel,			\
> > +		.scan_index =
> > _channel,					\
> 
> driver doesn't offer buffered interface, so no scan_index, scan_type 
> needed
> 
good point.
> > +		.scan_type =
> > {						\
> > +			.sign =
> > 'u',					\
> > +			.realbits =
> > 12,					\
> > +			.storagebits =
> > 32,				\
> > +			.endianness =
> > IIO_LE,				\
> > +		},
> > \
> > +		.info_mask_separate =
> > BIT(IIO_CHAN_INFO_RAW),		\
> > +	}
> > +
> > +static const struct iio_chan_spec const axp288_adc_channels[] = {
> > +	ADC_CHANNEL(IIO_TEMP, 0, AXP288_TS_ADC_H),
> > +	ADC_CHANNEL(IIO_TEMP, 1, AXP288_PMIC_ADC_H),
> > +	ADC_CHANNEL(IIO_TEMP, 2, AXP288_GP_ADC_H),
> 
> what unit are these values in?
> IIO wants milli degrees Celsius
> 
the consumer of these channels are kernel drivers. so i will remove
them from sysfs. the consumer drivers knows the scale. e.g. the final
system board temperature is read by ACPI opregion handler then to a
ACPI INT3403 thermal zone driver under /sys/class/thermal/, so the unit
is converted there.
> > +	ADC_CHANNEL(IIO_CURRENT, 3, AXP20X_BATT_CHRG_I_H),
> > +	ADC_CHANNEL(IIO_CURRENT, 4, AXP20X_BATT_DISCHRG_I_H),
> 
> there are no current channels documented in 
> Documentation/ABI/testing/sysfs-bus-iio yet, need to describe
> 
> what unit?
> 
will do. they are mV and mA
> > +	ADC_CHANNEL(IIO_VOLTAGE, 5, AXP20X_BATT_V_H),
> > +};
> > +
> > +#define ADC_MAP(_adc_channel_label,
> > \
> > +		     _consumer_dev_name,			\
> > +
> > _consumer_channel)				\
> > +	{							\
> > +		.adc_channel_label = _adc_channel_label,	\
> > +		.consumer_dev_name = _consumer_dev_name,	\
> > +		.consumer_channel =
> > _consumer_channel,		\
> > +	}
> > +
> > +/* for consumer drivers */
> 
> const probably
iio_map_array_register() does not take const

thanks for the review.

Jacob
> 
> > +static struct iio_map axp288_iio_default_maps[] = {
> > +	ADC_MAP("TS_PIN", "axp288-batt", "axp288-batt-temp"),
> > +	ADC_MAP("PMIC_TEMP", "axp288-pmic", "axp288-pmic-temp"),
> > +	ADC_MAP("GPADC", "axp288-gpadc", "axp288-system-temp"),
> > +	ADC_MAP("BATT_CHG_I", "axp288-chrg", "axp288-chrg-curr"),
> > +	ADC_MAP("BATT_DISCHRG_I", "axp288-chrg",
> > "axp288-chrg-d-curr"),
> > +	ADC_MAP("BATT_V", "axp288-batt", "axp288-batt-volt"),
> > +	{},
> > +};
> > +
> > +static int axp288_adc_read_raw(struct iio_dev *indio_dev,
> > +			struct iio_chan_spec const *chan,
> > +			int *val, int *val2, long m)
> > +{
> > +	int ret;
> > +	struct gpadc_info *info = iio_priv(indio_dev);
> > +	unsigned int th, tl;
> > +
> > +	mutex_lock(&indio_dev->mlock);
> > +
> > +	/* special case for GPADC sample */
> > +	if (chan->address == AXP288_GP_ADC_H)
> > +		regmap_write(info->regmap, AXP288_ADC_TS_PIN_CTRL,
> > +			ADC_TS_PIN_GPADC);
> > +
> > +	ret = regmap_read(info->regmap, chan->address, &th);
> > +	if (ret) {
> > +		dev_err(&indio_dev->dev, "sample raw data high
> > failed\n");
> > +		goto exit_done;
> > +	}
> > +
> > +	ret = regmap_read(info->regmap, chan->address + 1, &tl);
> > +	if (ret) {
> > +		dev_err(&indio_dev->dev, "sample raw data low
> > failed\n");
> > +		goto exit_done;
> > +	}
> > +
> > +	*val = (th << 4) + ((tl >> 4) & 0x0F);
> > +	ret = IIO_VAL_INT;
> > +
> > +exit_done:
> > +	if (chan->address == AXP288_GP_ADC_H)
> > +		regmap_write(info->regmap, AXP288_ADC_TS_PIN_CTRL,
> > +			ADC_TS_PIN_ON);
> > +
> > +	mutex_unlock(&indio_dev->mlock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int axp288_gpadc_enable(struct regmap *regmap, bool enable)
> > +{
> > +	unsigned int pin_on, adc_en;
> > +
> > +	if (enable) {
> > +		pin_on = ADC_TS_PIN_ON;
> > +		adc_en = ADC_EN_MASK;
> > +	} else {
> > +		pin_on = ~ADC_TS_PIN_ON;
> > +		adc_en = ~ADC_EN_MASK;
> > +	}
> > +	if (regmap_write(regmap, AXP288_ADC_TS_PIN_CTRL, pin_on))
> > +		return -EIO;
> > +
> > +	return regmap_write(regmap, AXP20X_ADC_EN1, ~ADC_EN_MASK);
> > +}
> > +
> > +static const struct iio_info axp288_iio_info = {
> > +	.read_raw = &axp288_adc_read_raw,
> > +	.driver_module = THIS_MODULE,
> > +};
> > +
> > +static int axp288_gpadc_probe(struct platform_device *pdev)
> > +{
> > +	int err;
> > +	struct gpadc_info *info;
> > +	struct iio_dev *indio_dev;
> > +	struct axp2xx_dev *axp2xx =
> > dev_get_drvdata(pdev->dev.parent);
> > +	int irq;
> > +
> > +	indio_dev = devm_iio_device_alloc(&pdev->dev,
> > sizeof(*info));
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	info = iio_priv(indio_dev);
> > +	irq = platform_get_irq(pdev, 0);
> > +	if (irq < 0) {
> > +		dev_err(&pdev->dev, "no irq resource?\n");
> > +		return irq;
> > +	}
> > +	info->irq = irq;
> > +	platform_set_drvdata(pdev, indio_dev);
> > +	info->regmap = axp2xx->regmap;
> > +	axp288_gpadc_enable(axp2xx->regmap, true);
> > +
> > +	indio_dev->dev.parent = &pdev->dev;
> > +	indio_dev->name = pdev->name;
> > +	indio_dev->channels = axp288_adc_channels;
> > +	indio_dev->num_channels = ARRAY_SIZE(axp288_adc_channels);
> > +	indio_dev->info = &axp288_iio_info;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	/* REVISIT: override default map with platform data */
> > +	err = iio_map_array_register(indio_dev,
> > axp288_iio_default_maps);
> > +	if (err)
> > +		goto err_disable_dev;
> > +
> > +	err = iio_device_register(indio_dev);
> > +	if (err < 0) {
> > +		dev_err(&pdev->dev, "unable to register iio
> > device\n");
> > +		goto err_array_unregister;
> > +	}
> > +	return 0;
> > +
> > +err_array_unregister:
> > +	iio_map_array_unregister(indio_dev);
> > +err_disable_dev:
> > +	axp288_gpadc_enable(axp2xx->regmap, false);
> > +	return err;
> > +}
> > +
> > +static int axp288_gpadc_remove(struct platform_device *pdev)
> > +{
> > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > +
> > +	iio_device_unregister(indio_dev);
> > +	iio_map_array_unregister(indio_dev);
> 
> axp288_gpadc_enable(axp2xx->regmap, false);
> 
> > +
> > +	return 0;
> > +}
> > +
> > +#if defined(CONFIG_PM_SLEEP) || defined(CONFIG_PM_RUNTIME)
> > +static int axp288_gpadc_suspend(struct device *dev)
> > +{
> > +	int ret;
> > +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > +	struct gpadc_info *info = iio_priv(indio_dev);
> > +
> > +	mutex_lock(&indio_dev->mlock);
> > +	ret = axp288_gpadc_enable(info->regmap, false);
> > +	mutex_unlock(&indio_dev->mlock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int axp288_gpadc_resume(struct device *dev)
> > +{
> > +	int ret;
> > +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > +	struct gpadc_info *info = iio_priv(indio_dev);
> > +
> > +	mutex_lock(&indio_dev->mlock);
> > +	ret = axp288_gpadc_enable(info->regmap, true);
> > +	mutex_unlock(&indio_dev->mlock);
> > +
> > +	return ret;
> > +}
> > +#endif
> > +
> > +static UNIVERSAL_DEV_PM_OPS(axp288_gpadc_pm_ops,
> > axp288_gpadc_suspend,
> > +			axp288_gpadc_resume, NULL);
> > +
> > +static struct platform_driver axp288_gpadc_driver = {
> > +	.probe = axp288_gpadc_probe,
> > +	.remove = axp288_gpadc_remove,
> > +	.driver = {
> > +		.name = "axp288_adc",
> > +		.owner = THIS_MODULE,
> > +		.pm = &axp288_gpadc_pm_ops,
> > +	},
> > +};
> > +
> > +module_platform_driver(axp288_gpadc_driver);
> > +
> > +MODULE_AUTHOR("Jacob Pan <jacob.jun.pan@linux.intel.com>");
> > +MODULE_DESCRIPTION("Dollar Cove Xpower AXP288 General Purpose ADC
> > Driver"); +MODULE_LICENSE("GPL");
> > 
> 

[Jacob Pan]

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

* Re: [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc
  2014-09-09 13:46   ` Peter Meerwald
  2014-09-11 12:05     ` Jacob Pan
@ 2014-09-19 16:43     ` Jacob Pan
  2014-09-19 17:31       ` Srinivas Pandruvada
  1 sibling, 1 reply; 24+ messages in thread
From: Jacob Pan @ 2014-09-19 16:43 UTC (permalink / raw)
  To: Peter Meerwald
  Cc: IIO, Srinivas Pandruvada, Lars-Peter Clausen, Hartmut Knaack,
	Ramakrishna Pallala

On Tue, 9 Sep 2014 15:46:31 +0200 (CEST)
Peter Meerwald <pmeerw@pmeerw.net> wrote:

> > +static int axp288_gpadc_remove(struct platform_device *pdev)
> > +{
> > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > +
> > +	iio_device_unregister(indio_dev);
> > +	iio_map_array_unregister(indio_dev);  
> 
> axp288_gpadc_enable(axp2xx->regmap, false);
on a second thought, we should not disable the adc even on driver
removal. otherwise, internal fuel gauge will be affected.

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

* Re: [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc
  2014-09-19 16:43     ` Jacob Pan
@ 2014-09-19 17:31       ` Srinivas Pandruvada
  2014-09-19 19:47         ` Jacob Pan
  0 siblings, 1 reply; 24+ messages in thread
From: Srinivas Pandruvada @ 2014-09-19 17:31 UTC (permalink / raw)
  To: Jacob Pan
  Cc: Peter Meerwald, IIO, Lars-Peter Clausen, Hartmut Knaack,
	Ramakrishna Pallala

On Fri, 2014-09-19 at 09:43 -0700, Jacob Pan wrote:
> On Tue, 9 Sep 2014 15:46:31 +0200 (CEST)
> Peter Meerwald <pmeerw@pmeerw.net> wrote:
> 
> > > +static int axp288_gpadc_remove(struct platform_device *pdev)
> > > +{
> > > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > > +
> > > +	iio_device_unregister(indio_dev);
> > > +	iio_map_array_unregister(indio_dev);  
> > 
> > axp288_gpadc_enable(axp2xx->regmap, false);
> on a second thought, we should not disable the adc even on driver
> removal. otherwise, internal fuel gauge will be affected.
As an independent driver it should disable when driver is removed and
bring back to state before init. You can check the state at init and
restore to that state on exit.
If the FG or other driver has dependency then either it should use the
driver model to take reference by not allowing removal of this driver.
Or use built in module so that it can never be removed.
 
Isn't it a platform specific dependency not universal?

Thanks,
Srinivas
 


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

* Re: [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc
  2014-09-19 17:31       ` Srinivas Pandruvada
@ 2014-09-19 19:47         ` Jacob Pan
  2014-09-21 12:22           ` Jonathan Cameron
  0 siblings, 1 reply; 24+ messages in thread
From: Jacob Pan @ 2014-09-19 19:47 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: Peter Meerwald, IIO, Lars-Peter Clausen, Hartmut Knaack,
	Ramakrishna Pallala

On Fri, 19 Sep 2014 10:31:10 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> On Fri, 2014-09-19 at 09:43 -0700, Jacob Pan wrote:
> > On Tue, 9 Sep 2014 15:46:31 +0200 (CEST)
> > Peter Meerwald <pmeerw@pmeerw.net> wrote:
> > 
> > > > +static int axp288_gpadc_remove(struct platform_device *pdev)
> > > > +{
> > > > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > > > +
> > > > +	iio_device_unregister(indio_dev);
> > > > +	iio_map_array_unregister(indio_dev);  
> > > 
> > > axp288_gpadc_enable(axp2xx->regmap, false);
> > on a second thought, we should not disable the adc even on driver
> > removal. otherwise, internal fuel gauge will be affected.
> As an independent driver it should disable when driver is removed and
> bring back to state before init. You can check the state at init and
> restore to that state on exit.
> If the FG or other driver has dependency then either it should use the
> driver model to take reference by not allowing removal of this driver.
> Or use built in module so that it can never be removed.
>  
I share you opinion. But in this case, ideally ADCs should have been
enabled by FW since it should be always on, independent of fuel gauge
driver.
> Isn't it a platform specific dependency not universal?
> 
it is more PMIC specific since its internal fuel gauge depends on ADC
to be available regardless of what platform.
> Thanks,
> Srinivas
>  
> 

[Jacob Pan]

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

* Re: [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc
  2014-09-19 19:47         ` Jacob Pan
@ 2014-09-21 12:22           ` Jonathan Cameron
  0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2014-09-21 12:22 UTC (permalink / raw)
  To: Jacob Pan, Srinivas Pandruvada
  Cc: Peter Meerwald, IIO, Lars-Peter Clausen, Hartmut Knaack,
	Ramakrishna Pallala

On 19/09/14 20:47, Jacob Pan wrote:
> On Fri, 19 Sep 2014 10:31:10 -0700
> Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> 
>> On Fri, 2014-09-19 at 09:43 -0700, Jacob Pan wrote:
>>> On Tue, 9 Sep 2014 15:46:31 +0200 (CEST)
>>> Peter Meerwald <pmeerw@pmeerw.net> wrote:
>>>
>>>>> +static int axp288_gpadc_remove(struct platform_device *pdev)
>>>>> +{
>>>>> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>>>> +
>>>>> +	iio_device_unregister(indio_dev);
>>>>> +	iio_map_array_unregister(indio_dev);  
>>>>
>>>> axp288_gpadc_enable(axp2xx->regmap, false);
>>> on a second thought, we should not disable the adc even on driver
>>> removal. otherwise, internal fuel gauge will be affected.
>> As an independent driver it should disable when driver is removed and
>> bring back to state before init. You can check the state at init and
>> restore to that state on exit.
>> If the FG or other driver has dependency then either it should use the
>> driver model to take reference by not allowing removal of this driver.
>> Or use built in module so that it can never be removed.
>>  
> I share you opinion. But in this case, ideally ADCs should have been
> enabled by FW since it should be always on, independent of fuel gauge
> driver.
>> Isn't it a platform specific dependency not universal?
>>
> it is more PMIC specific since its internal fuel gauge depends on ADC
> to be available regardless of what platform.
Whilst irritating this isn't exactly uncommon.
Probably just don't touch it but put a comment in the driver to that
effect so it is obvious this wasn't just an omission.

Jonathan
>> Thanks,
>> Srinivas
>>  
>>
> 
> [Jacob Pan]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

end of thread, other threads:[~2014-09-21 12:22 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09 13:02 [PATCH v2 0/4] Initial support for XPowers AXP288 PMIC Jacob Pan
2014-09-09 13:02 ` Jacob Pan
2014-09-09 13:02 ` [PATCH v2 1/4] mfd/axp20x: rename files to support more devices Jacob Pan
2014-09-09 13:02   ` Jacob Pan
2014-09-10  8:12   ` Lee Jones
2014-09-10  8:12     ` Lee Jones
2014-09-09 13:02 ` [PATCH v2 2/4] mfd/axp2xx: extend axp20x to support axp288 pmic Jacob Pan
2014-09-09 13:02   ` Jacob Pan
2014-09-10  8:25   ` Maxime Ripard
2014-09-10  8:25     ` Maxime Ripard
2014-09-10  9:13   ` Lee Jones
2014-09-10  9:13     ` Lee Jones
2014-09-10 20:11     ` Jacob Pan
2014-09-10 20:11       ` Jacob Pan
2014-09-09 13:02 ` [PATCH v2 3/4] regulator/axp20x: use axp2xx consolidated header Jacob Pan
2014-09-09 13:02   ` Jacob Pan
2014-09-09 13:02 ` [PATCH v2 4/4] iio/adc/axp288: add support for axp288 gpadc Jacob Pan
2014-09-09 13:02   ` Jacob Pan
2014-09-09 13:46   ` Peter Meerwald
2014-09-11 12:05     ` Jacob Pan
2014-09-19 16:43     ` Jacob Pan
2014-09-19 17:31       ` Srinivas Pandruvada
2014-09-19 19:47         ` Jacob Pan
2014-09-21 12:22           ` Jonathan Cameron

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.