2014-01-08 06:30:36

by Sachin Kamat

[permalink] [raw]
Subject: [PATCH 1/3] mfd: Add support for S2MPA01 device

Add the necessary entries required for S2MPA01 multi-function
device. While at it also convert whitespaces to tabs in core.h.

Signed-off-by: Sachin Kamat <[email protected]>
---
drivers/mfd/sec-core.c | 39 +++++++
include/linux/mfd/samsung/core.h | 16 ++-
include/linux/mfd/samsung/irq.h | 50 +++++++++
include/linux/mfd/samsung/s2mpa01.h | 192 +++++++++++++++++++++++++++++++++++
4 files changed, 292 insertions(+), 5 deletions(-)
create mode 100644 include/linux/mfd/samsung/s2mpa01.h

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index a139798b8065..d2e890d50a36 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -26,6 +26,7 @@
#include <linux/mfd/samsung/core.h>
#include <linux/mfd/samsung/irq.h>
#include <linux/mfd/samsung/rtc.h>
+#include <linux/mfd/samsung/s2mpa01.h>
#include <linux/mfd/samsung/s2mps11.h>
#include <linux/mfd/samsung/s5m8763.h>
#include <linux/mfd/samsung/s5m8767.h>
@@ -69,6 +70,13 @@ static const struct mfd_cell s2mps11_devs[] = {
}
};

