All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14
@ 2014-02-13  9:13 Krzysztof Kozlowski
  2014-02-13  9:13 ` [PATCH v2 01/14] mfd: sec: Add maximum RTC register for regmap config Krzysztof Kozlowski
                   ` (13 more replies)
  0 siblings, 14 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:13 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski

Hi,


This is second version of patchset adding support for S2MPS14 device to the
Samsung MFD driver family.


Changes since v1
================
1. Added Lee Jones' ACK-s.
2. Applied suggestions from review (Lee Jones).
3. Regulator: added __initconst to regulator_desc array (Yadwinder Singh Brar).
   This replaces the patch 7/15:
   "regulator: s2mps11: Choose number of supported regulators during probe"
   with:
   "regulator: s2mps11: Copy supported regulators from initconst"


Description
===========

The S2MPS14 is similar to S2MPS11 but it has fewer regulators, two
clocks instead of three and a little different registers layout.

The patchset is organized in following way:
1. Patches from 1 to 7 clean up the S2MPS1X/S5M876X drivers and prepare
   for adding S2MPS14 support (some symbol renaming is needed).
2. Patches from 8 to 10 add support for S2MPS14 to the MFD and regulator
   drivers. They depend on previous patches.
3. Patches 11 and 12 add opmode support for S2MPS14 regulator driver.
4. Patches 13 and 14 add support for S2MPS14 RTC and they depend on previous
   MFD and RTC patches.

Probably the best way to get everything working and merged into the linux-next
would be to obtain ACK-s from all maintainers and to put all the patches into
the mfd-next tree.


The patchset is based on linux-next: next-20140211 *with* patch:
mfd: sec-core: Fix possible NULL pointer dereference when i2c_new_dummy error


TODO
====
Add support for S2MPS14 to the S2MPS11 clock driver. The patch
is actually ready but it is based on the "Add support for clocks in S5M8767"
   http://thread.gmane.org/gmane.linux.kernel/1587881/focus=1587882
which didn't get their way into clk-next. I will wait for them.


Krzysztof Kozlowski (15):
  mfd: sec: Add maximum RTC register for regmap config
  mfd: sec: Select different RTC regmaps for devices
  mfd/rtc: sec/sec: Rename SEC* symbols to S5M
  rtc: s5m: Remove undocumented time init on first boot
  mfd: sec: Use consistent S2MPS11 RTC alarm interrupt indexes
  regulator: s2mps11: Constify regulator_desc array
  regulator: s2mps11: Copy supported regulators from initconst
  mfd: sec: Add support for S2MPS14
  regulator: s2mps11: Add support for S2MPS14 regulators
  Documentation: mfd: s2mps11: Document support for S2MPS14
  regulator: s2mps11: Add opmode for S2MPS14 regulators
  Documentation: mfd/regulator: s2mps11: Document the "op_mode"
    bindings
  rtc: s5m: Support different register layout
  rtc: s5m: Add support for S2MPS14 RTC
  Documentation: mfd: s2mps11: Describe S5M8767 and S2MPS14 clocks

 Documentation/devicetree/bindings/mfd/s2mps11.txt |   82 +++-
 drivers/mfd/sec-core.c                            |   57 ++-
 drivers/mfd/sec-irq.c                             |   97 ++++-
 drivers/regulator/s2mps11.c                       |  421 +++++++++++++++++----
 drivers/rtc/rtc-s5m.c                             |  284 +++++++++-----
 include/linux/mfd/samsung/core.h                  |    1 +
 include/linux/mfd/samsung/irq.h                   |   31 +-
 include/linux/mfd/samsung/rtc.h                   |  132 ++++---
 include/linux/mfd/samsung/s2mps14.h               |  171 +++++++++
 9 files changed, 1045 insertions(+), 231 deletions(-)
 create mode 100644 include/linux/mfd/samsung/s2mps14.h

-- 
1.7.9.5


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

* [PATCH v2 01/14] mfd: sec: Add maximum RTC register for regmap config
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
@ 2014-02-13  9:13 ` Krzysztof Kozlowski
  2014-02-13  9:13 ` [PATCH v2 02/14] mfd: sec: Select different RTC regmaps for devices Krzysztof Kozlowski
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:13 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski

Add maximum register to the regmap used by rtc-s5m driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/sec-core.c          |    2 ++
 include/linux/mfd/samsung/rtc.h |    2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 7c6ce2e4aaa6..6021c54f74cf 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -141,6 +141,8 @@ static const struct regmap_config s5m8767_regmap_config = {
 static const struct regmap_config sec_rtc_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
+
+	.max_register = SEC_RTC_REG_MAX,
 };
 
 #ifdef CONFIG_OF
diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h
index 94b7cd6d8891..4627f59ebd84 100644
--- a/include/linux/mfd/samsung/rtc.h
+++ b/include/linux/mfd/samsung/rtc.h
@@ -43,6 +43,8 @@ enum sec_rtc_reg {
 	SEC_RTC_STATUS,
 	SEC_WTSR_SMPL_CNTL,
 	SEC_RTC_UDR_CON,
+
+	SEC_RTC_REG_MAX,
 };
 
 #define RTC_I2C_ADDR		(0x0C >> 1)
-- 
1.7.9.5


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

* [PATCH v2 02/14] mfd: sec: Select different RTC regmaps for devices
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
  2014-02-13  9:13 ` [PATCH v2 01/14] mfd: sec: Add maximum RTC register for regmap config Krzysztof Kozlowski
@ 2014-02-13  9:13 ` Krzysztof Kozlowski
  2014-02-13  9:13 ` [PATCH v2 03/14] mfd/rtc: sec/s5m: Rename SEC* symbols to S5M Krzysztof Kozlowski
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:13 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski

This patch prepares for adding support for S2MPS14 RTC driver by
selecting different regmaps for S2MPS1X/S5M876X RTC devices.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/sec-core.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 6021c54f74cf..0efa69e123ee 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -199,7 +199,7 @@ static int sec_pmic_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
 	struct sec_platform_data *pdata = dev_get_platdata(&i2c->dev);
-	const struct regmap_config *regmap;
+	const struct regmap_config *regmap, *regmap_rtc;
 	struct sec_pmic_dev *sec_pmic;
 	int ret;
 
@@ -233,15 +233,25 @@ static int sec_pmic_probe(struct i2c_client *i2c,
 	switch (sec_pmic->device_type) {
 	case S2MPS11X:
 		regmap = &s2mps11_regmap_config;
+		/*
+		 * The rtc-s5m driver does not support S2MPS11 and there
+		 * is no mfd_cell for S2MPS11 RTC device.
+		 * However we must pass something to devm_regmap_init_i2c()
+		 * so use S5M-like regmap config even though it wouldn't work.
+		 */
+		regmap_rtc = &sec_rtc_regmap_config;
 		break;
 	case S5M8763X:
 		regmap = &s5m8763_regmap_config;
+		regmap_rtc = &sec_rtc_regmap_config;
 		break;
 	case S5M8767X:
 		regmap = &s5m8767_regmap_config;
+		regmap_rtc = &sec_rtc_regmap_config;
 		break;
 	default:
 		regmap = &sec_regmap_config;
+		regmap_rtc = &sec_rtc_regmap_config;
 		break;
 	}
 
@@ -260,8 +270,7 @@ static int sec_pmic_probe(struct i2c_client *i2c,
 	}
 	i2c_set_clientdata(sec_pmic->rtc, sec_pmic);
 
-	sec_pmic->regmap_rtc = devm_regmap_init_i2c(sec_pmic->rtc,
-			&sec_rtc_regmap_config);
+	sec_pmic->regmap_rtc = devm_regmap_init_i2c(sec_pmic->rtc, regmap_rtc);
 	if (IS_ERR(sec_pmic->regmap_rtc)) {
 		ret = PTR_ERR(sec_pmic->regmap_rtc);
 		dev_err(&i2c->dev, "Failed to allocate RTC register map: %d\n",
-- 
1.7.9.5


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

* [PATCH v2 03/14] mfd/rtc: sec/s5m: Rename SEC* symbols to S5M
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
  2014-02-13  9:13 ` [PATCH v2 01/14] mfd: sec: Add maximum RTC register for regmap config Krzysztof Kozlowski
  2014-02-13  9:13 ` [PATCH v2 02/14] mfd: sec: Select different RTC regmaps for devices Krzysztof Kozlowski
@ 2014-02-13  9:13 ` Krzysztof Kozlowski
  2014-02-13 10:11   ` Lee Jones
  2014-02-13  9:13 ` [PATCH v2 04/14] rtc: s5m: Remove undocumented time init on first boot Krzysztof Kozlowski
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:13 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Alessandro Zummo, rtc-linux

This patch prepares for adding support for S2MPS14 RTC device to the
rtc-s5m driver:
1. Renames SEC* symbols to S5M.
2. Adds S5M prefix to some of defines which are different between S5M876X
and S2MPS14.

This is only a rename-like patch, new code is not added.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux@googlegroups.com
---
 drivers/mfd/sec-core.c          |    2 +-
 drivers/rtc/rtc-s5m.c           |   64 ++++++++++++++++-----------------
 include/linux/mfd/samsung/rtc.h |   76 +++++++++++++++++++--------------------
 3 files changed, 71 insertions(+), 71 deletions(-)

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 0efa69e123ee..8504de82b7e0 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -142,7 +142,7 @@ static const struct regmap_config sec_rtc_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 
-	.max_register = SEC_RTC_REG_MAX,
+	.max_register = S5M_RTC_REG_MAX,
 };
 
 #ifdef CONFIG_OF
diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index 476af93543f6..d26e2480f8b3 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -30,10 +30,10 @@
 
 /*
  * Maximum number of retries for checking changes in UDR field
- * of SEC_RTC_UDR_CON register (to limit possible endless loop).
+ * of S5M_RTC_UDR_CON register (to limit possible endless loop).
  *
  * After writing to RTC registers (setting time or alarm) read the UDR field
- * in SEC_RTC_UDR_CON register. UDR is auto-cleared when data have
+ * in S5M_RTC_UDR_CON register. UDR is auto-cleared when data have
  * been transferred.
  */
 #define UDR_READ_RETRY_CNT	5
@@ -104,8 +104,8 @@ static inline int s5m8767_wait_for_udr_update(struct s5m_rtc_info *info)
 	unsigned int data;
 
 	do {
-		ret = regmap_read(info->regmap, SEC_RTC_UDR_CON, &data);
-	} while (--retry && (data & RTC_UDR_MASK) && !ret);
+		ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
+	} while (--retry && (data & S5M_RTC_UDR_MASK) && !ret);
 
 	if (!retry)
 		dev_err(info->dev, "waiting for UDR update, reached max number of retries\n");
@@ -118,16 +118,16 @@ static inline int s5m8767_rtc_set_time_reg(struct s5m_rtc_info *info)
 	int ret;
 	unsigned int data;
 
-	ret = regmap_read(info->regmap, SEC_RTC_UDR_CON, &data);
+	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
 	if (ret < 0) {
 		dev_err(info->dev, "failed to read update reg(%d)\n", ret);
 		return ret;
 	}
 
-	data |= RTC_TIME_EN_MASK;
-	data |= RTC_UDR_MASK;
+	data |= S5M_RTC_TIME_EN_MASK;
+	data |= S5M_RTC_UDR_MASK;
 
-	ret = regmap_write(info->regmap, SEC_RTC_UDR_CON, data);
+	ret = regmap_write(info->regmap, S5M_RTC_UDR_CON, data);
 	if (ret < 0) {
 		dev_err(info->dev, "failed to write update reg(%d)\n", ret);
 		return ret;
@@ -143,17 +143,17 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info)
 	int ret;
 	unsigned int data;
 
-	ret = regmap_read(info->regmap, SEC_RTC_UDR_CON, &data);
+	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
 	if (ret < 0) {
 		dev_err(info->dev, "%s: fail to read update reg(%d)\n",
 			__func__, ret);
 		return ret;
 	}
 
-	data &= ~RTC_TIME_EN_MASK;
-	data |= RTC_UDR_MASK;
+	data &= ~S5M_RTC_TIME_EN_MASK;
+	data |= S5M_RTC_UDR_MASK;
 
-	ret = regmap_write(info->regmap, SEC_RTC_UDR_CON, data);
+	ret = regmap_write(info->regmap, S5M_RTC_UDR_CON, data);
 	if (ret < 0) {
 		dev_err(info->dev, "%s: fail to write update reg(%d)\n",
 			__func__, ret);
@@ -203,7 +203,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	u8 data[8];
 	int ret;
 
-	ret = regmap_bulk_read(info->regmap, SEC_RTC_SEC, data, 8);
+	ret = regmap_bulk_read(info->regmap, S5M_RTC_SEC, data, 8);
 	if (ret < 0)
 		return ret;
 
@@ -251,7 +251,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm)
 		1900 + tm->tm_year, 1 + tm->tm_mon, tm->tm_mday,
 		tm->tm_hour, tm->tm_min, tm->tm_sec, tm->tm_wday);
 
-	ret = regmap_raw_write(info->regmap, SEC_RTC_SEC, data, 8);
+	ret = regmap_raw_write(info->regmap, S5M_RTC_SEC, data, 8);
 	if (ret < 0)
 		return ret;
 
@@ -267,20 +267,20 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	unsigned int val;
 	int ret, i;
 
-	ret = regmap_bulk_read(info->regmap, SEC_ALARM0_SEC, data, 8);
+	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
 	if (ret < 0)
 		return ret;
 
 	switch (info->device_type) {
 	case S5M8763X:
 		s5m8763_data_to_tm(data, &alrm->time);
-		ret = regmap_read(info->regmap, SEC_ALARM0_CONF, &val);
+		ret = regmap_read(info->regmap, S5M_ALARM0_CONF, &val);
 		if (ret < 0)
 			return ret;
 
 		alrm->enabled = !!val;
 
-		ret = regmap_read(info->regmap, SEC_RTC_STATUS, &val);
+		ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val);
 		if (ret < 0)
 			return ret;
 
@@ -303,7 +303,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 		}
 
 		alrm->pending = 0;
-		ret = regmap_read(info->regmap, SEC_RTC_STATUS, &val);
+		ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val);
 		if (ret < 0)
 			return ret;
 		break;
@@ -312,7 +312,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 		return -EINVAL;
 	}
 
-	if (val & ALARM0_STATUS)
+	if (val & S5M_ALARM0_STATUS)
 		alrm->pending = 1;
 	else
 		alrm->pending = 0;
@@ -326,7 +326,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
 	int ret, i;
 	struct rtc_time tm;
 
-	ret = regmap_bulk_read(info->regmap, SEC_ALARM0_SEC, data, 8);
+	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
 	if (ret < 0)
 		return ret;
 
