2014-02-13 09:14:35

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 00/14] mfd/regulator/rtc: sec: Add support for S2MPS14

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


2014-02-13 09:14:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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 <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Liam Girdwood <[email protected]>
---
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

2014-02-13 09:14:54

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 10/14] Documentation: mfd: s2mps11: Document support for S2MPS14

Add bindings documentation for S2MPS14 device to the s2mps11 driver.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Tomasz Figa <[email protected]>
Cc: [email protected]
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
---
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

2014-02-13 09:14:52

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 06/14] regulator: s2mps11: Constify regulator_desc array

Constify the regulator_desc 'regulators' array.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Liam Girdwood <[email protected]>
---
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

2014-02-13 09:14:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst

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 <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Yadwinder Singh Brar <[email protected]>
---
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

2014-02-13 09:16:30

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 14/14] rtc: s5m: Add support for S2MPS14 RTC

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 <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: [email protected]
---
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 <[email protected]>");
-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

2014-02-13 09:14:46

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators

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 <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Liam Girdwood <[email protected]>
---
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 <[email protected]>");
-MODULE_DESCRIPTION("SAMSUNG S2MPS11 Regulator Driver");
+MODULE_DESCRIPTION("SAMSUNG S2MPS11/S2MPS14 Regulator Driver");
MODULE_LICENSE("GPL");
--
1.7.9.5

2014-02-13 09:17:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 12/14] Documentation: mfd/regulator: s2mps11: Document the "op_mode" bindings

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 <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Tomasz Figa <[email protected]>
Cc: [email protected]
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
---
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

2014-02-13 09:17:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 13/14] rtc: s5m: Support different register layout

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 <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: [email protected]
---
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

2014-02-13 09:14:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 01/14] mfd: sec: Add maximum RTC register for regmap config

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

Signed-off-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Lee Jones <[email protected]>
---
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

2014-02-13 09:18:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 04/14] rtc: s5m: Remove undocumented time init on first boot

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 <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: [email protected]
---
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

2014-02-13 09:19:04

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 03/14] mfd/rtc: sec/s5m: Rename SEC* symbols to S5M

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 <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: [email protected]
---
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

2014-02-13 09:19:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 02/14] mfd: sec: Select different RTC regmaps for devices

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

Signed-off-by: Krzysztof Kozlowski <[email protected]>
Acked-by: Lee Jones <[email protected]>
---
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

2014-02-13 09:18:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 08/14] mfd: sec: Add support for S2MPS14

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 <[email protected]>
Acked-by: Lee Jones <[email protected]>
---
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

2014-02-13 09:18:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH v2 05/14] mfd: sec: Use consistent S2MPS11 RTC alarm interrupt indexes

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 <[email protected]>
Acked-by: Lee Jones <[email protected]>
---
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

2014-02-13 10:11:25

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v2 03/14] mfd/rtc: sec/s5m: Rename SEC* symbols to S5M

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 <[email protected]>
> Cc: Alessandro Zummo <[email protected]>
> Cc: [email protected]

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

2014-02-13 12:16:18

by Yadwinder Singh Brar

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

Hi,

On Thu, Feb 13, 2014 at 2:44 PM, Krzysztof Kozlowski
<[email protected]> 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

2014-02-13 12:21:40

by Yadwinder Singh Brar

[permalink] [raw]
Subject: Re: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst

Hi,

On Thu, Feb 13, 2014 at 2:44 PM, Krzysztof Kozlowski
<[email protected]> 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 <[email protected]>
> Signed-off-by: Chanwoo Choi <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: Liam Girdwood <[email protected]>
> Cc: Yadwinder Singh Brar <[email protected]>
> ---

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 <[email protected]>


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
>

2014-02-13 12:24:15

by Yadwinder Singh Brar

[permalink] [raw]
Subject: Re: [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators

On Thu, Feb 13, 2014 at 2:44 PM, Krzysztof Kozlowski
<[email protected]> 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 <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: Liam Girdwood <[email protected]>
> ---

Reviewed-by: Yadwinder Singh Brar <[email protected]>

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 <[email protected]>");
> -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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-02-13 12:35:30

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst

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
> <[email protected]> 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 <[email protected]>
> > Signed-off-by: Chanwoo Choi <[email protected]>
> > Cc: Mark Brown <[email protected]>
> > Cc: Liam Girdwood <[email protected]>
> > Cc: Yadwinder Singh Brar <[email protected]>
> > ---
>
> 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 <[email protected]>
>

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

Best regards,
Krzysztof


2014-02-13 12:37:24

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 07/14] regulator: s2mps11: Copy supported regulators from initconst

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 <[email protected]>
Signed-off-by: Chanwoo Choi <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Liam Girdwood <[email protected]>
Cc: Yadwinder Singh Brar <[email protected]>
Reviewed-by: Yadwinder Singh Brar <[email protected]>
---
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

2014-02-13 12:43:30

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

> 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 <[email protected]>
> Signed-off-by: Chanwoo Choi <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: Liam Girdwood <[email protected]>
> ---
> 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

2014-02-13 13:00:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators



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 <[email protected]>
> > Signed-off-by: Chanwoo Choi <[email protected]>
> > Cc: Mark Brown <[email protected]>
> > Cc: Liam Girdwood <[email protected]>
> > ---
> > 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 */
>

2014-02-13 14:55:16

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH v2 10/14] Documentation: mfd: s2mps11: Document support for S2MPS14

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 <[email protected]>
> Cc: Mark Brown <[email protected]>
> Cc: Liam Girdwood <[email protected]>
> Cc: Tomasz Figa <[email protected]>
> Cc: [email protected]
> Cc: Rob Herring <[email protected]>
> Cc: Pawel Moll <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Ian Campbell <[email protected]>
> Cc: Kumar Gala <[email protected]>
> ---
> 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 <[email protected]>

Best regards,
Tomasz

2014-02-13 18:25:58

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 07/14] regulator: s2mps11: Copy supported regulators from initconst

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.


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

2014-02-13 19:28:47

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators

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.


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

2014-02-13 19:28:44

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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.


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

2014-02-13 19:29:27

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst

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?


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

2014-02-14 07:33:46

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 09/14] regulator: s2mps11: Add support for S2MPS14 regulators

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.

2014-02-14 07:47:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 07/14] regulator: s2mps11: Copy supported regulators from initconst

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

2014-02-14 08:15:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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

2014-02-14 13:06:02

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators



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
> <[email protected]> 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

2014-02-15 10:09:45

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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.


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

2014-02-15 10:10:03

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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.


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

2014-02-17 08:07:41

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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

2014-02-17 08:09:52

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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

2014-02-18 00:36:36

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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.


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

2014-02-18 08:12:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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


2014-02-19 04:27:12

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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?


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

2014-02-19 10:09:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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


2014-02-19 13:06:00

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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.


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

2014-02-19 14:19:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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

2014-02-19 15:07:59

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 11/14] regulator: s2mps11: Add opmode for S2MPS14 regulators

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.


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