+static const struct mfd_cell s2mpa01_devs[] = {
+ {
+ .name = "s2mpa01-pmic",
+ },
+};
+
+
#ifdef CONFIG_OF
static struct of_device_id sec_dt_match[] = {
{ .compatible = "samsung,s5m8767-pmic",
@@ -77,10 +85,25 @@ static struct of_device_id sec_dt_match[] = {
{ .compatible = "samsung,s2mps11-pmic",
.data = (void *)S2MPS11X,
},
+ { .compatible = "samsung,s2mpa01-pmic",
+ .data = (void *)S2MPA01,
+ },
{},
};
#endif

+static bool s2mpa01_volatile(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case S2MPA01_REG_INT1M:
+ case S2MPA01_REG_INT2M:
+ case S2MPA01_REG_INT3M:
+ return false;
+ default:
+ return true;
+ }
+}
+
static bool s2mps11_volatile(struct device *dev, unsigned int reg)
{
switch (reg) {
@@ -111,6 +134,15 @@ static const struct regmap_config sec_regmap_config = {
.val_bits = 8,
};

+static const struct regmap_config s2mpa01_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = S2MPA01_REG_LDO_OVCB4,
+ .volatile_reg = s2mpa01_volatile,
+ .cache_type = REGCACHE_FLAT,
+};
+
static const struct regmap_config s2mps11_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
@@ -229,6 +261,9 @@ static int sec_pmic_probe(struct i2c_client *i2c,
}

switch (sec_pmic->device_type) {
+ case S2MPA01:
+ regmap = &s2mpa01_regmap_config;
+ break;
case S2MPS11X:
regmap = &s2mps11_regmap_config;
break;
@@ -283,6 +318,10 @@ static int sec_pmic_probe(struct i2c_client *i2c,
ret = mfd_add_devices(sec_pmic->dev, -1, s5m8767_devs,
ARRAY_SIZE(s5m8767_devs), NULL, 0, NULL);
break;
+ case S2MPA01:
+ ret = mfd_add_devices(sec_pmic->dev, -1, s2mpa01_devs,
+ ARRAY_SIZE(s2mpa01_devs), NULL, 0, NULL);
+ break;
case S2MPS11X:
ret = mfd_add_devices(sec_pmic->dev, -1, s2mps11_devs,
ARRAY_SIZE(s2mps11_devs), NULL, 0, NULL);
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index 41c9bde410c5..db165f8712d3 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -18,6 +18,7 @@ enum sec_device_type {
S5M8751X,
S5M8763X,
S5M8767X,
+ S2MPA01,
S2MPS11X,
};

@@ -92,7 +93,7 @@ struct sec_platform_data {
int buck3_default_idx;
int buck4_default_idx;

- int buck_ramp_delay;
+ int buck_ramp_delay;

int buck2_ramp_delay;
int buck34_ramp_delay;
@@ -100,10 +101,15 @@ struct sec_platform_data {
int buck16_ramp_delay;
int buck7810_ramp_delay;
int buck9_ramp_delay;
-
- bool buck2_ramp_enable;
- bool buck3_ramp_enable;
- bool buck4_ramp_enable;
+ int buck24_ramp_delay;
+ int buck3_ramp_delay;
+ int buck7_ramp_delay;
+ int buck8910_ramp_delay;
+
+ bool buck1_ramp_enable;
+ bool buck2_ramp_enable;
+ bool buck3_ramp_enable;
+ bool buck4_ramp_enable;
bool buck6_ramp_enable;

int buck2_init;
diff --git a/include/linux/mfd/samsung/irq.h b/include/linux/mfd/samsung/irq.h
index d43b4f9e7fb2..2ca965948cd4 100644
--- a/include/linux/mfd/samsung/irq.h
+++ b/include/linux/mfd/samsung/irq.h
@@ -13,6 +13,56 @@
#ifndef __LINUX_MFD_SEC_IRQ_H
#define __LINUX_MFD_SEC_IRQ_H

+enum s2mpa01_irq {
+ S2MPA01_IRQ_PWRONF,
+ S2MPA01_IRQ_PWRONR,
+ S2MPA01_IRQ_JIGONBF,
+ S2MPA01_IRQ_JIGONBR,
+ S2MPA01_IRQ_ACOKBF,
+ S2MPA01_IRQ_ACOKBR,
+ S2MPA01_IRQ_PWRON1S,
+ S2MPA01_IRQ_MRB,
+
+ S2MPA01_IRQ_RTC60S,
+ S2MPA01_IRQ_RTCA1,
+ S2MPA01_IRQ_RTCA0,
+ S2MPA01_IRQ_SMPL,
+ S2MPA01_IRQ_RTC1S,
+ S2MPA01_IRQ_WTSR,
+
+ S2MPA01_IRQ_INT120C,
+ S2MPA01_IRQ_INT140C,
+ S2MPA01_IRQ_LDO3_TSD,
+ S2MPA01_IRQ_B16_TSD,
+ S2MPA01_IRQ_B24_TSD,
+ S2MPA01_IRQ_B35_TSD,
+
+ S2MPA01_IRQ_NR,
+};
+
+#define S2MPA01_IRQ_PWRONF_MASK (1 << 0)
+#define S2MPA01_IRQ_PWRONR_MASK (1 << 1)
+#define S2MPA01_IRQ_JIGONBF_MASK (1 << 2)
+#define S2MPA01_IRQ_JIGONBR_MASK (1 << 3)
+#define S2MPA01_IRQ_ACOKBF_MASK (1 << 4)
+#define S2MPA01_IRQ_ACOKBR_MASK (1 << 5)
+#define S2MPA01_IRQ_PWRON1S_MASK (1 << 6)
+#define S2MPA01_IRQ_MRB_MASK (1 << 7)
+
+#define S2MPA01_IRQ_RTC60S_MASK (1 << 0)
+#define S2MPA01_IRQ_RTCA1_MASK (1 << 1)
+#define S2MPA01_IRQ_RTCA0_MASK (1 << 2)
+#define S2MPA01_IRQ_SMPL_MASK (1 << 3)
+#define S2MPA01_IRQ_RTC1S_MASK (1 << 4)
+#define S2MPA01_IRQ_WTSR_MASK (1 << 5)
+
+#define S2MPA01_IRQ_INT120C_MASK (1 << 0)
+#define S2MPA01_IRQ_INT140C_MASK (1 << 1)
+#define S2MPA01_IRQ_LDO3_TSD_MASK (1 << 2)
+#define S2MPA01_IRQ_B16_TSD_MASK (1 << 3)
+#define S2MPA01_IRQ_B24_TSD_MASK (1 << 4)
+#define S2MPA01_IRQ_B35_TSD_MASK (1 << 5)
+
enum s2mps11_irq {
S2MPS11_IRQ_PWRONF,
S2MPS11_IRQ_PWRONR,
diff --git a/include/linux/mfd/samsung/s2mpa01.h b/include/linux/mfd/samsung/s2mpa01.h
new file mode 100644
index 000000000000..fbc63bc0d6a2
--- /dev/null
+++ b/include/linux/mfd/samsung/s2mpa01.h
@@ -0,0 +1,192 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#ifndef __LINUX_MFD_S2MPA01_H
+#define __LINUX_MFD_S2MPA01_H
+
+/* S2MPA01 registers */
+enum s2mpa01_reg {
+ S2MPA01_REG_ID,
+ S2MPA01_REG_INT1,
+ S2MPA01_REG_INT2,
+ S2MPA01_REG_INT3,
+ S2MPA01_REG_INT1M,
+ S2MPA01_REG_INT2M,
+ S2MPA01_REG_INT3M,
+ S2MPA01_REG_ST1,
+ S2MPA01_REG_ST2,
+ S2MPA01_REG_PWRONSRC,
+ S2MPA01_REG_OFFSRC,
+ S2MPA01_REG_RTC_BUF,
+ S2MPA01_REG_CTRL1,
+ S2MPA01_REG_ETC_TEST,
+ S2MPA01_REG_RSVD1,
+ S2MPA01_REG_BU_CHG,
+ S2MPA01_REG_RAMP1,
+ S2MPA01_REG_RAMP2,
+ S2MPA01_REG_LDO_DSCH1,
+ S2MPA01_REG_LDO_DSCH2,
+ S2MPA01_REG_LDO_DSCH3,
+ S2MPA01_REG_LDO_DSCH4,
+ S2MPA01_REG_OTP_ADRL,
+ S2MPA01_REG_OTP_ADRH,
+ S2MPA01_REG_OTP_DATA,
+ S2MPA01_REG_MON1SEL,
+ S2MPA01_REG_MON2SEL,
+ S2MPA01_REG_LEE,
+ S2MPA01_REG_RSVD2,
+ S2MPA01_REG_RSVD3,
+ S2MPA01_REG_RSVD4,
+ S2MPA01_REG_RSVD5,
+ S2MPA01_REG_RSVD6,
+ S2MPA01_REG_TOP_RSVD,
+ S2MPA01_REG_DVS_SEL,
+ S2MPA01_REG_DVS_PTR,
+ S2MPA01_REG_DVS_DATA,
+ S2MPA01_REG_RSVD_NO,
+ S2MPA01_REG_UVLO,
+ S2MPA01_REG_LEE_NO,
+ S2MPA01_REG_B1CTRL1,
+ S2MPA01_REG_B1CTRL2,
+ S2MPA01_REG_B2CTRL1,
+ S2MPA01_REG_B2CTRL2,
+ S2MPA01_REG_B3CTRL1,
+ S2MPA01_REG_B3CTRL2,
+ S2MPA01_REG_B4CTRL1,
+ S2MPA01_REG_B4CTRL2,
+ S2MPA01_REG_B5CTRL1,
+ S2MPA01_REG_B5CTRL2,
+ S2MPA01_REG_B5CTRL3,
+ S2MPA01_REG_B5CTRL4,
+ S2MPA01_REG_B5CTRL5,
+ S2MPA01_REG_B5CTRL6,
+ S2MPA01_REG_B6CTRL1,
+ S2MPA01_REG_B6CTRL2,
+ S2MPA01_REG_B7CTRL1,
+ S2MPA01_REG_B7CTRL2,
+ S2MPA01_REG_B8CTRL1,
+ S2MPA01_REG_B8CTRL2,
+ S2MPA01_REG_B9CTRL1,
+ S2MPA01_REG_B9CTRL2,
+ S2MPA01_REG_B10CTRL1,
+ S2MPA01_REG_B10CTRL2,
+ S2MPA01_REG_L1CTRL,
+ S2MPA01_REG_L2CTRL,
+ S2MPA01_REG_L3CTRL,
+ S2MPA01_REG_L4CTRL,
+ S2MPA01_REG_L5CTRL,
+ S2MPA01_REG_L6CTRL,
+ S2MPA01_REG_L7CTRL,
+ S2MPA01_REG_L8CTRL,
+ S2MPA01_REG_L9CTRL,
+ S2MPA01_REG_L10CTRL,
+ S2MPA01_REG_L11CTRL,
+ S2MPA01_REG_L12CTRL,
+ S2MPA01_REG_L13CTRL,
+ S2MPA01_REG_L14CTRL,
+ S2MPA01_REG_L15CTRL,
+ S2MPA01_REG_L16CTRL,
+ S2MPA01_REG_L17CTRL,
+ S2MPA01_REG_L18CTRL,
+ S2MPA01_REG_L19CTRL,
+ S2MPA01_REG_L20CTRL,
+ S2MPA01_REG_L21CTRL,
+ S2MPA01_REG_L22CTRL,
+ S2MPA01_REG_L23CTRL,
+ S2MPA01_REG_L24CTRL,
+ S2MPA01_REG_L25CTRL,
+ S2MPA01_REG_L26CTRL,
+
+ S2MPA01_REG_LDO_OVCB1,
+ S2MPA01_REG_LDO_OVCB2,
+ S2MPA01_REG_LDO_OVCB3,
+ S2MPA01_REG_LDO_OVCB4,
+
+};
+
+/* S2MPA01 regulator ids */
+enum s2mpa01_regulators {
+ S2MPA01_LDO1,
+ S2MPA01_LDO2,
+ S2MPA01_LDO3,
+ S2MPA01_LDO4,
+ S2MPA01_LDO5,
+ S2MPA01_LDO6,
+ S2MPA01_LDO7,
+ S2MPA01_LDO8,
+ S2MPA01_LDO9,
+ S2MPA01_LDO10,
+ S2MPA01_LDO11,
+ S2MPA01_LDO12,
+ S2MPA01_LDO13,
+ S2MPA01_LDO14,
+ S2MPA01_LDO15,
+ S2MPA01_LDO16,
+ S2MPA01_LDO17,
+ S2MPA01_LDO18,
+ S2MPA01_LDO19,
+ S2MPA01_LDO20,
+ S2MPA01_LDO21,
+ S2MPA01_LDO22,
+ S2MPA01_LDO23,
+ S2MPA01_LDO24,
+ S2MPA01_LDO25,
+ S2MPA01_LDO26,
+
+ S2MPA01_BUCK1,
+ S2MPA01_BUCK2,
+ S2MPA01_BUCK3,
+ S2MPA01_BUCK4,
+ S2MPA01_BUCK5,
+ S2MPA01_BUCK6,
+ S2MPA01_BUCK7,
+ S2MPA01_BUCK8,
+ S2MPA01_BUCK9,
+ S2MPA01_BUCK10,
+
+ S2MPA01_REGULATOR_MAX,
+};
+
+#define S2MPA01_BUCK_MIN1 600000
+#define S2MPA01_BUCK_MIN2 800000
+#define S2MPA01_BUCK_MIN3 1000000
+#define S2MPA01_BUCK_MIN4 1500000
+#define S2MPA01_LDO_MIN 800000
+
+#define S2MPA01_BUCK_STEP1 6250
+#define S2MPA01_BUCK_STEP2 12500
+
+#define S2MPA01_LDO_STEP1 50000
+#define S2MPA01_LDO_STEP2 25000
+
+#define S2MPA01_LDO_VSEL_MASK 0x3F
+#define S2MPA01_BUCK_VSEL_MASK 0xFF
+#define S2MPA01_ENABLE_MASK (0x03 << S2MPA01_ENABLE_SHIFT)
+#define S2MPA01_ENABLE_SHIFT 0x06
+#define S2MPA01_LDO_N_VOLTAGES (S2MPA01_LDO_VSEL_MASK + 1)
+#define S2MPA01_BUCK_N_VOLTAGES (S2MPA01_BUCK_VSEL_MASK + 1)
+
+#define S2MPA01_RAMP_DELAY 12500 /* uV/us */
+
+#define S2MPA01_BUCK16_RAMP_SHIFT 4
+#define S2MPA01_BUCK24_RAMP_SHIFT 6
+#define S2MPA01_BUCK3_RAMP_SHIFT 4
+#define S2MPA01_BUCK5_RAMP_SHIFT 6
+#define S2MPA01_BUCK7_RAMP_SHIFT 2
+#define S2MPA01_BUCK8910_RAMP_SHIFT 0
+
+#define S2MPA01_BUCK1_RAMP_EN_SHIFT 3
+#define S2MPA01_BUCK2_RAMP_EN_SHIFT 2
+#define S2MPA01_BUCK3_RAMP_EN_SHIFT 1
+#define S2MPA01_BUCK4_RAMP_EN_SHIFT 0
+#define S2MPA01_PMIC_EN_SHIFT 6
+
+#endif /*__LINUX_MFD_S2MPA01_H */
--
1.7.9.5


2014-01-08 06:30:52

by Sachin Kamat

[permalink] [raw]
Subject: [PATCH 2/3] regulator: Add support for S2MPA01 regulator

Add support for S2MPA01 voltage and current regulator.

Signed-off-by: Sachin Kamat <[email protected]>
---
drivers/regulator/Kconfig | 7 +
drivers/regulator/Makefile | 1 +
drivers/regulator/s2mpa01.c | 497 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 505 insertions(+)
create mode 100644 drivers/regulator/s2mpa01.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 6a7932822e37..8f8e6710bb45 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -416,6 +416,13 @@ config REGULATOR_RC5T583
through regulator interface. The device supports multiple DCDC/LDO
outputs which can be controlled by i2c communication.

+config REGULATOR_S2MPA01
+ tristate "Samsung S2MPA01 voltage regulator"
+ depends on MFD_SEC_CORE
+ help
+ This driver controls Samsung S2MPA01 voltage output regulator
+ via I2C bus. S2MPA01 has 10 Bucks and 26 LDO outputs.
+
config REGULATOR_S2MPS11
tristate "Samsung S2MPS11 voltage regulator"
depends on MFD_SEC_CORE
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 979f9ddcf259..b3ece84289cf 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_REGULATOR_TPS51632) += tps51632-regulator.o
obj-$(CONFIG_REGULATOR_PCAP) += pcap-regulator.o
obj-$(CONFIG_REGULATOR_PCF50633) += pcf50633-regulator.o
obj-$(CONFIG_REGULATOR_RC5T583) += rc5t583-regulator.o
+obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
new file mode 100644
index 000000000000..f1c831d5fc87
--- /dev/null
+++ b/drivers/regulator/s2mpa01.c
@@ -0,0 +1,497 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/bug.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/samsung/core.h>
+#include <linux/mfd/samsung/s2mpa01.h>
+
+#define S2MPA01_REGULATOR_CNT ARRAY_SIZE(regulators)
+
+struct s2mpa01_info {
+ struct regulator_dev *rdev[S2MPA01_REGULATOR_MAX];
+
+ int ramp_delay24;
+ int ramp_delay3;
+ int ramp_delay5;
+ int ramp_delay16;
+ int ramp_delay7;
+ int ramp_delay8910;
+};
+
+static int get_ramp_delay(int ramp_delay)
+{
+ unsigned char cnt = 0;
+
+ ramp_delay /= 6250;
+
+ while (true) {
+ ramp_delay = ramp_delay >> 1;
+ if (ramp_delay == 0)
+ break;
+ cnt++;
+ }
+
+ if (cnt > 3)
+ cnt = 3;
+
+ return cnt;
+}
+
+static int s2mpa01_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
+ unsigned int old_selector,
+ unsigned int new_selector)
+{
+ struct s2mpa01_info *s2mpa01 = rdev_get_drvdata(rdev);
+ unsigned int ramp_delay = 0;
+ int old_volt, new_volt;
+
+ switch (rdev->desc->id) {
+ case S2MPA01_BUCK2:
+ case S2MPA01_BUCK4:
+ ramp_delay = s2mpa01->ramp_delay24;
+ break;
+ case S2MPA01_BUCK3:
+ ramp_delay = s2mpa01->ramp_delay3;
+ break;
+ case S2MPA01_BUCK5:
+ ramp_delay = s2mpa01->ramp_delay5;
+ break;
+ case S2MPA01_BUCK1:
+ case S2MPA01_BUCK6:
+ ramp_delay = s2mpa01->ramp_delay16;
+ break;
+ case S2MPA01_BUCK7:
+ ramp_delay = s2mpa01->ramp_delay7;
+ break;
+ case S2MPA01_BUCK8:
+ case S2MPA01_BUCK9:
+ case S2MPA01_BUCK10:
+ ramp_delay = s2mpa01->ramp_delay8910;
+ break;
+ }
+
+ if (ramp_delay == 0)
+ ramp_delay = rdev->desc->ramp_delay;
+
+ old_volt = rdev->desc->min_uV + (rdev->desc->uV_step * old_selector);
+ new_volt = rdev->desc->min_uV + (rdev->desc->uV_step * new_selector);
+
+ return DIV_ROUND_UP(abs(new_volt - old_volt), ramp_delay);
+}
+
+static int s2mpa01_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
+{
+ struct s2mpa01_info *s2mpa01 = rdev_get_drvdata(rdev);
+ unsigned int ramp_val, ramp_shift, ramp_reg = S2MPA01_REG_RAMP2;
+ unsigned int ramp_enable = 1, enable_shift = 0;
+ int ret;
+
+ switch (rdev->desc->id) {
+ case S2MPA01_BUCK1:
+ enable_shift = S2MPA01_BUCK1_RAMP_EN_SHIFT;
+ if (!ramp_delay) {
+ ramp_enable = 0;
+ break;
+ }
+
+ if (ramp_delay > s2mpa01->ramp_delay16)
+ s2mpa01->ramp_delay16 = ramp_delay;
+ else
+ ramp_delay = s2mpa01->ramp_delay16;
+
+ ramp_shift = S2MPA01_BUCK16_RAMP_SHIFT;
+ ramp_reg = S2MPA01_REG_RAMP1;
+ break;
+ case S2MPA01_BUCK2:
+ enable_shift = S2MPA01_BUCK2_RAMP_EN_SHIFT;
+ if (!ramp_delay) {
+ ramp_enable = 0;
+ break;
+ }
+
+ if (ramp_delay > s2mpa01->ramp_delay24)
+ s2mpa01->ramp_delay24 = ramp_delay;
+ else
+ ramp_delay = s2mpa01->ramp_delay24;
+
+ ramp_shift = S2MPA01_BUCK24_RAMP_SHIFT;
+ ramp_reg = S2MPA01_REG_RAMP1;
+ break;
+ case S2MPA01_BUCK3:
+ enable_shift = S2MPA01_BUCK3_RAMP_EN_SHIFT;
+ if (!ramp_delay) {
+ ramp_enable = 0;
+ break;
+ }
+
+ s2mpa01->ramp_delay3 = ramp_delay;
+ ramp_shift = S2MPA01_BUCK3_RAMP_SHIFT;
+ ramp_reg = S2MPA01_REG_RAMP1;
+ break;
+ case S2MPA01_BUCK4:
+ enable_shift = S2MPA01_BUCK4_RAMP_EN_SHIFT;
+ if (!ramp_delay) {
+ ramp_enable = 0;
+ break;
+ }
+
+ if (ramp_delay > s2mpa01->ramp_delay24)
+ s2mpa01->ramp_delay24 = ramp_delay;
+ else
+ ramp_delay = s2mpa01->ramp_delay24;
+
+ ramp_shift = S2MPA01_BUCK24_RAMP_SHIFT;
+ ramp_reg = S2MPA01_REG_RAMP1;
+ break;
+ case S2MPA01_BUCK5:
+ s2mpa01->ramp_delay5 = ramp_delay;
+ ramp_shift = S2MPA01_BUCK5_RAMP_SHIFT;
+ break;
+ case S2MPA01_BUCK6:
+ if (ramp_delay > s2mpa01->ramp_delay16)
+ s2mpa01->ramp_delay16 = ramp_delay;
+ else
+ ramp_delay = s2mpa01->ramp_delay16;
+
+ ramp_shift = S2MPA01_BUCK16_RAMP_SHIFT;
+ break;
+ case S2MPA01_BUCK7:
+ s2mpa01->ramp_delay7 = ramp_delay;
+ ramp_shift = S2MPA01_BUCK7_RAMP_SHIFT;
+ break;
+ case S2MPA01_BUCK8:
+ case S2MPA01_BUCK9:
+ case S2MPA01_BUCK10:
+ if (ramp_delay > s2mpa01->ramp_delay8910)
+ s2mpa01->ramp_delay8910 = ramp_delay;
+ else
+ ramp_delay = s2mpa01->ramp_delay8910;
+
+ ramp_shift = S2MPA01_BUCK8910_RAMP_SHIFT;
+ break;
+ default:
+ return 0;
+ }
+
+ if (!ramp_enable)
+ goto ramp_disable;
+
+ if (enable_shift) {
+ ret = regmap_update_bits(rdev->regmap, S2MPA01_REG_RAMP1,
+ 1 << enable_shift, 1 << enable_shift);
+ if (ret) {
+ dev_err(&rdev->dev, "failed to enable ramp rate\n");
+ return ret;
+ }
+ }
+
+ ramp_val = get_ramp_delay(ramp_delay);
+
+ return regmap_update_bits(rdev->regmap, ramp_reg, 0x3 << ramp_shift,
+ ramp_val << ramp_shift);
+
+ramp_disable:
+ return regmap_update_bits(rdev->regmap, S2MPA01_REG_RAMP1,
+ 1 << enable_shift, 0);
+}
+
+static struct regulator_ops s2mpa01_ldo_ops = {
+ .list_voltage = regulator_list_voltage_linear,
+ .map_voltage = regulator_map_voltage_linear,
+ .is_enabled = regulator_is_enabled_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+static struct regulator_ops s2mpa01_buck_ops = {
+ .list_voltage = regulator_list_voltage_linear,
+ .map_voltage = regulator_map_voltage_linear,
+ .is_enabled = regulator_is_enabled_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .set_voltage_time_sel = s2mpa01_regulator_set_voltage_time_sel,
+ .set_ramp_delay = s2mpa01_set_ramp_delay,
+};
+
+#define regulator_desc_ldo1(num) { \
+ .name = "LDO"#num, \
+ .id = S2MPA01_LDO##num, \
+ .ops = &s2mpa01_ldo_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = S2MPA01_LDO_MIN, \
+ .uV_step = S2MPA01_LDO_STEP1, \
+ .n_voltages = S2MPA01_LDO_N_VOLTAGES, \
+ .vsel_reg = S2MPA01_REG_L1CTRL + num - 1, \
+ .vsel_mask = S2MPA01_LDO_VSEL_MASK, \
+ .enable_reg = S2MPA01_REG_L1CTRL + num - 1, \
+ .enable_mask = S2MPA01_ENABLE_MASK \
+}
+#define regulator_desc_ldo2(num) { \
+ .name = "LDO"#num, \
+ .id = S2MPA01_LDO##num, \
+ .ops = &s2mpa01_ldo_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = S2MPA01_LDO_MIN, \
+ .uV_step = S2MPA01_LDO_STEP2, \
+ .n_voltages = S2MPA01_LDO_N_VOLTAGES, \
+ .vsel_reg = S2MPA01_REG_L1CTRL + num - 1, \
+ .vsel_mask = S2MPA01_LDO_VSEL_MASK, \
+ .enable_reg = S2MPA01_REG_L1CTRL + num - 1, \
+ .enable_mask = S2MPA01_ENABLE_MASK \
+}
+
+#define regulator_desc_buck1_4(num) { \
+ .name = "BUCK"#num, \
+ .id = S2MPA01_BUCK##num, \
+ .ops = &s2mpa01_buck_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = S2MPA01_BUCK_MIN1, \
+ .uV_step = S2MPA01_BUCK_STEP1, \
+ .n_voltages = S2MPA01_BUCK_N_VOLTAGES, \
+ .ramp_delay = S2MPA01_RAMP_DELAY, \
+ .vsel_reg = S2MPA01_REG_B1CTRL2 + (num - 1) * 2, \
+ .vsel_mask = S2MPA01_BUCK_VSEL_MASK, \
+ .enable_reg = S2MPA01_REG_B1CTRL1 + (num - 1) * 2, \
+ .enable_mask = S2MPA01_ENABLE_MASK \
+}
+
+#define regulator_desc_buck5 { \
+ .name = "BUCK5", \
+ .id = S2MPA01_BUCK5, \
+ .ops = &s2mpa01_buck_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = S2MPA01_BUCK_MIN2, \
+ .uV_step = S2MPA01_BUCK_STEP1, \
+ .n_voltages = S2MPA01_BUCK_N_VOLTAGES, \
+ .ramp_delay = S2MPA01_RAMP_DELAY, \
+ .vsel_reg = S2MPA01_REG_B5CTRL2, \
+ .vsel_mask = S2MPA01_BUCK_VSEL_MASK, \
+ .enable_reg = S2MPA01_REG_B5CTRL1, \
+ .enable_mask = S2MPA01_ENABLE_MASK \
+}
+
+#define regulator_desc_buck6_7(num) { \
+ .name = "BUCK"#num, \
+ .id = S2MPA01_BUCK##num, \
+ .ops = &s2mpa01_buck_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = S2MPA01_BUCK_MIN1, \
+ .uV_step = S2MPA01_BUCK_STEP1, \
+ .n_voltages = S2MPA01_BUCK_N_VOLTAGES, \
+ .ramp_delay = S2MPA01_RAMP_DELAY, \
+ .vsel_reg = S2MPA01_REG_B6CTRL2 + (num - 6) * 2, \
+ .vsel_mask = S2MPA01_BUCK_VSEL_MASK, \
+ .enable_reg = S2MPA01_REG_B6CTRL1 + (num - 6) * 2, \
+ .enable_mask = S2MPA01_ENABLE_MASK \
+}
+
+#define regulator_desc_buck8 { \
+ .name = "BUCK8", \
+ .id = S2MPA01_BUCK8, \
+ .ops = &s2mpa01_buck_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = S2MPA01_BUCK_MIN2, \
+ .uV_step = S2MPA01_BUCK_STEP2, \
+ .n_voltages = S2MPA01_BUCK_N_VOLTAGES, \
+ .ramp_delay = S2MPA01_RAMP_DELAY, \
+ .vsel_reg = S2MPA01_REG_B8CTRL2, \
+ .vsel_mask = S2MPA01_BUCK_VSEL_MASK, \
+ .enable_reg = S2MPA01_REG_B8CTRL1, \
+ .enable_mask = S2MPA01_ENABLE_MASK \
+}
+
+#define regulator_desc_buck9 { \
+ .name = "BUCK9", \
+ .id = S2MPA01_BUCK9, \
+ .ops = &s2mpa01_buck_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = S2MPA01_BUCK_MIN4, \
+ .uV_step = S2MPA01_BUCK_STEP2, \
+ .n_voltages = S2MPA01_BUCK_N_VOLTAGES, \
+ .ramp_delay = S2MPA01_RAMP_DELAY, \
+ .vsel_reg = S2MPA01_REG_B9CTRL2, \
+ .vsel_mask = S2MPA01_BUCK_VSEL_MASK, \
+ .enable_reg = S2MPA01_REG_B9CTRL1, \
+ .enable_mask = S2MPA01_ENABLE_MASK \
+}
+
+#define regulator_desc_buck10 { \
+ .name = "BUCK10", \
+ .id = S2MPA01_BUCK10, \
+ .ops = &s2mpa01_buck_ops, \
+ .type = REGULATOR_VOLTAGE, \
+ .owner = THIS_MODULE, \
+ .min_uV = S2MPA01_BUCK_MIN3, \
+ .uV_step = S2MPA01_BUCK_STEP2, \
+ .n_voltages = S2MPA01_BUCK_N_VOLTAGES, \
+ .ramp_delay = S2MPA01_RAMP_DELAY, \
+ .vsel_reg = S2MPA01_REG_B10CTRL2, \
+ .vsel_mask = S2MPA01_BUCK_VSEL_MASK, \
+ .enable_reg = S2MPA01_REG_B10CTRL1, \
+ .enable_mask = S2MPA01_ENABLE_MASK \
+}
+
+static struct regulator_desc regulators[] = {
+ regulator_desc_ldo2(1),
+ regulator_desc_ldo1(2),
+ regulator_desc_ldo1(3),
+ regulator_desc_ldo1(4),
+ regulator_desc_ldo1(5),
+ regulator_desc_ldo2(6),
+ regulator_desc_ldo1(7),
+ regulator_desc_ldo1(8),
+ regulator_desc_ldo1(9),
+ regulator_desc_ldo1(10),
+ regulator_desc_ldo2(11),
+ regulator_desc_ldo1(12),
+ regulator_desc_ldo1(13),
+ regulator_desc_ldo1(14),
+ regulator_desc_ldo1(15),
+ regulator_desc_ldo1(16),
+ regulator_desc_ldo1(17),
+ regulator_desc_ldo1(18),
+ regulator_desc_ldo1(19),
+ regulator_desc_ldo1(20),
+ regulator_desc_ldo1(21),
+ regulator_desc_ldo2(22),
+ regulator_desc_ldo2(23),
+ regulator_desc_ldo1(24),
+ regulator_desc_ldo1(25),
+ regulator_desc_ldo1(26),
+ regulator_desc_buck1_4(1),
+ regulator_desc_buck1_4(2),
+ regulator_desc_buck1_4(3),
+ regulator_desc_buck1_4(4),
+ regulator_desc_buck5,
+ regulator_desc_buck6_7(6),
+ regulator_desc_buck6_7(7),
+ regulator_desc_buck8,
+ regulator_desc_buck9,
+ regulator_desc_buck10,
+};
+
+static int s2mpa01_pmic_probe(struct platform_device *pdev)
+{
+ struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+ struct sec_platform_data *pdata = dev_get_platdata(iodev->dev);
+ struct of_regulator_match rdata[S2MPA01_REGULATOR_MAX];
+ struct device_node *reg_np = NULL;
+ struct regulator_config config = { };
+ struct s2mpa01_info *s2mpa01;
+ int i, ret;
+
+ s2mpa01 = devm_kzalloc(&pdev->dev, sizeof(*s2mpa01), GFP_KERNEL);
+ if (!s2mpa01)
+ return -ENOMEM;
+
+ if (!iodev->dev->of_node) {
+ if (pdata) {
+ goto common_reg;
+ } else {
+ dev_err(pdev->dev.parent,
+ "Platform data or DT node not supplied\n");
+ return -ENODEV;
+ }
+ }
+
+ for (i = 0; i < S2MPA01_REGULATOR_CNT; i++)
+ rdata[i].name = regulators[i].name;
+
+ reg_np = of_find_node_by_name(iodev->dev->of_node, "regulators");
+ if (!reg_np) {
+ dev_err(&pdev->dev, "could not find regulators sub-node\n");
+ return -EINVAL;
+ }
+
+ of_regulator_match(&pdev->dev, reg_np, rdata, S2MPA01_REGULATOR_MAX);
+
+common_reg:
+ platform_set_drvdata(pdev, s2mpa01);
+
+ config.dev = &pdev->dev;
+ config.regmap = iodev->regmap_pmic;
+ config.driver_data = s2mpa01;
+
+ for (i = 0; i < S2MPA01_REGULATOR_MAX; i++) {
+ if (!reg_np) {
+ config.init_data = pdata->regulators[i].initdata;
+ } else {
+ config.init_data = rdata[i].init_data;
+ config.of_node = rdata[i].of_node;
+ }
+
+ s2mpa01->rdev[i] = devm_regulator_register(&pdev->dev,
+ &regulators[i], &config);
+ if (IS_ERR(s2mpa01->rdev[i])) {
+ ret = PTR_ERR(s2mpa01->rdev[i]);
+ dev_err(&pdev->dev, "regulator init failed for %d\n",
+ i);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static const struct platform_device_id s2mpa01_pmic_id[] = {
+ { "s2mpa01-pmic", 0},
+ { },
+};
+MODULE_DEVICE_TABLE(platform, s2mpa01_pmic_id);
+
+static struct platform_driver s2mpa01_pmic_driver = {
+ .driver = {
+ .name = "s2mpa01-pmic",
+ .owner = THIS_MODULE,
+ },
+ .probe = s2mpa01_pmic_probe,
+ .id_table = s2mpa01_pmic_id,
+};
+
+static int __init s2mpa01_pmic_init(void)
+{
+ return platform_driver_register(&s2mpa01_pmic_driver);
+}
+subsys_initcall(s2mpa01_pmic_init);
+
+static void __exit s2mpa01_pmic_exit(void)
+{
+ platform_driver_unregister(&s2mpa01_pmic_driver);
+}
+module_exit(s2mpa01_pmic_exit);
+
+/* Module information */
+MODULE_AUTHOR("Sangbeom Kim <[email protected]>");
+MODULE_AUTHOR("Sachin Kamat <[email protected]>");
+MODULE_DESCRIPTION("SAMSUNG S2MPA01 Regulator Driver");
+MODULE_LICENSE("GPL");
--
1.7.9.5

2014-01-08 06:31:03

by Sachin Kamat

[permalink] [raw]
Subject: [PATCH 3/3] Documentation: mfd: Add binding document for S2MPA01

Added initial binding documentation for S2MPA01 MFD.

Signed-off-by: Sachin Kamat <[email protected]>
---
Documentation/devicetree/bindings/mfd/s2mpa01.txt | 91 +++++++++++++++++++++
1 file changed, 91 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/s2mpa01.txt

diff --git a/Documentation/devicetree/bindings/mfd/s2mpa01.txt b/Documentation/devicetree/bindings/mfd/s2mpa01.txt
new file mode 100644
index 000000000000..ae750a28821b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/s2mpa01.txt
@@ -0,0 +1,91 @@
+
+* Samsung S2MPA01 Voltage and Current Regulator
+
+The Samsung S2MPA01 is a multi-function device which includes high
+efficiency buck converters including Dual-Phase buck converter, various LDOs,
+and an RTC. It is interfaced to the host controller using an I2C interface.
+Each sub-block is addressed by the host system using different I2C slave
+addresses.
+
+Required properties:
+- compatible: Should be "samsung,s2mpa01-pmic".
+- 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 s2mpa01 are delivered to.
+- interrupts: Interrupt specifiers for interrupt sources.
+
+Optional nodes:
+- regulators: The regulators of s2mpa01 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 constraints....];
+ };
+
+ regulator-ramp-delay for BUCKs = [6250/12500(default)/25000/50000] uV/us
+
+ BUCK[1/2/3/4] supports disabling ramp delay on hardware, so explictly
+ regulator-ramp-delay = <0> can be used for them to disable ramp delay.
+ In the absence of the regulator-ramp-delay property, the default ramp
+ delay will be used.
+
+NOTE: Some BUCKs share the ramp rate setting i.e. same ramp value will be set
+for a particular group of BUCKs. So provide same regulator-ramp-delay<value>.
+Grouping of BUCKs sharing ramp rate setting is as follow : BUCK[1, 6],
+BUCK[2, 4], and BUCK[8, 9, 10]
+
+The regulator constraints inside the regulator nodes use the standard regulator
+bindings which are documented elsewhere.
+
+The following are the names of the regulators that the s2mpa01 PMIC block
+supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
+as per the datasheet of s2mpa01.
+
+ - LDOn
+ - valid values for n are 1 to 26
+ - Example: LDO1, LD02, LDO26
+ - BUCKn
+ - valid values for n are 1 to 10.
+ - Example: BUCK1, BUCK2, BUCK9
+
+Example:
+
+ s2mpa01_pmic@66 {
+ compatible = "samsung,s2mpa01-pmic";
+ reg = <0x66>;
+
+ regulators {
+ ldo1_reg: LDO1 {
+ regulator-name = "VDD_ALIVE";
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <1000000>;
+ };
+
+ ldo2_reg: LDO2 {
+ regulator-name = "VDDQ_MMC2";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-always-on;
+ };
+
+ buck1_reg: BUCK1 {
+ regulator-name = "vdd_mif";
+ regulator-min-microvolt = <950000>;
+ regulator-max-microvolt = <1350000>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ buck2_reg: BUCK2 {
+ regulator-name = "vdd_arm";
+ regulator-min-microvolt = <950000>;
+ regulator-max-microvolt = <1350000>;
+ regulator-always-on;
+ regulator-boot-on;
+ regulator-ramp-delay = <50000>;
+ };
+ };
+ };
--
1.7.9.5

2014-01-08 09:02:25

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/3] mfd: Add support for S2MPA01 device

> Add the necessary entries required for S2MPA01 multi-function
> device. While at it also convert whitespaces to tabs in core.h.
>
> Signed-off-by: Sachin Kamat <[email protected]>
> ---
> drivers/mfd/sec-core.c | 39 +++++++
> include/linux/mfd/samsung/core.h | 16 ++-
> include/linux/mfd/samsung/irq.h | 50 +++++++++
> include/linux/mfd/samsung/s2mpa01.h | 192 +++++++++++++++++++++++++++++++++++
> 4 files changed, 292 insertions(+), 5 deletions(-)
> create mode 100644 include/linux/mfd/samsung/s2mpa01.h
>
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index a139798b8065..d2e890d50a36 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -26,6 +26,7 @@
> #include <linux/mfd/samsung/core.h>
> #include <linux/mfd/samsung/irq.h>
> #include <linux/mfd/samsung/rtc.h>
> +#include <linux/mfd/samsung/s2mpa01.h>
> #include <linux/mfd/samsung/s2mps11.h>
> #include <linux/mfd/samsung/s5m8763.h>
> #include <linux/mfd/samsung/s5m8767.h>
> @@ -69,6 +70,13 @@ static const struct mfd_cell s2mps11_devs[] = {
> }
> };
>
> +static const struct mfd_cell s2mpa01_devs[] = {
> + {
> + .name = "s2mpa01-pmic",
> + },
> +};
> +
> +