@@ -337,14 +337,14 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
 
 	switch (info->device_type) {
 	case S5M8763X:
-		ret = regmap_write(info->regmap, SEC_ALARM0_CONF, 0);
+		ret = regmap_write(info->regmap, S5M_ALARM0_CONF, 0);
 		break;
 
 	case S5M8767X:
 		for (i = 0; i < 7; i++)
 			data[i] &= ~ALARM_ENABLE_MASK;
 
-		ret = regmap_raw_write(info->regmap, SEC_ALARM0_SEC, data, 8);
+		ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
 		if (ret < 0)
 			return ret;
 
@@ -366,7 +366,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
 	u8 alarm0_conf;
 	struct rtc_time tm;
 
-	ret = regmap_bulk_read(info->regmap, SEC_ALARM0_SEC, data, 8);
+	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
 	if (ret < 0)
 		return ret;
 
@@ -378,7 +378,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
 	switch (info->device_type) {
 	case S5M8763X:
 		alarm0_conf = 0x77;
-		ret = regmap_write(info->regmap, SEC_ALARM0_CONF, alarm0_conf);
+		ret = regmap_write(info->regmap, S5M_ALARM0_CONF, alarm0_conf);
 		break;
 
 	case S5M8767X:
@@ -393,7 +393,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
 		if (data[RTC_YEAR1] & 0x7f)
 			data[RTC_YEAR1] |= ALARM_ENABLE_MASK;
 
-		ret = regmap_raw_write(info->regmap, SEC_ALARM0_SEC, data, 8);
+		ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
 		if (ret < 0)
 			return ret;
 		ret = s5m8767_rtc_set_alarm_reg(info);
@@ -435,7 +435,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (ret < 0)
 		return ret;
 
-	ret = regmap_raw_write(info->regmap, SEC_ALARM0_SEC, data, 8);
+	ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
 	if (ret < 0)
 		return ret;
 
@@ -480,7 +480,7 @@ static const struct rtc_class_ops s5m_rtc_ops = {
 static void s5m_rtc_enable_wtsr(struct s5m_rtc_info *info, bool enable)
 {
 	int ret;
-	ret = regmap_update_bits(info->regmap, SEC_WTSR_SMPL_CNTL,
+	ret = regmap_update_bits(info->regmap, S5M_WTSR_SMPL_CNTL,
 				 WTSR_ENABLE_MASK,
 				 enable ? WTSR_ENABLE_MASK : 0);
 	if (ret < 0)
@@ -491,7 +491,7 @@ static void s5m_rtc_enable_wtsr(struct s5m_rtc_info *info, bool enable)
 static void s5m_rtc_enable_smpl(struct s5m_rtc_info *info, bool enable)
 {
 	int ret;
-	ret = regmap_update_bits(info->regmap, SEC_WTSR_SMPL_CNTL,
+	ret = regmap_update_bits(info->regmap, S5M_WTSR_SMPL_CNTL,
 				 SMPL_ENABLE_MASK,
 				 enable ? SMPL_ENABLE_MASK : 0);
 	if (ret < 0)
@@ -506,7 +506,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
 	int ret;
 	struct rtc_time tm;
 
-	ret = regmap_read(info->regmap, SEC_RTC_UDR_CON, &tp_read);
+	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &tp_read);
 	if (ret < 0) {
 		dev_err(info->dev, "%s: fail to read control reg(%d)\n",
 			__func__, ret);
@@ -518,7 +518,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
 	data[1] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
 
 	info->rtc_24hr_mode = 1;
-	ret = regmap_raw_write(info->regmap, SEC_ALARM0_CONF, data, 2);
+	ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2);
 	if (ret < 0) {
 		dev_err(info->dev, "%s: fail to write controlm reg(%d)\n",
 			__func__, ret);
@@ -540,7 +540,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
 		ret = s5m_rtc_set_time(info->dev, &tm);
 	}
 
-	ret = regmap_update_bits(info->regmap, SEC_RTC_UDR_CON,
+	ret = regmap_update_bits(info->regmap, S5M_RTC_UDR_CON,
 				 RTC_TCON_MASK, tp_read | RTC_TCON_MASK);
 	if (ret < 0)
 		dev_err(info->dev, "%s: fail to update TCON reg(%d)\n",
@@ -623,7 +623,7 @@ static void s5m_rtc_shutdown(struct platform_device *pdev)
 	if (info->wtsr_smpl) {
 		for (i = 0; i < 3; i++) {
 			s5m_rtc_enable_wtsr(info, false);
-			regmap_read(info->regmap, SEC_WTSR_SMPL_CNTL, &val);
+			regmap_read(info->regmap, S5M_WTSR_SMPL_CNTL, &val);
 			pr_debug("%s: WTSR_SMPL reg(0x%02x)\n", __func__, val);
 			if (val & WTSR_ENABLE_MASK)
 				pr_emerg("%s: fail to disable WTSR\n",
diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h
index 4627f59ebd84..bdf0573891d0 100644
--- a/include/linux/mfd/samsung/rtc.h
+++ b/include/linux/mfd/samsung/rtc.h
@@ -13,38 +13,38 @@
 #ifndef __LINUX_MFD_SEC_RTC_H
 #define __LINUX_MFD_SEC_RTC_H
 
-enum sec_rtc_reg {
-	SEC_RTC_SEC,
-	SEC_RTC_MIN,
-	SEC_RTC_HOUR,
-	SEC_RTC_WEEKDAY,
-	SEC_RTC_DATE,
-	SEC_RTC_MONTH,
-	SEC_RTC_YEAR1,
-	SEC_RTC_YEAR2,
-	SEC_ALARM0_SEC,
-	SEC_ALARM0_MIN,
-	SEC_ALARM0_HOUR,
-	SEC_ALARM0_WEEKDAY,
-	SEC_ALARM0_DATE,
-	SEC_ALARM0_MONTH,
-	SEC_ALARM0_YEAR1,
-	SEC_ALARM0_YEAR2,
-	SEC_ALARM1_SEC,
-	SEC_ALARM1_MIN,
-	SEC_ALARM1_HOUR,
-	SEC_ALARM1_WEEKDAY,
-	SEC_ALARM1_DATE,
-	SEC_ALARM1_MONTH,
-	SEC_ALARM1_YEAR1,
-	SEC_ALARM1_YEAR2,
-	SEC_ALARM0_CONF,
-	SEC_ALARM1_CONF,
-	SEC_RTC_STATUS,
-	SEC_WTSR_SMPL_CNTL,
-	SEC_RTC_UDR_CON,
+enum s5m_rtc_reg {
+	S5M_RTC_SEC,
+	S5M_RTC_MIN,
+	S5M_RTC_HOUR,
+	S5M_RTC_WEEKDAY,
+	S5M_RTC_DATE,
+	S5M_RTC_MONTH,
+	S5M_RTC_YEAR1,
+	S5M_RTC_YEAR2,
+	S5M_ALARM0_SEC,
+	S5M_ALARM0_MIN,
+	S5M_ALARM0_HOUR,
+	S5M_ALARM0_WEEKDAY,
+	S5M_ALARM0_DATE,
+	S5M_ALARM0_MONTH,
+	S5M_ALARM0_YEAR1,
+	S5M_ALARM0_YEAR2,
+	S5M_ALARM1_SEC,
+	S5M_ALARM1_MIN,
+	S5M_ALARM1_HOUR,
+	S5M_ALARM1_WEEKDAY,
+	S5M_ALARM1_DATE,
+	S5M_ALARM1_MONTH,
+	S5M_ALARM1_YEAR1,
+	S5M_ALARM1_YEAR2,
+	S5M_ALARM0_CONF,
+	S5M_ALARM1_CONF,
+	S5M_RTC_STATUS,
+	S5M_WTSR_SMPL_CNTL,
+	S5M_RTC_UDR_CON,
 
-	SEC_RTC_REG_MAX,
+	S5M_RTC_REG_MAX,
 };
 
 #define RTC_I2C_ADDR		(0x0C >> 1)
@@ -52,9 +52,9 @@ enum sec_rtc_reg {
 #define HOUR_12			(1 << 7)
 #define HOUR_AMPM		(1 << 6)
 #define HOUR_PM			(1 << 5)
-#define ALARM0_STATUS		(1 << 1)
-#define ALARM1_STATUS		(1 << 2)
-#define UPDATE_AD		(1 << 0)
+#define S5M_ALARM0_STATUS	(1 << 1)
+#define S5M_ALARM1_STATUS	(1 << 2)
+#define S5M_UPDATE_AD		(1 << 0)
 
 /* RTC Control Register */
 #define BCD_EN_SHIFT		0
@@ -62,12 +62,12 @@ enum sec_rtc_reg {
 #define MODEL24_SHIFT		1
 #define MODEL24_MASK		(1 << MODEL24_SHIFT)
 /* RTC Update Register1 */
-#define RTC_UDR_SHIFT		0
-#define RTC_UDR_MASK		(1 << RTC_UDR_SHIFT)
+#define S5M_RTC_UDR_SHIFT	0
+#define S5M_RTC_UDR_MASK	(1 << S5M_RTC_UDR_SHIFT)
 #define RTC_TCON_SHIFT		1
 #define RTC_TCON_MASK		(1 << RTC_TCON_SHIFT)
-#define RTC_TIME_EN_SHIFT	3
-#define RTC_TIME_EN_MASK	(1 << RTC_TIME_EN_SHIFT)
+#define S5M_RTC_TIME_EN_SHIFT	3
+#define S5M_RTC_TIME_EN_MASK	(1 << S5M_RTC_TIME_EN_SHIFT)
 
 /* RTC Hour register */
 #define HOUR_PM_SHIFT		6
-- 
1.7.9.5


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

* [PATCH v2 04/14] rtc: s5m: Remove undocumented time init on first boot
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (2 preceding siblings ...)
  2014-02-13  9:13 ` [PATCH v2 03/14] mfd/rtc: sec/s5m: Rename SEC* symbols to S5M Krzysztof Kozlowski
@ 2014-02-13  9:13 ` Krzysztof Kozlowski
  2014-02-13  9:13 ` [PATCH v2 05/14] mfd: sec: Use consistent S2MPS11 RTC alarm interrupt indexes Krzysztof Kozlowski
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:13 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Alessandro Zummo, rtc-linux

This patch removes the code for initializing time if this is first boot.

The code for detecting first boot uses undocumented field RTC_TCON in
RTC_UDR_CON register. According to S5M8767's datasheet this field is
reserved. On S2MPS14 it is not documented at all. On device first boot
the registers will be initialized with reset value (2000-01-01
00:00:00).

The code might work on S5M8763 but still this does not look like a task
for RTC driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux@googlegroups.com
---
 drivers/rtc/rtc-s5m.c |   30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index d26e2480f8b3..b1627e9ab8f0 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -502,16 +502,7 @@ static void s5m_rtc_enable_smpl(struct s5m_rtc_info *info, bool enable)
 static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
 {
 	u8 data[2];
-	unsigned int tp_read;
 	int ret;
-	struct rtc_time tm;
-
-	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &tp_read);
-	if (ret < 0) {
-		dev_err(info->dev, "%s: fail to read control reg(%d)\n",
-			__func__, ret);
-		return ret;
-	}
 
 	/* Set RTC control register : Binary mode, 24hour mode */
 	data[0] = (1 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
@@ -525,27 +516,6 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
 		return ret;
 	}
 
-	/* In first boot time, Set rtc time to 1/1/2012 00:00:00(SUN) */
-	if ((tp_read & RTC_TCON_MASK) == 0) {
-		dev_dbg(info->dev, "rtc init\n");
-		tm.tm_sec = 0;
-		tm.tm_min = 0;
-		tm.tm_hour = 0;
-		tm.tm_wday = 0;
-		tm.tm_mday = 1;
-		tm.tm_mon = 0;
-		tm.tm_year = 112;
-		tm.tm_yday = 0;
-		tm.tm_isdst = 0;
-		ret = s5m_rtc_set_time(info->dev, &tm);
-	}
-
-	ret = regmap_update_bits(info->regmap, S5M_RTC_UDR_CON,
-				 RTC_TCON_MASK, tp_read | RTC_TCON_MASK);
-	if (ret < 0)
-		dev_err(info->dev, "%s: fail to update TCON reg(%d)\n",
-			__func__, ret);
-
 	return ret;
 }
 
-- 
1.7.9.5


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

* [PATCH v2 05/14] mfd: sec: Use consistent S2MPS11 RTC alarm interrupt indexes
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (3 preceding siblings ...)
  2014-02-13  9:13 ` [PATCH v2 04/14] rtc: s5m: Remove undocumented time init on first boot Krzysztof Kozlowski
@ 2014-02-13  9:13 ` Krzysztof Kozlowski
  2014-02-13  9:13 ` [PATCH v2 06/14] regulator: s2mps11: Constify regulator_desc array Krzysztof Kozlowski
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:13 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski

The S2MPS11 RTC has two alarms: alarm0 and alarm1 (corresponding
interrupts are named similarly). Use consistent names for interrupts to
limit possible errors.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/sec-irq.c           |    8 ++++----
 include/linux/mfd/samsung/irq.h |    4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index 4de494f51d40..e403c293b437 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -59,13 +59,13 @@ static const struct regmap_irq s2mps11_irqs[] = {
 		.reg_offset = 1,
 		.mask = S2MPS11_IRQ_RTC60S_MASK,
 	},
-	[S2MPS11_IRQ_RTCA1] = {
+	[S2MPS11_IRQ_RTCA0] = {
 		.reg_offset = 1,
-		.mask = S2MPS11_IRQ_RTCA1_MASK,
+		.mask = S2MPS11_IRQ_RTCA0_MASK,
 	},
-	[S2MPS11_IRQ_RTCA2] = {
+	[S2MPS11_IRQ_RTCA1] = {
 		.reg_offset = 1,
-		.mask = S2MPS11_IRQ_RTCA2_MASK,
+		.mask = S2MPS11_IRQ_RTCA1_MASK,
 	},
 	[S2MPS11_IRQ_SMPL] = {
 		.reg_offset = 1,
diff --git a/include/linux/mfd/samsung/irq.h b/include/linux/mfd/samsung/irq.h
index d43b4f9e7fb2..abe1a6aae3b7 100644
--- a/include/linux/mfd/samsung/irq.h
+++ b/include/linux/mfd/samsung/irq.h
@@ -24,8 +24,8 @@ enum s2mps11_irq {
 	S2MPS11_IRQ_MRB,
 
 	S2MPS11_IRQ_RTC60S,
+	S2MPS11_IRQ_RTCA0,
 	S2MPS11_IRQ_RTCA1,
-	S2MPS11_IRQ_RTCA2,
 	S2MPS11_IRQ_SMPL,
 	S2MPS11_IRQ_RTC1S,
 	S2MPS11_IRQ_WTSR,
@@ -47,7 +47,7 @@ enum s2mps11_irq {
 
 #define S2MPS11_IRQ_RTC60S_MASK		(1 << 0)
 #define S2MPS11_IRQ_RTCA1_MASK		(1 << 1)
-#define S2MPS11_IRQ_RTCA2_MASK		(1 << 2)
+#define S2MPS11_IRQ_RTCA0_MASK		(1 << 2)
 #define S2MPS11_IRQ_SMPL_MASK		(1 << 3)
 #define S2MPS11_IRQ_RTC1S_MASK		(1 << 4)
 #define S2MPS11_IRQ_WTSR_MASK		(1 << 5)
-- 
1.7.9.5


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

* [PATCH v2 06/14] regulator: s2mps11: Constify regulator_desc array
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (4 preceding siblings ...)
  2014-02-13  9:13 ` [PATCH v2 05/14] mfd: sec: Use consistent S2MPS11 RTC alarm interrupt indexes Krzysztof Kozlowski
@ 2014-02-13  9:13 ` Krzysztof Kozlowski
  2014-02-13  9:14 ` [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst Krzysztof Kozlowski
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:13 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Mark Brown, Liam Girdwood

Constify the regulator_desc 'regulators' array.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
---
 drivers/regulator/s2mps11.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 89966213315c..d44bd5b3fe8e 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -345,7 +345,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-static struct regulator_desc regulators[] = {
+static const struct regulator_desc regulators[] = {
 	regulator_desc_ldo2(1),
 	regulator_desc_ldo1(2),
 	regulator_desc_ldo1(3),
-- 
1.7.9.5


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

* [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (5 preceding siblings ...)
  2014-02-13  9:13 ` [PATCH v2 06/14] regulator: s2mps11: Constify regulator_desc array Krzysztof Kozlowski
@ 2014-02-13  9:14 ` Krzysztof Kozlowski
  2014-02-13 12:21   ` Yadwinder Singh Brar
  2014-02-13 19:07   ` [PATCH v2 " Mark Brown
  2014-02-13  9:14 ` [PATCH v2 08/14] mfd: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (6 subsequent siblings)
  13 siblings, 2 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:14 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Chanwoo Choi, Mark Brown, Liam Girdwood,
	Yadwinder Singh Brar

Add __initconst to 'regulator_desc' array with supported regulators.
During probe choose how many and which regulators will be supported
according to device ID. Then copy the 'regulator_desc' array to
allocated memory so the regulator core can use it.

Additionally allocate array of of_regulator_match() dynamically (based
on number of regulators) instead of allocation on the stack.

This is needed for supporting different devices in s2mps11
driver and actually prepares the regulator driver for supporting the
S2MPS14 device.

Code for supporting the S2MPS14 device will add its own array of
'regulator_desc' (also marked as __initconst). This way memory footprint
of the driver will be reduced (approximately 'regulators_desc' array for
S2MPS11 occupies 5 kB on 32-bit ARM, for S2MPS14 will occupy 3 kB).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Yadwinder Singh Brar <yadi.brar01@gmail.com>
---
 drivers/regulator/s2mps11.c |   74 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index d44bd5b3fe8e..246b25d58c2b 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -25,10 +25,9 @@
 #include <linux/mfd/samsung/core.h>
 #include <linux/mfd/samsung/s2mps11.h>
 
-#define S2MPS11_REGULATOR_CNT ARRAY_SIZE(regulators)
-
 struct s2mps11_info {
-	struct regulator_dev *rdev[S2MPS11_REGULATOR_MAX];
+	struct regulator_dev **rdev;
+	unsigned int rdev_num;
 
 	int ramp_delay2;
 	int ramp_delay34;
@@ -345,7 +344,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-static const struct regulator_desc regulators[] = {
+static const struct regulator_desc s2mps11_regulators[] __initconst = {
 	regulator_desc_ldo2(1),
 	regulator_desc_ldo1(2),
 	regulator_desc_ldo1(3),
@@ -396,21 +395,62 @@ static const struct regulator_desc regulators[] = {
 	regulator_desc_buck10,
 };
 
+/*
+ * Allocates memory under 'regulators' pointer and copies there array
+ * of regulator_desc for given device.
+ *
+ * Returns number of regulators or negative ERRNO on error.
+ */
+static int __init
+s2mps11_pmic_init_regulators_desc(struct platform_device *pdev,
+		struct regulator_desc **regulators)
+{
+	const struct regulator_desc *regulators_init;
+	enum sec_device_type dev_type;
+	int rdev_num;
+
+	dev_type = platform_get_device_id(pdev)->driver_data;
+	switch (dev_type) {
+	case S2MPS11X:
+		rdev_num = ARRAY_SIZE(s2mps11_regulators);
+		regulators_init = s2mps11_regulators;
+		break;
+	default:
+		dev_err(&pdev->dev, "Invalid device type: %u\n", dev_type);
+		return -EINVAL;
+	};
+
+	*regulators = devm_kzalloc(&pdev->dev, sizeof(**regulators)*rdev_num,
+			GFP_KERNEL);
+	if (!*regulators)
+		return -ENOMEM;
+
+	memcpy(*regulators, regulators_init, sizeof(**regulators)*rdev_num);
+
+	return rdev_num;
+}
+
 static int s2mps11_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[S2MPS11_REGULATOR_MAX];
+	struct sec_platform_data *pdata = iodev->pdata;
+	struct of_regulator_match *rdata = NULL;
 	struct device_node *reg_np = NULL;
 	struct regulator_config config = { };
 	struct s2mps11_info *s2mps11;
 	int i, ret;
+	struct regulator_desc *regulators = NULL;
+	unsigned int rdev_num;
 
 	s2mps11 = devm_kzalloc(&pdev->dev, sizeof(struct s2mps11_info),
 				GFP_KERNEL);
 	if (!s2mps11)
 		return -ENOMEM;
 
+	rdev_num = s2mps11_pmic_init_regulators_desc(pdev, &regulators);
+	if (rdev_num < 0)
+		return rdev_num;
+
 	if (!iodev->dev->of_node) {
 		if (pdata) {
 			goto common_reg;
@@ -421,7 +461,16 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
 		}
 	}
 
-	for (i = 0; i < S2MPS11_REGULATOR_CNT; i++)
+	s2mps11->rdev = devm_kzalloc(&pdev->dev,
+			sizeof(*s2mps11->rdev)*rdev_num, GFP_KERNEL);
+	if (!s2mps11->rdev)
+		return -ENOMEM;
+
+	rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata)*rdev_num, GFP_KERNEL);
+	if (!rdata)
+		return -ENOMEM;
+
+	for (i = 0; i < rdev_num; i++)
 		rdata[i].name = regulators[i].name;
 
 	reg_np = of_find_node_by_name(iodev->dev->of_node, "regulators");
@@ -430,15 +479,16 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	of_regulator_match(&pdev->dev, reg_np, rdata, S2MPS11_REGULATOR_MAX);
+	of_regulator_match(&pdev->dev, reg_np, rdata, rdev_num);
 
 common_reg:
 	platform_set_drvdata(pdev, s2mps11);
+	s2mps11->rdev_num = rdev_num;
 
 	config.dev = &pdev->dev;
 	config.regmap = iodev->regmap_pmic;
 	config.driver_data = s2mps11;
-	for (i = 0; i < S2MPS11_REGULATOR_MAX; i++) {
+	for (i = 0; i < rdev_num; i++) {
 		if (!reg_np) {
 			config.init_data = pdata->regulators[i].initdata;
 			config.of_node = pdata->regulators[i].reg_node;
@@ -457,11 +507,15 @@ common_reg:
 		}
 	}
 
+	/* rdata was needed only for of_regulator_match() during probe */
+	if (rdata)
+		devm_kfree(&pdev->dev, rdata);
+
 	return 0;
 }
 
 static const struct platform_device_id s2mps11_pmic_id[] = {
-	{ "s2mps11-pmic", 0},
+	{ "s2mps11-pmic", S2MPS11X},
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, s2mps11_pmic_id);
-- 
1.7.9.5


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

* [PATCH v2 08/14] mfd: sec: Add support for S2MPS14
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (6 preceding siblings ...)
  2014-02-13  9:14 ` [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst Krzysztof Kozlowski
@ 2014-02-13  9:14 ` Krzysztof Kozlowski
  2014-02-13  9:14 ` [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators Krzysztof Kozlowski
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:14 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski

Add support for S2MPS14 PMIC device to the MFD sec-core driver.
The S2MPS14 is similar to S2MPS11 but it has fewer regulators, two
clocks instead of three and a little different registers layout.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/sec-core.c              |   48 +++++++++--
 drivers/mfd/sec-irq.c               |   89 +++++++++++++++++++-
 include/linux/mfd/samsung/core.h    |    1 +
 include/linux/mfd/samsung/irq.h     |   27 +++++++
 include/linux/mfd/samsung/rtc.h     |   56 +++++++++++--
 include/linux/mfd/samsung/s2mps14.h |  152 +++++++++++++++++++++++++++++++++++
 6 files changed, 361 insertions(+), 12 deletions(-)
 create mode 100644 include/linux/mfd/samsung/s2mps14.h

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 8504de82b7e0..6e5343255633 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -27,6 +27,7 @@
 #include <linux/mfd/samsung/irq.h>
 #include <linux/mfd/samsung/rtc.h>
 #include <linux/mfd/samsung/s2mps11.h>
+#include <linux/mfd/samsung/s2mps14.h>
 #include <linux/mfd/samsung/s5m8763.h>
 #include <linux/mfd/samsung/s5m8767.h>
 #include <linux/regmap.h>
@@ -69,6 +70,16 @@ static const struct mfd_cell s2mps11_devs[] = {
 	}
 };
 
+static const struct mfd_cell s2mps14_devs[] = {
+	{
+		.name = "s2mps14-pmic",
+	}, {
+		.name = "s2mps14-rtc",
+	}, {
+		.name = "s2mps14-clk",
+	}
+};
+
 #ifdef CONFIG_OF
 static struct of_device_id sec_dt_match[] = {
 	{	.compatible = "samsung,s5m8767-pmic",
@@ -77,6 +88,9 @@ static struct of_device_id sec_dt_match[] = {
 	{	.compatible = "samsung,s2mps11-pmic",
 		.data = (void *)S2MPS11X,
 	},
+	{	.compatible = "samsung,s2mps14-pmic",
+		.data = (void *)S2MPS14X,
+	},
 	{},
 };
 #endif
@@ -120,6 +134,15 @@ static const struct regmap_config s2mps11_regmap_config = {
 	.cache_type = REGCACHE_FLAT,
 };
 
+static const struct regmap_config s2mps14_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = S2MPS14_REG_LDODSCH3,
+	.volatile_reg = s2mps11_volatile,
+	.cache_type = REGCACHE_FLAT,
+};
+
 static const struct regmap_config s5m8763_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -138,13 +161,20 @@ static const struct regmap_config s5m8767_regmap_config = {
 	.cache_type = REGCACHE_FLAT,
 };
 
-static const struct regmap_config sec_rtc_regmap_config = {
+static const struct regmap_config s5m_rtc_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 
 	.max_register = S5M_RTC_REG_MAX,
 };
 
+static const struct regmap_config s2mps14_rtc_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = S2MPS_RTC_REG_MAX,
+};
+
 #ifdef CONFIG_OF
 /*
  * Only the common platform data elements for s5m8767 are parsed here from the
@@ -239,19 +269,23 @@ static int sec_pmic_probe(struct i2c_client *i2c,
 		 * However we must pass something to devm_regmap_init_i2c()
 		 * so use S5M-like regmap config even though it wouldn't work.
 		 */
-		regmap_rtc = &sec_rtc_regmap_config;
+		regmap_rtc = &s5m_rtc_regmap_config;
+		break;
+	case S2MPS14X:
+		regmap = &s2mps14_regmap_config;
+		regmap_rtc = &s2mps14_rtc_regmap_config;
 		break;
 	case S5M8763X:
 		regmap = &s5m8763_regmap_config;
-		regmap_rtc = &sec_rtc_regmap_config;
+		regmap_rtc = &s5m_rtc_regmap_config;
 		break;
 	case S5M8767X:
 		regmap = &s5m8767_regmap_config;
-		regmap_rtc = &sec_rtc_regmap_config;
+		regmap_rtc = &s5m_rtc_regmap_config;
 		break;
 	default:
 		regmap = &sec_regmap_config;
-		regmap_rtc = &sec_rtc_regmap_config;
+		regmap_rtc = &s5m_rtc_regmap_config;
 		break;
 	}
 
@@ -302,6 +336,10 @@ static int sec_pmic_probe(struct i2c_client *i2c,
 		ret = mfd_add_devices(sec_pmic->dev, -1, s2mps11_devs,
 				      ARRAY_SIZE(s2mps11_devs), NULL, 0, NULL);
 		break;
+	case S2MPS14X:
+		ret = mfd_add_devices(sec_pmic->dev, -1, s2mps14_devs,
+				      ARRAY_SIZE(s2mps14_devs), NULL, 0, NULL);
+		break;
 	default:
 		/* If this happens the probe function is problem */
 		BUG();
diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
index e403c293b437..64e7913aadc6 100644
--- a/drivers/mfd/sec-irq.c
+++ b/drivers/mfd/sec-irq.c
@@ -1,7 +1,7 @@
 /*
  * sec-irq.c
  *
- * Copyright (c) 2011 Samsung Electronics Co., Ltd
+ * Copyright (c) 2011-2014 Samsung Electronics Co., Ltd
  *              http://www.samsung.com
  *
  *  This program is free software; you can redistribute  it and/or modify it
@@ -19,6 +19,7 @@
 #include <linux/mfd/samsung/core.h>
 #include <linux/mfd/samsung/irq.h>
 #include <linux/mfd/samsung/s2mps11.h>
+#include <linux/mfd/samsung/s2mps14.h>
 #include <linux/mfd/samsung/s5m8763.h>
 #include <linux/mfd/samsung/s5m8767.h>
 
@@ -89,6 +90,76 @@ static const struct regmap_irq s2mps11_irqs[] = {
 	},
 };
 
+static const struct regmap_irq s2mps14_irqs[] = {
+	[S2MPS14_IRQ_PWRONF] = {
+		.reg_offset = 0,
+		.mask = S2MPS11_IRQ_PWRONF_MASK,
+	},
+	[S2MPS14_IRQ_PWRONR] = {
+		.reg_offset = 0,
+		.mask = S2MPS11_IRQ_PWRONR_MASK,
+	},
+	[S2MPS14_IRQ_JIGONBF] = {
+		.reg_offset = 0,
+		.mask = S2MPS11_IRQ_JIGONBF_MASK,
+	},
+	[S2MPS14_IRQ_JIGONBR] = {
+		.reg_offset = 0,
+		.mask = S2MPS11_IRQ_JIGONBR_MASK,
+	},
+	[S2MPS14_IRQ_ACOKBF] = {
+		.reg_offset = 0,
+		.mask = S2MPS11_IRQ_ACOKBF_MASK,
+	},
+	[S2MPS14_IRQ_ACOKBR] = {
+		.reg_offset = 0,
+		.mask = S2MPS11_IRQ_ACOKBR_MASK,
+	},
+	[S2MPS14_IRQ_PWRON1S] = {
+		.reg_offset = 0,
+		.mask = S2MPS11_IRQ_PWRON1S_MASK,
+	},
+	[S2MPS14_IRQ_MRB] = {
+		.reg_offset = 0,
+		.mask = S2MPS11_IRQ_MRB_MASK,
+	},
+	[S2MPS14_IRQ_RTC60S] = {
+		.reg_offset = 1,
+		.mask = S2MPS11_IRQ_RTC60S_MASK,
+	},
+	[S2MPS14_IRQ_RTCA1] = {
+		.reg_offset = 1,
+		.mask = S2MPS11_IRQ_RTCA1_MASK,
+	},
+	[S2MPS14_IRQ_RTCA0] = {
+		.reg_offset = 1,
+		.mask = S2MPS11_IRQ_RTCA0_MASK,
+	},
+	[S2MPS14_IRQ_SMPL] = {
+		.reg_offset = 1,
+		.mask = S2MPS11_IRQ_SMPL_MASK,
+	},
+	[S2MPS14_IRQ_RTC1S] = {
+		.reg_offset = 1,
+		.mask = S2MPS11_IRQ_RTC1S_MASK,
+	},
+	[S2MPS14_IRQ_WTSR] = {
+		.reg_offset = 1,
+		.mask = S2MPS11_IRQ_WTSR_MASK,
+	},
+	[S2MPS14_IRQ_INT120C] = {
+		.reg_offset = 2,
+		.mask = S2MPS11_IRQ_INT120C_MASK,
+	},
+	[S2MPS14_IRQ_INT140C] = {
+		.reg_offset = 2,
+		.mask = S2MPS11_IRQ_INT140C_MASK,
+	},
+	[S2MPS14_IRQ_TSD] = {
+		.reg_offset = 2,
+		.mask = S2MPS14_IRQ_TSD_MASK,
+	},
+};
 
 static const struct regmap_irq s5m8767_irqs[] = {
 	[S5M8767_IRQ_PWRR] = {
@@ -246,6 +317,16 @@ static const struct regmap_irq_chip s2mps11_irq_chip = {
 	.ack_base = S2MPS11_REG_INT1,
 };
 
+static const struct regmap_irq_chip s2mps14_irq_chip = {
+	.name = "s2mps14",
+	.irqs = s2mps14_irqs,
+	.num_irqs = ARRAY_SIZE(s2mps14_irqs),
+	.num_regs = 3,
+	.status_base = S2MPS14_REG_INT1,
+	.mask_base = S2MPS14_REG_INT1M,
+	.ack_base = S2MPS14_REG_INT1,
+};
+
 static const struct regmap_irq_chip s5m8767_irq_chip = {
 	.name = "s5m8767",
 	.irqs = s5m8767_irqs,
@@ -297,6 +378,12 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic)
 				  sec_pmic->irq_base, &s2mps11_irq_chip,
 				  &sec_pmic->irq_data);
 		break;
+	case S2MPS14X:
+		ret = regmap_add_irq_chip(sec_pmic->regmap_pmic, sec_pmic->irq,
+				  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+				  sec_pmic->irq_base, &s2mps14_irq_chip,
+				  &sec_pmic->irq_data);
+		break;
 	default:
 		dev_err(sec_pmic->dev, "Unknown device type %d\n",
 			sec_pmic->device_type);
diff --git a/include/linux/mfd/samsung/core.h b/include/linux/mfd/samsung/core.h
index 55510444b9fd..e517b12f290f 100644
--- a/include/linux/mfd/samsung/core.h
+++ b/include/linux/mfd/samsung/core.h
@@ -19,6 +19,7 @@ enum sec_device_type {
 	S5M8763X,
 	S5M8767X,
 	S2MPS11X,
+	S2MPS14X,
 };
 
 /**
diff --git a/include/linux/mfd/samsung/irq.h b/include/linux/mfd/samsung/irq.h
index abe1a6aae3b7..0065f6f1daf4 100644
--- a/include/linux/mfd/samsung/irq.h
+++ b/include/linux/mfd/samsung/irq.h
@@ -55,6 +55,33 @@ enum s2mps11_irq {
 #define S2MPS11_IRQ_INT120C_MASK	(1 << 0)
 #define S2MPS11_IRQ_INT140C_MASK	(1 << 1)
 
+enum s2mps14_irq {
+	S2MPS14_IRQ_PWRONF,
+	S2MPS14_IRQ_PWRONR,
+	S2MPS14_IRQ_JIGONBF,
+	S2MPS14_IRQ_JIGONBR,
+	S2MPS14_IRQ_ACOKBF,
+	S2MPS14_IRQ_ACOKBR,
+	S2MPS14_IRQ_PWRON1S,
+	S2MPS14_IRQ_MRB,
+
+	S2MPS14_IRQ_RTC60S,
+	S2MPS14_IRQ_RTCA1,
+	S2MPS14_IRQ_RTCA0,
+	S2MPS14_IRQ_SMPL,
+	S2MPS14_IRQ_RTC1S,
+	S2MPS14_IRQ_WTSR,
+
+	S2MPS14_IRQ_INT120C,
+	S2MPS14_IRQ_INT140C,
+	S2MPS14_IRQ_TSD,
+
+	S2MPS14_IRQ_NR,
+};
+
+/* Masks for interrupts are the same as in s2mps11 */
+#define S2MPS14_IRQ_TSD_MASK		(1 << 2)
+
 enum s5m8767_irq {
 	S5M8767_IRQ_PWRR,
 	S5M8767_IRQ_PWRF,
diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h
index bdf0573891d0..fa0fc32c763a 100644
--- a/include/linux/mfd/samsung/rtc.h
+++ b/include/linux/mfd/samsung/rtc.h
@@ -1,12 +1,17 @@
-/*  rtc.h
+/* rtc.h
  *
- * Copyright (c) 2011 Samsung Electronics Co., Ltd
+ * Copyright (c) 2011-2014 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.
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
  *
  */
 
@@ -47,15 +52,50 @@ enum s5m_rtc_reg {
 	S5M_RTC_REG_MAX,
 };
 
+enum s2mps_rtc_reg {
+	S2MPS_RTC_CTRL,
+	S2MPS_WTSR_SMPL_CNTL,
+	S2MPS_RTC_UDR_CON,
+	S2MPS_RSVD,
+	S2MPS_RTC_SEC,
+	S2MPS_RTC_MIN,
+	S2MPS_RTC_HOUR,
+	S2MPS_RTC_WEEKDAY,
+	S2MPS_RTC_DATE,
+	S2MPS_RTC_MONTH,
+	S2MPS_RTC_YEAR,
+	S2MPS_ALARM0_SEC,
+	S2MPS_ALARM0_MIN,
+	S2MPS_ALARM0_HOUR,
+	S2MPS_ALARM0_WEEKDAY,
+	S2MPS_ALARM0_DATE,
+	S2MPS_ALARM0_MONTH,
+	S2MPS_ALARM0_YEAR,
+	S2MPS_ALARM1_SEC,
+	S2MPS_ALARM1_MIN,
+	S2MPS_ALARM1_HOUR,
+	S2MPS_ALARM1_WEEKDAY,
+	S2MPS_ALARM1_DATE,
+	S2MPS_ALARM1_MONTH,
+	S2MPS_ALARM1_YEAR,
+	S2MPS_OFFSRC,
+
+	S2MPS_RTC_REG_MAX,
+};
+
 #define RTC_I2C_ADDR		(0x0C >> 1)
 
 #define HOUR_12			(1 << 7)
 #define HOUR_AMPM		(1 << 6)
 #define HOUR_PM			(1 << 5)
+
 #define S5M_ALARM0_STATUS	(1 << 1)
 #define S5M_ALARM1_STATUS	(1 << 2)
 #define S5M_UPDATE_AD		(1 << 0)
 
+#define S2MPS_ALARM0_STATUS	(1 << 2)
+#define S2MPS_ALARM1_STATUS	(1 << 1)
+
 /* RTC Control Register */
 #define BCD_EN_SHIFT		0
 #define BCD_EN_MASK		(1 << BCD_EN_SHIFT)
@@ -64,6 +104,10 @@ enum s5m_rtc_reg {
 /* RTC Update Register1 */
 #define S5M_RTC_UDR_SHIFT	0
 #define S5M_RTC_UDR_MASK	(1 << S5M_RTC_UDR_SHIFT)
+#define S2MPS_RTC_WUDR_SHIFT	4
+#define S2MPS_RTC_WUDR_MASK	(1 << S2MPS_RTC_WUDR_SHIFT)
+#define S2MPS_RTC_RUDR_SHIFT	0
+#define S2MPS_RTC_RUDR_MASK	(1 << S2MPS_RTC_RUDR_SHIFT)
 #define RTC_TCON_SHIFT		1
 #define RTC_TCON_MASK		(1 << RTC_TCON_SHIFT)
 #define S5M_RTC_TIME_EN_SHIFT	3
diff --git a/include/linux/mfd/samsung/s2mps14.h b/include/linux/mfd/samsung/s2mps14.h
new file mode 100644
index 000000000000..ec1e0857ddde
--- /dev/null
+++ b/include/linux/mfd/samsung/s2mps14.h
@@ -0,0 +1,152 @@
+/*
+ * s2mps14.h
+ *
+ * Copyright (c) 2014 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __LINUX_MFD_S2MPS14_H
+#define __LINUX_MFD_S2MPS14_H
+
+/* S2MPS14 registers */
+enum s2mps14_reg {
+	S2MPS14_REG_ID,
+	S2MPS14_REG_INT1,
+	S2MPS14_REG_INT2,
+	S2MPS14_REG_INT3,
+	S2MPS14_REG_INT1M,
+	S2MPS14_REG_INT2M,
+	S2MPS14_REG_INT3M,
+	S2MPS14_REG_ST1,
+	S2MPS14_REG_ST2,
+	S2MPS14_REG_PWRONSRC,
+	S2MPS14_REG_OFFSRC,
+	S2MPS14_REG_BU_CHG,
+	S2MPS14_REG_RTCCTRL,
+	S2MPS14_REG_CTRL1,
+	S2MPS14_REG_CTRL2,
+	S2MPS14_REG_RSVD1,
+	S2MPS14_REG_RSVD2,
+	S2MPS14_REG_RSVD3,
+	S2MPS14_REG_RSVD4,
+	S2MPS14_REG_RSVD5,
+	S2MPS14_REG_RSVD6,
+	S2MPS14_REG_CTRL3,
+	S2MPS14_REG_RSVD7,
+	S2MPS14_REG_RSVD8,
+	S2MPS14_REG_WRSTBI,
+	S2MPS14_REG_B1CTRL1,
+	S2MPS14_REG_B1CTRL2,
+	S2MPS14_REG_B2CTRL1,
+	S2MPS14_REG_B2CTRL2,
+	S2MPS14_REG_B3CTRL1,
+	S2MPS14_REG_B3CTRL2,
+	S2MPS14_REG_B4CTRL1,
+	S2MPS14_REG_B4CTRL2,
+	S2MPS14_REG_B5CTRL1,
+	S2MPS14_REG_B5CTRL2,
+	S2MPS14_REG_L1CTRL,
+	S2MPS14_REG_L2CTRL,
+	S2MPS14_REG_L3CTRL,
+	S2MPS14_REG_L4CTRL,
+	S2MPS14_REG_L5CTRL,
+	S2MPS14_REG_L6CTRL,
+	S2MPS14_REG_L7CTRL,
+	S2MPS14_REG_L8CTRL,
+	S2MPS14_REG_L9CTRL,
+	S2MPS14_REG_L10CTRL,
+	S2MPS14_REG_L11CTRL,
+	S2MPS14_REG_L12CTRL,
+	S2MPS14_REG_L13CTRL,
+	S2MPS14_REG_L14CTRL,
+	S2MPS14_REG_L15CTRL,
+	S2MPS14_REG_L16CTRL,
+	S2MPS14_REG_L17CTRL,
+	S2MPS14_REG_L18CTRL,
+	S2MPS14_REG_L19CTRL,
+	S2MPS14_REG_L20CTRL,
+	S2MPS14_REG_L21CTRL,
+	S2MPS14_REG_L22CTRL,
+	S2MPS14_REG_L23CTRL,
+	S2MPS14_REG_L24CTRL,
+	S2MPS14_REG_L25CTRL,
+	S2MPS14_REG_LDODSCH1,
+	S2MPS14_REG_LDODSCH2,
+	S2MPS14_REG_LDODSCH3,
+};
+
+/* S2MPS14 regulator ids */
+enum s2mps14_regulators {
+	S2MPS14_LDO1,
+	S2MPS14_LDO2,
+	S2MPS14_LDO3,
+	S2MPS14_LDO4,
+	S2MPS14_LDO5,
+	S2MPS14_LDO6,
+	S2MPS14_LDO7,
+	S2MPS14_LDO8,
+	S2MPS14_LDO9,
+	S2MPS14_LDO10,
+	S2MPS14_LDO11,
+	S2MPS14_LDO12,
+	S2MPS14_LDO13,
+	S2MPS14_LDO14,
+	S2MPS14_LDO15,
+	S2MPS14_LDO16,
+	S2MPS14_LDO17,
+	S2MPS14_LDO18,
+	S2MPS14_LDO19,
+	S2MPS14_LDO20,
+	S2MPS14_LDO21,
+	S2MPS14_LDO22,
+	S2MPS14_LDO23,
+	S2MPS14_LDO24,
+	S2MPS14_LDO25,
+	S2MPS14_BUCK1,
+	S2MPS14_BUCK2,
+	S2MPS14_BUCK3,
+	S2MPS14_BUCK4,
+	S2MPS14_BUCK5,
+
+	S2MPS14_REGULATOR_MAX,
+};
+
+/* Regulator constraints for BUCKx */
+#define S2MPS14_BUCK1235_MIN_600MV	600000
+#define S2MPS14_BUCK4_MIN_1400MV	1400000
+#define S2MPS14_BUCK1235_STEP_6_25MV	6250
+#define S2MPS14_BUCK4_STEP_12_5MV	12500
+#define S2MPS14_BUCK1235_START_SEL	0x20
+#define S2MPS14_BUCK4_START_SEL		0x40
+/*
+ * Default ramp delay in uv/us. Datasheet says that ramp delay can be
+ * controlled however it does not specify which register is used for that.
+ * Let's assume that default value will be set.
+ */
+#define S2MPS14_BUCK_RAMP_DELAY		12500
+
+/* Regulator constraints for different types of LDOx */
+#define S2MPS14_LDO_MIN_800MV		800000
+#define S2MPS14_LDO_MIN_1800MV		1800000
+#define S2MPS14_LDO_STEP_12_5MV		12500
+#define S2MPS14_LDO_STEP_25MV		25000
+
+#define S2MPS14_LDO_VSEL_MASK		0x3F
+#define S2MPS14_BUCK_VSEL_MASK		0xFF
+#define S2MPS14_ENABLE_MASK		(0x03 << S2MPS14_ENABLE_SHIFT)
+#define S2MPS14_ENABLE_SHIFT		6
+#define S2MPS14_LDO_N_VOLTAGES		(S2MPS14_LDO_VSEL_MASK + 1)
+#define S2MPS14_BUCK_N_VOLTAGES		(S2MPS14_BUCK_VSEL_MASK + 1)
+
+#endif /*  __LINUX_MFD_S2MPS14_H */
-- 
1.7.9.5


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

* [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (7 preceding siblings ...)
  2014-02-13  9:14 ` [PATCH v2 08/14] mfd: sec: Add support for S2MPS14 Krzysztof Kozlowski
@ 2014-02-13  9:14 ` Krzysztof Kozlowski
  2014-02-13 12:24   ` Yadwinder Singh Brar
  2014-02-13 19:10   ` Mark Brown
  2014-02-13  9:14 ` [PATCH v2 10/14] Documentation: mfd: s2mps11: Document support for S2MPS14 Krzysztof Kozlowski
                   ` (4 subsequent siblings)
  13 siblings, 2 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:14 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Mark Brown, Liam Girdwood

Add support for S2MPS14 PMIC regulators to s2mps11 driver. The S2MPS14
has fewer BUCK-s and LDO-s than S2MPS11. It also does not support
controlling the BUCK ramp delay.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
---
 drivers/regulator/s2mps11.c |  252 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 191 insertions(+), 61 deletions(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 246b25d58c2b..f56ac6f776ae 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -1,13 +1,18 @@
 /*
  * s2mps11.c
  *
- * Copyright (c) 2012 Samsung Electronics Co., Ltd
+ * Copyright (c) 2012-2014 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.
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
  *
  */
 
@@ -24,6 +29,7 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/mfd/samsung/core.h>
 #include <linux/mfd/samsung/s2mps11.h>
+#include <linux/mfd/samsung/s2mps14.h>
 
 struct s2mps11_info {
 	struct regulator_dev **rdev;
@@ -235,7 +241,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.set_ramp_delay		= s2mps11_set_ramp_delay,
 };
 
-#define regulator_desc_ldo1(num)	{		\
+#define regulator_desc_s2mps11_ldo1(num)	{		\
 	.name		= "LDO"#num,			\
 	.id		= S2MPS11_LDO##num,		\
 	.ops		= &s2mps11_ldo_ops,		\
@@ -249,7 +255,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_reg	= S2MPS11_REG_L1CTRL + num - 1,	\
 	.enable_mask	= S2MPS11_ENABLE_MASK		\
 }
-#define regulator_desc_ldo2(num)	{		\
+#define regulator_desc_s2mps11_ldo2(num) {		\
 	.name		= "LDO"#num,			\
 	.id		= S2MPS11_LDO##num,		\
 	.ops		= &s2mps11_ldo_ops,		\
@@ -264,7 +270,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK		\
 }
 
-#define regulator_desc_buck1_4(num)	{			\
+#define regulator_desc_s2mps11_buck1_4(num) {			\
 	.name		= "BUCK"#num,				\
 	.id		= S2MPS11_BUCK##num,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -280,7 +286,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck5	{				\
+#define regulator_desc_s2mps11_buck5 {				\
 	.name		= "BUCK5",				\
 	.id		= S2MPS11_BUCK5,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -296,7 +302,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck6_8(num)	{			\
+#define regulator_desc_s2mps11_buck6_8(num) {			\
 	.name		= "BUCK"#num,				\
 	.id		= S2MPS11_BUCK##num,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -312,7 +318,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck9	{				\
+#define regulator_desc_s2mps11_buck9 {				\
 	.name		= "BUCK9",				\
 	.id		= S2MPS11_BUCK9,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -328,7 +334,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-#define regulator_desc_buck10	{				\
+#define regulator_desc_s2mps11_buck10 {				\
 	.name		= "BUCK10",				\
 	.id		= S2MPS11_BUCK10,			\
 	.ops		= &s2mps11_buck_ops,			\
@@ -345,54 +351,173 @@ static struct regulator_ops s2mps11_buck_ops = {
 }
 
 static const struct regulator_desc s2mps11_regulators[] __initconst = {
-	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_ldo2(27),
-	regulator_desc_ldo1(28),
-	regulator_desc_ldo1(29),
-	regulator_desc_ldo1(30),
-	regulator_desc_ldo1(31),
-	regulator_desc_ldo1(32),
-	regulator_desc_ldo1(33),
-	regulator_desc_ldo1(34),
-	regulator_desc_ldo1(35),
-	regulator_desc_ldo1(36),
-	regulator_desc_ldo1(37),
-	regulator_desc_ldo1(38),
-	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_8(6),
-	regulator_desc_buck6_8(7),
-	regulator_desc_buck6_8(8),
-	regulator_desc_buck9,
-	regulator_desc_buck10,
+	regulator_desc_s2mps11_ldo2(1),
+	regulator_desc_s2mps11_ldo1(2),
+	regulator_desc_s2mps11_ldo1(3),
+	regulator_desc_s2mps11_ldo1(4),
+	regulator_desc_s2mps11_ldo1(5),
+	regulator_desc_s2mps11_ldo2(6),
+	regulator_desc_s2mps11_ldo1(7),
+	regulator_desc_s2mps11_ldo1(8),
+	regulator_desc_s2mps11_ldo1(9),
+	regulator_desc_s2mps11_ldo1(10),
+	regulator_desc_s2mps11_ldo2(11),
+	regulator_desc_s2mps11_ldo1(12),
+	regulator_desc_s2mps11_ldo1(13),
+	regulator_desc_s2mps11_ldo1(14),
+	regulator_desc_s2mps11_ldo1(15),
+	regulator_desc_s2mps11_ldo1(16),
+	regulator_desc_s2mps11_ldo1(17),
+	regulator_desc_s2mps11_ldo1(18),
+	regulator_desc_s2mps11_ldo1(19),
+	regulator_desc_s2mps11_ldo1(20),
+	regulator_desc_s2mps11_ldo1(21),
+	regulator_desc_s2mps11_ldo2(22),
+	regulator_desc_s2mps11_ldo2(23),
+	regulator_desc_s2mps11_ldo1(24),
+	regulator_desc_s2mps11_ldo1(25),
+	regulator_desc_s2mps11_ldo1(26),
+	regulator_desc_s2mps11_ldo2(27),
+	regulator_desc_s2mps11_ldo1(28),
+	regulator_desc_s2mps11_ldo1(29),
+	regulator_desc_s2mps11_ldo1(30),
+	regulator_desc_s2mps11_ldo1(31),
+	regulator_desc_s2mps11_ldo1(32),
+	regulator_desc_s2mps11_ldo1(33),
+	regulator_desc_s2mps11_ldo1(34),
+	regulator_desc_s2mps11_ldo1(35),
+	regulator_desc_s2mps11_ldo1(36),
+	regulator_desc_s2mps11_ldo1(37),
+	regulator_desc_s2mps11_ldo1(38),
+	regulator_desc_s2mps11_buck1_4(1),
+	regulator_desc_s2mps11_buck1_4(2),
+	regulator_desc_s2mps11_buck1_4(3),
+	regulator_desc_s2mps11_buck1_4(4),
+	regulator_desc_s2mps11_buck5,
+	regulator_desc_s2mps11_buck6_8(6),
+	regulator_desc_s2mps11_buck6_8(7),
+	regulator_desc_s2mps11_buck6_8(8),
+	regulator_desc_s2mps11_buck9,
+	regulator_desc_s2mps11_buck10,
+};
+
+static struct regulator_ops s2mps14_reg_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,
+};
+
+#define regulator_desc_s2mps14_ldo1(num) {		\
+	.name		= "LDO"#num,			\
+	.id		= S2MPS14_LDO##num,		\
+	.ops		= &s2mps14_reg_ops,		\
+	.type		= REGULATOR_VOLTAGE,		\
+	.owner		= THIS_MODULE,			\
+	.min_uV		= S2MPS14_LDO_MIN_800MV,	\
+	.uV_step	= S2MPS14_LDO_STEP_25MV,	\
+	.n_voltages	= S2MPS14_LDO_N_VOLTAGES,	\
+	.vsel_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.vsel_mask	= S2MPS14_LDO_VSEL_MASK,	\
+	.enable_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK		\
+}
+#define regulator_desc_s2mps14_ldo2(num) {		\
+	.name		= "LDO"#num,			\
+	.id		= S2MPS14_LDO##num,		\
+	.ops		= &s2mps14_reg_ops,		\
+	.type		= REGULATOR_VOLTAGE,		\
+	.owner		= THIS_MODULE,			\
+	.min_uV		= S2MPS14_LDO_MIN_1800MV,	\
+	.uV_step	= S2MPS14_LDO_STEP_25MV,	\
+	.n_voltages	= S2MPS14_LDO_N_VOLTAGES,	\
+	.vsel_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.vsel_mask	= S2MPS14_LDO_VSEL_MASK,	\
+	.enable_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK		\
+}
+#define regulator_desc_s2mps14_ldo3(num) {		\
+	.name		= "LDO"#num,			\
+	.id		= S2MPS14_LDO##num,		\
+	.ops		= &s2mps14_reg_ops,		\
+	.type		= REGULATOR_VOLTAGE,		\
+	.owner		= THIS_MODULE,			\
+	.min_uV		= S2MPS14_LDO_MIN_800MV,	\
+	.uV_step	= S2MPS14_LDO_STEP_12_5MV,	\
+	.n_voltages	= S2MPS14_LDO_N_VOLTAGES,	\
+	.vsel_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.vsel_mask	= S2MPS14_LDO_VSEL_MASK,	\
+	.enable_reg	= S2MPS14_REG_L1CTRL + num - 1,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK		\
+}
+#define regulator_desc_s2mps14_buck1235(num) {			\
+	.name		= "BUCK"#num,				\
+	.id		= S2MPS14_BUCK##num,			\
+	.ops		= &s2mps14_reg_ops,			\
+	.type		= REGULATOR_VOLTAGE,			\
+	.owner		= THIS_MODULE,				\
+	.min_uV		= S2MPS14_BUCK1235_MIN_600MV,		\
+	.uV_step	= S2MPS14_BUCK1235_STEP_6_25MV,		\
+	.n_voltages	= S2MPS14_BUCK_N_VOLTAGES,		\
+	.linear_min_sel = S2MPS14_BUCK1235_START_SEL,		\
+	.ramp_delay	= S2MPS14_BUCK_RAMP_DELAY,		\
+	.vsel_reg	= S2MPS14_REG_B1CTRL2 + (num - 1) * 2,	\
+	.vsel_mask	= S2MPS14_BUCK_VSEL_MASK,		\
+	.enable_reg	= S2MPS14_REG_B1CTRL1 + (num - 1) * 2,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK			\
+}
+#define regulator_desc_s2mps14_buck4(num) {			\
+	.name		= "BUCK"#num,				\
+	.id		= S2MPS14_BUCK##num,			\
+	.ops		= &s2mps14_reg_ops,			\
+	.type		= REGULATOR_VOLTAGE,			\
+	.owner		= THIS_MODULE,				\
+	.min_uV		= S2MPS14_BUCK4_MIN_1400MV,		\
+	.uV_step	= S2MPS14_BUCK4_STEP_12_5MV,		\
+	.n_voltages	= S2MPS14_BUCK_N_VOLTAGES,		\
+	.linear_min_sel = S2MPS14_BUCK4_START_SEL,		\
+	.ramp_delay	= S2MPS14_BUCK_RAMP_DELAY,		\
+	.vsel_reg	= S2MPS14_REG_B1CTRL2 + (num - 1) * 2,	\
+	.vsel_mask	= S2MPS14_BUCK_VSEL_MASK,		\
+	.enable_reg	= S2MPS14_REG_B1CTRL1 + (num - 1) * 2,	\
+	.enable_mask	= S2MPS14_ENABLE_MASK			\
+}
+
+static const struct regulator_desc s2mps14_regulators[] __initconst = {
+	regulator_desc_s2mps14_ldo3(1),
+	regulator_desc_s2mps14_ldo3(2),
+	regulator_desc_s2mps14_ldo1(3),
+	regulator_desc_s2mps14_ldo1(4),
+	regulator_desc_s2mps14_ldo3(5),
+	regulator_desc_s2mps14_ldo3(6),
+	regulator_desc_s2mps14_ldo1(7),
+	regulator_desc_s2mps14_ldo2(8),
+	regulator_desc_s2mps14_ldo3(9),
+	regulator_desc_s2mps14_ldo3(10),
+	regulator_desc_s2mps14_ldo1(11),
+	regulator_desc_s2mps14_ldo2(12),
+	regulator_desc_s2mps14_ldo2(13),
+	regulator_desc_s2mps14_ldo2(14),
+	regulator_desc_s2mps14_ldo2(15),
+	regulator_desc_s2mps14_ldo2(16),
+	regulator_desc_s2mps14_ldo2(17),
+	regulator_desc_s2mps14_ldo2(18),
+	regulator_desc_s2mps14_ldo1(19),
+	regulator_desc_s2mps14_ldo1(20),
+	regulator_desc_s2mps14_ldo1(21),
+	regulator_desc_s2mps14_ldo3(22),
+	regulator_desc_s2mps14_ldo1(23),
+	regulator_desc_s2mps14_ldo2(24),
+	regulator_desc_s2mps14_ldo2(25),
+	regulator_desc_s2mps14_buck1235(1),
+	regulator_desc_s2mps14_buck1235(2),
+	regulator_desc_s2mps14_buck1235(3),
+	regulator_desc_s2mps14_buck4(4),
+	regulator_desc_s2mps14_buck1235(5),
 };
 
 /*
@@ -415,6 +540,10 @@ s2mps11_pmic_init_regulators_desc(struct platform_device *pdev,
 		rdev_num = ARRAY_SIZE(s2mps11_regulators);
 		regulators_init = s2mps11_regulators;
 		break;
+	case S2MPS14X:
+		rdev_num = ARRAY_SIZE(s2mps14_regulators);
+		regulators_init = s2mps14_regulators;
+		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device type: %u\n", dev_type);
 		return -EINVAL;
@@ -516,6 +645,7 @@ common_reg:
 
 static const struct platform_device_id s2mps11_pmic_id[] = {
 	{ "s2mps11-pmic", S2MPS11X},
+	{ "s2mps14-pmic", S2MPS14X},
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, s2mps11_pmic_id);
@@ -543,5 +673,5 @@ module_exit(s2mps11_pmic_exit);
 
 /* Module information */
 MODULE_AUTHOR("Sangbeom Kim <sbkim73@samsung.com>");
-MODULE_DESCRIPTION("SAMSUNG S2MPS11 Regulator Driver");
+MODULE_DESCRIPTION("SAMSUNG S2MPS11/S2MPS14 Regulator Driver");
 MODULE_LICENSE("GPL");
-- 
1.7.9.5


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

* [PATCH v2 10/14] Documentation: mfd: s2mps11: Document support for S2MPS14
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (8 preceding siblings ...)
  2014-02-13  9:14 ` [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators Krzysztof Kozlowski
@ 2014-02-13  9:14 ` Krzysztof Kozlowski
  2014-02-13 14:55   ` Tomasz Figa
  2014-02-13  9:14 ` [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators Krzysztof Kozlowski
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:14 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Mark Brown, Liam Girdwood, Tomasz Figa,
	devicetree, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala

Add bindings documentation for S2MPS14 device to the s2mps11 driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
---
 Documentation/devicetree/bindings/mfd/s2mps11.txt |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/s2mps11.txt b/Documentation/devicetree/bindings/mfd/s2mps11.txt
index 15ee89c3cc7b..f69bec294f02 100644
--- a/Documentation/devicetree/bindings/mfd/s2mps11.txt
+++ b/Documentation/devicetree/bindings/mfd/s2mps11.txt
@@ -1,5 +1,5 @@
 
-* Samsung S2MPS11 Voltage and Current Regulator
+* Samsung S2MPS11 and S2MPS14 Voltage and Current Regulator
 
 The Samsung S2MPS11 is a multi-function device which includes voltage and
 current regulators, RTC, charger controller and other sub-blocks. It is
@@ -7,7 +7,7 @@ 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,s2mps11-pmic".
+- compatible: Should be "samsung,s2mps11-pmic" or "samsung,s2mps14-pmic".
 - reg: Specifies the I2C slave address of the pmic block. It should be 0x66.
 
 Optional properties:
@@ -59,10 +59,14 @@ supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
 as per the datasheet of s2mps11.
 
 	- LDOn
-		  - valid values for n are 1 to 38
+		  - valid values for n are:
+			- S2MPS11: 1 to 38
+			- S2MPS14: 1 to 25
 		  - Example: LDO1, LD02, LDO28
 	- BUCKn
-		  - valid values for n are 1 to 10.
+		  - valid values for n are:
+			- S2MPS11: 1 to 10
+			- S2MPS14: 1 to 5
 		  - Example: BUCK1, BUCK2, BUCK9
 
 Example:
-- 
1.7.9.5


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

* [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (9 preceding siblings ...)
  2014-02-13  9:14 ` [PATCH v2 10/14] Documentation: mfd: s2mps11: Document support for S2MPS14 Krzysztof Kozlowski
@ 2014-02-13  9:14 ` Krzysztof Kozlowski
  2014-02-13 12:16   ` Yadwinder Singh Brar
                     ` (2 more replies)
  2014-02-13  9:14 ` [PATCH v2 12/14] Documentation: mfd/regulator: s2mps11: Document the "op_mode" bindings Krzysztof Kozlowski
                   ` (2 subsequent siblings)
  13 siblings, 3 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:14 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Chanwoo Choi, Mark Brown, Liam Girdwood

S2MPS11/S2MPS14 regulators support different modes of operation:
 - Always off;
 - On/Off controlled by pin/GPIO (PWREN/LDOEN/EMMCEN);
 - Always on;
This is very similar to S5M8767 regulator driver which also supports
opmodes (although S5M8767 have also low-power mode).

This patch adds parsing the operation mode from DTS by reading a
"op_mode" property from regulator child node.

The op_mode is then used for enabling the S2MPS14 regulators.
On S2MPS11 the DTS "op_mode" property is parsed but not used for
enabling, as this was not tested.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
---
 drivers/regulator/s2mps11.c         |   97 ++++++++++++++++++++++++++++++++++-
 include/linux/mfd/samsung/s2mps14.h |   19 +++++++
 2 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index f56ac6f776ae..4a203ef9a605 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -34,6 +34,7 @@
 struct s2mps11_info {
 	struct regulator_dev **rdev;
 	unsigned int rdev_num;
+	struct sec_opmode_data *opmode;
 
 	int ramp_delay2;
 	int ramp_delay34;
@@ -43,6 +44,48 @@ struct s2mps11_info {
 	int ramp_delay9;
 };
 
+/* LDO_EN/BUCK_EN register values for enabling/disabling regulator */
+static unsigned int s2mps14_opmode_reg[4] = {
+	[S2MPS14_REGULATOR_OPMODE_OFF]		= 0x0,
+	[S2MPS14_REGULATOR_OPMODE_ON]		= 0x3,
+	[S2MPS14_REGULATOR_OPMODE_RESERVED]	= 0x2,
+	[S2MPS14_REGULATOR_OPMODE_SUSPEND]	= 0x1,
+};
+
+static int s2mps14_get_opmode(struct regulator_dev *rdev)
+{
+	int i, reg_id = rdev_get_id(rdev);
+	int mode = -EINVAL;
+	struct s2mps11_info *s2mps11 = rdev_get_drvdata(rdev);
+
+	for (i = 0; i < s2mps11->rdev_num; i++) {
+		if (s2mps11->opmode[i].id == reg_id) {
+			mode = s2mps11->opmode[i].mode;
+			break;
+		}
+	}
+
+	if (mode == -EINVAL) {
+		dev_warn(rdev_get_dev(rdev),
+				"No op_mode in the driver for regulator %s\n",
+				rdev->desc->name);
+		return mode;
+	}
+
+	return s2mps14_opmode_reg[mode] << S2MPS14_ENCTRL_SHIFT;
+}
+
+static int s2mps14_reg_enable(struct regulator_dev *rdev)
+{
+	int enable_ctrl = s2mps14_get_opmode(rdev);
+
+	if (enable_ctrl < 0)
+		return enable_ctrl;
+
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+			S2MPS14_ENCTRL_MASK, enable_ctrl);
+}
+
 static int get_ramp_delay(int ramp_delay)
 {
 	unsigned char cnt = 0;
@@ -405,7 +448,7 @@ static struct regulator_ops s2mps14_reg_ops = {
 	.list_voltage		= regulator_list_voltage_linear,
 	.map_voltage		= regulator_map_voltage_linear,
 	.is_enabled		= regulator_is_enabled_regmap,
-	.enable			= regulator_enable_regmap,
+	.enable			= s2mps14_reg_enable,
 	.disable		= regulator_disable_regmap,
 	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
 	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
@@ -520,6 +563,53 @@ static const struct regulator_desc s2mps14_regulators[] __initconst = {
 	regulator_desc_s2mps14_buck1235(5),
 };
 
+static inline void s2mps11_dt_read_opmode(struct platform_device *pdev,
+		struct device_node *np, unsigned int *mode)
+{
+	if (of_property_read_u32(np, "op_mode",	mode)) {
+		dev_warn(&pdev->dev, "no op_mode property property at %s\n",
+				np->full_name);
+		*mode = S2MPS14_REGULATOR_OPMODE_ON;
+	} else if (*mode >= S2MPS14_REGULATOR_OPMODE_MAX ||
+			*mode == S2MPS14_REGULATOR_OPMODE_RESERVED) {
+		dev_warn(&pdev->dev, "wrong op_mode value at %s\n",
+				np->full_name);
+		*mode = S2MPS14_REGULATOR_OPMODE_ON;
+	}
+	/* else: 'mode' was read from DTS and it is valid */
+}
+
+/*
+ * Returns allocated array with opmodes for regulators. The opmodes are read
+ * from DTS.
+ */
+static struct sec_opmode_data *
+s2mps11_pmic_dt_parse_opmode(struct platform_device *pdev,
+		unsigned int rdev_num, struct of_regulator_match *rdata,
+		const struct regulator_desc *regulators)
+{
+	struct sec_opmode_data *rmode;
+	int i;
+
+	rmode = devm_kzalloc(&pdev->dev, sizeof(*rmode)*rdev_num, GFP_KERNEL);
+	if (!rmode) {
+		dev_err(&pdev->dev,
+			"could not allocate memory for regulator mode\n");
+		return NULL;
+	}
+
+	for (i = 0; i < rdev_num; i++) {
+		/*
+		 * The index of rdata and regulators is the same, but this
+		 * may not be equal to ID of regulator.
+		 */
+		rmode[i].id = regulators[i].id;
+		s2mps11_dt_read_opmode(pdev, rdata[i].of_node, &rmode[i].mode);
+	}
+
+	return rmode;
+}
+
 /*
  * Allocates memory under 'regulators' pointer and copies there array
  * of regulator_desc for given device.
@@ -609,9 +699,14 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
 	}
 
 	of_regulator_match(&pdev->dev, reg_np, rdata, rdev_num);
+	pdata->opmode = s2mps11_pmic_dt_parse_opmode(pdev, rdev_num, rdata,
+				regulators);
+	if (!pdata->opmode)
+		return -ENOMEM;
 
 common_reg:
 	platform_set_drvdata(pdev, s2mps11);
+	s2mps11->opmode = pdata->opmode;
 	s2mps11->rdev_num = rdev_num;
 
 	config.dev = &pdev->dev;
diff --git a/include/linux/mfd/samsung/s2mps14.h b/include/linux/mfd/samsung/s2mps14.h
index ec1e0857ddde..2d36f75a6301 100644
--- a/include/linux/mfd/samsung/s2mps14.h
+++ b/include/linux/mfd/samsung/s2mps14.h
@@ -149,4 +149,23 @@ enum s2mps14_regulators {
 #define S2MPS14_LDO_N_VOLTAGES		(S2MPS14_LDO_VSEL_MASK + 1)
 #define S2MPS14_BUCK_N_VOLTAGES		(S2MPS14_BUCK_VSEL_MASK + 1)
 
+#define S2MPS14_ENCTRL_SHIFT		6
+#define S2MPS14_ENCTRL_MASK		(0x3 << S2MPS14_ENCTRL_SHIFT)
+
+/*
+ * Values of regulator operation modes match device tree bindings.
+ */
+enum s2mps14_regulator_opmode {
+	S2MPS14_REGULATOR_OPMODE_OFF		= 0,
+	S2MPS14_REGULATOR_OPMODE_ON		= 1,
+	/*
+	 * Reserved for compatibility with S5M8767 where this
+	 * is a low power mode.
+	 */
+	S2MPS14_REGULATOR_OPMODE_RESERVED	= 2,
+	S2MPS14_REGULATOR_OPMODE_SUSPEND	= 3,
+
+	S2MPS14_REGULATOR_OPMODE_MAX,
+};
+
 #endif /*  __LINUX_MFD_S2MPS14_H */
-- 
1.7.9.5


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

* [PATCH v2 12/14] Documentation: mfd/regulator: s2mps11: Document the "op_mode" bindings
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (10 preceding siblings ...)
  2014-02-13  9:14 ` [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators Krzysztof Kozlowski
@ 2014-02-13  9:14 ` Krzysztof Kozlowski
  2014-02-13  9:14 ` [PATCH v2 13/14] rtc: s5m: Support different register layout Krzysztof Kozlowski
  2014-02-13  9:14 ` [PATCH v2 14/14] rtc: s5m: Add support for S2MPS14 RTC Krzysztof Kozlowski
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:14 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Chanwoo Choi, Mark Brown, Liam Girdwood,
	Tomasz Figa, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala

Document the "op_mode" properties parsed from DTS by s2mps11 driver.

S2MPS11/S2MPS14 regulators support different modes of operation:
 - Always off;
 - On/Off controlled by pin/GPIO (PWREN/LDOEN/EMMCEN);
 - Always on;

This is very similar to S5M8767 regulator driver which also supports
opmodes (although S5M8767 have also low-power mode).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
---
 Documentation/devicetree/bindings/mfd/s2mps11.txt |   46 +++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/s2mps11.txt b/Documentation/devicetree/bindings/mfd/s2mps11.txt
index f69bec294f02..ffad6bfe2ebf 100644
--- a/Documentation/devicetree/bindings/mfd/s2mps11.txt
+++ b/Documentation/devicetree/bindings/mfd/s2mps11.txt
@@ -54,6 +54,15 @@ BUCK[3, 4], and BUCK[7, 8, 10]
 The regulator constraints inside the regulator nodes use the standard regulator
 bindings which are documented elsewhere.
 
+On S2MPS14 chipset the driver additionally supports "op_mode" properties for
+each regulator.
+ - op_mode: describes the different operating modes of the regulators with
+	power mode change in SOC. The different possible values are,
+		0 - always off mode
+		1 - on in normal mode
+		3 - suspend mode
+		(NOTE: value of 2 is reserved)
+
 The following are the names of the regulators that the s2mps11 pmic block
 supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
 as per the datasheet of s2mps11.
@@ -112,3 +121,40 @@ Example:
 			};
 		};
 	};
+
+	s2mps14_pmic@66 {
+		compatible = "samsung,s2mps14-pmic";
+		reg = <0x66>;
+
+		s2m_osc: clocks {
+			compatible = "samsung,s2mps14-clk";
+			#clock-cells = 1;
+			clock-output-names = "xx", "", "zz";
+		};
+
+		regulators {
+			ldo1_reg: LDO1 {
+				regulator-name = "VAP_ALIVE_1.0V";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				op_mode = <1>; /* Normal Mode */
+			};
+
+			ldo2_reg: LDO2 {
+				regulator-name = "VAP_M1_1.2V";
+				regulator-min-microvolt = <1200000>;
+				regulator-max-microvolt = <1200000>;
+				regulator-always-on;
+				op_mode = <1>; /* Normal Mode */
+			};
+
+			buck1_reg: BUCK1 {
+				regulator-name = "VAP_MIF_1.0V";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				op_mode = <3>; /* Standby Mode */
+			};
+		};
+	};
-- 
1.7.9.5


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

* [PATCH v2 13/14] rtc: s5m: Support different register layout
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (11 preceding siblings ...)
  2014-02-13  9:14 ` [PATCH v2 12/14] Documentation: mfd/regulator: s2mps11: Document the "op_mode" bindings Krzysztof Kozlowski
@ 2014-02-13  9:14 ` Krzysztof Kozlowski
  2014-02-13  9:14 ` [PATCH v2 14/14] rtc: s5m: Add support for S2MPS14 RTC Krzysztof Kozlowski
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:14 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Alessandro Zummo, rtc-linux

This patch prepares for adding support for S2MPS14 RTC device to the
rtc-s5m driver:
1. Adds a map of registers used by the driver which differ between
the chipsets (S5M876X and S2MPS14).
2. Moves code of checking for alarm pending to separate function.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux@googlegroups.com
---
 drivers/rtc/rtc-s5m.c |  157 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 109 insertions(+), 48 deletions(-)

diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index b1627e9ab8f0..6a1290f8709a 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013 Samsung Electronics Co., Ltd
+ * Copyright (c) 2013-2014 Samsung Electronics Co., Ltd
  *	http://www.samsung.com
  *
  *  Copyright (C) 2013 Google, Inc
@@ -38,6 +38,42 @@
  */
 #define UDR_READ_RETRY_CNT	5
 
+/* Registers used by the driver which are different between chipsets. */
+struct s5m_rtc_reg_config {
+	/* Number of registers used for setting time/alarm0/alarm1 */
+	unsigned int regs_count;
+	/* First register for time, seconds */
+	unsigned int time;
+	/* RTC control register */
+	unsigned int ctrl;
+	/* First register for alarm 0, seconds */
+	unsigned int alarm0;
+	/* First register for alarm 1, seconds */
+	unsigned int alarm1;
+	/* SMPL/WTSR register */
+	unsigned int smpl_wtsr;
+	/*
+	 * Register for update flag (UDR). Typically setting UDR field to 1
+	 * will enable update of time or alarm register. Then it will be
+	 * auto-cleared after successful update.
+	 */
+	unsigned int rtc_udr_update;
+	/* Mask for UDR field in 'rtc_udr_update' register */
+	unsigned int rtc_udr_mask;
+};
+
+/* Register map for S5M8763 and S5M8767 */
+static const struct s5m_rtc_reg_config s5m_rtc_regs = {
+	.regs_count		= 8,
+	.time			= S5M_RTC_SEC,
+	.ctrl			= S5M_ALARM1_CONF,
+	.alarm0			= S5M_ALARM0_SEC,
+	.alarm1			= S5M_ALARM1_SEC,
+	.smpl_wtsr		= S5M_WTSR_SMPL_CNTL,
+	.rtc_udr_update		= S5M_RTC_UDR_CON,
+	.rtc_udr_mask		= S5M_RTC_UDR_MASK,
+};
+
 struct s5m_rtc_info {
 	struct device *dev;
 	struct sec_pmic_dev *s5m87xx;
@@ -47,6 +83,7 @@ struct s5m_rtc_info {
 	int device_type;
 	int rtc_24hr_mode;
 	bool wtsr_smpl;
+	const struct s5m_rtc_reg_config	*regs;
 };
 
 static void s5m8767_data_to_tm(u8 *data, struct rtc_time *tm,
@@ -104,8 +141,9 @@ static inline int s5m8767_wait_for_udr_update(struct s5m_rtc_info *info)
 	unsigned int data;
 
 	do {
-		ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
-	} while (--retry && (data & S5M_RTC_UDR_MASK) && !ret);
+		ret = regmap_read(info->regmap, info->regs->rtc_udr_update,
+				&data);
+	} while (--retry && (data & info->regs->rtc_udr_mask) && !ret);
 
 	if (!retry)
 		dev_err(info->dev, "waiting for UDR update, reached max number of retries\n");
@@ -113,21 +151,47 @@ static inline int s5m8767_wait_for_udr_update(struct s5m_rtc_info *info)
 	return ret;
 }
 
+static inline int s5m_check_peding_alarm_interrupt(struct s5m_rtc_info *info,
+		struct rtc_wkalrm *alarm)
+{
+	int ret;
+	unsigned int val;
+
+	switch (info->device_type) {
+	case S5M8767X:
+	case S5M8763X:
+		ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val);
+		val &= S5M_ALARM0_STATUS;
+		break;
+	default:
+		return -EINVAL;
+	}
+	if (ret < 0)
+		return ret;
+
+	if (val)
+		alarm->pending = 1;
+	else
+		alarm->pending = 0;
+
+	return 0;
+}
+
 static inline int s5m8767_rtc_set_time_reg(struct s5m_rtc_info *info)
 {
 	int ret;
 	unsigned int data;
 
-	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
+	ret = regmap_read(info->regmap, info->regs->rtc_udr_update, &data);
 	if (ret < 0) {
 		dev_err(info->dev, "failed to read update reg(%d)\n", ret);
 		return ret;
 	}
 
 	data |= S5M_RTC_TIME_EN_MASK;
-	data |= S5M_RTC_UDR_MASK;
+	data |= info->regs->rtc_udr_mask;
 
-	ret = regmap_write(info->regmap, S5M_RTC_UDR_CON, data);
+	ret = regmap_write(info->regmap, info->regs->rtc_udr_update, data);
 	if (ret < 0) {
 		dev_err(info->dev, "failed to write update reg(%d)\n", ret);
 		return ret;
@@ -143,7 +207,7 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info)
 	int ret;
 	unsigned int data;
 
-	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
+	ret = regmap_read(info->regmap, info->regs->rtc_udr_update, &data);
 	if (ret < 0) {
 		dev_err(info->dev, "%s: fail to read update reg(%d)\n",
 			__func__, ret);
@@ -151,9 +215,9 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info)
 	}
 
 	data &= ~S5M_RTC_TIME_EN_MASK;
-	data |= S5M_RTC_UDR_MASK;
+	data |= info->regs->rtc_udr_mask;
 
-	ret = regmap_write(info->regmap, S5M_RTC_UDR_CON, data);
+	ret = regmap_write(info->regmap, info->regs->rtc_udr_update, data);
 	if (ret < 0) {
 		dev_err(info->dev, "%s: fail to write update reg(%d)\n",
 			__func__, ret);
@@ -200,10 +264,11 @@ static void s5m8763_tm_to_data(struct rtc_time *tm, u8 *data)
 static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct s5m_rtc_info *info = dev_get_drvdata(dev);
-	u8 data[8];
+	u8 data[info->regs->regs_count];
 	int ret;
 
-	ret = regmap_bulk_read(info->regmap, S5M_RTC_SEC, data, 8);
+	ret = regmap_bulk_read(info->regmap, info->regs->time, data,
+			info->regs->regs_count);
 	if (ret < 0)
 		return ret;
 
@@ -230,7 +295,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm)
 static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct s5m_rtc_info *info = dev_get_drvdata(dev);
-	u8 data[8];
+	u8 data[info->regs->regs_count];
 	int ret = 0;
 
 	switch (info->device_type) {
@@ -251,7 +316,8 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm)
 		1900 + tm->tm_year, 1 + tm->tm_mon, tm->tm_mday,
 		tm->tm_hour, tm->tm_min, tm->tm_sec, tm->tm_wday);
 
-	ret = regmap_raw_write(info->regmap, S5M_RTC_SEC, data, 8);
+	ret = regmap_raw_write(info->regmap, info->regs->time, data,
+			info->regs->regs_count);
 	if (ret < 0)
 		return ret;
 
@@ -263,11 +329,12 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm)
 static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
 	struct s5m_rtc_info *info = dev_get_drvdata(dev);
-	u8 data[8];
+	u8 data[info->regs->regs_count];
 	unsigned int val;
 	int ret, i;
 
-	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
+	ret = regmap_bulk_read(info->regmap, info->regs->alarm0, data,
+			info->regs->regs_count);
 	if (ret < 0)
 		return ret;
 
@@ -279,54 +346,42 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 			return ret;
 
 		alrm->enabled = !!val;
-
-		ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val);
-		if (ret < 0)
-			return ret;
-
 		break;
 
 	case S5M8767X:
 		s5m8767_data_to_tm(data, &alrm->time, info->rtc_24hr_mode);
-		dev_dbg(dev, "%s: %d/%d/%d %d:%d:%d(%d)\n", __func__,
-			1900 + alrm->time.tm_year, 1 + alrm->time.tm_mon,
-			alrm->time.tm_mday, alrm->time.tm_hour,
-			alrm->time.tm_min, alrm->time.tm_sec,
-			alrm->time.tm_wday);
-
 		alrm->enabled = 0;
-		for (i = 0; i < 7; i++) {
+		for (i = 0; i < info->regs->regs_count; i++) {
 			if (data[i] & ALARM_ENABLE_MASK) {
 				alrm->enabled = 1;
 				break;
 			}
 		}
-
-		alrm->pending = 0;
-		ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val);
-		if (ret < 0)
-			return ret;
 		break;
 
 	default:
 		return -EINVAL;
 	}
 
-	if (val & S5M_ALARM0_STATUS)
-		alrm->pending = 1;
-	else
-		alrm->pending = 0;
+	dev_dbg(dev, "%s: %d/%d/%d %d:%d:%d(%d)\n", __func__,
+		1900 + alrm->time.tm_year, 1 + alrm->time.tm_mon,
+		alrm->time.tm_mday, alrm->time.tm_hour,
+		alrm->time.tm_min, alrm->time.tm_sec,
+		alrm->time.tm_wday);
+
+	ret = s5m_check_peding_alarm_interrupt(info, alrm);
 
 	return 0;
 }
 
 static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
 {
-	u8 data[8];
+	u8 data[info->regs->regs_count];
 	int ret, i;
 	struct rtc_time tm;
 
-	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
+	ret = regmap_bulk_read(info->regmap, info->regs->alarm0, data,
+			info->regs->regs_count);
 	if (ret < 0)
 		return ret;
 
@@ -341,10 +396,11 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
 		break;
 
 	case S5M8767X:
-		for (i = 0; i < 7; i++)
+		for (i = 0; i < info->regs->regs_count; i++)
 			data[i] &= ~ALARM_ENABLE_MASK;
 
-		ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
+		ret = regmap_raw_write(info->regmap, info->regs->alarm0, data,
+				info->regs->regs_count);
 		if (ret < 0)
 			return ret;
 
@@ -362,11 +418,12 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
 static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
 {
 	int ret;
-	u8 data[8];
+	u8 data[info->regs->regs_count];
 	u8 alarm0_conf;
 	struct rtc_time tm;
 
-	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
+	ret = regmap_bulk_read(info->regmap, info->regs->alarm0, data,
+			info->regs->regs_count);
 	if (ret < 0)
 		return ret;
 
@@ -393,7 +450,8 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
 		if (data[RTC_YEAR1] & 0x7f)
 			data[RTC_YEAR1] |= ALARM_ENABLE_MASK;
 
-		ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
+		ret = regmap_raw_write(info->regmap, info->regs->alarm0, data,
+				info->regs->regs_count);
 		if (ret < 0)
 			return ret;
 		ret = s5m8767_rtc_set_alarm_reg(info);
@@ -410,7 +468,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
 static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
 	struct s5m_rtc_info *info = dev_get_drvdata(dev);
-	u8 data[8];
+	u8 data[info->regs->regs_count];
 	int ret;
 
 	switch (info->device_type) {
@@ -435,7 +493,8 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (ret < 0)
 		return ret;
 
-	ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
+	ret = regmap_raw_write(info->regmap, info->regs->alarm0, data,
+			info->regs->regs_count);
 	if (ret < 0)
 		return ret;
 
@@ -480,7 +539,7 @@ static const struct rtc_class_ops s5m_rtc_ops = {
 static void s5m_rtc_enable_wtsr(struct s5m_rtc_info *info, bool enable)
 {
 	int ret;
-	ret = regmap_update_bits(info->regmap, S5M_WTSR_SMPL_CNTL,
+	ret = regmap_update_bits(info->regmap, info->regs->smpl_wtsr,
 				 WTSR_ENABLE_MASK,
 				 enable ? WTSR_ENABLE_MASK : 0);
 	if (ret < 0)
@@ -491,7 +550,7 @@ static void s5m_rtc_enable_wtsr(struct s5m_rtc_info *info, bool enable)
 static void s5m_rtc_enable_smpl(struct s5m_rtc_info *info, bool enable)
 {
 	int ret;
-	ret = regmap_update_bits(info->regmap, S5M_WTSR_SMPL_CNTL,
+	ret = regmap_update_bits(info->regmap, info->regs->smpl_wtsr,
 				 SMPL_ENABLE_MASK,
 				 enable ? SMPL_ENABLE_MASK : 0);
 	if (ret < 0)
@@ -545,11 +604,13 @@ static int s5m_rtc_probe(struct platform_device *pdev)
 	case S5M8763X:
 		info->irq = regmap_irq_get_virq(s5m87xx->irq_data,
 				S5M8763_IRQ_ALARM0);
+		info->regs = &s5m_rtc_regs;
 		break;
 
 	case S5M8767X:
 		info->irq = regmap_irq_get_virq(s5m87xx->irq_data,
 				S5M8767_IRQ_RTCA1);
+		info->regs = &s5m_rtc_regs;
 		break;
 
 	default:
@@ -593,7 +654,7 @@ static void s5m_rtc_shutdown(struct platform_device *pdev)
 	if (info->wtsr_smpl) {
 		for (i = 0; i < 3; i++) {
 			s5m_rtc_enable_wtsr(info, false);
-			regmap_read(info->regmap, S5M_WTSR_SMPL_CNTL, &val);
+			regmap_read(info->regmap, info->regs->smpl_wtsr, &val);
 			pr_debug("%s: WTSR_SMPL reg(0x%02x)\n", __func__, val);
 			if (val & WTSR_ENABLE_MASK)
 				pr_emerg("%s: fail to disable WTSR\n",
-- 
1.7.9.5


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

* [PATCH v2 14/14] rtc: s5m: Add support for S2MPS14 RTC
  2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
                   ` (12 preceding siblings ...)
  2014-02-13  9:14 ` [PATCH v2 13/14] rtc: s5m: Support different register layout Krzysztof Kozlowski
@ 2014-02-13  9:14 ` Krzysztof Kozlowski
  13 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13  9:14 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Alessandro Zummo, rtc-linux

Add support for S2MPS14 to the rtc-s5m driver. Differences in S2MPS14
(in comparison to S5M8767):
 - Layout of registers;
 - Lack of century support for time and alarms (7 registers used for
   storing time/alarm);
 - Two buffer control registers: WUDR and RUDR;
 - No register for enabling writing time;
 - RTC interrupts are reported in main PMIC I2C device;

This patch also adds missing mfd_cell for RTC in the MFD core driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux@googlegroups.com
---
 drivers/rtc/rtc-s5m.c |   89 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 78 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index 6a1290f8709a..6e4faffe4b5b 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -17,16 +17,14 @@
 
 #include <linux/module.h>
 #include <linux/i2c.h>
-#include <linux/slab.h>
 #include <linux/bcd.h>
-#include <linux/bitops.h>
 #include <linux/regmap.h>
 #include <linux/rtc.h>
-#include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/samsung/core.h>
 #include <linux/mfd/samsung/irq.h>
 #include <linux/mfd/samsung/rtc.h>
+#include <linux/mfd/samsung/s2mps14.h>
 
 /*
  * Maximum number of retries for checking changes in UDR field
@@ -74,6 +72,21 @@ static const struct s5m_rtc_reg_config s5m_rtc_regs = {
 	.rtc_udr_mask		= S5M_RTC_UDR_MASK,
 };
 
+/*
+ * Register map for S2MPS14.
+ * It may be also suitable for S2MPS11 but this was not tested.
+ */
+static const struct s5m_rtc_reg_config s2mps_rtc_regs = {
+	.regs_count		= 7,
+	.time			= S2MPS_RTC_SEC,
+	.ctrl			= S2MPS_RTC_CTRL,
+	.alarm0			= S2MPS_ALARM0_SEC,
+	.alarm1			= S2MPS_ALARM1_SEC,
+	.smpl_wtsr		= S2MPS_WTSR_SMPL_CNTL,
+	.rtc_udr_update		= S2MPS_RTC_UDR_CON,
+	.rtc_udr_mask		= S2MPS_RTC_WUDR_MASK,
+};
+
 struct s5m_rtc_info {
 	struct device *dev;
 	struct sec_pmic_dev *s5m87xx;
@@ -163,6 +176,11 @@ static inline int s5m_check_peding_alarm_interrupt(struct s5m_rtc_info *info,
 		ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val);
 		val &= S5M_ALARM0_STATUS;
 		break;
+	case S2MPS14X:
+		ret = regmap_read(info->s5m87xx->regmap_pmic, S2MPS14_REG_ST2,
+				&val);
+		val &= S2MPS_ALARM0_STATUS;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -188,8 +206,9 @@ static inline int s5m8767_rtc_set_time_reg(struct s5m_rtc_info *info)
 		return ret;
 	}
 
-	data |= S5M_RTC_TIME_EN_MASK;
 	data |= info->regs->rtc_udr_mask;
+	if (info->device_type == S5M8763X || info->device_type == S5M8767X)
+		data |= S5M_RTC_TIME_EN_MASK;
 
 	ret = regmap_write(info->regmap, info->regs->rtc_udr_update, data);
 	if (ret < 0) {
@@ -214,8 +233,18 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info)
 		return ret;
 	}
 
-	data &= ~S5M_RTC_TIME_EN_MASK;
 	data |= info->regs->rtc_udr_mask;
+	switch (info->device_type) {
+	case S5M8763X:
+	case S5M8767X:
+		data &= ~S5M_RTC_TIME_EN_MASK;
+		break;
+	case S2MPS14X:
+		data |= S2MPS_RTC_RUDR_MASK;
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	ret = regmap_write(info->regmap, info->regs->rtc_udr_update, data);
 	if (ret < 0) {
@@ -267,6 +296,17 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	u8 data[info->regs->regs_count];
 	int ret;
 
+	if (info->device_type == S2MPS14X) {
+		ret = regmap_update_bits(info->regmap,
+				info->regs->rtc_udr_update,
+				S2MPS_RTC_RUDR_MASK, S2MPS_RTC_RUDR_MASK);
+		if (ret) {
+			dev_err(dev,
+				"Failed to prepare registers for time reading: %d\n",
+				ret);
+			return ret;
+		}
+	}
 	ret = regmap_bulk_read(info->regmap, info->regs->time, data,
 			info->regs->regs_count);
 	if (ret < 0)
@@ -278,6 +318,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm)
 		break;
 
 	case S5M8767X:
+	case S2MPS14X:
 		s5m8767_data_to_tm(data, tm, info->rtc_24hr_mode);
 		break;
 
@@ -303,6 +344,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm)
 		s5m8763_tm_to_data(tm, data);
 		break;
 	case S5M8767X:
+	case S2MPS14X:
 		ret = s5m8767_tm_to_data(tm, data);
 		break;
 	default:
@@ -349,6 +391,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 		break;
 
 	case S5M8767X:
+	case S2MPS14X:
 		s5m8767_data_to_tm(data, &alrm->time, info->rtc_24hr_mode);
 		alrm->enabled = 0;
 		for (i = 0; i < info->regs->regs_count; i++) {
@@ -396,6 +439,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
 		break;
 
 	case S5M8767X:
+	case S2MPS14X:
 		for (i = 0; i < info->regs->regs_count; i++)
 			data[i] &= ~ALARM_ENABLE_MASK;
 
@@ -439,6 +483,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
 		break;
 
 	case S5M8767X:
+	case S2MPS14X:
 		data[RTC_SEC] |= ALARM_ENABLE_MASK;
 		data[RTC_MIN] |= ALARM_ENABLE_MASK;
 		data[RTC_HOUR] |= ALARM_ENABLE_MASK;
@@ -477,6 +522,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 		break;
 
 	case S5M8767X:
+	case S2MPS14X:
 		s5m8767_tm_to_data(&alrm->time, data);
 		break;
 
@@ -563,12 +609,26 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
 	u8 data[2];
 	int ret;
 
-	/* Set RTC control register : Binary mode, 24hour mode */
-	data[0] = (1 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
-	data[1] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
+	switch (info->device_type) {
+	case S5M8763X:
+	case S5M8767X:
+		/* Set RTC control register : Binary mode, 24hour mode */
+		data[0] = (1 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
+		data[1] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
+
+		ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2);
+		break;
+
+	case S2MPS14X:
+		data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
+		ret = regmap_write(info->regmap, info->regs->ctrl, data[0]);
+		break;
+
+	default:
+		return -EINVAL;
+	}
 
 	info->rtc_24hr_mode = 1;
-	ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2);
 	if (ret < 0) {
 		dev_err(info->dev, "%s: fail to write controlm reg(%d)\n",
 			__func__, ret);
@@ -613,6 +673,12 @@ static int s5m_rtc_probe(struct platform_device *pdev)
 		info->regs = &s5m_rtc_regs;
 		break;
 
+	case S2MPS14X:
+		info->irq = regmap_irq_get_virq(s5m87xx->irq_data,
+				S2MPS14_IRQ_RTCA0);
+		info->regs = &s2mps_rtc_regs;
+		break;
+
 	default:
 		ret = -EINVAL;
 		dev_err(&pdev->dev, "Unsupported device type: %d\n", ret);
@@ -697,7 +763,8 @@ static int s5m_rtc_suspend(struct device *dev)
 static SIMPLE_DEV_PM_OPS(s5m_rtc_pm_ops, s5m_rtc_suspend, s5m_rtc_resume);
 
 static const struct platform_device_id s5m_rtc_id[] = {
-	{ "s5m-rtc", 0 },
+	{ "s5m-rtc",		S5M8767X },
+	{ "s2mps14-rtc",	S2MPS14X },
 };
 
 static struct platform_driver s5m_rtc_driver = {
@@ -715,6 +782,6 @@ module_platform_driver(s5m_rtc_driver);
 
 /* Module information */
 MODULE_AUTHOR("Sangbeom Kim <sbkim73@samsung.com>");
-MODULE_DESCRIPTION("Samsung S5M RTC driver");
+MODULE_DESCRIPTION("Samsung S5M/S2MPS14 RTC driver");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:s5m-rtc");
-- 
1.7.9.5


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

* Re: [PATCH v2 03/14] mfd/rtc: sec/s5m: Rename SEC* symbols to S5M
  2014-02-13  9:13 ` [PATCH v2 03/14] mfd/rtc: sec/s5m: Rename SEC* symbols to S5M Krzysztof Kozlowski
@ 2014-02-13 10:11   ` Lee Jones
  0 siblings, 0 replies; 43+ messages in thread
From: Lee Jones @ 2014-02-13 10:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, linux-kernel, linux-samsung-soc,
	Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Alessandro Zummo, rtc-linux

On Thu, 13 Feb 2014, Krzysztof Kozlowski wrote:

> This patch prepares for adding support for S2MPS14 RTC device to the
> rtc-s5m driver:
> 1. Renames SEC* symbols to S5M.
> 2. Adds S5M prefix to some of defines which are different between S5M876X
> and S2MPS14.
> 
> This is only a rename-like patch, new code is not added.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: rtc-linux@googlegroups.com

I Acked the MFD parts of this already.

> ---
>  drivers/mfd/sec-core.c          |    2 +-
>  drivers/rtc/rtc-s5m.c           |   64 ++++++++++++++++-----------------
>  include/linux/mfd/samsung/rtc.h |   76 +++++++++++++++++++--------------------
>  3 files changed, 71 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index 0efa69e123ee..8504de82b7e0 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -142,7 +142,7 @@ static const struct regmap_config sec_rtc_regmap_config = {
>  	.reg_bits = 8,
>  	.val_bits = 8,
>  
> -	.max_register = SEC_RTC_REG_MAX,
> +	.max_register = S5M_RTC_REG_MAX,
>  };
>  
>  #ifdef CONFIG_OF
> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
> index 476af93543f6..d26e2480f8b3 100644
> --- a/drivers/rtc/rtc-s5m.c
> +++ b/drivers/rtc/rtc-s5m.c
> @@ -30,10 +30,10 @@
>  
>  /*
>   * Maximum number of retries for checking changes in UDR field
> - * of SEC_RTC_UDR_CON register (to limit possible endless loop).
> + * of S5M_RTC_UDR_CON register (to limit possible endless loop).
>   *
>   * After writing to RTC registers (setting time or alarm) read the UDR field
> - * in SEC_RTC_UDR_CON register. UDR is auto-cleared when data have
> + * in S5M_RTC_UDR_CON register. UDR is auto-cleared when data have
>   * been transferred.
>   */
>  #define UDR_READ_RETRY_CNT	5
> @@ -104,8 +104,8 @@ static inline int s5m8767_wait_for_udr_update(struct s5m_rtc_info *info)
>  	unsigned int data;
>  
>  	do {
> -		ret = regmap_read(info->regmap, SEC_RTC_UDR_CON, &data);
> -	} while (--retry && (data & RTC_UDR_MASK) && !ret);
> +		ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
> +	} while (--retry && (data & S5M_RTC_UDR_MASK) && !ret);
>  
>  	if (!retry)
>  		dev_err(info->dev, "waiting for UDR update, reached max number of retries\n");
> @@ -118,16 +118,16 @@ static inline int s5m8767_rtc_set_time_reg(struct s5m_rtc_info *info)
>  	int ret;
>  	unsigned int data;
>  
> -	ret = regmap_read(info->regmap, SEC_RTC_UDR_CON, &data);
> +	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
>  	if (ret < 0) {
>  		dev_err(info->dev, "failed to read update reg(%d)\n", ret);
>  		return ret;
>  	}
>  
> -	data |= RTC_TIME_EN_MASK;
> -	data |= RTC_UDR_MASK;
> +	data |= S5M_RTC_TIME_EN_MASK;
> +	data |= S5M_RTC_UDR_MASK;
>  
> -	ret = regmap_write(info->regmap, SEC_RTC_UDR_CON, data);
> +	ret = regmap_write(info->regmap, S5M_RTC_UDR_CON, data);
>  	if (ret < 0) {
>  		dev_err(info->dev, "failed to write update reg(%d)\n", ret);
>  		return ret;
> @@ -143,17 +143,17 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info)
>  	int ret;
>  	unsigned int data;
>  
> -	ret = regmap_read(info->regmap, SEC_RTC_UDR_CON, &data);
> +	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &data);
>  	if (ret < 0) {
>  		dev_err(info->dev, "%s: fail to read update reg(%d)\n",
>  			__func__, ret);
>  		return ret;
>  	}
>  
> -	data &= ~RTC_TIME_EN_MASK;
> -	data |= RTC_UDR_MASK;
> +	data &= ~S5M_RTC_TIME_EN_MASK;
> +	data |= S5M_RTC_UDR_MASK;
>  
> -	ret = regmap_write(info->regmap, SEC_RTC_UDR_CON, data);
> +	ret = regmap_write(info->regmap, S5M_RTC_UDR_CON, data);
>  	if (ret < 0) {
>  		dev_err(info->dev, "%s: fail to write update reg(%d)\n",
>  			__func__, ret);
> @@ -203,7 +203,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	u8 data[8];
>  	int ret;
>  
> -	ret = regmap_bulk_read(info->regmap, SEC_RTC_SEC, data, 8);
> +	ret = regmap_bulk_read(info->regmap, S5M_RTC_SEC, data, 8);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -251,7 +251,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  		1900 + tm->tm_year, 1 + tm->tm_mon, tm->tm_mday,
>  		tm->tm_hour, tm->tm_min, tm->tm_sec, tm->tm_wday);
>  
> -	ret = regmap_raw_write(info->regmap, SEC_RTC_SEC, data, 8);
> +	ret = regmap_raw_write(info->regmap, S5M_RTC_SEC, data, 8);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -267,20 +267,20 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	unsigned int val;
>  	int ret, i;
>  
> -	ret = regmap_bulk_read(info->regmap, SEC_ALARM0_SEC, data, 8);
> +	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
>  	if (ret < 0)
>  		return ret;
>  
>  	switch (info->device_type) {
>  	case S5M8763X:
>  		s5m8763_data_to_tm(data, &alrm->time);
> -		ret = regmap_read(info->regmap, SEC_ALARM0_CONF, &val);
> +		ret = regmap_read(info->regmap, S5M_ALARM0_CONF, &val);
>  		if (ret < 0)
>  			return ret;
>  
>  		alrm->enabled = !!val;
>  
> -		ret = regmap_read(info->regmap, SEC_RTC_STATUS, &val);
> +		ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val);
>  		if (ret < 0)
>  			return ret;
>  
> @@ -303,7 +303,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  		}
>  
>  		alrm->pending = 0;
> -		ret = regmap_read(info->regmap, SEC_RTC_STATUS, &val);
> +		ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val);
>  		if (ret < 0)
>  			return ret;
>  		break;
> @@ -312,7 +312,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  		return -EINVAL;
>  	}
>  
> -	if (val & ALARM0_STATUS)
> +	if (val & S5M_ALARM0_STATUS)
>  		alrm->pending = 1;
>  	else
>  		alrm->pending = 0;
> @@ -326,7 +326,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
>  	int ret, i;
>  	struct rtc_time tm;
>  
> -	ret = regmap_bulk_read(info->regmap, SEC_ALARM0_SEC, data, 8);
> +	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -337,14 +337,14 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
>  
>  	switch (info->device_type) {
>  	case S5M8763X:
> -		ret = regmap_write(info->regmap, SEC_ALARM0_CONF, 0);
> +		ret = regmap_write(info->regmap, S5M_ALARM0_CONF, 0);
>  		break;
>  
>  	case S5M8767X:
>  		for (i = 0; i < 7; i++)
>  			data[i] &= ~ALARM_ENABLE_MASK;
>  
> -		ret = regmap_raw_write(info->regmap, SEC_ALARM0_SEC, data, 8);
> +		ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
>  		if (ret < 0)
>  			return ret;
>  
> @@ -366,7 +366,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
>  	u8 alarm0_conf;
>  	struct rtc_time tm;
>  
> -	ret = regmap_bulk_read(info->regmap, SEC_ALARM0_SEC, data, 8);
> +	ret = regmap_bulk_read(info->regmap, S5M_ALARM0_SEC, data, 8);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -378,7 +378,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
>  	switch (info->device_type) {
>  	case S5M8763X:
>  		alarm0_conf = 0x77;
> -		ret = regmap_write(info->regmap, SEC_ALARM0_CONF, alarm0_conf);
> +		ret = regmap_write(info->regmap, S5M_ALARM0_CONF, alarm0_conf);
>  		break;
>  
>  	case S5M8767X:
> @@ -393,7 +393,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
>  		if (data[RTC_YEAR1] & 0x7f)
>  			data[RTC_YEAR1] |= ALARM_ENABLE_MASK;
>  
> -		ret = regmap_raw_write(info->regmap, SEC_ALARM0_SEC, data, 8);
> +		ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
>  		if (ret < 0)
>  			return ret;
>  		ret = s5m8767_rtc_set_alarm_reg(info);
> @@ -435,7 +435,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = regmap_raw_write(info->regmap, SEC_ALARM0_SEC, data, 8);
> +	ret = regmap_raw_write(info->regmap, S5M_ALARM0_SEC, data, 8);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -480,7 +480,7 @@ static const struct rtc_class_ops s5m_rtc_ops = {
>  static void s5m_rtc_enable_wtsr(struct s5m_rtc_info *info, bool enable)
>  {
>  	int ret;
> -	ret = regmap_update_bits(info->regmap, SEC_WTSR_SMPL_CNTL,
> +	ret = regmap_update_bits(info->regmap, S5M_WTSR_SMPL_CNTL,
>  				 WTSR_ENABLE_MASK,
>  				 enable ? WTSR_ENABLE_MASK : 0);
>  	if (ret < 0)
> @@ -491,7 +491,7 @@ static void s5m_rtc_enable_wtsr(struct s5m_rtc_info *info, bool enable)
>  static void s5m_rtc_enable_smpl(struct s5m_rtc_info *info, bool enable)
>  {
>  	int ret;
> -	ret = regmap_update_bits(info->regmap, SEC_WTSR_SMPL_CNTL,
> +	ret = regmap_update_bits(info->regmap, S5M_WTSR_SMPL_CNTL,
>  				 SMPL_ENABLE_MASK,
>  				 enable ? SMPL_ENABLE_MASK : 0);
>  	if (ret < 0)
> @@ -506,7 +506,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
>  	int ret;
>  	struct rtc_time tm;
>  
> -	ret = regmap_read(info->regmap, SEC_RTC_UDR_CON, &tp_read);
> +	ret = regmap_read(info->regmap, S5M_RTC_UDR_CON, &tp_read);
>  	if (ret < 0) {
>  		dev_err(info->dev, "%s: fail to read control reg(%d)\n",
>  			__func__, ret);
> @@ -518,7 +518,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
>  	data[1] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
>  
>  	info->rtc_24hr_mode = 1;
> -	ret = regmap_raw_write(info->regmap, SEC_ALARM0_CONF, data, 2);
> +	ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2);
>  	if (ret < 0) {
>  		dev_err(info->dev, "%s: fail to write controlm reg(%d)\n",
>  			__func__, ret);
> @@ -540,7 +540,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info)
>  		ret = s5m_rtc_set_time(info->dev, &tm);
>  	}
>  
> -	ret = regmap_update_bits(info->regmap, SEC_RTC_UDR_CON,
> +	ret = regmap_update_bits(info->regmap, S5M_RTC_UDR_CON,
>  				 RTC_TCON_MASK, tp_read | RTC_TCON_MASK);
>  	if (ret < 0)
>  		dev_err(info->dev, "%s: fail to update TCON reg(%d)\n",
> @@ -623,7 +623,7 @@ static void s5m_rtc_shutdown(struct platform_device *pdev)
>  	if (info->wtsr_smpl) {
>  		for (i = 0; i < 3; i++) {
>  			s5m_rtc_enable_wtsr(info, false);
> -			regmap_read(info->regmap, SEC_WTSR_SMPL_CNTL, &val);
> +			regmap_read(info->regmap, S5M_WTSR_SMPL_CNTL, &val);
>  			pr_debug("%s: WTSR_SMPL reg(0x%02x)\n", __func__, val);
>  			if (val & WTSR_ENABLE_MASK)
>  				pr_emerg("%s: fail to disable WTSR\n",
> diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h
> index 4627f59ebd84..bdf0573891d0 100644
> --- a/include/linux/mfd/samsung/rtc.h
> +++ b/include/linux/mfd/samsung/rtc.h
> @@ -13,38 +13,38 @@
>  #ifndef __LINUX_MFD_SEC_RTC_H
>  #define __LINUX_MFD_SEC_RTC_H
>  
> -enum sec_rtc_reg {
> -	SEC_RTC_SEC,
> -	SEC_RTC_MIN,
> -	SEC_RTC_HOUR,
> -	SEC_RTC_WEEKDAY,
> -	SEC_RTC_DATE,
> -	SEC_RTC_MONTH,
> -	SEC_RTC_YEAR1,
> -	SEC_RTC_YEAR2,
> -	SEC_ALARM0_SEC,
> -	SEC_ALARM0_MIN,
> -	SEC_ALARM0_HOUR,
> -	SEC_ALARM0_WEEKDAY,
> -	SEC_ALARM0_DATE,
> -	SEC_ALARM0_MONTH,
> -	SEC_ALARM0_YEAR1,
> -	SEC_ALARM0_YEAR2,
> -	SEC_ALARM1_SEC,
> -	SEC_ALARM1_MIN,
> -	SEC_ALARM1_HOUR,
> -	SEC_ALARM1_WEEKDAY,
> -	SEC_ALARM1_DATE,
> -	SEC_ALARM1_MONTH,
> -	SEC_ALARM1_YEAR1,
> -	SEC_ALARM1_YEAR2,
> -	SEC_ALARM0_CONF,
> -	SEC_ALARM1_CONF,
> -	SEC_RTC_STATUS,
> -	SEC_WTSR_SMPL_CNTL,
> -	SEC_RTC_UDR_CON,
> +enum s5m_rtc_reg {
> +	S5M_RTC_SEC,
> +	S5M_RTC_MIN,
> +	S5M_RTC_HOUR,
> +	S5M_RTC_WEEKDAY,
> +	S5M_RTC_DATE,
> +	S5M_RTC_MONTH,
> +	S5M_RTC_YEAR1,
> +	S5M_RTC_YEAR2,
> +	S5M_ALARM0_SEC,
> +	S5M_ALARM0_MIN,
> +	S5M_ALARM0_HOUR,
> +	S5M_ALARM0_WEEKDAY,
> +	S5M_ALARM0_DATE,
> +	S5M_ALARM0_MONTH,
> +	S5M_ALARM0_YEAR1,
> +	S5M_ALARM0_YEAR2,
> +	S5M_ALARM1_SEC,
> +	S5M_ALARM1_MIN,
> +	S5M_ALARM1_HOUR,
> +	S5M_ALARM1_WEEKDAY,
> +	S5M_ALARM1_DATE,
> +	S5M_ALARM1_MONTH,
> +	S5M_ALARM1_YEAR1,
> +	S5M_ALARM1_YEAR2,
> +	S5M_ALARM0_CONF,
> +	S5M_ALARM1_CONF,
> +	S5M_RTC_STATUS,
> +	S5M_WTSR_SMPL_CNTL,
> +	S5M_RTC_UDR_CON,
>  
> -	SEC_RTC_REG_MAX,
> +	S5M_RTC_REG_MAX,
>  };
>  
>  #define RTC_I2C_ADDR		(0x0C >> 1)
> @@ -52,9 +52,9 @@ enum sec_rtc_reg {
>  #define HOUR_12			(1 << 7)
>  #define HOUR_AMPM		(1 << 6)
>  #define HOUR_PM			(1 << 5)
> -#define ALARM0_STATUS		(1 << 1)
> -#define ALARM1_STATUS		(1 << 2)
> -#define UPDATE_AD		(1 << 0)
> +#define S5M_ALARM0_STATUS	(1 << 1)
> +#define S5M_ALARM1_STATUS	(1 << 2)
> +#define S5M_UPDATE_AD		(1 << 0)
>  
>  /* RTC Control Register */
>  #define BCD_EN_SHIFT		0
> @@ -62,12 +62,12 @@ enum sec_rtc_reg {
>  #define MODEL24_SHIFT		1
>  #define MODEL24_MASK		(1 << MODEL24_SHIFT)
>  /* RTC Update Register1 */
> -#define RTC_UDR_SHIFT		0
> -#define RTC_UDR_MASK		(1 << RTC_UDR_SHIFT)
> +#define S5M_RTC_UDR_SHIFT	0
> +#define S5M_RTC_UDR_MASK	(1 << S5M_RTC_UDR_SHIFT)
>  #define RTC_TCON_SHIFT		1
>  #define RTC_TCON_MASK		(1 << RTC_TCON_SHIFT)
> -#define RTC_TIME_EN_SHIFT	3
> -#define RTC_TIME_EN_MASK	(1 << RTC_TIME_EN_SHIFT)
> +#define S5M_RTC_TIME_EN_SHIFT	3
> +#define S5M_RTC_TIME_EN_MASK	(1 << S5M_RTC_TIME_EN_SHIFT)
>  
>  /* RTC Hour register */
>  #define HOUR_PM_SHIFT		6

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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-13  9:14 ` [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators Krzysztof Kozlowski
@ 2014-02-13 12:16   ` Yadwinder Singh Brar
  2014-02-14 13:05     ` Krzysztof Kozlowski
  2014-02-13 12:43   ` Lee Jones
  2014-02-13 19:28   ` Mark Brown
  2 siblings, 1 reply; 43+ messages in thread
From: Yadwinder Singh Brar @ 2014-02-13 12:16 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Mark Brown,
	Liam Girdwood

Hi,

On Thu, Feb 13, 2014 at 2:44 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> S2MPS11/S2MPS14 regulators support different modes of operation:
>  - Always off;
>  - On/Off controlled by pin/GPIO (PWREN/LDOEN/EMMCEN);
>  - Always on;
> This is very similar to S5M8767 regulator driver which also supports
> opmodes (although S5M8767 have also low-power mode).
>
> This patch adds parsing the operation mode from DTS by reading a
> "op_mode" property from regulator child node.
>

First thing since "op_mode" is not generic property, I think it should
be appended with some driver specific prefix.

But IMHO its quite generic property used and required by many other
PMICs(almost all used by Samsung).
I would like to use this opportunity to discuss about adding it as
generic regulator constraint(as initial_mode)
by providing a default mapping of generic Regulator operating
modes(kernel specific) to operating modes supported by hardware in
regulator driver itself.

Regards,
Yadwinder

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

* Re: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst
  2014-02-13  9:14 ` [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst Krzysztof Kozlowski
@ 2014-02-13 12:21   ` Yadwinder Singh Brar
  2014-02-13 12:35     ` Krzysztof Kozlowski
  2014-02-13 12:37     ` [PATCH " Krzysztof Kozlowski
  2014-02-13 19:07   ` [PATCH v2 " Mark Brown
  1 sibling, 2 replies; 43+ messages in thread
From: Yadwinder Singh Brar @ 2014-02-13 12:21 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Mark Brown,
	Liam Girdwood

Hi,

On Thu, Feb 13, 2014 at 2:44 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> Add __initconst to 'regulator_desc' array with supported regulators.
> During probe choose how many and which regulators will be supported
> according to device ID. Then copy the 'regulator_desc' array to
> allocated memory so the regulator core can use it.
>
> Additionally allocate array of of_regulator_match() dynamically (based
> on number of regulators) instead of allocation on the stack.
>
> This is needed for supporting different devices in s2mps11
> driver and actually prepares the regulator driver for supporting the
> S2MPS14 device.
>
> Code for supporting the S2MPS14 device will add its own array of
> 'regulator_desc' (also marked as __initconst). This way memory footprint
> of the driver will be reduced (approximately 'regulators_desc' array for
> S2MPS11 occupies 5 kB on 32-bit ARM, for S2MPS14 will occupy 3 kB).
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Yadwinder Singh Brar <yadi.brar01@gmail.com>
> ---

Just observed one trivial thing that in this patch, spacing is not
provided before and after multiplication binary operator ( _ * _ ),
which is recommended by linux spacing convention.

Other than that it looks fine to me, pls feel free to add

Reviewed-by: Yadwinder Singh Brar <yadi.brar@samsung.com>


Regards,
Yadwinder

>  drivers/regulator/s2mps11.c |   74 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 64 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
> index d44bd5b3fe8e..246b25d58c2b 100644
> --- a/drivers/regulator/s2mps11.c
> +++ b/drivers/regulator/s2mps11.c
> @@ -25,10 +25,9 @@
>  #include <linux/mfd/samsung/core.h>
>  #include <linux/mfd/samsung/s2mps11.h>
>
> -#define S2MPS11_REGULATOR_CNT ARRAY_SIZE(regulators)
> -
>  struct s2mps11_info {
> -       struct regulator_dev *rdev[S2MPS11_REGULATOR_MAX];
> +       struct regulator_dev **rdev;
> +       unsigned int rdev_num;
>
>         int ramp_delay2;
>         int ramp_delay34;
> @@ -345,7 +344,7 @@ static struct regulator_ops s2mps11_buck_ops = {
>         .enable_mask    = S2MPS11_ENABLE_MASK                   \
>  }
>
> -static const struct regulator_desc regulators[] = {
> +static const struct regulator_desc s2mps11_regulators[] __initconst = {
>         regulator_desc_ldo2(1),
>         regulator_desc_ldo1(2),
>         regulator_desc_ldo1(3),
> @@ -396,21 +395,62 @@ static const struct regulator_desc regulators[] = {
>         regulator_desc_buck10,
>  };
>
> +/*
> + * Allocates memory under 'regulators' pointer and copies there array
> + * of regulator_desc for given device.
> + *
> + * Returns number of regulators or negative ERRNO on error.
> + */
> +static int __init
> +s2mps11_pmic_init_regulators_desc(struct platform_device *pdev,
> +               struct regulator_desc **regulators)
> +{
> +       const struct regulator_desc *regulators_init;
> +       enum sec_device_type dev_type;
> +       int rdev_num;
> +
> +       dev_type = platform_get_device_id(pdev)->driver_data;
> +       switch (dev_type) {
> +       case S2MPS11X:
> +               rdev_num = ARRAY_SIZE(s2mps11_regulators);
> +               regulators_init = s2mps11_regulators;
> +               break;
> +       default:
> +               dev_err(&pdev->dev, "Invalid device type: %u\n", dev_type);
> +               return -EINVAL;
> +       };
> +
> +       *regulators = devm_kzalloc(&pdev->dev, sizeof(**regulators)*rdev_num,
> +                       GFP_KERNEL);
> +       if (!*regulators)
> +               return -ENOMEM;
> +
> +       memcpy(*regulators, regulators_init, sizeof(**regulators)*rdev_num);
> +
> +       return rdev_num;
> +}
> +
>  static int s2mps11_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[S2MPS11_REGULATOR_MAX];
> +       struct sec_platform_data *pdata = iodev->pdata;
> +       struct of_regulator_match *rdata = NULL;
>         struct device_node *reg_np = NULL;
>         struct regulator_config config = { };
>         struct s2mps11_info *s2mps11;
>         int i, ret;
> +       struct regulator_desc *regulators = NULL;
> +       unsigned int rdev_num;
>
>         s2mps11 = devm_kzalloc(&pdev->dev, sizeof(struct s2mps11_info),
>                                 GFP_KERNEL);
>         if (!s2mps11)
>                 return -ENOMEM;
>
> +       rdev_num = s2mps11_pmic_init_regulators_desc(pdev, &regulators);
> +       if (rdev_num < 0)
> +               return rdev_num;
> +
>         if (!iodev->dev->of_node) {
>                 if (pdata) {
>                         goto common_reg;
> @@ -421,7 +461,16 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
>                 }
>         }
>
> -       for (i = 0; i < S2MPS11_REGULATOR_CNT; i++)
> +       s2mps11->rdev = devm_kzalloc(&pdev->dev,
> +                       sizeof(*s2mps11->rdev)*rdev_num, GFP_KERNEL);
> +       if (!s2mps11->rdev)
> +               return -ENOMEM;
> +
> +       rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata)*rdev_num, GFP_KERNEL);
> +       if (!rdata)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < rdev_num; i++)
>                 rdata[i].name = regulators[i].name;
>
>         reg_np = of_find_node_by_name(iodev->dev->of_node, "regulators");
> @@ -430,15 +479,16 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> -       of_regulator_match(&pdev->dev, reg_np, rdata, S2MPS11_REGULATOR_MAX);
> +       of_regulator_match(&pdev->dev, reg_np, rdata, rdev_num);
>
>  common_reg:
>         platform_set_drvdata(pdev, s2mps11);
> +       s2mps11->rdev_num = rdev_num;
>
>         config.dev = &pdev->dev;
>         config.regmap = iodev->regmap_pmic;
>         config.driver_data = s2mps11;
> -       for (i = 0; i < S2MPS11_REGULATOR_MAX; i++) {
> +       for (i = 0; i < rdev_num; i++) {
>                 if (!reg_np) {
>                         config.init_data = pdata->regulators[i].initdata;
>                         config.of_node = pdata->regulators[i].reg_node;
> @@ -457,11 +507,15 @@ common_reg:
>                 }
>         }
>
> +       /* rdata was needed only for of_regulator_match() during probe */
> +       if (rdata)
> +               devm_kfree(&pdev->dev, rdata);
> +
>         return 0;
>  }
>
>  static const struct platform_device_id s2mps11_pmic_id[] = {
> -       { "s2mps11-pmic", 0},
> +       { "s2mps11-pmic", S2MPS11X},
>         { },
>  };
>  MODULE_DEVICE_TABLE(platform, s2mps11_pmic_id);
> --
> 1.7.9.5
>

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

* Re: [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators
  2014-02-13  9:14 ` [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators Krzysztof Kozlowski
@ 2014-02-13 12:24   ` Yadwinder Singh Brar
  2014-02-13 19:10   ` Mark Brown
  1 sibling, 0 replies; 43+ messages in thread
From: Yadwinder Singh Brar @ 2014-02-13 12:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Mark Brown, Liam Girdwood

On Thu, Feb 13, 2014 at 2:44 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> Add support for S2MPS14 PMIC regulators to s2mps11 driver. The S2MPS14
> has fewer BUCK-s and LDO-s than S2MPS11. It also does not support
> controlling the BUCK ramp delay.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> ---

Reviewed-by: Yadwinder Singh Brar <yadi.brar@samsung.com>

Regards,
Yadwinder

>  drivers/regulator/s2mps11.c |  252 ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 191 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
> index 246b25d58c2b..f56ac6f776ae 100644
> --- a/drivers/regulator/s2mps11.c
> +++ b/drivers/regulator/s2mps11.c
> @@ -1,13 +1,18 @@
>  /*
>   * s2mps11.c
>   *
> - * Copyright (c) 2012 Samsung Electronics Co., Ltd
> + * Copyright (c) 2012-2014 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.
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
>   *
>   */
>
> @@ -24,6 +29,7 @@
>  #include <linux/regulator/of_regulator.h>
>  #include <linux/mfd/samsung/core.h>
>  #include <linux/mfd/samsung/s2mps11.h>
> +#include <linux/mfd/samsung/s2mps14.h>
>
>  struct s2mps11_info {
>         struct regulator_dev **rdev;
> @@ -235,7 +241,7 @@ static struct regulator_ops s2mps11_buck_ops = {
>         .set_ramp_delay         = s2mps11_set_ramp_delay,
>  };
>
> -#define regulator_desc_ldo1(num)       {               \
> +#define regulator_desc_s2mps11_ldo1(num)       {               \
>         .name           = "LDO"#num,                    \
>         .id             = S2MPS11_LDO##num,             \
>         .ops            = &s2mps11_ldo_ops,             \
> @@ -249,7 +255,7 @@ static struct regulator_ops s2mps11_buck_ops = {
>         .enable_reg     = S2MPS11_REG_L1CTRL + num - 1, \
>         .enable_mask    = S2MPS11_ENABLE_MASK           \
>  }
> -#define regulator_desc_ldo2(num)       {               \
> +#define regulator_desc_s2mps11_ldo2(num) {             \
>         .name           = "LDO"#num,                    \
>         .id             = S2MPS11_LDO##num,             \
>         .ops            = &s2mps11_ldo_ops,             \
> @@ -264,7 +270,7 @@ static struct regulator_ops s2mps11_buck_ops = {
>         .enable_mask    = S2MPS11_ENABLE_MASK           \
>  }
>
> -#define regulator_desc_buck1_4(num)    {                       \
> +#define regulator_desc_s2mps11_buck1_4(num) {                  \
>         .name           = "BUCK"#num,                           \
>         .id             = S2MPS11_BUCK##num,                    \
>         .ops            = &s2mps11_buck_ops,                    \
> @@ -280,7 +286,7 @@ static struct regulator_ops s2mps11_buck_ops = {
>         .enable_mask    = S2MPS11_ENABLE_MASK                   \
>  }
>
> -#define regulator_desc_buck5   {                               \
> +#define regulator_desc_s2mps11_buck5 {                         \
>         .name           = "BUCK5",                              \
>         .id             = S2MPS11_BUCK5,                        \
>         .ops            = &s2mps11_buck_ops,                    \
> @@ -296,7 +302,7 @@ static struct regulator_ops s2mps11_buck_ops = {
>         .enable_mask    = S2MPS11_ENABLE_MASK                   \
>  }
>
> -#define regulator_desc_buck6_8(num)    {                       \
> +#define regulator_desc_s2mps11_buck6_8(num) {                  \
>         .name           = "BUCK"#num,                           \
>         .id             = S2MPS11_BUCK##num,                    \
>         .ops            = &s2mps11_buck_ops,                    \
> @@ -312,7 +318,7 @@ static struct regulator_ops s2mps11_buck_ops = {
>         .enable_mask    = S2MPS11_ENABLE_MASK                   \
>  }
>
> -#define regulator_desc_buck9   {                               \
> +#define regulator_desc_s2mps11_buck9 {                         \
>         .name           = "BUCK9",                              \
>         .id             = S2MPS11_BUCK9,                        \
>         .ops            = &s2mps11_buck_ops,                    \
> @@ -328,7 +334,7 @@ static struct regulator_ops s2mps11_buck_ops = {
>         .enable_mask    = S2MPS11_ENABLE_MASK                   \
>  }
>
> -#define regulator_desc_buck10  {                               \
> +#define regulator_desc_s2mps11_buck10 {                                \
>         .name           = "BUCK10",                             \
>         .id             = S2MPS11_BUCK10,                       \
>         .ops            = &s2mps11_buck_ops,                    \
> @@ -345,54 +351,173 @@ static struct regulator_ops s2mps11_buck_ops = {
>  }
>
>  static const struct regulator_desc s2mps11_regulators[] __initconst = {
> -       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_ldo2(27),
> -       regulator_desc_ldo1(28),
> -       regulator_desc_ldo1(29),
> -       regulator_desc_ldo1(30),
> -       regulator_desc_ldo1(31),
> -       regulator_desc_ldo1(32),
> -       regulator_desc_ldo1(33),
> -       regulator_desc_ldo1(34),
> -       regulator_desc_ldo1(35),
> -       regulator_desc_ldo1(36),
> -       regulator_desc_ldo1(37),
> -       regulator_desc_ldo1(38),
> -       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_8(6),
> -       regulator_desc_buck6_8(7),
> -       regulator_desc_buck6_8(8),
> -       regulator_desc_buck9,
> -       regulator_desc_buck10,
> +       regulator_desc_s2mps11_ldo2(1),
> +       regulator_desc_s2mps11_ldo1(2),
> +       regulator_desc_s2mps11_ldo1(3),
> +       regulator_desc_s2mps11_ldo1(4),
> +       regulator_desc_s2mps11_ldo1(5),
> +       regulator_desc_s2mps11_ldo2(6),
> +       regulator_desc_s2mps11_ldo1(7),
> +       regulator_desc_s2mps11_ldo1(8),
> +       regulator_desc_s2mps11_ldo1(9),
> +       regulator_desc_s2mps11_ldo1(10),
> +       regulator_desc_s2mps11_ldo2(11),
> +       regulator_desc_s2mps11_ldo1(12),
> +       regulator_desc_s2mps11_ldo1(13),
> +       regulator_desc_s2mps11_ldo1(14),
> +       regulator_desc_s2mps11_ldo1(15),
> +       regulator_desc_s2mps11_ldo1(16),
> +       regulator_desc_s2mps11_ldo1(17),
> +       regulator_desc_s2mps11_ldo1(18),
> +       regulator_desc_s2mps11_ldo1(19),
> +       regulator_desc_s2mps11_ldo1(20),
> +       regulator_desc_s2mps11_ldo1(21),
> +       regulator_desc_s2mps11_ldo2(22),
> +       regulator_desc_s2mps11_ldo2(23),
> +       regulator_desc_s2mps11_ldo1(24),
> +       regulator_desc_s2mps11_ldo1(25),
> +       regulator_desc_s2mps11_ldo1(26),
> +       regulator_desc_s2mps11_ldo2(27),
> +       regulator_desc_s2mps11_ldo1(28),
> +       regulator_desc_s2mps11_ldo1(29),
> +       regulator_desc_s2mps11_ldo1(30),
> +       regulator_desc_s2mps11_ldo1(31),
> +       regulator_desc_s2mps11_ldo1(32),
> +       regulator_desc_s2mps11_ldo1(33),
> +       regulator_desc_s2mps11_ldo1(34),
> +       regulator_desc_s2mps11_ldo1(35),
> +       regulator_desc_s2mps11_ldo1(36),
> +       regulator_desc_s2mps11_ldo1(37),
> +       regulator_desc_s2mps11_ldo1(38),
> +       regulator_desc_s2mps11_buck1_4(1),
> +       regulator_desc_s2mps11_buck1_4(2),
> +       regulator_desc_s2mps11_buck1_4(3),
> +       regulator_desc_s2mps11_buck1_4(4),
> +       regulator_desc_s2mps11_buck5,
> +       regulator_desc_s2mps11_buck6_8(6),
> +       regulator_desc_s2mps11_buck6_8(7),
> +       regulator_desc_s2mps11_buck6_8(8),
> +       regulator_desc_s2mps11_buck9,
> +       regulator_desc_s2mps11_buck10,
> +};
> +
> +static struct regulator_ops s2mps14_reg_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,
> +};
> +
> +#define regulator_desc_s2mps14_ldo1(num) {             \
> +       .name           = "LDO"#num,                    \
> +       .id             = S2MPS14_LDO##num,             \
> +       .ops            = &s2mps14_reg_ops,             \
> +       .type           = REGULATOR_VOLTAGE,            \
> +       .owner          = THIS_MODULE,                  \
> +       .min_uV         = S2MPS14_LDO_MIN_800MV,        \
> +       .uV_step        = S2MPS14_LDO_STEP_25MV,        \
> +       .n_voltages     = S2MPS14_LDO_N_VOLTAGES,       \
> +       .vsel_reg       = S2MPS14_REG_L1CTRL + num - 1, \
> +       .vsel_mask      = S2MPS14_LDO_VSEL_MASK,        \
> +       .enable_reg     = S2MPS14_REG_L1CTRL + num - 1, \
> +       .enable_mask    = S2MPS14_ENABLE_MASK           \
> +}
> +#define regulator_desc_s2mps14_ldo2(num) {             \
> +       .name           = "LDO"#num,                    \
> +       .id             = S2MPS14_LDO##num,             \
> +       .ops            = &s2mps14_reg_ops,             \
> +       .type           = REGULATOR_VOLTAGE,            \
> +       .owner          = THIS_MODULE,                  \
> +       .min_uV         = S2MPS14_LDO_MIN_1800MV,       \
> +       .uV_step        = S2MPS14_LDO_STEP_25MV,        \
> +       .n_voltages     = S2MPS14_LDO_N_VOLTAGES,       \
> +       .vsel_reg       = S2MPS14_REG_L1CTRL + num - 1, \
> +       .vsel_mask      = S2MPS14_LDO_VSEL_MASK,        \
> +       .enable_reg     = S2MPS14_REG_L1CTRL + num - 1, \
> +       .enable_mask    = S2MPS14_ENABLE_MASK           \
> +}
> +#define regulator_desc_s2mps14_ldo3(num) {             \
> +       .name           = "LDO"#num,                    \
> +       .id             = S2MPS14_LDO##num,             \
> +       .ops            = &s2mps14_reg_ops,             \
> +       .type           = REGULATOR_VOLTAGE,            \
> +       .owner          = THIS_MODULE,                  \
> +       .min_uV         = S2MPS14_LDO_MIN_800MV,        \
> +       .uV_step        = S2MPS14_LDO_STEP_12_5MV,      \
> +       .n_voltages     = S2MPS14_LDO_N_VOLTAGES,       \
> +       .vsel_reg       = S2MPS14_REG_L1CTRL + num - 1, \
> +       .vsel_mask      = S2MPS14_LDO_VSEL_MASK,        \
> +       .enable_reg     = S2MPS14_REG_L1CTRL + num - 1, \
> +       .enable_mask    = S2MPS14_ENABLE_MASK           \
> +}
> +#define regulator_desc_s2mps14_buck1235(num) {                 \
> +       .name           = "BUCK"#num,                           \
> +       .id             = S2MPS14_BUCK##num,                    \
> +       .ops            = &s2mps14_reg_ops,                     \
> +       .type           = REGULATOR_VOLTAGE,                    \
> +       .owner          = THIS_MODULE,                          \
> +       .min_uV         = S2MPS14_BUCK1235_MIN_600MV,           \
> +       .uV_step        = S2MPS14_BUCK1235_STEP_6_25MV,         \
> +       .n_voltages     = S2MPS14_BUCK_N_VOLTAGES,              \
> +       .linear_min_sel = S2MPS14_BUCK1235_START_SEL,           \
> +       .ramp_delay     = S2MPS14_BUCK_RAMP_DELAY,              \
> +       .vsel_reg       = S2MPS14_REG_B1CTRL2 + (num - 1) * 2,  \
> +       .vsel_mask      = S2MPS14_BUCK_VSEL_MASK,               \
> +       .enable_reg     = S2MPS14_REG_B1CTRL1 + (num - 1) * 2,  \
> +       .enable_mask    = S2MPS14_ENABLE_MASK                   \
> +}
> +#define regulator_desc_s2mps14_buck4(num) {                    \
> +       .name           = "BUCK"#num,                           \
> +       .id             = S2MPS14_BUCK##num,                    \
> +       .ops            = &s2mps14_reg_ops,                     \
> +       .type           = REGULATOR_VOLTAGE,                    \
> +       .owner          = THIS_MODULE,                          \
> +       .min_uV         = S2MPS14_BUCK4_MIN_1400MV,             \
> +       .uV_step        = S2MPS14_BUCK4_STEP_12_5MV,            \
> +       .n_voltages     = S2MPS14_BUCK_N_VOLTAGES,              \
> +       .linear_min_sel = S2MPS14_BUCK4_START_SEL,              \
> +       .ramp_delay     = S2MPS14_BUCK_RAMP_DELAY,              \
> +       .vsel_reg       = S2MPS14_REG_B1CTRL2 + (num - 1) * 2,  \
> +       .vsel_mask      = S2MPS14_BUCK_VSEL_MASK,               \
> +       .enable_reg     = S2MPS14_REG_B1CTRL1 + (num - 1) * 2,  \
> +       .enable_mask    = S2MPS14_ENABLE_MASK                   \
> +}
> +
> +static const struct regulator_desc s2mps14_regulators[] __initconst = {
> +       regulator_desc_s2mps14_ldo3(1),
> +       regulator_desc_s2mps14_ldo3(2),
> +       regulator_desc_s2mps14_ldo1(3),
> +       regulator_desc_s2mps14_ldo1(4),
> +       regulator_desc_s2mps14_ldo3(5),
> +       regulator_desc_s2mps14_ldo3(6),
> +       regulator_desc_s2mps14_ldo1(7),
> +       regulator_desc_s2mps14_ldo2(8),
> +       regulator_desc_s2mps14_ldo3(9),
> +       regulator_desc_s2mps14_ldo3(10),
> +       regulator_desc_s2mps14_ldo1(11),
> +       regulator_desc_s2mps14_ldo2(12),
> +       regulator_desc_s2mps14_ldo2(13),
> +       regulator_desc_s2mps14_ldo2(14),
> +       regulator_desc_s2mps14_ldo2(15),
> +       regulator_desc_s2mps14_ldo2(16),
> +       regulator_desc_s2mps14_ldo2(17),
> +       regulator_desc_s2mps14_ldo2(18),
> +       regulator_desc_s2mps14_ldo1(19),
> +       regulator_desc_s2mps14_ldo1(20),
> +       regulator_desc_s2mps14_ldo1(21),
> +       regulator_desc_s2mps14_ldo3(22),
> +       regulator_desc_s2mps14_ldo1(23),
> +       regulator_desc_s2mps14_ldo2(24),
> +       regulator_desc_s2mps14_ldo2(25),
> +       regulator_desc_s2mps14_buck1235(1),
> +       regulator_desc_s2mps14_buck1235(2),
> +       regulator_desc_s2mps14_buck1235(3),
> +       regulator_desc_s2mps14_buck4(4),
> +       regulator_desc_s2mps14_buck1235(5),
>  };
>
>  /*
> @@ -415,6 +540,10 @@ s2mps11_pmic_init_regulators_desc(struct platform_device *pdev,
>                 rdev_num = ARRAY_SIZE(s2mps11_regulators);
>                 regulators_init = s2mps11_regulators;
>                 break;
> +       case S2MPS14X:
> +               rdev_num = ARRAY_SIZE(s2mps14_regulators);
> +               regulators_init = s2mps14_regulators;
> +               break;
>         default:
>                 dev_err(&pdev->dev, "Invalid device type: %u\n", dev_type);
>                 return -EINVAL;
> @@ -516,6 +645,7 @@ common_reg:
>
>  static const struct platform_device_id s2mps11_pmic_id[] = {
>         { "s2mps11-pmic", S2MPS11X},
> +       { "s2mps14-pmic", S2MPS14X},
>         { },
>  };
>  MODULE_DEVICE_TABLE(platform, s2mps11_pmic_id);
> @@ -543,5 +673,5 @@ module_exit(s2mps11_pmic_exit);
>
>  /* Module information */
>  MODULE_AUTHOR("Sangbeom Kim <sbkim73@samsung.com>");
> -MODULE_DESCRIPTION("SAMSUNG S2MPS11 Regulator Driver");
> +MODULE_DESCRIPTION("SAMSUNG S2MPS11/S2MPS14 Regulator Driver");
>  MODULE_LICENSE("GPL");
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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] 43+ messages in thread

* Re: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst
  2014-02-13 12:21   ` Yadwinder Singh Brar
@ 2014-02-13 12:35     ` Krzysztof Kozlowski
  2014-02-13 12:37     ` [PATCH " Krzysztof Kozlowski
  1 sibling, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13 12:35 UTC (permalink / raw)
  To: Yadwinder Singh Brar
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Mark Brown,
	Liam Girdwood

On Thu, 2014-02-13 at 17:51 +0530, Yadwinder Singh Brar wrote:
> Hi,
> 
> On Thu, Feb 13, 2014 at 2:44 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > Add __initconst to 'regulator_desc' array with supported regulators.
> > During probe choose how many and which regulators will be supported
> > according to device ID. Then copy the 'regulator_desc' array to
> > allocated memory so the regulator core can use it.
> >
> > Additionally allocate array of of_regulator_match() dynamically (based
> > on number of regulators) instead of allocation on the stack.
> >
> > This is needed for supporting different devices in s2mps11
> > driver and actually prepares the regulator driver for supporting the
> > S2MPS14 device.
> >
> > Code for supporting the S2MPS14 device will add its own array of
> > 'regulator_desc' (also marked as __initconst). This way memory footprint
> > of the driver will be reduced (approximately 'regulators_desc' array for
> > S2MPS11 occupies 5 kB on 32-bit ARM, for S2MPS14 will occupy 3 kB).
> >
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Liam Girdwood <lgirdwood@gmail.com>
> > Cc: Yadwinder Singh Brar <yadi.brar01@gmail.com>
> > ---
> 
> Just observed one trivial thing that in this patch, spacing is not
> provided before and after multiplication binary operator ( _ * _ ),
> which is recommended by linux spacing convention.
> 
> Other than that it looks fine to me, pls feel free to add
> 
> Reviewed-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
> 

Thanks for review. I'll send a patch with proper spacing around '*'.

Best regards,
Krzysztof




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

* [PATCH 07/14] regulator: s2mps11: Copy supported regulators from initconst
  2014-02-13 12:21   ` Yadwinder Singh Brar
  2014-02-13 12:35     ` Krzysztof Kozlowski
@ 2014-02-13 12:37     ` Krzysztof Kozlowski
  2014-02-13 18:05       ` Mark Brown
  1 sibling, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13 12:37 UTC (permalink / raw)
  To: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Krzysztof Kozlowski, Chanwoo Choi, Mark Brown, Liam Girdwood,
	Yadwinder Singh Brar

Add __initconst to 'regulator_desc' array with supported regulators.
During probe choose how many and which regulators will be supported
according to device ID. Then copy the 'regulator_desc' array to
allocated memory so the regulator core can use it.

Additionally allocate array of of_regulator_match() dynamically (based
on number of regulators) instead of allocation on the stack.

This is needed for supporting different devices in s2mps11
driver and actually prepares the regulator driver for supporting the
S2MPS14 device.

Code for supporting the S2MPS14 device will add its own array of
'regulator_desc' (also marked as __initconst). This way memory footprint
of the driver will be reduced (approximately 'regulators_desc' array for
S2MPS11 occupies 5 kB on 32-bit ARM, for S2MPS14 will occupy 3 kB).

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Yadwinder Singh Brar <yadi.brar01@gmail.com>
Reviewed-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
---
 drivers/regulator/s2mps11.c |   75 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 65 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index d44bd5b3fe8e..8504ab29aa9f 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -25,10 +25,9 @@
 #include <linux/mfd/samsung/core.h>
 #include <linux/mfd/samsung/s2mps11.h>
 
-#define S2MPS11_REGULATOR_CNT ARRAY_SIZE(regulators)
-
 struct s2mps11_info {
-	struct regulator_dev *rdev[S2MPS11_REGULATOR_MAX];
+	struct regulator_dev **rdev;
+	unsigned int rdev_num;
 
 	int ramp_delay2;
 	int ramp_delay34;
@@ -345,7 +344,7 @@ static struct regulator_ops s2mps11_buck_ops = {
 	.enable_mask	= S2MPS11_ENABLE_MASK			\
 }
 
-static const struct regulator_desc regulators[] = {
+static const struct regulator_desc s2mps11_regulators[] __initconst = {
 	regulator_desc_ldo2(1),
 	regulator_desc_ldo1(2),
 	regulator_desc_ldo1(3),
@@ -396,21 +395,62 @@ static const struct regulator_desc regulators[] = {
 	regulator_desc_buck10,
 };
 
+/*
+ * Allocates memory under 'regulators' pointer and copies there array
+ * of regulator_desc for given device.
+ *
+ * Returns number of regulators or negative ERRNO on error.
+ */
+static int __init
+s2mps11_pmic_init_regulators_desc(struct platform_device *pdev,
+		struct regulator_desc **regulators)
+{
+	const struct regulator_desc *regulators_init;
+	enum sec_device_type dev_type;
+	int rdev_num;
+
+	dev_type = platform_get_device_id(pdev)->driver_data;
+	switch (dev_type) {
+	case S2MPS11X:
+		rdev_num = ARRAY_SIZE(s2mps11_regulators);
+		regulators_init = s2mps11_regulators;
+		break;
+	default:
+		dev_err(&pdev->dev, "Invalid device type: %u\n", dev_type);
+		return -EINVAL;
+	};
+
+	*regulators = devm_kzalloc(&pdev->dev,
+			sizeof(**regulators) * rdev_num, GFP_KERNEL);
+	if (!*regulators)
+		return -ENOMEM;
+
+	memcpy(*regulators, regulators_init, sizeof(**regulators) * rdev_num);
+
+	return rdev_num;
+}
+
 static int s2mps11_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[S2MPS11_REGULATOR_MAX];
+	struct sec_platform_data *pdata = iodev->pdata;
+	struct of_regulator_match *rdata = NULL;
 	struct device_node *reg_np = NULL;
 	struct regulator_config config = { };
 	struct s2mps11_info *s2mps11;
 	int i, ret;
+	struct regulator_desc *regulators = NULL;
+	unsigned int rdev_num;
 
 	s2mps11 = devm_kzalloc(&pdev->dev, sizeof(struct s2mps11_info),
 				GFP_KERNEL);
 	if (!s2mps11)
 		return -ENOMEM;
 
+	rdev_num = s2mps11_pmic_init_regulators_desc(pdev, &regulators);
+	if (rdev_num < 0)
+		return rdev_num;
+
 	if (!iodev->dev->of_node) {
 		if (pdata) {
 			goto common_reg;
@@ -421,7 +461,17 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
 		}
 	}
 
-	for (i = 0; i < S2MPS11_REGULATOR_CNT; i++)
+	s2mps11->rdev = devm_kzalloc(&pdev->dev,
+			sizeof(*s2mps11->rdev) * rdev_num, GFP_KERNEL);
+	if (!s2mps11->rdev)
+		return -ENOMEM;
+
+	rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata) * rdev_num,
+			GFP_KERNEL);
+	if (!rdata)
+		return -ENOMEM;
+
+	for (i = 0; i < rdev_num; i++)
 		rdata[i].name = regulators[i].name;
 
 	reg_np = of_find_node_by_name(iodev->dev->of_node, "regulators");
@@ -430,15 +480,16 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	of_regulator_match(&pdev->dev, reg_np, rdata, S2MPS11_REGULATOR_MAX);
+	of_regulator_match(&pdev->dev, reg_np, rdata, rdev_num);
 
 common_reg:
 	platform_set_drvdata(pdev, s2mps11);
+	s2mps11->rdev_num = rdev_num;
 
 	config.dev = &pdev->dev;
 	config.regmap = iodev->regmap_pmic;
 	config.driver_data = s2mps11;
-	for (i = 0; i < S2MPS11_REGULATOR_MAX; i++) {
+	for (i = 0; i < rdev_num; i++) {
 		if (!reg_np) {
 			config.init_data = pdata->regulators[i].initdata;
 			config.of_node = pdata->regulators[i].reg_node;
@@ -457,11 +508,15 @@ common_reg:
 		}
 	}
 
+	/* rdata was needed only for of_regulator_match() during probe */
+	if (rdata)
+		devm_kfree(&pdev->dev, rdata);
+
 	return 0;
 }
 
 static const struct platform_device_id s2mps11_pmic_id[] = {
-	{ "s2mps11-pmic", 0},
+	{ "s2mps11-pmic", S2MPS11X},
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, s2mps11_pmic_id);
-- 
1.7.9.5


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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-13  9:14 ` [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators Krzysztof Kozlowski
  2014-02-13 12:16   ` Yadwinder Singh Brar
@ 2014-02-13 12:43   ` Lee Jones
  2014-02-13 13:00     ` Krzysztof Kozlowski
  2014-02-13 19:28   ` Mark Brown
  2 siblings, 1 reply; 43+ messages in thread
From: Lee Jones @ 2014-02-13 12:43 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, linux-kernel, linux-samsung-soc,
	Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Mark Brown, Liam Girdwood

> S2MPS11/S2MPS14 regulators support different modes of operation:
>  - Always off;
>  - On/Off controlled by pin/GPIO (PWREN/LDOEN/EMMCEN);
>  - Always on;
> This is very similar to S5M8767 regulator driver which also supports
> opmodes (although S5M8767 have also low-power mode).
> 
> This patch adds parsing the operation mode from DTS by reading a
> "op_mode" property from regulator child node.
> 
> The op_mode is then used for enabling the S2MPS14 regulators.
> On S2MPS11 the DTS "op_mode" property is parsed but not used for
> enabling, as this was not tested.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> ---
>  drivers/regulator/s2mps11.c         |   97 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/samsung/s2mps14.h |   19 +++++++
>  2 files changed, 115 insertions(+), 1 deletion(-)

<snip>

> +++ b/include/linux/mfd/samsung/s2mps14.h
> @@ -149,4 +149,23 @@ enum s2mps14_regulators {
>  #define S2MPS14_LDO_N_VOLTAGES		(S2MPS14_LDO_VSEL_MASK + 1)
>  #define S2MPS14_BUCK_N_VOLTAGES		(S2MPS14_BUCK_VSEL_MASK + 1)
>  
> +#define S2MPS14_ENCTRL_SHIFT		6
> +#define S2MPS14_ENCTRL_MASK		(0x3 << S2MPS14_ENCTRL_SHIFT)
> +
> +/*
> + * Values of regulator operation modes match device tree bindings.
> + */
> +enum s2mps14_regulator_opmode {
> +	S2MPS14_REGULATOR_OPMODE_OFF		= 0,
> +	S2MPS14_REGULATOR_OPMODE_ON		= 1,
> +	/*
> +	 * Reserved for compatibility with S5M8767 where this
> +	 * is a low power mode.
> +	 */
> +	S2MPS14_REGULATOR_OPMODE_RESERVED	= 2,
> +	S2MPS14_REGULATOR_OPMODE_SUSPEND	= 3,

You don't need to number these like this. If you want to force the
numbering to start at '0' initialise the top value, then the rest
should be sequential.

> +	S2MPS14_REGULATOR_OPMODE_MAX,
> +};
> +
>  #endif /*  __LINUX_MFD_S2MPS14_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] 43+ messages in thread

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-13 12:43   ` Lee Jones
@ 2014-02-13 13:00     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-13 13:00 UTC (permalink / raw)
  To: Lee Jones
  Cc: Sangbeom Kim, Samuel Ortiz, linux-kernel, linux-samsung-soc,
	Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Chanwoo Choi, Mark Brown, Liam Girdwood



On Thu, 2014-02-13 at 12:43 +0000, Lee Jones wrote:
> > S2MPS11/S2MPS14 regulators support different modes of operation:
> >  - Always off;
> >  - On/Off controlled by pin/GPIO (PWREN/LDOEN/EMMCEN);
> >  - Always on;
> > This is very similar to S5M8767 regulator driver which also supports
> > opmodes (although S5M8767 have also low-power mode).
> > 
> > This patch adds parsing the operation mode from DTS by reading a
> > "op_mode" property from regulator child node.
> > 
> > The op_mode is then used for enabling the S2MPS14 regulators.
> > On S2MPS11 the DTS "op_mode" property is parsed but not used for
> > enabling, as this was not tested.
> > 
> > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: Liam Girdwood <lgirdwood@gmail.com>
> > ---
> >  drivers/regulator/s2mps11.c         |   97 ++++++++++++++++++++++++++++++++++-
> >  include/linux/mfd/samsung/s2mps14.h |   19 +++++++
> >  2 files changed, 115 insertions(+), 1 deletion(-)
> 
> <snip>
> 
> > +++ b/include/linux/mfd/samsung/s2mps14.h
> > @@ -149,4 +149,23 @@ enum s2mps14_regulators {
> >  #define S2MPS14_LDO_N_VOLTAGES		(S2MPS14_LDO_VSEL_MASK + 1)
> >  #define S2MPS14_BUCK_N_VOLTAGES		(S2MPS14_BUCK_VSEL_MASK + 1)
> >  
> > +#define S2MPS14_ENCTRL_SHIFT		6
> > +#define S2MPS14_ENCTRL_MASK		(0x3 << S2MPS14_ENCTRL_SHIFT)
> > +
> > +/*
> > + * Values of regulator operation modes match device tree bindings.
> > + */
> > +enum s2mps14_regulator_opmode {
> > +	S2MPS14_REGULATOR_OPMODE_OFF		= 0,
> > +	S2MPS14_REGULATOR_OPMODE_ON		= 1,
> > +	/*
> > +	 * Reserved for compatibility with S5M8767 where this
> > +	 * is a low power mode.
> > +	 */
> > +	S2MPS14_REGULATOR_OPMODE_RESERVED	= 2,
> > +	S2MPS14_REGULATOR_OPMODE_SUSPEND	= 3,
> 
> You don't need to number these like this. If you want to force the
> numbering to start at '0' initialise the top value, then the rest
> should be sequential.

Yes, I know. I wanted to emphasize the relationship to opmode entries in
DTS (to prevent adding new enum value somewhere between them). The code
somehow self-documents that the enum should only be extended, not
modified.

Best regards,
Krzysztof

> 
> > +	S2MPS14_REGULATOR_OPMODE_MAX,
> > +};
> > +
> >  #endif /*  __LINUX_MFD_S2MPS14_H */
> 


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

* Re: [PATCH v2 10/14] Documentation: mfd: s2mps11: Document support for S2MPS14
  2014-02-13  9:14 ` [PATCH v2 10/14] Documentation: mfd: s2mps11: Document support for S2MPS14 Krzysztof Kozlowski
@ 2014-02-13 14:55   ` Tomasz Figa
  0 siblings, 0 replies; 43+ messages in thread
From: Tomasz Figa @ 2014-02-13 14:55 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc
  Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
	Mark Brown, Liam Girdwood, devicetree, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala

Hi,

On 13.02.2014 10:14, Krzysztof Kozlowski wrote:
> Add bindings documentation for S2MPS14 device to the s2mps11 driver.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> ---
>   Documentation/devicetree/bindings/mfd/s2mps11.txt |   12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/s2mps11.txt b/Documentation/devicetree/bindings/mfd/s2mps11.txt
> index 15ee89c3cc7b..f69bec294f02 100644
> --- a/Documentation/devicetree/bindings/mfd/s2mps11.txt
> +++ b/Documentation/devicetree/bindings/mfd/s2mps11.txt
> @@ -1,5 +1,5 @@
>
> -* Samsung S2MPS11 Voltage and Current Regulator
> +* Samsung S2MPS11 and S2MPS14 Voltage and Current Regulator
>
>   The Samsung S2MPS11 is a multi-function device which includes voltage and
>   current regulators, RTC, charger controller and other sub-blocks. It is
> @@ -7,7 +7,7 @@ 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,s2mps11-pmic".
> +- compatible: Should be "samsung,s2mps11-pmic" or "samsung,s2mps14-pmic".
>   - reg: Specifies the I2C slave address of the pmic block. It should be 0x66.
>
>   Optional properties:
> @@ -59,10 +59,14 @@ supports. Note: The 'n' in LDOn and BUCKn represents the LDO or BUCK number
>   as per the datasheet of s2mps11.
>
>   	- LDOn
> -		  - valid values for n are 1 to 38
> +		  - valid values for n are:
> +			- S2MPS11: 1 to 38
> +			- S2MPS14: 1 to 25
>   		  - Example: LDO1, LD02, LDO28
>   	- BUCKn
> -		  - valid values for n are 1 to 10.
> +		  - valid values for n are:
> +			- S2MPS11: 1 to 10
> +			- S2MPS14: 1 to 5
>   		  - Example: BUCK1, BUCK2, BUCK9
>
>   Example:
>

Acked-by: Tomasz Figa <t.figa@samsung.com>

Best regards,
Tomasz

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

* Re: [PATCH 07/14] regulator: s2mps11: Copy supported regulators from initconst
  2014-02-13 12:37     ` [PATCH " Krzysztof Kozlowski
@ 2014-02-13 18:05       ` Mark Brown
  0 siblings, 0 replies; 43+ messages in thread
From: Mark Brown @ 2014-02-13 18:05 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Yadwinder Singh Brar

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

On Thu, Feb 13, 2014 at 01:37:03PM +0100, Krzysztof Kozlowski wrote:
> Add __initconst to 'regulator_desc' array with supported regulators.
> During probe choose how many and which regulators will be supported
> according to device ID. Then copy the 'regulator_desc' array to
> allocated memory so the regulator core can use it.

Please don't bury individual patches in the middle of replies to other
patch serieses, that makes things more confusing and difficult to
manage.  Please also try to avoid sending only one patch from a series
to some people - at least also CC everyone on the cover letter so that
it's clear what the overall goal is and what the dependencies are.

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

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

* Re: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst
  2014-02-13  9:14 ` [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst Krzysztof Kozlowski
  2014-02-13 12:21   ` Yadwinder Singh Brar
@ 2014-02-13 19:07   ` Mark Brown
  2014-02-14  7:46     ` Krzysztof Kozlowski
  1 sibling, 1 reply; 43+ messages in thread
From: Mark Brown @ 2014-02-13 19:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Yadwinder Singh Brar

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

On Thu, Feb 13, 2014 at 10:14:00AM +0100, Krzysztof Kozlowski wrote:

> -	for (i = 0; i < S2MPS11_REGULATOR_CNT; i++)
> +	s2mps11->rdev = devm_kzalloc(&pdev->dev,
> +			sizeof(*s2mps11->rdev)*rdev_num, GFP_KERNEL);
> +	if (!s2mps11->rdev)
> +		return -ENOMEM;

If we're using managed allocations do we actually need to keep the rdev
table at all?  We only normally use it to free.

> +	rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata)*rdev_num, GFP_KERNEL);
> +	if (!rdata)
> +		return -ENOMEM;
> +

> +	/* rdata was needed only for of_regulator_match() during probe */
> +	if (rdata)
> +		devm_kfree(&pdev->dev, rdata);
> +

If this is always going to be freed within the probe path (in the same
function indeed) why is it a managed allocaton at all?

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

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

* Re: [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators
  2014-02-13  9:14 ` [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators Krzysztof Kozlowski
  2014-02-13 12:24   ` Yadwinder Singh Brar
@ 2014-02-13 19:10   ` Mark Brown
  2014-02-14  7:33     ` Krzysztof Kozlowski
  1 sibling, 1 reply; 43+ messages in thread
From: Mark Brown @ 2014-02-13 19:10 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Liam Girdwood

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

On Thu, Feb 13, 2014 at 10:14:02AM +0100, Krzysztof Kozlowski wrote:
> Add support for S2MPS14 PMIC regulators to s2mps11 driver. The S2MPS14
> has fewer BUCK-s and LDO-s than S2MPS11. It also does not support
> controlling the BUCK ramp delay.

How different are the other Samsung PMICs?  They share the core driver
code and this makes the driver more parameterisable so perhaps it means
the others could be merged in here too.

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

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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-13  9:14 ` [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators Krzysztof Kozlowski
  2014-02-13 12:16   ` Yadwinder Singh Brar
  2014-02-13 12:43   ` Lee Jones
@ 2014-02-13 19:28   ` Mark Brown
  2014-02-14  8:15     ` Krzysztof Kozlowski
  2 siblings, 1 reply; 43+ messages in thread
From: Mark Brown @ 2014-02-13 19:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood

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

On Thu, Feb 13, 2014 at 10:14:04AM +0100, Krzysztof Kozlowski wrote:
> S2MPS11/S2MPS14 regulators support different modes of operation:
>  - Always off;
>  - On/Off controlled by pin/GPIO (PWREN/LDOEN/EMMCEN);
>  - Always on;
> This is very similar to S5M8767 regulator driver which also supports
> opmodes (although S5M8767 have also low-power mode).

What is the difference between always off/on and the normal enable
control?  This sounds like a fairly standard register control that can
be overridden by a GPIO.  The usual way of specifying that is by keying
off the presence of the GPIO.

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

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

* Re: [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators
  2014-02-13 19:10   ` Mark Brown
@ 2014-02-14  7:33     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-14  7:33 UTC (permalink / raw)
  To: Mark Brown
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Liam Girdwood

On Thu, 2014-02-13 at 19:10 +0000, Mark Brown wrote:
> On Thu, Feb 13, 2014 at 10:14:02AM +0100, Krzysztof Kozlowski wrote:
> > Add support for S2MPS14 PMIC regulators to s2mps11 driver. The S2MPS14
> > has fewer BUCK-s and LDO-s than S2MPS11. It also does not support
> > controlling the BUCK ramp delay.
> 
> How different are the other Samsung PMICs?  They share the core driver
> code and this makes the driver more parameterisable so perhaps it means
> the others could be merged in here too.

With some effort the s2mps11 and s5m8767 regulator drivers can be
merged. The s5m8767 shares most of the features with s2mps11 and adds
some new stuff (i.e. GPIO control over BUCK DVS, low power mode for
opmode). Anyway it was easier to add support for S2MPS14 to S2MPS11 than
to merge everything now.


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

* Re: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst
  2014-02-13 19:07   ` [PATCH v2 " Mark Brown
@ 2014-02-14  7:46     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-14  7:46 UTC (permalink / raw)
  To: Mark Brown
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Yadwinder Singh Brar

On Thu, 2014-02-13 at 19:07 +0000, Mark Brown wrote:
> On Thu, Feb 13, 2014 at 10:14:00AM +0100, Krzysztof Kozlowski wrote:
> 
> > -	for (i = 0; i < S2MPS11_REGULATOR_CNT; i++)
> > +	s2mps11->rdev = devm_kzalloc(&pdev->dev,
> > +			sizeof(*s2mps11->rdev)*rdev_num, GFP_KERNEL);
> > +	if (!s2mps11->rdev)
> > +		return -ENOMEM;
> 
> If we're using managed allocations do we actually need to keep the rdev
> table at all?  We only normally use it to free.

You're right, the "s2mps11->rdev" is not needed at all.

> 
> > +	rdata = devm_kzalloc(&pdev->dev, sizeof(*rdata)*rdev_num, GFP_KERNEL);
> > +	if (!rdata)
> > +		return -ENOMEM;
> > +
> 
> > +	/* rdata was needed only for of_regulator_match() during probe */
> > +	if (rdata)
> > +		devm_kfree(&pdev->dev, rdata);
> > +
> 
> If this is always going to be freed within the probe path (in the same
> function indeed) why is it a managed allocaton at all?

Actually no good reason, simplifies a little the return statements on
error conditions. I'll use kzalloc() and kfree().

Best regards,
Krzysztof


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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-13 19:28   ` Mark Brown
@ 2014-02-14  8:15     ` Krzysztof Kozlowski
  2014-02-14 20:59       ` Mark Brown
  0 siblings, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-14  8:15 UTC (permalink / raw)
  To: Mark Brown
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood

On Thu, 2014-02-13 at 19:28 +0000, Mark Brown wrote:
> On Thu, Feb 13, 2014 at 10:14:04AM +0100, Krzysztof Kozlowski wrote:
> > S2MPS11/S2MPS14 regulators support different modes of operation:
> >  - Always off;
> >  - On/Off controlled by pin/GPIO (PWREN/LDOEN/EMMCEN);
> >  - Always on;
> > This is very similar to S5M8767 regulator driver which also supports
> > opmodes (although S5M8767 have also low-power mode).
> 
> What is the difference between always off/on and the normal enable
> control?  This sounds like a fairly standard register control that can
> be overridden by a GPIO.  The usual way of specifying that is by keying
> off the presence of the GPIO.

My initial idea was to do this similarly to the S5M8767 regulator (where
there is also 4th mode: low power). The presence of GPIO in DTS can
simplify the bindings but on the other hand it wouldn't be compatible
with S5M8767 regulator driver. This may complicate the merge of these
drivers.

What is your opinion on this - should I abandon the "op_mode" idea and
use presence of GPIO?

Best regards,
Krzysztof


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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-13 12:16   ` Yadwinder Singh Brar
@ 2014-02-14 13:05     ` Krzysztof Kozlowski
  2014-02-14 21:05       ` Mark Brown
  0 siblings, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-14 13:05 UTC (permalink / raw)
  To: Yadwinder Singh Brar
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Mark Brown,
	Liam Girdwood, Tomasz Figa



On Thu, 2014-02-13 at 17:46 +0530, Yadwinder Singh Brar wrote:
> Hi,
> 
> On Thu, Feb 13, 2014 at 2:44 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > S2MPS11/S2MPS14 regulators support different modes of operation:
> >  - Always off;
> >  - On/Off controlled by pin/GPIO (PWREN/LDOEN/EMMCEN);
> >  - Always on;
> > This is very similar to S5M8767 regulator driver which also supports
> > opmodes (although S5M8767 have also low-power mode).
> >
> > This patch adds parsing the operation mode from DTS by reading a
> > "op_mode" property from regulator child node.
> >
> 
> First thing since "op_mode" is not generic property, I think it should
> be appended with some driver specific prefix.
> 
> But IMHO its quite generic property used and required by many other
> PMICs(almost all used by Samsung).
> I would like to use this opportunity to discuss about adding it as
> generic regulator constraint(as initial_mode)
> by providing a default mapping of generic Regulator operating
> modes(kernel specific) to operating modes supported by hardware in
> regulator driver itself.
> 
> Regards,
> Yadwinder

Hi,

I was thinking about this. This relates also to ideas pointed by Mark:
 - Maybe s2mps11 and s5m8767 regulator drivers could be merged into one;
 - The external control should be determined by presence of attribute
with gpios.

The S5M8767 has following operation modes (except on/off):
 - external control by GPIO;
 - On/Off controlled by PWREN;
 - low-power mode;
 - low-power mode controlled by PWREN;
Although not all are present for each regulator.

The S2MPS14 is easier:
 - external control by GPIO;
 - On/Off controlled by PWREN;

A generic solution for operating mode of regulators (not only s2mps11
and s5m8767) could cover all of these above or just a subset, for
example regulator bindings could look like:
 - regulator-mode-suspend; /* PWR controls: on/off or low-power mode */
 - regulator-mode-low-power; /* Low power mode */


What do you think?

Best regards,
Krzysztof


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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-14  8:15     ` Krzysztof Kozlowski
@ 2014-02-14 20:59       ` Mark Brown
  2014-02-17  8:09         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 43+ messages in thread
From: Mark Brown @ 2014-02-14 20:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood

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

On Fri, Feb 14, 2014 at 09:15:12AM +0100, Krzysztof Kozlowski wrote:

> My initial idea was to do this similarly to the S5M8767 regulator (where
> there is also 4th mode: low power). The presence of GPIO in DTS can
> simplify the bindings but on the other hand it wouldn't be compatible
> with S5M8767 regulator driver. This may complicate the merge of these
> drivers.

They can always use separate ops.

> What is your opinion on this - should I abandon the "op_mode" idea and
> use presence of GPIO?

Yes, that's better - especially since the framework has support for
enable GPIOs.  It can do things like handle cases where the hardware has
tied the enables for several regulators together so they all need to be
enabled and disabled as one.

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

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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-14 13:05     ` Krzysztof Kozlowski
@ 2014-02-14 21:05       ` Mark Brown
  2014-02-17  8:07         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 43+ messages in thread
From: Mark Brown @ 2014-02-14 21:05 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

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

On Fri, Feb 14, 2014 at 02:05:56PM +0100, Krzysztof Kozlowski wrote:
> On Thu, 2014-02-13 at 17:46 +0530, Yadwinder Singh Brar wrote:

>  - low-power mode;
>  - low-power mode controlled by PWREN;
> Although not all are present for each regulator.

What exactly is low power mode and how does it interact with the enable?
The name suggests it's a more efficient mode for use with low current
drain, is that right?

> A generic solution for operating mode of regulators (not only s2mps11
> and s5m8767) could cover all of these above or just a subset, for
> example regulator bindings could look like:
>  - regulator-mode-suspend; /* PWR controls: on/off or low-power mode */
>  - regulator-mode-low-power; /* Low power mode */

Those properties have awfully generic names and part of what I was
wondering above is if the low power mode maps onto the idle or standby
modes that the API defines?  We don't cover those in the bindings yet
since they are unfortunately fuzzy but perhaps we need to do so.

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

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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-14 21:05       ` Mark Brown
@ 2014-02-17  8:07         ` Krzysztof Kozlowski
  2014-02-18  0:35           ` Mark Brown
  0 siblings, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-17  8:07 UTC (permalink / raw)
  To: Mark Brown
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

On Fri, 2014-02-14 at 21:05 +0000, Mark Brown wrote:
> On Fri, Feb 14, 2014 at 02:05:56PM +0100, Krzysztof Kozlowski wrote:
> > On Thu, 2014-02-13 at 17:46 +0530, Yadwinder Singh Brar wrote:
> 
> >  - low-power mode;
> >  - low-power mode controlled by PWREN;
> > Although not all are present for each regulator.
> 
> What exactly is low power mode and how does it interact with the enable?
> The name suggests it's a more efficient mode for use with low current
> drain, is that right?

Yes, it is (i.e. ground current on battery load typically reduced from
8mA to 1.5mA). In this mode for Buck5 and LDO-s the regulator is enabled
but output current is limited to 5 mA.

> 
> > A generic solution for operating mode of regulators (not only s2mps11
> > and s5m8767) could cover all of these above or just a subset, for
> > example regulator bindings could look like:
> >  - regulator-mode-suspend; /* PWR controls: on/off or low-power mode */
> >  - regulator-mode-low-power; /* Low power mode */
> 
> Those properties have awfully generic names and part of what I was
> wondering above is if the low power mode maps onto the idle or standby
> modes that the API defines?  We don't cover those in the bindings yet
> since they are unfortunately fuzzy but perhaps we need to do so.

The low power maps to REGULATOR_MODE_IDLE or REGULATOR_MODE_STANDBY
(depending on the understanding of "more efficient" and "most efficient"
for light loads). However the suspend mode of S5M8767/S2MPS14 is more
like automatic regulator disable by SoC.

Best regards,
Krzysztof



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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-14 20:59       ` Mark Brown
@ 2014-02-17  8:09         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-17  8:09 UTC (permalink / raw)
  To: Mark Brown
  Cc: Sangbeom Kim, Samuel Ortiz, Lee Jones, linux-kernel,
	linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood

On Fri, 2014-02-14 at 20:59 +0000, Mark Brown wrote:
> On Fri, Feb 14, 2014 at 09:15:12AM +0100, Krzysztof Kozlowski wrote:
> 
> > My initial idea was to do this similarly to the S5M8767 regulator (where
> > there is also 4th mode: low power). The presence of GPIO in DTS can
> > simplify the bindings but on the other hand it wouldn't be compatible
> > with S5M8767 regulator driver. This may complicate the merge of these
> > drivers.
> 
> They can always use separate ops.
> 
> > What is your opinion on this - should I abandon the "op_mode" idea and
> > use presence of GPIO?
> 
> Yes, that's better - especially since the framework has support for
> enable GPIOs.  It can do things like handle cases where the hardware has
> tied the enables for several regulators together so they all need to be
> enabled and disabled as one.

OK. I'll rewrite this patch and send later when S2MPS14 will be
accepted.

Best regards,
Krzysztof


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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-17  8:07         ` Krzysztof Kozlowski
@ 2014-02-18  0:35           ` Mark Brown
  2014-02-18  8:12             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 43+ messages in thread
From: Mark Brown @ 2014-02-18  0:35 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

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

On Mon, Feb 17, 2014 at 09:07:34AM +0100, Krzysztof Kozlowski wrote:

> The low power maps to REGULATOR_MODE_IDLE or REGULATOR_MODE_STANDBY
> (depending on the understanding of "more efficient" and "most efficient"
> for light loads). However the suspend mode of S5M8767/S2MPS14 is more
> like automatic regulator disable by SoC.

I don't understand the above?  Are you saying that suspend mode actually
turns off the regulator or something else?  If it's a separate setting
for suspend mode then it should be using the core suspend mode stuff.

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

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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-18  0:35           ` Mark Brown
@ 2014-02-18  8:12             ` Krzysztof Kozlowski
  2014-02-19  4:08               ` Mark Brown
  0 siblings, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-18  8:12 UTC (permalink / raw)
  To: Mark Brown
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

On Tue, 2014-02-18 at 09:35 +0900, Mark Brown wrote:
> On Mon, Feb 17, 2014 at 09:07:34AM +0100, Krzysztof Kozlowski wrote:
> 
> > The low power maps to REGULATOR_MODE_IDLE or REGULATOR_MODE_STANDBY
> > (depending on the understanding of "more efficient" and "most efficient"
> > for light loads). However the suspend mode of S5M8767/S2MPS14 is more
> > like automatic regulator disable by SoC.
> 
> I don't understand the above?  Are you saying that suspend mode actually
> turns off the regulator or something else? If it's a separate setting
> for suspend mode then it should be using the core suspend mode stuff.

No, it is similar to external control (by GPIO) except that regulator is
controlled by PWREN pin. The PMIC's PWREN is not a GPIO, but instead it
is directly connected to AP (for Exynos 4212: XPWRRGTON). In AP's normal
mode the XPWRRGTON/PWREN is high. In sleep mode *AP* sets it low.

Best regards,
Krzysztof




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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-18  8:12             ` Krzysztof Kozlowski
@ 2014-02-19  4:08               ` Mark Brown
  2014-02-19 10:09                 ` Krzysztof Kozlowski
  2014-02-19 14:19                 ` Krzysztof Kozlowski
  0 siblings, 2 replies; 43+ messages in thread
From: Mark Brown @ 2014-02-19  4:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

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

On Tue, Feb 18, 2014 at 09:12:09AM +0100, Krzysztof Kozlowski wrote:
> On Tue, 2014-02-18 at 09:35 +0900, Mark Brown wrote:

> > I don't understand the above?  Are you saying that suspend mode actually
> > turns off the regulator or something else? If it's a separate setting
> > for suspend mode then it should be using the core suspend mode stuff.

> No, it is similar to external control (by GPIO) except that regulator is
> controlled by PWREN pin. The PMIC's PWREN is not a GPIO, but instead it
> is directly connected to AP (for Exynos 4212: XPWRRGTON). In AP's normal
> mode the XPWRRGTON/PWREN is high. In sleep mode *AP* sets it low.

How is that different to suspend mode then?

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

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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-19  4:08               ` Mark Brown
@ 2014-02-19 10:09                 ` Krzysztof Kozlowski
  2014-02-19 12:16                   ` Mark Brown
  2014-02-19 14:19                 ` Krzysztof Kozlowski
  1 sibling, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-19 10:09 UTC (permalink / raw)
  To: Mark Brown
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

On Wed, 2014-02-19 at 13:08 +0900, Mark Brown wrote:
> On Tue, Feb 18, 2014 at 09:12:09AM +0100, Krzysztof Kozlowski wrote:
> > On Tue, 2014-02-18 at 09:35 +0900, Mark Brown wrote:
> 
> > > I don't understand the above?  Are you saying that suspend mode actually
> > > turns off the regulator or something else? If it's a separate setting
> > > for suspend mode then it should be using the core suspend mode stuff.
> 
> > No, it is similar to external control (by GPIO) except that regulator is
> > controlled by PWREN pin. The PMIC's PWREN is not a GPIO, but instead it
> > is directly connected to AP (for Exynos 4212: XPWRRGTON). In AP's normal
> > mode the XPWRRGTON/PWREN is high. In sleep mode *AP* sets it low.
> 
> How is that different to suspend mode then?

Now I see... there is no difference. It seems that the whole idea of
opmode can be replaced with suspend modes and regulator modes.

I can't only find a way to set this from DTS. There are no bindings for
regulation_constraints->state_{disk,mem,standby}.

Should the driver set manually after obtaining init_data from DTS?

Best regards,
Krzysztof




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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-19 10:09                 ` Krzysztof Kozlowski
@ 2014-02-19 12:16                   ` Mark Brown
  0 siblings, 0 replies; 43+ messages in thread
From: Mark Brown @ 2014-02-19 12:16 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

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

On Wed, Feb 19, 2014 at 11:09:40AM +0100, Krzysztof Kozlowski wrote:

> I can't only find a way to set this from DTS. There are no bindings for
> regulation_constraints->state_{disk,mem,standby}.

> Should the driver set manually after obtaining init_data from DTS?

Someone should work out a suitably abstract way of defining what these
things mean and create bindings - that's not happened yet.  It's not
clear that what's in the code at the minute (which is a very direct
mapping onto Linux stuff) are general things that would apply well to
other OSs and there's not been much demand for this in general,
especially given the tendency for system designs to become more dynamic
and use case driven (or for these things to not be configurable by
software at all making the whole thing moot).

Something that defined a single suspend mode configuration suitable for
all modes would be a bit easier I think, and/or a good work through of
how things are part of the hardware rather than software configuration.

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

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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-19  4:08               ` Mark Brown
  2014-02-19 10:09                 ` Krzysztof Kozlowski
@ 2014-02-19 14:19                 ` Krzysztof Kozlowski
  2014-02-19 15:07                   ` Mark Brown
  1 sibling, 1 reply; 43+ messages in thread
From: Krzysztof Kozlowski @ 2014-02-19 14:19 UTC (permalink / raw)
  To: Mark Brown
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

On Wed, 2014-02-19 at 13:08 +0900, Mark Brown wrote:
> On Tue, Feb 18, 2014 at 09:12:09AM +0100, Krzysztof Kozlowski wrote:
> > On Tue, 2014-02-18 at 09:35 +0900, Mark Brown wrote:
> 
> > > I don't understand the above?  Are you saying that suspend mode actually
> > > turns off the regulator or something else? If it's a separate setting
> > > for suspend mode then it should be using the core suspend mode stuff.
> 
> > No, it is similar to external control (by GPIO) except that regulator is
> > controlled by PWREN pin. The PMIC's PWREN is not a GPIO, but instead it
> > is directly connected to AP (for Exynos 4212: XPWRRGTON). In AP's normal
> > mode the XPWRRGTON/PWREN is high. In sleep mode *AP* sets it low.
> 
> How is that different to suspend mode then?

I found two differences: performance (no need to send I2C commands) and
possible issues during resume.

Example: regulator which should be disabled during suspend to memory and
enabled for normal system operation.

As I understand the suspend mode (correct me if I'm wrong), the
regulator core during suspend to mem:
1. Calls suspend_set_state().
2. rstate->disabled is true so the ops->set_suspend_disable() is called.
3. The ops->set_suspend_disable() function (implemented by the driver)
disables the regulator (e.g. through I2C commands /regmap/).
4. During resume the regulator is enabled normal way (ops->enable(), I2C
again).

Possible problems:
A. What happens if some driver using this regulator resumes earlier then
regulator_suspend_finish()?
B. What happens if resuming regulator requires some other driver to be
resumed earlier (e.g. I2C bus)? If regulator resumes before I2C bus then
calling ops->enable() would fail.


The S5M8767 suspend mode (controlled by PWREN pin) works differently:
1. To enable regulator set regulator mode to "On/Off controller by
PWREN". During normal mode it will be enabled.
2. During suspend the regulator won't be suspended or disabled by
regulator core but instead the SoC will set PWREN to low and PMIC will
disable the regulator. No need to send I2C commands.
3. During resume the PWREN will be set to high and PMIC will enable the
regulator before resuming other drivers. No need to send I2C commands as
well.


Best regards,
Krzysztof



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

* Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators
  2014-02-19 14:19                 ` Krzysztof Kozlowski
@ 2014-02-19 15:07                   ` Mark Brown
  0 siblings, 0 replies; 43+ messages in thread
From: Mark Brown @ 2014-02-19 15:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Yadwinder Singh Brar, Sangbeom Kim, Samuel Ortiz, Lee Jones,
	linux-kernel, linux-samsung-soc, Kyungmin Park, Marek Szyprowski,
	Bartlomiej Zolnierkiewicz, Chanwoo Choi, Liam Girdwood,
	Tomasz Figa

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

On Wed, Feb 19, 2014 at 03:19:00PM +0100, Krzysztof Kozlowski wrote:

> As I understand the suspend mode (correct me if I'm wrong), the
> regulator core during suspend to mem:
> 1. Calls suspend_set_state().
> 2. rstate->disabled is true so the ops->set_suspend_disable() is called.
> 3. The ops->set_suspend_disable() function (implemented by the driver)
> disables the regulator (e.g. through I2C commands /regmap/).
> 4. During resume the regulator is enabled normal way (ops->enable(), I2C
> again).

No, set_suspend_disable() should *not* disable the regulator, it should
configure what the regulator will do when the hardware enters suspend
mode.  If we were just disabling the regulator there would be no point
in having a special operation, we could just use the normal operation.

> Possible problems:
> A. What happens if some driver using this regulator resumes earlier then
> regulator_suspend_finish()?

Nothing, if the system is not in suspend mode then configuring suspend
mode will have no impact.

> B. What happens if resuming regulator requires some other driver to be
> resumed earlier (e.g. I2C bus)? If regulator resumes before I2C bus then
> calling ops->enable() would fail.

It is expected that as part of exiting suspend mode the hardware will
revert to normal operational mode.

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

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

end of thread, other threads:[~2014-02-19 15:07 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-13  9:13 [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14 Krzysztof Kozlowski
2014-02-13  9:13 ` [PATCH v2 01/14] mfd: sec: Add maximum RTC register for regmap config Krzysztof Kozlowski
2014-02-13  9:13 ` [PATCH v2 02/14] mfd: sec: Select different RTC regmaps for devices Krzysztof Kozlowski
2014-02-13  9:13 ` [PATCH v2 03/14] mfd/rtc: sec/s5m: Rename SEC* symbols to S5M Krzysztof Kozlowski
2014-02-13 10:11   ` Lee Jones
2014-02-13  9:13 ` [PATCH v2 04/14] rtc: s5m: Remove undocumented time init on first boot Krzysztof Kozlowski
2014-02-13  9:13 ` [PATCH v2 05/14] mfd: sec: Use consistent S2MPS11 RTC alarm interrupt indexes Krzysztof Kozlowski
2014-02-13  9:13 ` [PATCH v2 06/14] regulator: s2mps11: Constify regulator_desc array Krzysztof Kozlowski
2014-02-13  9:14 ` [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst Krzysztof Kozlowski
2014-02-13 12:21   ` Yadwinder Singh Brar
2014-02-13 12:35     ` Krzysztof Kozlowski
2014-02-13 12:37     ` [PATCH " Krzysztof Kozlowski
2014-02-13 18:05       ` Mark Brown
2014-02-13 19:07   ` [PATCH v2 " Mark Brown
2014-02-14  7:46     ` Krzysztof Kozlowski
2014-02-13  9:14 ` [PATCH v2 08/14] mfd: sec: Add support for S2MPS14 Krzysztof Kozlowski
2014-02-13  9:14 ` [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators Krzysztof Kozlowski
2014-02-13 12:24   ` Yadwinder Singh Brar
2014-02-13 19:10   ` Mark Brown
2014-02-14  7:33     ` Krzysztof Kozlowski
2014-02-13  9:14 ` [PATCH v2 10/14] Documentation: mfd: s2mps11: Document support for S2MPS14 Krzysztof Kozlowski
2014-02-13 14:55   ` Tomasz Figa
2014-02-13  9:14 ` [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators Krzysztof Kozlowski
2014-02-13 12:16   ` Yadwinder Singh Brar
2014-02-14 13:05     ` Krzysztof Kozlowski
2014-02-14 21:05       ` Mark Brown
2014-02-17  8:07         ` Krzysztof Kozlowski
2014-02-18  0:35           ` Mark Brown
2014-02-18  8:12             ` Krzysztof Kozlowski
2014-02-19  4:08               ` Mark Brown
2014-02-19 10:09                 ` Krzysztof Kozlowski
2014-02-19 12:16                   ` Mark Brown
2014-02-19 14:19                 ` Krzysztof Kozlowski
2014-02-19 15:07                   ` Mark Brown
2014-02-13 12:43   ` Lee Jones
2014-02-13 13:00     ` Krzysztof Kozlowski
2014-02-13 19:28   ` Mark Brown
2014-02-14  8:15     ` Krzysztof Kozlowski
2014-02-14 20:59       ` Mark Brown
2014-02-17  8:09         ` Krzysztof Kozlowski
2014-02-13  9:14 ` [PATCH v2 12/14] Documentation: mfd/regulator: s2mps11: Document the "op_mode" bindings Krzysztof Kozlowski
2014-02-13  9:14 ` [PATCH v2 13/14] rtc: s5m: Support different register layout Krzysztof Kozlowski
2014-02-13  9:14 ` [PATCH v2 14/14] rtc: s5m: Add support for S2MPS14 RTC Krzysztof Kozlowski

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.