2013-04-04 16:19:51

by Tomasz Figa

[permalink] [raw]
Subject: [PATCH 0/3] regulator: max8998: Add support for Device Tree

This series adds Device Tree support to max8998 MFD driver.

First patch reworks max8998-irq driver to use IRQ domains. Second patch
prepares platform data structure to ease generating it at runtime from
data parsed from device tree. Third patch implements Device Tree
binding and adds necessary documentation.

Tested on Universal C210 board.

Tomasz Figa (3):
mfd: Add irq domain support for max8998 interrupts
regulator: max8998: Use arrays for specifying voltages in platform
data
mfd: max8998: Add support for Device Tree

Documentation/devicetree/bindings/mfd/max8998.txt | 111 ++++++++++
arch/arm/mach-exynos/mach-universal_c210.c | 8 +-
arch/arm/mach-s5pv210/mach-aquila.c | 8 +-
arch/arm/mach-s5pv210/mach-goni.c | 8 +-
drivers/mfd/Kconfig | 1 +
drivers/mfd/max8998-irq.c | 61 +++---
drivers/mfd/max8998.c | 76 ++++++-
drivers/regulator/max8998.c | 254 ++++++++++++++++------
drivers/rtc/rtc-max8998.c | 17 +-
include/linux/mfd/max8998-private.h | 6 +-
include/linux/mfd/max8998.h | 20 +-
11 files changed, 435 insertions(+), 135 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt

--
1.8.1.5


2013-04-04 16:19:57

by Tomasz Figa

[permalink] [raw]
Subject: [PATCH 1/3] mfd: Add irq domain support for max8998 interrupts

This patch adds irq domain support for max8998 interrupts.

The reverse mapping method used is linear mapping since the sub-drivers
of max8998 such as regulator and charger drivers can use the max8998
irq_domain to get the virtual irq number for max8998 interrupts.

All uses of irq_base in platform data and max8997 driver private data
are removed.

Signed-off-by: Tomasz Figa <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/mfd/Kconfig | 1 +
drivers/mfd/max8998-irq.c | 61 ++++++++++++++++++++++---------------
drivers/mfd/max8998.c | 1 -
drivers/rtc/rtc-max8998.c | 15 +++++++--
include/linux/mfd/max8998-private.h | 4 ++-
include/linux/mfd/max8998.h | 2 --
6 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index c346941..3ab3a11 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -585,6 +585,7 @@ config MFD_MAX8998
bool "Maxim Semiconductor MAX8998/National LP3974 PMIC Support"
depends on I2C=y && GENERIC_HARDIRQS
select MFD_CORE
+ select IRQ_DOMAIN
help
Say yes here to support for Maxim Semiconductor MAX8998 and
National Semiconductor LP3974. This is a Power Management IC.
diff --git a/drivers/mfd/max8998-irq.c b/drivers/mfd/max8998-irq.c
index 5919710..f770abf 100644
--- a/drivers/mfd/max8998-irq.c
+++ b/drivers/mfd/max8998-irq.c
@@ -14,6 +14,7 @@
#include <linux/device.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
+#include <linux/irqdomain.h>
#include <linux/mfd/max8998-private.h>