Extra '/n' here.

> #ifdef CONFIG_OF
> static struct of_device_id sec_dt_match[] = {
> { .compatible = "samsung,s5m8767-pmic",
> @@ -77,10 +85,25 @@ static struct of_device_id sec_dt_match[] = {
> { .compatible = "samsung,s2mps11-pmic",
> .data = (void *)S2MPS11X,
> },
> + { .compatible = "samsung,s2mpa01-pmic",
> + .data = (void *)S2MPA01,
> + },

Can you use the same format at the rest of the file.

I'm happy for you to fix the entire struct in this patch.

Rest of the file uses overlapping format:

}, {
.compatible = "samsung,s2mpa01-pmic",
},


<snip>

> @@ -92,7 +93,7 @@ struct sec_platform_data {
> int buck3_default_idx;
> int buck4_default_idx;
>
> - int buck_ramp_delay;
> + int buck_ramp_delay;

What's this? Are you fixing (or breaking) white space here?

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2014-01-08 09:21:22

by Sachin Kamat

[permalink] [raw]
Subject: Re: [PATCH 1/3] mfd: Add support for S2MPA01 device

Hi Lee,

Thanks for the review.

On 8 January 2014 14:32, Lee Jones <[email protected]> wrote:
>> Add the necessary entries required for S2MPA01 multi-function
>> device. While at it also convert whitespaces to tabs in core.h.
>>
>> Signed-off-by: Sachin Kamat <[email protected]>
>> ---
>> drivers/mfd/sec-core.c | 39 +++++++
>> include/linux/mfd/samsung/core.h | 16 ++-
>> include/linux/mfd/samsung/irq.h | 50 +++++++++
>> include/linux/mfd/samsung/s2mpa01.h | 192 +++++++++++++++++++++++++++++++++++
>> 4 files changed, 292 insertions(+), 5 deletions(-)
>> create mode 100644 include/linux/mfd/samsung/s2mpa01.h
>>
>> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
>> index a139798b8065..d2e890d50a36 100644
>> --- a/drivers/mfd/sec-core.c
>> +++ b/drivers/mfd/sec-core.c
>> @@ -26,6 +26,7 @@
>> #include <linux/mfd/samsung/core.h>
>> #include <linux/mfd/samsung/irq.h>
>> #include <linux/mfd/samsung/rtc.h>
>> +#include <linux/mfd/samsung/s2mpa01.h>
>> #include <linux/mfd/samsung/s2mps11.h>
>> #include <linux/mfd/samsung/s5m8763.h>
>> #include <linux/mfd/samsung/s5m8767.h>
>> @@ -69,6 +70,13 @@ static const struct mfd_cell s2mps11_devs[] = {
>> }
>> };
>>
>> +static const struct mfd_cell s2mpa01_devs[] = {
>> + {
>> + .name = "s2mpa01-pmic",
>> + },
>> +};
>> +
>> +
>
> Extra '/n' here.

Right. Will remove it.

>
>> #ifdef CONFIG_OF
>> static struct of_device_id sec_dt_match[] = {
>> { .compatible = "samsung,s5m8767-pmic",
>> @@ -77,10 +85,25 @@ static struct of_device_id sec_dt_match[] = {
>> { .compatible = "samsung,s2mps11-pmic",
>> .data = (void *)S2MPS11X,
>> },
>> + { .compatible = "samsung,s2mpa01-pmic",
>> + .data = (void *)S2MPA01,
>> + },
>
> Can you use the same format at the rest of the file.
>
> I'm happy for you to fix the entire struct in this patch.
>
> Rest of the file uses overlapping format:

Sure. I formatted it as per the current structure without noticing the others.
Will change it.

>
> }, {
> .compatible = "samsung,s2mpa01-pmic",
> },
>
>
> <snip>
>
>> @@ -92,7 +93,7 @@ struct sec_platform_data {
>> int buck3_default_idx;
>> int buck4_default_idx;
>>
>> - int buck_ramp_delay;
>> + int buck_ramp_delay;
>
> What's this? Are you fixing (or breaking) white space here?

I didn't get you. I have converted spaces to tabs here. See some problem?


--
With warm regards,
Sachin

2014-01-08 09:27:39

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/3] mfd: Add support for S2MPA01 device

> >> @@ -92,7 +93,7 @@ struct sec_platform_data {
> >> int buck3_default_idx;
> >> int buck4_default_idx;
> >>
> >> - int buck_ramp_delay;
> >> + int buck_ramp_delay;
> >
> > What's this? Are you fixing (or breaking) white space here?
>
> I didn't get you. I have converted spaces to tabs here. See some problem?

No problem. It was unclear in the patch, as white space changes do not
show up in my mail client.

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2014-01-08 09:41:21

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH 3/3] Documentation: mfd: Add binding document for S2MPA01

On Wed, Jan 08, 2014 at 06:26:30AM +0000, Sachin Kamat wrote:
> Added initial binding documentation for S2MPA01 MFD.
>
> Signed-off-by: Sachin Kamat <[email protected]>
> ---
> Documentation/devicetree/bindings/mfd/s2mpa01.txt | 91 +++++++++++++++++++++
> 1 file changed, 91 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/s2mpa01.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/s2mpa01.txt b/Documentation/devicetree/bindings/mfd/s2mpa01.txt
> new file mode 100644
> index 000000000000..ae750a28821b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/s2mpa01.txt
> @@ -0,0 +1,91 @@
> +
> +* Samsung S2MPA01 Voltage and Current Regulator
> +
> +The Samsung S2MPA01 is a multi-function device which includes high
> +efficiency buck converters including Dual-Phase buck converter, various LDOs,
> +and an RTC. It is interfaced to the host controller using an I2C interface.
> +Each sub-block is addressed by the host system using different I2C slave
> +addresses.
> +
> +Required properties:
> +- compatible: Should be "samsung,s2mpa01-pmic".
> +- 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 s2mpa01 are delivered to.
> +- interrupts: Interrupt specifiers for interrupt sources.

That sounds a bit odd. How many interrupts might there be? What do they
signal?

> +
> +Optional nodes:
> +- regulators: The regulators of s2mpa01 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 constraints....];
> + };
> +

Why not just say that the regulators node contains regulators in the
usual format, with reference to the regulator binding?

> + regulator-ramp-delay for BUCKs = [6250/12500(default)/25000/50000] uV/us
> +
> + BUCK[1/2/3/4] supports disabling ramp delay on hardware, so explictly
> + regulator-ramp-delay = <0> can be used for them to disable ramp delay.
> + In the absence of the regulator-ramp-delay property, the default ramp
> + delay will be used.
> +
> +NOTE: Some BUCKs share the ramp rate setting i.e. same ramp value will be set
> +for a particular group of BUCKs. So provide same regulator-ramp-delay<value>.
> +Grouping of BUCKs sharing ramp rate setting is as follow : BUCK[1, 6],
> +BUCK[2, 4], and BUCK[8, 9, 10]

It would probably be better to have a heading like "Properties for BUCK
regulator nodes" for all of the above.


> +
> +The regulator constraints inside the regulator nodes use the standard regulator
> +bindings which are documented elsewhere.

As mentioned above, state this at the beginning and point to the
regulator bindings.

> +
> +The following are the names of the regulators that the s2mpa01 PMIC block
> +supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
> +as per the datasheet of s2mpa01.

So these are what the nodes should be called? (rather than values for
regulator-name).

Otherwise this looks ok to me.

Thanks,
Mark.

2014-01-08 12:30:12

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/3] regulator: Add support for S2MPA01 regulator

On Wed, Jan 08, 2014 at 11:56:29AM +0530, Sachin Kamat wrote:

> + if (!iodev->dev->of_node) {
> + if (pdata) {
> + goto common_reg;
> + } else {
> + dev_err(pdev->dev.parent,
> + "Platform data or DT node not supplied\n");
> + return -ENODEV;
> + }
> + }

The driver shouldn't need these to probe, it should be able to start in
read only mode with no configuration.

> +static int __init s2mpa01_pmic_init(void)
> +{
> + return platform_driver_register(&s2mpa01_pmic_driver);
> +}
> +subsys_initcall(s2mpa01_pmic_init);

module_platform_driver().


Attachments:
(No filename) (560.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-01-09 04:47:03

by Sachin Kamat

[permalink] [raw]
Subject: Re: [PATCH 3/3] Documentation: mfd: Add binding document for S2MPA01

On 8 January 2014 15:11, Mark Rutland <[email protected]> wrote:
> On Wed, Jan 08, 2014 at 06:26:30AM +0000, Sachin Kamat wrote:
>> Added initial binding documentation for S2MPA01 MFD.
>>
>> Signed-off-by: Sachin Kamat <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mfd/s2mpa01.txt | 91 +++++++++++++++++++++
>> 1 file changed, 91 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/s2mpa01.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/s2mpa01.txt b/Documentation/devicetree/bindings/mfd/s2mpa01.txt
>> new file mode 100644
>> index 000000000000..ae750a28821b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/s2mpa01.txt
>> @@ -0,0 +1,91 @@
>> +
>> +* Samsung S2MPA01 Voltage and Current Regulator
>> +
>> +The Samsung S2MPA01 is a multi-function device which includes high
>> +efficiency buck converters including Dual-Phase buck converter, various LDOs,
>> +and an RTC. It is interfaced to the host controller using an I2C interface.
>> +Each sub-block is addressed by the host system using different I2C slave
>> +addresses.
>> +
>> +Required properties:
>> +- compatible: Should be "samsung,s2mpa01-pmic".
>> +- 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 s2mpa01 are delivered to.
>> +- interrupts: Interrupt specifiers for interrupt sources.
>
> That sounds a bit odd. How many interrupts might there be? What do they
> signal?

Actually there is only one interrupt line for sending interrupts to
the AP for conditons like sudden
voltage drop, etc. I will make the above sentence singular.

>> +
>> +Optional nodes:
>> +- regulators: The regulators of s2mpa01 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 constraints....];
>> + };
>> +
>
> Why not just say that the regulators node contains regulators in the
> usual format, with reference to the regulator binding?

