regulator: Support AP watchdog reset operation for s5m8767a

PATCH Applied Sangbeom Kim 2 participants 5 messages Jun 13, 2012 – Jun 17, 2012

Subsystems (3)

Multifunction devices (MFD) Voltage and current regulator framework Samsung multifunction PMIC device drivers

lore

Share & tools

lore

Sangbeom Kim

author

2012-06-13 08:28 UTC #0 lore raw

PATCH 5/5 regulator: Support AP watchdog reset operation for s5m8767a

After AP watchdog reset, S5M8767A should be reset too. If AP is set on lowest voltage, AP can't execute boot sequence. This patch support watchdog reset of s5m8767.

Signed-off-by Sangbeom Kim <[email protected]>

---
 drivers/regulator/s5m8767.c          |  137 +++++++++++++++++++++++-----------
 include/linux/mfd/s5m87xx/s5m-core.h |    5 +
 2 files changed, 98 insertions(+), 44 deletions(-)

drivers/regulator/s5m8767.c +93 ~~-44~~

index 770e4df..533bde6 100644
@@ -41,6 +41,7 @@ struct s5m8767_info {
 	u8 buck3_vol[8];
 	u8 buck4_vol[8];
 	int buck_gpios[3];
+	int buck_ds[3];
 	int buck_gpioindex;
 };

@@ -283,17 +284,17 @@ static int s5m8767_get_voltage_register(struct regulator_dev *rdev, int *_reg)
 		reg = S5M8767_REG_BUCK1CTRL2;
 		break;
 	case S5M8767_BUCK2:
-		reg = S5M8767_REG_BUCK2DVS1;
+		reg = S5M8767_REG_BUCK2DVS2;
 		if (s5m8767->buck2_gpiodvs)
 			reg += s5m8767->buck_gpioindex;
 		break;
 	case S5M8767_BUCK3:
-		reg = S5M8767_REG_BUCK3DVS1;
+		reg = S5M8767_REG_BUCK3DVS2;
 		if (s5m8767->buck3_gpiodvs)
 			reg += s5m8767->buck_gpioindex;
 		break;
 	case S5M8767_BUCK4:
-		reg = S5M8767_REG_BUCK4DVS1;
+		reg = S5M8767_REG_BUCK4DVS2;
 		if (s5m8767->buck4_gpiodvs)
 			reg += s5m8767->buck_gpioindex;
 		break;

@@ -512,7 +513,7 @@ static __devinit int s5m8767_pmic_probe(struct platform_device *pdev)
 	struct regulator_config config = { };
 	struct regulator_dev **rdev;
 	struct s5m8767_info *s5m8767;