struct max8998_irq_data {
@@ -99,7 +100,8 @@ static struct max8998_irq_data max8998_irqs[] = {
static inline struct max8998_irq_data *
irq_to_max8998_irq(struct max8998_dev *max8998, int irq)
{
- return &max8998_irqs[irq - max8998->irq_base];
+ struct irq_data *data = irq_get_irq_data(irq);
+ return &max8998_irqs[data->hwirq];
}

static void max8998_irq_lock(struct irq_data *data)
@@ -176,8 +178,10 @@ static irqreturn_t max8998_irq_thread(int irq, void *data)

/* Report */
for (i = 0; i < MAX8998_IRQ_NR; i++) {
- if (irq_reg[max8998_irqs[i].reg - 1] & max8998_irqs[i].mask)
- handle_nested_irq(max8998->irq_base + i);
+ if (irq_reg[max8998_irqs[i].reg - 1] & max8998_irqs[i].mask) {
+ irq = irq_linear_revmap(max8998->irq_domain, i);
+ handle_nested_irq(irq);
+ }
}

return IRQ_HANDLED;
@@ -185,27 +189,40 @@ static irqreturn_t max8998_irq_thread(int irq, void *data)

int max8998_irq_resume(struct max8998_dev *max8998)
{
- if (max8998->irq && max8998->irq_base)
- max8998_irq_thread(max8998->irq_base, max8998);
+ if (max8998->irq && max8998->irq_domain)
+ max8998_irq_thread(0, max8998);
+ return 0;
+}
+
+static int max8998_irq_domain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ struct max8997_dev *max8998 = d->host_data;
+
+ irq_set_chip_data(irq, max8998);
+ irq_set_chip_and_handler(irq, &max8998_irq_chip, handle_edge_irq);
+ irq_set_nested_thread(irq, 1);
+#ifdef CONFIG_ARM
+ set_irq_flags(irq, IRQF_VALID);
+#else
+ irq_set_noprobe(irq);
+#endif
return 0;
}

+static struct irq_domain_ops max8998_irq_domain_ops = {
+ .map = max8998_irq_domain_map,
+};
+
int max8998_irq_init(struct max8998_dev *max8998)
{
int i;
- int cur_irq;
int ret;
+ struct irq_domain *domain;

if (!max8998->irq) {
dev_warn(max8998->dev,
"No interrupt specified, no interrupts\n");
- max8998->irq_base = 0;
- return 0;
- }
-
- if (!max8998->irq_base) {
- dev_err(max8998->dev,
- "No interrupt base specified, no interrupts\n");
return 0;
}

@@ -221,19 +238,13 @@ int max8998_irq_init(struct max8998_dev *max8998)
max8998_write_reg(max8998->i2c, MAX8998_REG_STATUSM1, 0xff);
max8998_write_reg(max8998->i2c, MAX8998_REG_STATUSM2, 0xff);

- /* register with genirq */
- for (i = 0; i < MAX8998_IRQ_NR; i++) {
- cur_irq = i + max8998->irq_base;
- irq_set_chip_data(cur_irq, max8998);
- irq_set_chip_and_handler(cur_irq, &max8998_irq_chip,
- handle_edge_irq);
- irq_set_nested_thread(cur_irq, 1);
-#ifdef CONFIG_ARM
- set_irq_flags(cur_irq, IRQF_VALID);
-#else
- irq_set_noprobe(cur_irq);
-#endif
+ domain = irq_domain_add_linear(NULL, MAX8998_IRQ_NR,
+ &max8998_irq_domain_ops, max8998);
+ if (!domain) {
+ dev_err(max8998->dev, "could not create irq domain\n");
+ return -ENODEV;
}
+ max8998->irq_domain = domain;

ret = request_threaded_irq(max8998->irq, NULL, max8998_irq_thread,
IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index d7218cc..e9bd352 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -146,7 +146,6 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
max8998->type = id->driver_data;
if (pdata) {
max8998->ono = pdata->ono;
- max8998->irq_base = pdata->irq_base;
max8998->wakeup = pdata->wakeup;
}
mutex_init(&max8998->iolock);
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index 8f234a0..f854983 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -16,6 +16,7 @@
#include <linux/i2c.h>
#include <linux/slab.h>
#include <linux/bcd.h>
+#include <linux/irqdomain.h>
#include <linux/rtc.h>
#include <linux/platform_device.h>
#include <linux/mfd/max8998.h>
@@ -263,7 +264,6 @@ static int max8998_rtc_probe(struct platform_device *pdev)
info->dev = &pdev->dev;
info->max8998 = max8998;
info->rtc = max8998->rtc;
- info->irq = max8998->irq_base + MAX8998_IRQ_ALARM0;

platform_set_drvdata(pdev, info);

@@ -276,6 +276,15 @@ static int max8998_rtc_probe(struct platform_device *pdev)
goto out_rtc;
}

+ if (!max8998->irq_domain)
+ goto no_irq;
+
+ info->irq = irq_create_mapping(max8998->irq_domain, MAX8998_IRQ_ALARM0);
+ if (!info->irq) {
+ dev_warn(&pdev->dev, "Failed to map alarm IRQ\n");
+ goto no_irq;
+ }
+
ret = request_threaded_irq(info->irq, NULL, max8998_rtc_alarm_irq, 0,
"rtc-alarm0", info);

@@ -283,6 +292,7 @@ static int max8998_rtc_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
info->irq, ret);

+no_irq:
dev_info(&pdev->dev, "RTC CHIP NAME: %s\n", pdev->id_entry->name);
if (pdata->rtc_delay) {
info->lp3974_bug_workaround = true;
@@ -303,7 +313,8 @@ static int max8998_rtc_remove(struct platform_device *pdev)
struct max8998_rtc_info *info = platform_get_drvdata(pdev);

if (info) {
- free_irq(info->irq, info);
+ if (info->irq)
+ free_irq(info->irq, info);
rtc_device_unregister(info->rtc_dev);
kfree(info);
}
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index effa5d3..e81077a 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -132,6 +132,8 @@ enum {

#define MAX8998_ENRAMP (1 << 4)

+struct irq_domain;
+
/**
* struct max8998_dev - max8998 master device for sub-drivers
* @dev: master device of the chip (can be used to access platform data)
@@ -153,7 +155,7 @@ struct max8998_dev {
struct mutex iolock;
struct mutex irqlock;

- int irq_base;
+ struct irq_domain *irq_domain;
int irq;
int ono;
u8 irq_masks_cur[MAX8998_NUM_IRQ_REGS];
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 6823548..9df60ba 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -68,7 +68,6 @@ struct max8998_regulator_data {
* struct max8998_board - packages regulator init data
* @regulators: array of defined regulators
* @num_regulators: number of regulators used
- * @irq_base: base IRQ number for max8998, required for IRQs
* @ono: power onoff IRQ number for max8998
* @buck_voltage_lock: Do NOT change the values of the following six
* registers set by buck?_voltage?. The voltage of BUCK1/2 cannot
@@ -100,7 +99,6 @@ struct max8998_regulator_data {
struct max8998_platform_data {
struct max8998_regulator_data *regulators;
int num_regulators;
- int irq_base;
int ono;
bool buck_voltage_lock;
int buck1_voltage1;
--
1.8.1.5

2013-04-04 16:20:03

by Tomasz Figa

[permalink] [raw]
Subject: [PATCH 2/3] regulator: max8998: Use arrays for specifying voltages in platform data

This patch modifies the platform data of max8998 to use arrays for
specifying predefined voltages of buck1 and buck2 instead of separate
field for each voltage.

This allows to simplify the code a bit and will help in adding support
for Device Tree, which will be introduced in further patch.

Signed-off-by: Tomasz Figa <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
arch/arm/mach-exynos/mach-universal_c210.c | 8 +--
arch/arm/mach-s5pv210/mach-aquila.c | 8 +--
arch/arm/mach-s5pv210/mach-goni.c | 8 +--
drivers/regulator/max8998.c | 96 +++++++++---------------------
include/linux/mfd/max8998.h | 16 ++---
5 files changed, 39 insertions(+), 97 deletions(-)

diff --git a/arch/arm/mach-exynos/mach-universal_c210.c b/arch/arm/mach-exynos/mach-universal_c210.c
index 497fcb7..366abb3 100644
--- a/arch/arm/mach-exynos/mach-universal_c210.c
+++ b/arch/arm/mach-exynos/mach-universal_c210.c
@@ -540,15 +540,11 @@ static struct max8998_regulator_data lp3974_regulators[] = {
static struct max8998_platform_data universal_lp3974_pdata = {
.num_regulators = ARRAY_SIZE(lp3974_regulators),
.regulators = lp3974_regulators,
- .buck1_voltage1 = 1100000, /* INT */
- .buck1_voltage2 = 1000000,
- .buck1_voltage3 = 1100000,
- .buck1_voltage4 = 1000000,
+ .buck1_voltage = { 1100000, 1000000, 1100000, 1000000 },
.buck1_set1 = EXYNOS4_GPX0(5),
.buck1_set2 = EXYNOS4_GPX0(6),
- .buck2_voltage1 = 1200000, /* G3D */
- .buck2_voltage2 = 1100000,
.buck1_default_idx = 0,
+ .buck2_voltage = { 1200000, 1100000 },
.buck2_set3 = EXYNOS4_GPE2(0),
.buck2_default_idx = 0,
.wakeup = true,
diff --git a/arch/arm/mach-s5pv210/mach-aquila.c b/arch/arm/mach-s5pv210/mach-aquila.c
index 11900a8..7e6c718 100644
--- a/arch/arm/mach-s5pv210/mach-aquila.c
+++ b/arch/arm/mach-s5pv210/mach-aquila.c
@@ -377,12 +377,8 @@ static struct max8998_platform_data aquila_max8998_pdata = {
.buck1_set1 = S5PV210_GPH0(3),
.buck1_set2 = S5PV210_GPH0(4),
.buck2_set3 = S5PV210_GPH0(5),
- .buck1_voltage1 = 1200000,
- .buck1_voltage2 = 1200000,
- .buck1_voltage3 = 1200000,
- .buck1_voltage4 = 1200000,
- .buck2_voltage1 = 1200000,
- .buck2_voltage2 = 1200000,
+ .buck1_voltage = { 1200000, 1200000, 1200000, 1200000 },
+ .buck2_voltage = { 1200000, 1200000 },
};
#endif

diff --git a/arch/arm/mach-s5pv210/mach-goni.c b/arch/arm/mach-s5pv210/mach-goni.c
index e373de4..2ae61d4 100644
--- a/arch/arm/mach-s5pv210/mach-goni.c
+++ b/arch/arm/mach-s5pv210/mach-goni.c
@@ -580,12 +580,8 @@ static struct max8998_platform_data goni_max8998_pdata = {
.buck1_set1 = S5PV210_GPH0(3),
.buck1_set2 = S5PV210_GPH0(4),
.buck2_set3 = S5PV210_GPH0(5),
- .buck1_voltage1 = 1200000,
- .buck1_voltage2 = 1200000,
- .buck1_voltage3 = 1200000,
- .buck1_voltage4 = 1200000,
- .buck2_voltage1 = 1200000,
- .buck2_voltage2 = 1200000,
+ .buck1_voltage = { 1200000, 1200000, 1200000, 1200000 },
+ .buck2_voltage = { 1200000, 1200000 },
};
#endif

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index a57a1b1..8c45b93 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -630,6 +630,7 @@ static int max8998_pmic_probe(struct platform_device *pdev)
struct max8998_data *max8998;
struct i2c_client *i2c;
int i, ret, size;
+ unsigned int v;

if (!pdata) {
dev_err(pdev->dev.parent, "No platform init data supplied\n");
@@ -688,53 +689,21 @@ static int max8998_pmic_probe(struct platform_device *pdev)
gpio_request(pdata->buck1_set2, "MAX8998 BUCK1_SET2");
gpio_direction_output(pdata->buck1_set2,
(max8998->buck1_idx >> 1) & 0x1);
- /* Set predefined value for BUCK1 register 1 */
- i = 0;
- while (buck12_voltage_map_desc.min +
- buck12_voltage_map_desc.step*i
- < pdata->buck1_voltage1)
- i++;
- max8998->buck1_vol[0] = i;
- ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE1, i);
- if (ret)
- goto err_out;
-
- /* Set predefined value for BUCK1 register 2 */
- i = 0;
- while (buck12_voltage_map_desc.min +
- buck12_voltage_map_desc.step*i
- < pdata->buck1_voltage2)
- i++;
-
- max8998->buck1_vol[1] = i;
- ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE2, i);
- if (ret)
- goto err_out;
-
- /* Set predefined value for BUCK1 register 3 */
- i = 0;
- while (buck12_voltage_map_desc.min +
- buck12_voltage_map_desc.step*i
- < pdata->buck1_voltage3)
- i++;
-
- max8998->buck1_vol[2] = i;
- ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE3, i);
- if (ret)
- goto err_out;
-
- /* Set predefined value for BUCK1 register 4 */
- i = 0;
- while (buck12_voltage_map_desc.min +
- buck12_voltage_map_desc.step*i
- < pdata->buck1_voltage4)
- i++;
-
- max8998->buck1_vol[3] = i;
- ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE4, i);
- if (ret)
- goto err_out;

+ /* Set predefined values for BUCK1 registers */
+ for (v = 0; v < ARRAY_SIZE(pdata->buck1_voltage); ++v) {
+ i = 0;
+ while (buck12_voltage_map_desc.min +
+ buck12_voltage_map_desc.step*i
+ < pdata->buck1_voltage[v])
+ i++;
+
+ max8998->buck1_vol[v] = i;
+ ret = max8998_write_reg(i2c,
+ MAX8998_REG_BUCK1_VOLTAGE1 + v, i);
+ if (ret)
+ goto err_out;
+ }
}