OK. I will update this (as also suggested by your later comment).

>
>> + regulator-ramp-delay for BUCKs = [6250/12500(default)/25000/50000] uV/us
>> +
>> + BUCK[1/2/3/4] supports disabling ramp delay on hardware, so explictly
>> + regulator-ramp-delay = <0> can be used for them to disable ramp delay.
>> + In the absence of the regulator-ramp-delay property, the default ramp
>> + delay will be used.
>> +
>> +NOTE: Some BUCKs share the ramp rate setting i.e. same ramp value will be set
>> +for a particular group of BUCKs. So provide same regulator-ramp-delay<value>.
>> +Grouping of BUCKs sharing ramp rate setting is as follow : BUCK[1, 6],
>> +BUCK[2, 4], and BUCK[8, 9, 10]
>
> It would probably be better to have a heading like "Properties for BUCK
> regulator nodes" for all of the above.

OK.

>
>> +
>> +The regulator constraints inside the regulator nodes use the standard regulator
>> +bindings which are documented elsewhere.
>
> As mentioned above, state this at the beginning and point to the
> regulator bindings.

OK.

>> +
>> +The following are the names of the regulators that the s2mpa01 PMIC block
>> +supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
>> +as per the datasheet of s2mpa01.
>
> So these are what the nodes should be called? (rather than values for
> regulator-name).