-	int i, ret, size;
+	int i, ret, size, buck_init;
 
 	if (!pdata) {
 		dev_err(pdev->dev.parent, "Platform data not supplied\n");

@@ -563,12 +564,37 @@ static __devinit int s5m8767_pmic_probe(struct platform_device *pdev)
 	s5m8767->buck_gpios[0] = pdata->buck_gpios[0];
 	s5m8767->buck_gpios[1] = pdata->buck_gpios[1];
 	s5m8767->buck_gpios[2] = pdata->buck_gpios[2];
+	s5m8767->buck_ds[0] = pdata->buck_ds[0];
+	s5m8767->buck_ds[1] = pdata->buck_ds[1];
+	s5m8767->buck_ds[2] = pdata->buck_ds[2];
+
 	s5m8767->ramp_delay = pdata->buck_ramp_delay;
 	s5m8767->buck2_ramp = pdata->buck2_ramp_enable;
 	s5m8767->buck3_ramp = pdata->buck3_ramp_enable;
 	s5m8767->buck4_ramp = pdata->buck4_ramp_enable;
 	s5m8767->opmode = pdata->opmode;
 
+	buck_init = s5m8767_convert_voltage_to_sel(&buck_voltage_val2,
+						pdata->buck2_init,
+						pdata->buck2_init +
+						buck_voltage_val2.step);
+
+	s5m_reg_write(s5m8767->iodev, S5M8767_REG_BUCK2DVS2, buck_init);
+
+	buck_init = s5m8767_convert_voltage_to_sel(&buck_voltage_val2,
+						pdata->buck3_init,
+						pdata->buck3_init +
+						buck_voltage_val2.step);
+
+	s5m_reg_write(s5m8767->iodev, S5M8767_REG_BUCK3DVS2, buck_init);
+
+	buck_init = s5m8767_convert_voltage_to_sel(&buck_voltage_val2,
+						pdata->buck4_init,
+						pdata->buck4_init +
+						buck_voltage_val2.step);
+
+	s5m_reg_write(s5m8767->iodev, S5M8767_REG_BUCK4DVS2, buck_init);
+
 	for (i = 0; i < 8; i++) {
 		if (s5m8767->buck2_gpiodvs) {
 			s5m8767->buck2_vol[i] =

@@ -598,48 +624,71 @@ static __devinit int s5m8767_pmic_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (pdata->buck2_gpiodvs || pdata->buck3_gpiodvs ||
-		pdata->buck4_gpiodvs) {
-		if (gpio_is_valid(pdata->buck_gpios[0]) &&
-			gpio_is_valid(pdata->buck_gpios[1]) &&
-			gpio_is_valid(pdata->buck_gpios[2])) {
-			ret = gpio_request(pdata->buck_gpios[0],
-						"S5M8767 SET1");
-			if (ret == -EBUSY)
-				dev_warn(&pdev->dev, "Duplicated gpio request for SET1\n");
-
-			ret = gpio_request(pdata->buck_gpios[1],
-					   "S5M8767 SET2");
-			if (ret == -EBUSY)
-				dev_warn(&pdev->dev, "Duplicated gpio request for SET2\n");
-
-			ret = gpio_request(pdata->buck_gpios[2],
-					   "S5M8767 SET3");
-			if (ret == -EBUSY)
-				dev_warn(&pdev->dev, "Duplicated gpio request for SET3\n");
-			/* SET1 GPIO */
-			gpio_direction_output(pdata->buck_gpios[0],
-					(s5m8767->buck_gpioindex >> 2) & 0x1);
-			/* SET2 GPIO */
-			gpio_direction_output(pdata->buck_gpios[1],
-					(s5m8767->buck_gpioindex >> 1) & 0x1);
-			/* SET3 GPIO */
-			gpio_direction_output(pdata->buck_gpios[2],
-					(s5m8767->buck_gpioindex >> 0) & 0x1);
-			ret = 0;
-		} else {
-			dev_err(&pdev->dev, "GPIO NOT VALID\n");
-			ret = -EINVAL;
-			return ret;
-		}
+	if (gpio_is_valid(pdata->buck_gpios[0]) &&
+		gpio_is_valid(pdata->buck_gpios[1]) &&
+		gpio_is_valid(pdata->buck_gpios[2])) {
+		ret = gpio_request(pdata->buck_gpios[0], "S5M8767 SET1");
+		if (ret == -EBUSY)
+			dev_warn(&pdev->dev, "Duplicated gpio request"
+				" for SET1\n");
+
+		ret = gpio_request(pdata->buck_gpios[1], "S5M8767 SET2");
+		if (ret == -EBUSY)
+			dev_warn(&pdev->dev, "Duplicated gpio request"
+				" for SET2\n");
+
+		ret = gpio_request(pdata->buck_gpios[2], "S5M8767 SET3");
+		if (ret == -EBUSY)
+			dev_warn(&pdev->dev, "Duplicated gpio request"
+					" for SET3\n");
+		/* SET1 GPIO */
+		gpio_direction_output(pdata->buck_gpios[0],
+				(s5m8767->buck_gpioindex >> 2) & 0x1);
+		/* SET2 GPIO */
+		gpio_direction_output(pdata->buck_gpios[1],
+				(s5m8767->buck_gpioindex >> 1) & 0x1);
+		/* SET3 GPIO */
+		gpio_direction_output(pdata->buck_gpios[2],
+				(s5m8767->buck_gpioindex >> 0) & 0x1);
+		ret = 0;
+
+	} else {
+		dev_err(&pdev->dev, "GPIO NOT VALID\n");
+		ret = -EINVAL;
+		return ret;
 	}
 
-	s5m_reg_update(s5m8767->iodev, S5M8767_REG_BUCK2CTRL,
-			(pdata->buck2_gpiodvs) ? (1 << 1) : (0 << 1), 1 << 1);
-	s5m_reg_update(s5m8767->iodev, S5M8767_REG_BUCK3CTRL,
-			(pdata->buck3_gpiodvs) ? (1 << 1) : (0 << 1), 1 << 1);
-	s5m_reg_update(s5m8767->iodev, S5M8767_REG_BUCK4CTRL,
-			(pdata->buck4_gpiodvs) ? (1 << 1) : (0 << 1), 1 << 1);
+	ret = gpio_request(pdata->buck_ds[0], "S5M8767 DS2");
+	if (ret == -EBUSY)
+		dev_warn(&pdev->dev, "Duplicated gpio request for DS2\n");
+
+	ret = gpio_request(pdata->buck_ds[1], "S5M8767 DS3");
+	if (ret == -EBUSY)
+		dev_warn(&pdev->dev, "Duplicated gpio request for DS3\n");
+
+	ret = gpio_request(pdata->buck_ds[2], "S5M8767 DS4");
+	if (ret == -EBUSY)
+		dev_warn(&pdev->dev, "Duplicated gpio request for DS4\n");
+
+	/* DS2 GPIO */
+	gpio_direction_output(pdata->buck_ds[0], 0x0);
+	/* DS3 GPIO */
+	gpio_direction_output(pdata->buck_ds[1], 0x0);
+	/* DS4 GPIO */
+	gpio_direction_output(pdata->buck_ds[2], 0x0);
+
+	if (pdata->buck2_gpiodvs || pdata->buck3_gpiodvs ||
+	   pdata->buck4_gpiodvs) {
+		s5m_reg_update(s5m8767->iodev, S5M8767_REG_BUCK2CTRL,
+				(pdata->buck2_gpiodvs) ? (1 << 1) : (0 << 1),
+				1 << 1);
+		s5m_reg_update(s5m8767->iodev, S5M8767_REG_BUCK3CTRL,
+				(pdata->buck3_gpiodvs) ? (1 << 1) : (0 << 1),
+				1 << 1);
+		s5m_reg_update(s5m8767->iodev, S5M8767_REG_BUCK4CTRL,
+				(pdata->buck4_gpiodvs) ? (1 << 1) : (0 << 1),
+				1 << 1);
+	}
 
 	/* Initialize GPIO DVS registers */
 	for (i = 0; i < 8; i++) {

include/linux/mfd/s5m87xx/s5m-core.h +5 -0

index 21603b4..0b2e0ed 100644
@@ -347,6 +347,7 @@ struct s5m_platform_data {
 	bool				buck_voltage_lock;
 
 	int				buck_gpios[3];
+	int				buck_ds[3];
 	int				buck2_voltage[8];
 	bool				buck2_gpiodvs;
 	int				buck3_voltage[8];

@@ -369,6 +370,10 @@ struct s5m_platform_data {
 	bool                            buck2_ramp_enable;
 	bool                            buck3_ramp_enable;
 	bool                            buck4_ramp_enable;
+
+	int				buck2_init;
+	int				buck3_init;
+	int				buck4_init;
 };
 
 #endif /*  __LINUX_MFD_S5M_CORE_H */

Signature · 1 line

1.7.1

Mark Brown

2012-06-13 17:46 UTC #1 lore raw

Quoted 3 lines · Sangbeom Kim

After AP watchdog reset, S5M8767A should be reset too. If AP is set on lowest voltage, AP can't execute boot sequence. This patch support watchdog reset of s5m8767.

It's really hard to see how this patch addresses the issue above - could you explain in more detail, please?

Sangbeom Kim

author

2012-06-14 00:01 UTC #2 lore raw

Hi Mark,
I'm sorry for insufficient explanation.
Basically, PMIC can't know status of ap reset.
Problem can be happened like below.

- AP Bootable lowest arm voltage: 0.9v
- AP arm voltage table for DVFS: 0.8v, 0.9v, 1.0v
- During AP works on 0.8v, watchdog reset is asserted
- AP can't boot, because vdd arm is still 0.8v

Solution
- Basic concept: After ap watchdog reset, GPIO configuration is changed by default value
- S5M8767A has function of voltage control with gpio (8 levels with 3 gpios)
- Set bootable voltage on level 0 -> can work with default gpio configuration
- In the probing, Change voltage control level from level 0 to level 1
- Execute normal dvfs operation on level 1
- After watchdog reset, ap gpio is set by default value
- PMIC operation mode is changed by ap reset (level1 -> level0)
- AP can execute normal boot

If you have any question, email me again.
Thanks,
Sangbeom.

Quoted 14 lines

-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Thursday, June 14, 2012 2:46 AM
To: Sangbeom Kim
Cc: 'Liam Girdwood'; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 5/5] regulator: Support AP watchdog reset operation for s5m8767a
Quoted 3 lines · Sangbeom Kim
After AP watchdog reset, S5M8767A should be reset too. If AP is set on lowest voltage, AP can't execute boot sequence. This patch support watchdog reset of s5m8767.
It's really hard to see how this patch addresses the issue above - could you explain in more detail, please?

Mark Brown

2012-06-17 18:53 UTC #3 lore raw

Quoted 9 lines · Sangbeom Kim

Solution
- Basic concept: After ap watchdog reset, GPIO configuration is changed by default value
- S5M8767A has function of voltage control with gpio (8 levels with 3 gpios)
- Set bootable voltage on level 0 -> can work with default gpio configuration
- In the probing, Change voltage control level from level 0 to level 1
- Execute normal dvfs operation on level 1
- After watchdog reset, ap gpio is set by default value
- PMIC operation mode is changed by ap reset (level1 -> level0)
- AP can execute normal boot

OK, so basically what you're doing is fixing one of the GPIO selected voltage levels so that when the AP comes out of reset it'll drive the PMIC into a suitable level for it to boot? That makes sense - can you please resend with this in the changelog.

Sangbeom Kim

author

2012-06-17 23:56 UTC #4 lore raw

Quoted 13 lines · Mark Brown

Quoted 4 lines
Solution
- Basic concept: After ap watchdog reset, GPIO configuration is changed by default value
- S5M8767A has function of voltage control with gpio (8 levels with 3
Quoted 8 lines
gpios)
- Set bootable voltage on level 0 -> can work with default gpio configuration
- In the probing, Change voltage control level from level 0 to level 1
- Execute normal dvfs operation on level 1
- After watchdog reset, ap gpio is set by default value
- PMIC operation mode is changed by ap reset (level1 -> level0)
- AP can execute normal boot

Quoted 3 lines

OK, so basically what you're doing is fixing one of the GPIO selected voltage levels so that when the AP comes out of reset it'll drive the PMIC into a suitable level for it to boot? That makes sense

That's right.
The voltage of S5M8767A can be controlled by GPIO. So, I try to check the status of gpio.
I will resend above patch with adding detail commit message.

Thanks,
Sangbeom.