if (gpio_is_valid(pdata->buck2_set3)) {
@@ -750,27 +719,20 @@ static int max8998_pmic_probe(struct platform_device *pdev)
gpio_direction_output(pdata->buck2_set3,
max8998->buck2_idx & 0x1);

- /* BUCK2 register 1 */
- i = 0;
- while (buck12_voltage_map_desc.min +
- buck12_voltage_map_desc.step*i
- < pdata->buck2_voltage1)
- i++;
- max8998->buck2_vol[0] = i;
- ret = max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOLTAGE1, i);
- if (ret)
- goto err_out;
-
- /* BUCK2 register 2 */
- i = 0;
- while (buck12_voltage_map_desc.min +
- buck12_voltage_map_desc.step*i
- < pdata->buck2_voltage2)
- i++;
- max8998->buck2_vol[1] = i;
- ret = max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOLTAGE2, i);
- if (ret)
- goto err_out;
+ /* Set predefined values for BUCK2 registers */
+ for (v = 0; v < ARRAY_SIZE(pdata->buck2_voltage); ++v) {
+ i = 0;
+ while (buck12_voltage_map_desc.min +
+ buck12_voltage_map_desc.step*i
+ < pdata->buck2_voltage[v])
+ i++;
+
+ max8998->buck2_vol[v] = i;
+ ret = max8998_write_reg(i2c,
+ MAX8998_REG_BUCK2_VOLTAGE1 + v, i);
+ if (ret)
+ goto err_out;
+ }
}

for (i = 0; i < pdata->num_regulators; i++) {
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 9df60ba..e8b9ba7 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -72,12 +72,8 @@ struct max8998_regulator_data {
* @buck_voltage_lock: Do NOT change the values of the following six
* registers set by buck?_voltage?. The voltage of BUCK1/2 cannot
* be other than the preset values.
- * @buck1_voltage1: BUCK1 DVS mode 1 voltage register
- * @buck1_voltage2: BUCK1 DVS mode 2 voltage register
- * @buck1_voltage3: BUCK1 DVS mode 3 voltage register
- * @buck1_voltage4: BUCK1 DVS mode 4 voltage register
- * @buck2_voltage1: BUCK2 DVS mode 1 voltage register
- * @buck2_voltage2: BUCK2 DVS mode 2 voltage register
+ * @buck1_voltage: BUCK1 DVS mode 1 voltage registers
+ * @buck2_voltage: BUCK2 DVS mode 2 voltage registers
* @buck1_set1: BUCK1 gpio pin 1 to set output voltage
* @buck1_set2: BUCK1 gpio pin 2 to set output voltage
* @buck1_default_idx: Default for BUCK1 gpio pin 1, 2
@@ -101,12 +97,8 @@ struct max8998_platform_data {
int num_regulators;
int ono;
bool buck_voltage_lock;
- int buck1_voltage1;
- int buck1_voltage2;
- int buck1_voltage3;
- int buck1_voltage4;
- int buck2_voltage1;
- int buck2_voltage2;
+ int buck1_voltage[4];
+ int buck2_voltage[2];
int buck1_set1;
int buck1_set2;
int buck1_default_idx;
--
1.8.1.5

2013-04-04 16:20:11

by Tomasz Figa

[permalink] [raw]
Subject: [PATCH 3/3] mfd: max8998: Add support for Device Tree

This patch adds Device Tree support to max8998 driver.

Signed-off-by: Tomasz Figa <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
Documentation/devicetree/bindings/mfd/max8998.txt | 111 +++++++++++++++
drivers/mfd/max8998.c | 75 +++++++++-
drivers/regulator/max8998.c | 158 +++++++++++++++++++++-
drivers/rtc/rtc-max8998.c | 2 +-
include/linux/mfd/max8998-private.h | 2 +
include/linux/mfd/max8998.h | 2 +
6 files changed, 343 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt

diff --git a/Documentation/devicetree/bindings/mfd/max8998.txt b/Documentation/devicetree/bindings/mfd/max8998.txt
new file mode 100644
index 0000000..80ac378
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max8998.txt
@@ -0,0 +1,111 @@
+* Maxim MAX8998, National/TI LP3974 Voltage and Current Regulator
+
+The Maxim MAX8998 is a multi-function device which includes volatage and
+current regulators, rtc, charger controller and other sub-blocks. It is
+interfaced to the host controller using a i2c interface. Each sub-block is
+addressed by the host system using different i2c slave address. This document
+describes the bindings for 'pmic' sub-block of max8998.
+
+Required properties:
+- compatible: Should be one of the following:
+ - "maxim,max8998" for Maxim MAX8998
+ - "national,lp3974" or "ti,lp3974" for National/TI LP3974.
+- reg: Specifies the i2c slave address of the pmic block. It should be 0x66.
+
+Optional properties:
+- interrupt-parent: Specifies the phandle of the interrupt controller to which
+ the interrupts from max8998 are delivered to.
+- interrupts: Interrupt specifiers for two interrupt sources.
+ - First interrupt specifier is for 'irq1' interrupt.
+ - Second interrupt specifier is for 'alert' interrupt.
+- max8998,pmic-buck1-dvs-gpios: GPIO specifiers for two host gpios used
+ for buck 1 dvs. The format of the gpio specifier depends in the gpio
+ controller.
+- max8998,pmic-buck2-dvs-gpio: GPIO specifier for host gpio used
+ for buck 2 dvs. The format of the gpio specifier depends in the gpio
+ controller.
+- max8998,pmic-buck1-default-dvs-idx: Default voltage setting selected from
+ the possible 4 options selectable by the dvs gpios. The value of this
+ property should be between 0 and 3. If not specified or if out of range, the
+ default value of this property is set to 0.
+- max8998,pmic-buck2-default-dvs-idx: Default voltage setting selected from
+ the possible 2 options selectable by the dvs gpios. The value of this
+ property should be between 0 and 1. If not specified or if out of range, the
+ default value of this property is set to 0.
+- max8998,pmic-buck-voltage-lock: If present, disallows changing of
+ preprogrammed buck dvfs voltages.
+
+Additional properties required if max8998,pmic-buck1-dvs-gpios is defined:
+- max8998,pmic-buck1-dvs-voltage: A set of 4 voltage values in micro-volt (uV)
+ units for buck1 when changing voltage using gpio dvs.
+
+Additional properties required if max8998,pmic-buck2-dvs-gpio is defined:
+- max8998,pmic-buck2-dvs-voltage: A set of 2 voltage values in micro-volt (uV)
+ units for buck2 when changing voltage using gpio dvs.
+
+Regulators: The regulators of max8998 that have to be instantiated should be
+included in a sub-node named 'regulators'. Regulator nodes included in this
+sub-node should be of the format as listed below.
+
+ regulator_name {
+ standard regulator bindings here
+ };
+
+The following are the names of the regulators that the max8998 pmic block
+supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
+as per the datasheet of max8998.
+
+ - LDOn
+ - valid values for n are 2 to 17
+ - Example: LDO2, LDO10, LDO17
+ - BUCKn
+ - valid values for n are 1 to 4.
+ - Example: BUCK1, BUCK2, BUCK3, BUCK4
+
+ - ENVICHG: Battery Charging Current Monitor Output. This is a fixed
+ voltage type regulator
+
+ - ESAFEOUT1: (ldo19)
+ - ESAFEOUT2: (ld020)
+
+ - EN32KHz AP: 32KHz clock output for application processor
+ - EN32KHz CP: 32KHz clock output for call processor
+
+The bindings inside the regulator nodes use the standard regulator bindings
+which are documented elsewhere.
+
+Example:
+
+ [email protected] {
+ compatible = "maxim,max8998-pmic";
+ interrupt-parent = <&wakeup_eint>;
+ reg = <0x66>;
+ interrupts = <4 0>, <3 0>;
+
+ max8998,pmic-buck1-default-dvs-idx = <0>;
+ max8998,pmic-buck1-dvs-gpios = <&gpx0 0 1 0 0>, /* SET1 */
+ <&gpx0 1 1 0 0>; /* SET2 */
+ max8998,pmic-buck1-dvs-voltage = <1350000>, <1300000>,
+ <1000000>, <950000>;
+
+ max8998,pmic-buck2-default-dvs-idx = <0>;
+ max8998,pmic-buck2-dvs-gpio = <&gpx0 0 3 0 0>; /* SET3 */
+ max8998,pmic-buck2-dvs-voltage = <1350000>, <1300000>;
+
+ regulators {
+ ldo2_reg: LDO2 {
+ regulator-name = "VDD_ALIVE_1.1V";
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-always-on;
+ };
+
+ buck1_reg: BUCK1 {
+ regulator-name = "VDD_ARM_1.2V";
+ regulator-min-microvolt = <950000>;
+ regulator-max-microvolt = <1350000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+ };
+ };
diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index e9bd352..d4a8753 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -20,12 +20,14 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/

+#include <linux/err.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
+#include <linux/of_irq.h>
#include <linux/pm_runtime.h>
#include <linux/mutex.h>
#include <linux/mfd/core.h>
@@ -128,6 +130,65 @@ int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
}
EXPORT_SYMBOL(max8998_update_reg);

+#ifdef CONFIG_OF
+static struct of_device_id max8998_dt_match[] = {
+ { .compatible = "maxim,max8998", .data = (void *)TYPE_MAX8998 },
+ { .compatible = "national,lp3974", .data = (void *)TYPE_LP3974 },
+ { .compatible = "ti,lp3974", .data = (void *)TYPE_LP3974 },
+ {},
+};
+MODULE_DEVICE_TABLE(of, max8998_dt_match);
+
+/*
+ * Only the common platform data elements for max8998 are parsed here from the
+ * device tree. Other sub-modules of max8998 such as pmic, rtc and others have
+ * to parse their own platform data elements from device tree.
+ *
+ * The max8998 platform data structure is instantiated here and the drivers for
+ * the sub-modules need not instantiate another instance while parsing their
+ * platform data.
+ */
+static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
+ struct device *dev)
+{
+ struct max8998_platform_data *pd;
+
+ pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+ if (!pd) {
+ dev_err(dev, "could not allocate memory for pdata\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ pd->ono = irq_of_parse_and_map(dev->of_node, 1);
+
+ /*
+ * ToDo: the 'wakeup' member in the platform data is more of a linux
+ * specfic information. Hence, there is no binding for that yet and
+ * not parsed here.
+ */
+
+ return pd;
+}
+#else
+static struct max8998_platform_data *max8998_i2c_parse_dt_pdata(
+ struct device *dev)
+{
+ return 0;
+}
+#endif
+
+static inline int max8998_i2c_get_driver_data(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ if (i2c->dev.of_node) {
+ const struct of_device_id *match;
+ match = of_match_node(max8998_dt_match, i2c->dev.of_node);
+ return (int)match->data;
+ }
+
+ return (int)id->driver_data;
+}
+
static int max8998_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
@@ -139,11 +200,20 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
if (max8998 == NULL)
return -ENOMEM;

+ if (i2c->dev.of_node) {
+ pdata = max8998_i2c_parse_dt_pdata(&i2c->dev);
+ if (IS_ERR(pdata)) {
+ ret = PTR_ERR(pdata);
+ goto err;
+ }
+ }
+
i2c_set_clientdata(i2c, max8998);
max8998->dev = &i2c->dev;
max8998->i2c = i2c;
max8998->irq = i2c->irq;
- max8998->type = id->driver_data;
+ max8998->type = max8998_i2c_get_driver_data(i2c, id);
+ max8998->pdata = pdata;
if (pdata) {
max8998->ono = pdata->ono;
max8998->wakeup = pdata->wakeup;
@@ -157,7 +227,7 @@ static int max8998_i2c_probe(struct i2c_client *i2c,

pm_runtime_set_active(max8998->dev);

- switch (id->driver_data) {
+ switch (max8998->type) {
case TYPE_LP3974:
ret = mfd_add_devices(max8998->dev, -1,
lp3974_devs, ARRAY_SIZE(lp3974_devs),
@@ -313,6 +383,7 @@ static struct i2c_driver max8998_i2c_driver = {
.name = "max8998",
.owner = THIS_MODULE,
.pm = &max8998_pm,
+ .of_match_table = of_match_ptr(max8998_dt_match),
},
.probe = max8998_i2c_probe,
.remove = max8998_i2c_remove,
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 8c45b93..df37b3a 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -28,8 +28,10 @@
#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/mutex.h>
+#include <linux/of_gpio.h>
#include <linux/platform_device.h>
#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
#include <linux/mfd/max8998.h>
#include <linux/mfd/max8998-private.h>

@@ -589,13 +591,13 @@ static struct regulator_desc regulators[] = {
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
}, {
- .name = "EN32KHz AP",
+ .name = "EN32KHz-AP",
.id = MAX8998_EN32KHZ_AP,
.ops = &max8998_others_ops,
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
}, {
- .name = "EN32KHz CP",
+ .name = "EN32KHz-CP",
.id = MAX8998_EN32KHZ_CP,
.ops = &max8998_others_ops,
.type = REGULATOR_VOLTAGE,
@@ -621,10 +623,150 @@ static struct regulator_desc regulators[] = {
}
};

+#ifdef CONFIG_OF
+static int max8998_pmic_dt_parse_dvs_gpio(struct max8998_dev *iodev,
+ struct max8998_platform_data *pdata,
+ struct device_node *pmic_np)
+{
+ int gpio;
+
+ gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 0);
+ if (!gpio_is_valid(gpio)) {
+ dev_err(iodev->dev, "invalid buck1 gpio[0]: %d\n", gpio);
+ return -EINVAL;
+ }
+ pdata->buck1_set1 = gpio;
+
+ gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck1-dvs-gpios", 1);
+ if (!gpio_is_valid(gpio)) {
+ dev_err(iodev->dev, "invalid buck1 gpio[1]: %d\n", gpio);
+ return -EINVAL;
+ }
+ pdata->buck1_set2 = gpio;
+
+ gpio = of_get_named_gpio(pmic_np, "max8998,pmic-buck2-dvs-gpio", 0);
+ if (!gpio_is_valid(gpio)) {
+ dev_err(iodev->dev, "invalid buck 2 gpio: %d\n", gpio);
+ return -EINVAL;
+ }
+ pdata->buck2_set3 = gpio;
+
+ return 0;
+}
+
+static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
+ struct max8998_platform_data *pdata)
+{
+ struct device_node *pmic_np, *regulators_np, *reg_np;
+ struct max8998_regulator_data *rdata;
+ unsigned int i, ret;
+
+ pmic_np = iodev->dev->of_node;
+ if (!pmic_np) {
+ dev_err(iodev->dev, "could not find pmic sub-node\n");
+ return -ENODEV;
+ }
+
+ regulators_np = of_find_node_by_name(pmic_np, "regulators");
+ if (!regulators_np) {
+ dev_err(iodev->dev, "could not find regulators sub-node\n");
+ return -EINVAL;
+ }
+
+ /* count the number of regulators to be supported in pmic */
+ pdata->num_regulators = 0;
+ for_each_child_of_node(regulators_np, reg_np)
+ pdata->num_regulators++;
+
+ rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
+ pdata->num_regulators, GFP_KERNEL);
+ if (!rdata) {
+ dev_err(iodev->dev, "could not allocate memory for "
+ "regulator data\n");
+ return -ENOMEM;
+ }
+
+ pdata->regulators = rdata;
+ for_each_child_of_node(regulators_np, reg_np) {
+ for (i = 0; i < ARRAY_SIZE(regulators); i++)
+ if (!of_node_cmp(reg_np->name, regulators[i].name))
+ break;
+
+ if (i == ARRAY_SIZE(regulators)) {
+ dev_warn(iodev->dev, "don't know how to configure "
+ "regulator %s\n", reg_np->name);
+ --pdata->num_regulators;
+ continue;
+ }
+
+ rdata->id = regulators[i].id;
+ rdata->initdata = of_get_regulator_init_data(
+ iodev->dev, reg_np);
+ rdata->reg_node = reg_np;
+ rdata++;
+ }
+
+ ret = max8998_pmic_dt_parse_dvs_gpio(iodev, pdata, pmic_np);
+ if (ret)
+ return -EINVAL;
+
+ if (of_find_property(pmic_np, "max8998,pmic-buck-voltage-lock", NULL))
+ pdata->buck_voltage_lock = true;
+
+ if (of_property_read_u32(pmic_np,
+ "max8998,pmic-buck1-default-dvs-idx",
+ &pdata->buck1_default_idx)) {
+ pdata->buck1_default_idx = 0;
+ } else {
+ if (pdata->buck1_default_idx >= 4) {
+ pdata->buck1_default_idx = 0;
+ dev_info(iodev->dev, "invalid value for "
+ "default dvs index, using 0 instead\n");
+ }
+ }
+
+ if (of_property_read_u32(pmic_np,
+ "max8998,pmic-buck2-default-dvs-idx",
+ &pdata->buck2_default_idx)) {
+ pdata->buck2_default_idx = 0;
+ } else {
+ if (pdata->buck2_default_idx >= 2) {
+ pdata->buck2_default_idx = 0;
+ dev_info(iodev->dev, "invalid value for "
+ "default dvs index, using 0 instead\n");
+ }
+ }
+
+ if (of_property_read_u32_array(pmic_np,
+ "max8998,pmic-buck1-dvs-voltage",
+ pdata->buck1_voltage,
+ ARRAY_SIZE(pdata->buck1_voltage))) {
+ dev_err(iodev->dev, "buck1 voltages not specified\n");
+ return -EINVAL;
+ }
+
+ if (of_property_read_u32_array(pmic_np,
+ "max8998,pmic-buck2-dvs-voltage",
+ pdata->buck2_voltage,
+ ARRAY_SIZE(pdata->buck2_voltage))) {
+ dev_err(iodev->dev, "buck2 voltages not specified\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+#else
+static int max8998_pmic_dt_parse_pdata(struct max8998_dev *iodev,
+ struct max8998_platform_data *pdata)
+{
+ return 0;
+}
+#endif /* CONFIG_OF */
+
static int max8998_pmic_probe(struct platform_device *pdev)
{
struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
- struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
+ struct max8998_platform_data *pdata = iodev->pdata;
struct regulator_config config = { };
struct regulator_dev **rdev;
struct max8998_data *max8998;
@@ -637,6 +779,12 @@ static int max8998_pmic_probe(struct platform_device *pdev)
return -ENODEV;
}

+ if (iodev->dev->of_node) {
+ ret = max8998_pmic_dt_parse_pdata(iodev, pdata);
+ if (ret)
+ return ret;
+ }
+
max8998 = devm_kzalloc(&pdev->dev, sizeof(struct max8998_data),
GFP_KERNEL);
if (!max8998)
@@ -750,13 +898,15 @@ static int max8998_pmic_probe(struct platform_device *pdev)
}

config.dev = max8998->dev;
+ config.of_node = pdata->regulators[i].reg_node;
config.init_data = pdata->regulators[i].initdata;
config.driver_data = max8998;

rdev[i] = regulator_register(&regulators[index], &config);
if (IS_ERR(rdev[i])) {
ret = PTR_ERR(rdev[i]);
- dev_err(max8998->dev, "regulator init failed\n");
+ dev_err(max8998->dev, "regulator %s init failed\n",
+ regulators[index].name);
rdev[i] = NULL;
goto err;
}
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index f854983..66374ac 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -253,7 +253,7 @@ static const struct rtc_class_ops max8998_rtc_ops = {
static int max8998_rtc_probe(struct platform_device *pdev)
{
struct max8998_dev *max8998 = dev_get_drvdata(pdev->dev.parent);
- struct max8998_platform_data *pdata = dev_get_platdata(max8998->dev);
+ struct max8998_platform_data *pdata = max8998->pdata;
struct max8998_rtc_info *info;
int ret;

diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index e81077a..f02676e 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -137,6 +137,7 @@ struct irq_domain;
/**
* struct max8998_dev - max8998 master device for sub-drivers
* @dev: master device of the chip (can be used to access platform data)
+ * @pdata: platform data for the driver and subdrivers
* @i2c: i2c client private data for regulator
* @rtc: i2c client private data for rtc
* @iolock: mutex for serializing io access
@@ -150,6 +151,7 @@ struct irq_domain;
*/
struct max8998_dev {
struct device *dev;
+ struct max8998_platform_data *pdata;
struct i2c_client *i2c;
struct i2c_client *rtc;
struct mutex iolock;
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index e8b9ba7..bb4de8b 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -58,10 +58,12 @@ enum {
* max8998_regulator_data - regulator data
* @id: regulator id
* @initdata: regulator init data (contraints, supplies, ...)
+ * @reg_node: DT node of regulator (unused on non-DT platforms)
*/
struct max8998_regulator_data {
int id;
struct regulator_init_data *initdata;
+ struct device_node *reg_node;
};

/**
--
1.8.1.5

2013-04-15 12:42:35

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 0/3] regulator: max8998: Add support for Device Tree

Hi Mark,

On Thursday 04 of April 2013 18:19:20 Tomasz Figa wrote:
> This series adds Device Tree support to max8998 MFD driver.
>
> First patch reworks max8998-irq driver to use IRQ domains. Second patch
> prepares platform data structure to ease generating it at runtime from
> data parsed from device tree. Third patch implements Device Tree
> binding and adds necessary documentation.
>
> Tested on Universal C210 board.
>
> Tomasz Figa (3):
> mfd: Add irq domain support for max8998 interrupts
> regulator: max8998: Use arrays for specifying voltages in platform
> data
> mfd: max8998: Add support for Device Tree
>
> Documentation/devicetree/bindings/mfd/max8998.txt | 111 ++++++++++
> arch/arm/mach-exynos/mach-universal_c210.c | 8 +-
> arch/arm/mach-s5pv210/mach-aquila.c | 8 +-
> arch/arm/mach-s5pv210/mach-goni.c | 8 +-
> drivers/mfd/Kconfig | 1 +
> drivers/mfd/max8998-irq.c | 61 +++---
> drivers/mfd/max8998.c | 76 ++++++-
> drivers/regulator/max8998.c | 254
> ++++++++++++++++------ drivers/rtc/rtc-max8998.c |
> 17 +-
> include/linux/mfd/max8998-private.h | 6 +-
> include/linux/mfd/max8998.h | 20 +-
> 11 files changed, 435 insertions(+), 135 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/max8998.txt

Any chance to still have this series merged for 3.10?

Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Kernel and System Framework