Yes. That is the convention followed here.

>
> Otherwise this looks ok to me.
>

Thanks for reviewing.

--
With warm regards,
Sachin

2014-01-09 05:09:37

by Sachin Kamat

[permalink] [raw]
Subject: Re: [PATCH 2/3] regulator: Add support for S2MPA01 regulator

Hi Mark,

Thanks for reviewing.

On 8 January 2014 18:00, Mark Brown <[email protected]> wrote:
> On Wed, Jan 08, 2014 at 11:56:29AM +0530, Sachin Kamat wrote:
>
>> + if (!iodev->dev->of_node) {
>> + if (pdata) {
>> + goto common_reg;
>> + } else {
>> + dev_err(pdev->dev.parent,
>> + "Platform data or DT node not supplied\n");
>> + return -ENODEV;
>> + }
>> + }
>
> The driver shouldn't need these to probe, it should be able to start in
> read only mode with no configuration.

OK.

>
>> +static int __init s2mpa01_pmic_init(void)
>> +{
>> + return platform_driver_register(&s2mpa01_pmic_driver);
>> +}
>> +subsys_initcall(s2mpa01_pmic_init);
>
> module_platform_driver().

Just a small doubt here. Wouldn't changing this from subsys init to module init
cause problems to devices like mmc which would want the regulators available
(where they are not already enabled by the boot loader)?

--
With warm regards,
Sachin

2014-01-09 10:34:53

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/3] regulator: Add support for S2MPA01 regulator

On Thu, Jan 09, 2014 at 10:39:34AM +0530, Sachin Kamat wrote:
> On 8 January 2014 18:00, Mark Brown <[email protected]> wrote:

> >> +subsys_initcall(s2mpa01_pmic_init);

> > module_platform_driver().

> Just a small doubt here. Wouldn't changing this from subsys init to module init
> cause problems to devices like mmc which would want the regulators available
> (where they are not already enabled by the boot loader)?

This is resolved by using deferred probe.


Attachments:
(No filename) (466.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments