All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1
@ 2019-02-16  9:45 Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 1/9] adc: exynos-adc: Fix wrong bit operation used to stop the ADC Krzysztof Kozlowski
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

Hi,

Changes since v2
================
1. Add Lukasz review tags.
2. Patch 7: Return on error, as suggested by Simon.
2. Patch 3: Use IS_ENABLED() to run revision detection only
   once - either during late display board or misc_init_r.

Changes since v1
================
1. Move fixes to beginning of patchset.
2. Patch 3: Rework the idea - split revision detection to be executed later.
3. Patch 4: New patch.
4. Patch 6: Apply Simon's comments.
5. Patch 6: Do not delay when changing voltage if regulator is disabled.
6. Patch 6: Do not delay when disabling the regulator.


Description
===========
Odroid HC1 does not reboot properly (at least from SD card but
I do not expect difference on eMMC), if LDO4/VDD_ADC was turned
off by Linux kernel.  This condition happens so far always, because
Linux kernel did not enable ADC on Odroid HC1, therefore the
VDD_ADC regulator was turned off as unused.

The issue is in detection of revision which later is used to load
proper DTB.

The revision is obtained by ADC read of a voltage depending on VDD_ADC.
Therefore:
1. VDD_ADC has to be turned on (but board detection happens before
   power is initialized),
2. Turning VDD_ADC should wait with ramp delay,
3. Reading the value from ADC should wait for it to stabilize.

Tested on Odroid XU3-Lite and Odroid HC1.

Commends and testing are welcomed.

Best regards,
Krzysztof

Krzysztof Kozlowski (9):
  adc: exynos-adc: Fix wrong bit operation used to stop the ADC
  power: regulator: s2mps11: Fix step for LDO27 and LDO35
  arm: exynos: Detect revision later, when all resources are ready
  arm: exynos: odroid-xu3: Display info late to have proper type
  arm: exynos: Wait till ADC stabilizes before checking Odroid HC1
    revision
  regulator: Add support for ramp delay
  power: regulator: s2mps11: Add enable delay
  arm: dts: exynos: Add supply for ADC block to Odroid XU3 family
  arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid
    XU3 family

 arch/arm/dts/exynos5422-odroidxu3.dts         | 20 +++++++
 board/samsung/common/board.c                  | 24 ++++++++-
 board/samsung/common/exynos5-dt-types.c       | 54 +++++++++++++++++--
 board/samsung/odroid/odroid.c                 |  8 +++
 configs/odroid-xu3_defconfig                  |  2 +
 .../regulator/regulator.txt                   |  2 +
 drivers/adc/exynos-adc.c                      |  2 +-
 drivers/power/regulator/regulator-uclass.c    | 47 +++++++++++++++-
 drivers/power/regulator/s2mps11_regulator.c   | 15 +++++-
 include/power/regulator.h                     |  2 +
 include/samsung/misc.h                        |  1 +
 11 files changed, 167 insertions(+), 10 deletions(-)

-- 
2.17.1

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

* [U-Boot] [PATCH v3 1/9] adc: exynos-adc: Fix wrong bit operation used to stop the ADC
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 2/9] power: regulator: s2mps11: Fix step for LDO27 and LDO35 Krzysztof Kozlowski
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

When stopping the ADC_V2_CON1_STC_EN should be cleared.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/adc/exynos-adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/adc/exynos-adc.c b/drivers/adc/exynos-adc.c
index d33e3d632afc..12c49fc8cefb 100644
--- a/drivers/adc/exynos-adc.c
+++ b/drivers/adc/exynos-adc.c
@@ -62,7 +62,7 @@ int exynos_adc_stop(struct udevice *dev)
 
 	/* Stop conversion */
 	cfg = readl(&regs->con1);
-	cfg |= ~ADC_V2_CON1_STC_EN;
+	cfg &= ~ADC_V2_CON1_STC_EN;
 
 	writel(cfg, &regs->con1);
 
-- 
2.17.1

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

* [U-Boot] [PATCH v3 2/9] power: regulator: s2mps11: Fix step for LDO27 and LDO35
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 1/9] adc: exynos-adc: Fix wrong bit operation used to stop the ADC Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 3/9] arm: exynos: Detect revision later, when all resources are ready Krzysztof Kozlowski
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

LDO27 and LDO35 have 25 mV step, not 50 mV.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Lukasz Majewski <lukma@denx.de>
---
 drivers/power/regulator/s2mps11_regulator.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/power/regulator/s2mps11_regulator.c b/drivers/power/regulator/s2mps11_regulator.c
index ced504eb1476..723d27f67c9a 100644
--- a/drivers/power/regulator/s2mps11_regulator.c
+++ b/drivers/power/regulator/s2mps11_regulator.c
@@ -346,6 +346,8 @@ static int s2mps11_ldo_hex2volt(int ldo, int hex)
 	case 11:
 	case 22:
 	case 23:
+	case 27:
+	case 35:
 		uV = hex * S2MPS11_LDO_STEP + S2MPS11_LDO_UV_MIN;
 		break;
 	default:
@@ -366,6 +368,8 @@ static int s2mps11_ldo_volt2hex(int ldo, int uV)
 	case 11:
 	case 22:
 	case 23:
+	case 27:
+	case 35:
 		hex = (uV - S2MPS11_LDO_UV_MIN) / S2MPS11_LDO_STEP;
 		break;
 	default:
-- 
2.17.1

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

* [U-Boot] [PATCH v3 3/9] arm: exynos: Detect revision later, when all resources are ready
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 1/9] adc: exynos-adc: Fix wrong bit operation used to stop the ADC Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 2/9] power: regulator: s2mps11: Fix step for LDO27 and LDO35 Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-17 22:34   ` Lukasz Majewski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 4/9] arm: exynos: odroid-xu3: Display info late to have proper type Krzysztof Kozlowski
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

Detection of board revision is done early - before power setup.  In case of
Odroid XU3/XU4/HC1 family, the detection is done using ADC which
is supplied by LDO4/VDD_ADC regulator.  This regulator could be turned
off (e.g. by kernel before reboot).  If ADC is used early, the
regulators are not yet available and the detection won't work.

Split the revision detection out of set_board_type() into separate
function called later - either when displaying board info (in late mode)
or during misc_init_r.  The idea is that set_board_type() will be called
early so its method of detection are limited to flattened device tree
(exynos5-dt-types.c for Exynos5) or GPIO (odroid.c for Exynos4412).  The
newly added set_board_revision() can be called only later, when
resources like regulator are available.

This is necessary to fix the detection of Odroid HC1 after reboot, if
kernel turned off the LDO4 regulator.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 board/samsung/common/board.c            | 22 +++++++++++++++++++++-
 board/samsung/common/exynos5-dt-types.c | 16 +++++++++++++---
 board/samsung/odroid/odroid.c           |  8 ++++++++
 include/samsung/misc.h                  |  1 +
 4 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c
index 96228a86a117..ef2204742e1d 100644
--- a/board/samsung/common/board.c
+++ b/board/samsung/common/board.c
@@ -253,7 +253,18 @@ int board_eth_init(bd_t *bis)
 int checkboard(void)
 {
 	if (IS_ENABLED(CONFIG_BOARD_TYPES)) {
-		const char *board_info = get_board_type();
+		const char *board_info;
+
+		if (IS_ENABLED(CONFIG_DISPLAY_BOARDINFO_LATE)) {
+			/*
+			 * Printing type requires having revision, although
+			 * this will succeed only if done late.
+			 * Otherwise revision will be set in misc_init_r().
+			 */
+			set_board_revision();
+		}
+
+		board_info = get_board_type();
 
 		if (board_info)
 			printf("Type:  %s\n", board_info);
@@ -287,6 +298,15 @@ int board_late_init(void)
 #ifdef CONFIG_MISC_INIT_R
 int misc_init_r(void)
 {
+	if (!IS_ENABLED(CONFIG_DISPLAY_BOARDINFO_LATE)) {
+		/*
+		 * If revision was not set by late display boardinfo,
+		 * set it here. At this point regulators should be already
+		 * available.
+		 */
+		set_board_revision();
+	}
+
 #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
 	set_board_info();
 #endif
diff --git a/board/samsung/common/exynos5-dt-types.c b/board/samsung/common/exynos5-dt-types.c
index 7a86e9187768..7c1271d6547a 100644
--- a/board/samsung/common/exynos5-dt-types.c
+++ b/board/samsung/common/exynos5-dt-types.c
@@ -192,8 +192,11 @@ const char *get_board_type(void)
 
 /**
  * set_board_type() - set board type in gd->board_type.
- * As default type set EXYNOS5_BOARD_GENERIC, if detect Odroid,
- * then set its proper type.
+ * As default type set EXYNOS5_BOARD_GENERIC. If Odroid is detected,
+ * set its proper type based on device tree.
+ *
+ * This might be called early when some more specific ways to detect revision
+ * are not yet available.
  */
 void set_board_type(void)
 {
@@ -211,8 +214,15 @@ void set_board_type(void)
 		gd->board_type = of_match->data;
 		break;
 	}
+}
 
-	/* If Odroid, then check its revision */
+/**
+ * set_board_revision() - set detailed board type in gd->board_type.
+ * Should be called when resources (e.g. regulators) are available
+ * so ADC can be used to detect the specific revision of a board.
+ */
+void set_board_revision(void)
+{
 	if (board_is_odroidxu3())
 		gd->board_type = odroid_get_board_type();
 }
diff --git a/board/samsung/odroid/odroid.c b/board/samsung/odroid/odroid.c
index 552333fe869d..4be8cc9826c3 100644
--- a/board/samsung/odroid/odroid.c
+++ b/board/samsung/odroid/odroid.c
@@ -54,6 +54,14 @@ void set_board_type(void)
 		gd->board_type = ODROID_TYPE_U3;
 }
 
+void set_board_revision(void)
+{
+	/*
+	 * Revision already set by set_board_type() because it can be
+	 * executed early.
+	 */
+}
+
 const char *get_board_type(void)
 {
 	const char *board_type[] = {"u3", "x2"};
diff --git a/include/samsung/misc.h b/include/samsung/misc.h
index 017560c25662..4ff28a1df0e8 100644
--- a/include/samsung/misc.h
+++ b/include/samsung/misc.h
@@ -33,6 +33,7 @@ char *get_dfu_alt_system(char *interface, char *devstr);
 char *get_dfu_alt_boot(char *interface, char *devstr);
 #endif
 void set_board_type(void);
+void set_board_revision(void);
 const char *get_board_type(void);
 
 #endif /* __SAMSUNG_MISC_COMMON_H__ */
-- 
2.17.1

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

* [U-Boot] [PATCH v3 4/9] arm: exynos: odroid-xu3: Display info late to have proper type
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
                   ` (2 preceding siblings ...)
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 3/9] arm: exynos: Detect revision later, when all resources are ready Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision Krzysztof Kozlowski
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

Printing the "Type" of board requires proper detection of revision which
can happen only late because regulators are needed.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 board/samsung/common/board.c | 2 +-
 configs/odroid-xu3_defconfig | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c
index ef2204742e1d..4ffbd7254205 100644
--- a/board/samsung/common/board.c
+++ b/board/samsung/common/board.c
@@ -249,7 +249,7 @@ int board_eth_init(bd_t *bis)
 	return 0;
 }
 
-#ifdef CONFIG_DISPLAY_BOARDINFO
+#if defined(CONFIG_DISPLAY_BOARDINFO) || defined(CONFIG_DISPLAY_BOARDINFO_LATE)
 int checkboard(void)
 {
 	if (IS_ENABLED(CONFIG_BOARD_TYPES)) {
diff --git a/configs/odroid-xu3_defconfig b/configs/odroid-xu3_defconfig
index f6f05b294833..57aca5e015fc 100644
--- a/configs/odroid-xu3_defconfig
+++ b/configs/odroid-xu3_defconfig
@@ -11,6 +11,8 @@ CONFIG_FIT=y
 CONFIG_FIT_BEST_MATCH=y
 CONFIG_SILENT_CONSOLE=y
 CONFIG_CONSOLE_MUX=y
+# CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_MISC_INIT_R=y
 CONFIG_SYS_PROMPT="ODROID-XU3 # "
 CONFIG_CMD_THOR_DOWNLOAD=y
-- 
2.17.1

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

* [U-Boot] [PATCH v3 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
                   ` (3 preceding siblings ...)
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 4/9] arm: exynos: odroid-xu3: Display info late to have proper type Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay Krzysztof Kozlowski
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

Fix detection of Odroid HC1 (Exynos5422) after reboot if kernel disabled
the LDO4/VDD_ADC regulator.

The LDO4 supplies both ADC block and the ADC input AIN9.  Voltage on
AIN9 will rise slowly, so be patient and wait for it to stabilize.

First reads on Odroid HC1 return 305, 1207, 1297 and finally 1308
(reference value is 1309).

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 board/samsung/common/exynos5-dt-types.c | 38 ++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/board/samsung/common/exynos5-dt-types.c b/board/samsung/common/exynos5-dt-types.c
index 7c1271d6547a..516c32923e44 100644
--- a/board/samsung/common/exynos5-dt-types.c
+++ b/board/samsung/common/exynos5-dt-types.c
@@ -57,12 +57,48 @@ static unsigned int odroid_get_rev(void)
 	return 0;
 }
 
+/*
+ * Read ADC at least twice and check the resuls.  If regulator providing voltage
+ * on to measured point was just turned on, first reads might require time
+ * to stabilize.
+ */
+static int odroid_get_adc_val(unsigned int *adcval)
+{
+	unsigned int adcval_prev = 0;
+	int ret, retries = 20;
+
+	ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN,
+				      &adcval_prev);
+	if (ret)
+		return ret;
+
+	while (retries--) {
+		mdelay(5);
+
+		ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN,
+					      adcval);
+		if (ret)
+			return ret;
+
+		/*
+		 * If difference between ADC reads is less than 3%,
+		 * accept the result
+		 */
+		if ((100 * abs(*adcval - adcval_prev) / adcval_prev) < 3)
+			return ret;
+
+		adcval_prev = *adcval;
+	}
+
+	return ret;
+}
+
 static int odroid_get_board_type(void)
 {
 	unsigned int adcval;
 	int ret, i;
 
-	ret = adc_channel_single_shot("adc", CONFIG_ODROID_REV_AIN, &adcval);
+	ret = odroid_get_adc_val(&adcval);
 	if (ret)
 		goto rev_default;
 
-- 
2.17.1

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

* [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
                   ` (4 preceding siblings ...)
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-18 14:03   ` Torsten Duwe
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 7/9] power: regulator: s2mps11: Add enable delay Krzysztof Kozlowski
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

Changing voltage and enabling regulator might require delays so the
regulator stabilizes at expected level.

Add support for "regulator-ramp-delay" binding which can introduce
required time to both enabling the regulator and to changing the
voltage.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 .../regulator/regulator.txt                   |  2 +
 drivers/power/regulator/regulator-uclass.c    | 47 ++++++++++++++++++-
 include/power/regulator.h                     |  2 +
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/doc/device-tree-bindings/regulator/regulator.txt b/doc/device-tree-bindings/regulator/regulator.txt
index 65b69c427899..4ba642b7c77f 100644
--- a/doc/device-tree-bindings/regulator/regulator.txt
+++ b/doc/device-tree-bindings/regulator/regulator.txt
@@ -35,6 +35,7 @@ Optional properties:
 - regulator-max-microamp: a maximum allowed Current value
 - regulator-always-on: regulator should never be disabled
 - regulator-boot-on: enabled by bootloader/firmware
+- regulator-ramp-delay: ramp delay for regulator (in uV/us)
 
 Note
 The "regulator-name" constraint is used for setting the device's uclass
@@ -60,4 +61,5 @@ ldo0 {
 	regulator-max-microamp = <100000>;
 	regulator-always-on;
 	regulator-boot-on;
+	regulator-ramp-delay = <12000>;
 };
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c
index 6f355b969a6d..363c6e6441fa 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -35,10 +35,22 @@ int regulator_get_value(struct udevice *dev)
 	return ops->get_value(dev);
 }
 
+static void regulator_set_value_delay(struct udevice *dev, int old_uV,
+				      int new_uV, unsigned int ramp_delay)
+{
+	int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
+
+	debug("regulator %s: delay %u us (%d uV -> %d uV)\n", dev->name, delay,
+	      old_uV, new_uV);
+
+	udelay(delay);
+}
+
 int regulator_set_value(struct udevice *dev, int uV)
 {
 	const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
 	struct dm_regulator_uclass_platdata *uc_pdata;
+	int ret, old_uV = uV, is_enabled = 0;
 
 	uc_pdata = dev_get_uclass_platdata(dev);
 	if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV)
@@ -49,7 +61,20 @@ int regulator_set_value(struct udevice *dev, int uV)
 	if (!ops || !ops->set_value)
 		return -ENOSYS;
 
-	return ops->set_value(dev, uV);
+	if (uc_pdata->ramp_delay) {
+		is_enabled = regulator_get_enable(dev);
+		old_uV = regulator_get_value(dev);
+	}
+
+	ret = ops->set_value(dev, uV);
+
+	if (!ret) {
+		if (uc_pdata->ramp_delay && old_uV > 0 && is_enabled)
+			regulator_set_value_delay(dev, old_uV, uV,
+						  uc_pdata->ramp_delay);
+	}
+
+	return ret;
 }
 
 /*
@@ -107,6 +132,7 @@ int regulator_set_enable(struct udevice *dev, bool enable)
 {
 	const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
 	struct dm_regulator_uclass_platdata *uc_pdata;
+	int ret, old_enable = 0;
 
 	if (!ops || !ops->set_enable)
 		return -ENOSYS;
@@ -115,7 +141,22 @@ int regulator_set_enable(struct udevice *dev, bool enable)
 	if (!enable && uc_pdata->always_on)
 		return -EACCES;
 
-	return ops->set_enable(dev, enable);
+	if (uc_pdata->ramp_delay)
+		old_enable = regulator_get_enable(dev);
+
+	ret = ops->set_enable(dev, enable);
+	if (!ret) {
+		if (uc_pdata->ramp_delay && !old_enable && enable) {
+			int uV = regulator_get_value(dev);
+
+			if (uV > 0) {
+				regulator_set_value_delay(dev, 0, uV,
+							  uc_pdata->ramp_delay);
+			}
+		}
+	}
+
+	return ret;
 }
 
 int regulator_set_enable_if_allowed(struct udevice *dev, bool enable)
@@ -335,6 +376,8 @@ static int regulator_pre_probe(struct udevice *dev)
 						-ENODATA);
 	uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on");
 	uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
+	uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
+						    0);
 
 	/* Those values are optional (-ENODATA if unset) */
 	if ((uc_pdata->min_uV != -ENODATA) &&
diff --git a/include/power/regulator.h b/include/power/regulator.h
index 314160a894b7..6c6e2cd4f996 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -150,6 +150,7 @@ enum regulator_flag {
  * @always_on* - bool type, true or false
  * @boot_on*   - bool type, true or false
  * TODO(sjg at chromium.org): Consider putting the above two into @flags
+ * @ramp_delay - Time to settle down after voltage change (unit: uV/us)
  * @flags:     - flags value (see REGULATOR_FLAG_...)
  * @name**     - fdt regulator name - should be taken from the device tree
  * ctrl_reg:   - Control register offset used to enable/disable regulator
@@ -169,6 +170,7 @@ struct dm_regulator_uclass_platdata {
 	int max_uV;
 	int min_uA;
 	int max_uA;
+	unsigned int ramp_delay;
 	bool always_on;
 	bool boot_on;
 	const char *name;
-- 
2.17.1

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

* [U-Boot] [PATCH v3 7/9] power: regulator: s2mps11: Add enable delay
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
                   ` (5 preceding siblings ...)
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 8/9] arm: dts: exynos: Add supply for ADC block to Odroid XU3 family Krzysztof Kozlowski
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

According to datasheet, the output on LDO regulators will start
appearing after 10-15 us.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/power/regulator/s2mps11_regulator.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/power/regulator/s2mps11_regulator.c b/drivers/power/regulator/s2mps11_regulator.c
index 723d27f67c9a..67d1f9689de3 100644
--- a/drivers/power/regulator/s2mps11_regulator.c
+++ b/drivers/power/regulator/s2mps11_regulator.c
@@ -551,7 +551,16 @@ static int ldo_get_enable(struct udevice *dev)
 
 static int ldo_set_enable(struct udevice *dev, bool enable)
 {
-	return s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable);
+	int ret;
+
+	ret = s2mps11_ldo_enable(dev, PMIC_OP_SET, &enable);
+	if (ret)
+		return ret;
+
+	/* Wait the "enable delay" for voltage to start to rise */
+	udelay(15);
+
+	return 0;
 }
 
 static int ldo_get_mode(struct udevice *dev)
-- 
2.17.1

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

* [U-Boot] [PATCH v3 8/9] arm: dts: exynos: Add supply for ADC block to Odroid XU3 family
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
                   ` (6 preceding siblings ...)
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 7/9] power: regulator: s2mps11: Add enable delay Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 9/9] arm: dts: exynos: Add ramp delay property to LDO regulators " Krzysztof Kozlowski
  2019-02-24 12:55 ` [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Anand Moon
  9 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

The ADC block requires VDD supply to be on so provide one.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Lukasz Majewski <lukma@denx.de>
---
 arch/arm/dts/exynos5422-odroidxu3.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts
index e859dd1b981a..9dfae90667cf 100644
--- a/arch/arm/dts/exynos5422-odroidxu3.dts
+++ b/arch/arm/dts/exynos5422-odroidxu3.dts
@@ -32,6 +32,7 @@
 
 	adc at 12D10000 {
 		u-boot,dm-pre-reloc;
+		vdd-supply = <&ldo4_reg>;
 		status = "okay";
 	};
 
-- 
2.17.1

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

* [U-Boot] [PATCH v3 9/9] arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid XU3 family
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
                   ` (7 preceding siblings ...)
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 8/9] arm: dts: exynos: Add supply for ADC block to Odroid XU3 family Krzysztof Kozlowski
@ 2019-02-16  9:45 ` Krzysztof Kozlowski
  2019-02-24 12:55 ` [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Anand Moon
  9 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-16  9:45 UTC (permalink / raw)
  To: u-boot

Add startup time to LDO regulators of S2MPS11 PMIC on Odroid XU3/XU4/HC1
family of boards to be sure the voltage is proper before relying on the
regulator.

The datasheet for all the S2MPS1x family is inconsistent here and does
not specify unambiguously the value of ramp delay for LDO.  It mentions
30 mV/us in one timing diagram but then omits it completely in LDO
regulator characteristics table (it is specified for bucks).

However the vendor kernels for Galaxy S5 and Odroid XU3 use values of 12
mV/us or 24 mV/us.

Without the ramp delay value the consumers do not wait for voltage
settle after changing it.  Although the proper value of ramp delay for
LDOs is unknown, it seems safer to use at least some value from
reference kernel than to leave it unset.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Lukasz Majewski <lukma@denx.de>
---
 arch/arm/dts/exynos5422-odroidxu3.dts | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/dts/exynos5422-odroidxu3.dts b/arch/arm/dts/exynos5422-odroidxu3.dts
index 9dfae90667cf..04ecc404f907 100644
--- a/arch/arm/dts/exynos5422-odroidxu3.dts
+++ b/arch/arm/dts/exynos5422-odroidxu3.dts
@@ -45,6 +45,7 @@
 					regulator-name = "vdd_ldo1";
 					regulator-min-microvolt = <1000000>;
 					regulator-max-microvolt = <1000000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -52,18 +53,21 @@
 					regulator-name = "vddq_mmc0";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-ramp-delay = <12000>;
 				};
 
 				ldo4_reg: LDO4 {
 					regulator-name = "vdd_adc";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-ramp-delay = <12000>;
 				};
 
 				ldo5_reg: LDO5 {
 					regulator-name = "vdd_ldo5";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -71,6 +75,7 @@
 					regulator-name = "vdd_ldo6";
 					regulator-min-microvolt = <1000000>;
 					regulator-max-microvolt = <1000000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -78,6 +83,7 @@
 					regulator-name = "vdd_ldo7";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -85,6 +91,7 @@
 					regulator-name = "vdd_ldo8";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -92,6 +99,7 @@
 					regulator-name = "vdd_ldo9";
 					regulator-min-microvolt = <3000000>;
 					regulator-max-microvolt = <3000000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -99,6 +107,7 @@
 					regulator-name = "vdd_ldo10";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -106,6 +115,7 @@
 					regulator-name = "vdd_ldo11";
 					regulator-min-microvolt = <1000000>;
 					regulator-max-microvolt = <1000000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -113,6 +123,7 @@
 					regulator-name = "vdd_ldo12";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -120,12 +131,14 @@
 					regulator-name = "vddq_mmc2";
 					regulator-min-microvolt = <2800000>;
 					regulator-max-microvolt = <2800000>;
+					regulator-ramp-delay = <12000>;
 				};
 
 				ldo15_reg: LDO15 {
 					regulator-name = "vdd_ldo15";
 					regulator-min-microvolt = <3300000>;
 					regulator-max-microvolt = <3300000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -133,6 +146,7 @@
 					regulator-name = "vdd_ldo16";
 					regulator-min-microvolt = <2200000>;
 					regulator-max-microvolt = <2200000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -140,6 +154,7 @@
 					regulator-name = "vdd_ldo17";
 					regulator-min-microvolt = <3300000>;
 					regulator-max-microvolt = <3300000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -147,18 +162,21 @@
 					regulator-name = "vdd_emmc_1V8";
 					regulator-min-microvolt = <1800000>;
 					regulator-max-microvolt = <1800000>;
+					regulator-ramp-delay = <12000>;
 				};
 
 				ldo19_reg: LDO19 {
 					regulator-name = "vdd_sd";
 					regulator-min-microvolt = <2800000>;
 					regulator-max-microvolt = <2800000>;
+					regulator-ramp-delay = <12000>;
 				};
 
 				ldo24_reg: LDO24 {
 					regulator-name = "tsp_io";
 					regulator-min-microvolt = <2800000>;
 					regulator-max-microvolt = <2800000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
@@ -166,6 +184,7 @@
 					regulator-name = "vdd_ldo26";
 					regulator-min-microvolt = <3000000>;
 					regulator-max-microvolt = <3000000>;
+					regulator-ramp-delay = <12000>;
 					regulator-always-on;
 				};
 
-- 
2.17.1

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

* [U-Boot] [PATCH v3 3/9] arm: exynos: Detect revision later, when all resources are ready
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 3/9] arm: exynos: Detect revision later, when all resources are ready Krzysztof Kozlowski
@ 2019-02-17 22:34   ` Lukasz Majewski
  0 siblings, 0 replies; 18+ messages in thread
From: Lukasz Majewski @ 2019-02-17 22:34 UTC (permalink / raw)
  To: u-boot

On Sat, 16 Feb 2019 10:45:42 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> Detection of board revision is done early - before power setup.  In
> case of Odroid XU3/XU4/HC1 family, the detection is done using ADC
> which is supplied by LDO4/VDD_ADC regulator.  This regulator could be
> turned off (e.g. by kernel before reboot).  If ADC is used early, the
> regulators are not yet available and the detection won't work.
> 
> Split the revision detection out of set_board_type() into separate
> function called later - either when displaying board info (in late
> mode) or during misc_init_r.  The idea is that set_board_type() will
> be called early so its method of detection are limited to flattened
> device tree (exynos5-dt-types.c for Exynos5) or GPIO (odroid.c for
> Exynos4412).  The newly added set_board_revision() can be called only
> later, when resources like regulator are available.
> 
> This is necessary to fix the detection of Odroid HC1 after reboot, if
> kernel turned off the LDO4 regulator.
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  board/samsung/common/board.c            | 22 +++++++++++++++++++++-
>  board/samsung/common/exynos5-dt-types.c | 16 +++++++++++++---
>  board/samsung/odroid/odroid.c           |  8 ++++++++
>  include/samsung/misc.h                  |  1 +
>  4 files changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/board/samsung/common/board.c
> b/board/samsung/common/board.c index 96228a86a117..ef2204742e1d 100644
> --- a/board/samsung/common/board.c
> +++ b/board/samsung/common/board.c
> @@ -253,7 +253,18 @@ int board_eth_init(bd_t *bis)
>  int checkboard(void)
>  {
>  	if (IS_ENABLED(CONFIG_BOARD_TYPES)) {
> -		const char *board_info = get_board_type();
> +		const char *board_info;
> +
> +		if (IS_ENABLED(CONFIG_DISPLAY_BOARDINFO_LATE)) {
> +			/*
> +			 * Printing type requires having revision,
> although
> +			 * this will succeed only if done late.
> +			 * Otherwise revision will be set in
> misc_init_r().
> +			 */
> +			set_board_revision();
> +		}
> +
> +		board_info = get_board_type();
>  
>  		if (board_info)
>  			printf("Type:  %s\n", board_info);
> @@ -287,6 +298,15 @@ int board_late_init(void)
>  #ifdef CONFIG_MISC_INIT_R
>  int misc_init_r(void)
>  {
> +	if (!IS_ENABLED(CONFIG_DISPLAY_BOARDINFO_LATE)) {
> +		/*
> +		 * If revision was not set by late display boardinfo,
> +		 * set it here. At this point regulators should be
> already
> +		 * available.
> +		 */
> +		set_board_revision();
> +	}
> +
>  #ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
>  	set_board_info();
>  #endif
> diff --git a/board/samsung/common/exynos5-dt-types.c
> b/board/samsung/common/exynos5-dt-types.c index
> 7a86e9187768..7c1271d6547a 100644 ---
> a/board/samsung/common/exynos5-dt-types.c +++
> b/board/samsung/common/exynos5-dt-types.c @@ -192,8 +192,11 @@ const
> char *get_board_type(void) 
>  /**
>   * set_board_type() - set board type in gd->board_type.
> - * As default type set EXYNOS5_BOARD_GENERIC, if detect Odroid,
> - * then set its proper type.
> + * As default type set EXYNOS5_BOARD_GENERIC. If Odroid is detected,
> + * set its proper type based on device tree.
> + *
> + * This might be called early when some more specific ways to detect
> revision
> + * are not yet available.
>   */
>  void set_board_type(void)
>  {
> @@ -211,8 +214,15 @@ void set_board_type(void)
>  		gd->board_type = of_match->data;
>  		break;
>  	}
> +}
>  
> -	/* If Odroid, then check its revision */
> +/**
> + * set_board_revision() - set detailed board type in gd->board_type.
> + * Should be called when resources (e.g. regulators) are available
> + * so ADC can be used to detect the specific revision of a board.
> + */
> +void set_board_revision(void)
> +{
>  	if (board_is_odroidxu3())
>  		gd->board_type = odroid_get_board_type();
>  }
> diff --git a/board/samsung/odroid/odroid.c
> b/board/samsung/odroid/odroid.c index 552333fe869d..4be8cc9826c3
> 100644 --- a/board/samsung/odroid/odroid.c
> +++ b/board/samsung/odroid/odroid.c
> @@ -54,6 +54,14 @@ void set_board_type(void)
>  		gd->board_type = ODROID_TYPE_U3;
>  }
>  
> +void set_board_revision(void)
> +{
> +	/*
> +	 * Revision already set by set_board_type() because it can be
> +	 * executed early.
> +	 */
> +}
> +
>  const char *get_board_type(void)
>  {
>  	const char *board_type[] = {"u3", "x2"};
> diff --git a/include/samsung/misc.h b/include/samsung/misc.h
> index 017560c25662..4ff28a1df0e8 100644
> --- a/include/samsung/misc.h
> +++ b/include/samsung/misc.h
> @@ -33,6 +33,7 @@ char *get_dfu_alt_system(char *interface, char
> *devstr); char *get_dfu_alt_boot(char *interface, char *devstr);
>  #endif
>  void set_board_type(void);
> +void set_board_revision(void);
>  const char *get_board_type(void);
>  
>  #endif /* __SAMSUNG_MISC_COMMON_H__ */

Reviewed-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190217/f71435e4/attachment.sig>

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

* [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay Krzysztof Kozlowski
@ 2019-02-18 14:03   ` Torsten Duwe
  2019-02-18 14:28     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Torsten Duwe @ 2019-02-18 14:03 UTC (permalink / raw)
  To: u-boot

On Sat, Feb 16, 2019 at 10:45:45AM +0100, Krzysztof Kozlowski wrote:
> Changing voltage and enabling regulator might require delays so the
> regulator stabilizes at expected level.
> 
> Add support for "regulator-ramp-delay" binding which can introduce
> required time to both enabling the regulator and to changing the
> voltage.

I'm surprised that such a thing doesn't exist already.

> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

> --- a/doc/device-tree-bindings/regulator/regulator.txt
> +++ b/doc/device-tree-bindings/regulator/regulator.txt
> @@ -35,6 +35,7 @@ Optional properties:
>  - regulator-max-microamp: a maximum allowed Current value
>  - regulator-always-on: regulator should never be disabled
>  - regulator-boot-on: enabled by bootloader/firmware
> +- regulator-ramp-delay: ramp delay for regulator (in uV/us)

I guess you mean s/V, not V/s; at least the code suggests so.
But my main point is: is the required delay always a linear
function of the voltage jump? Depending on the dampening and
load on the rail this could be an overshoot and settle, no?

So I suggest to make that an array with 2 elements: a fixed part
and a time per voltage change. Does that make sense?

	Torsten

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

* [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay
  2019-02-18 14:03   ` Torsten Duwe
@ 2019-02-18 14:28     ` Krzysztof Kozlowski
  2019-02-18 15:26       ` Torsten Duwe
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-18 14:28 UTC (permalink / raw)
  To: u-boot

On Mon, 18 Feb 2019 at 15:03, Torsten Duwe <duwe@lst.de> wrote:
>
> On Sat, Feb 16, 2019 at 10:45:45AM +0100, Krzysztof Kozlowski wrote:
> > Changing voltage and enabling regulator might require delays so the
> > regulator stabilizes at expected level.
> >
> > Add support for "regulator-ramp-delay" binding which can introduce
> > required time to both enabling the regulator and to changing the
> > voltage.
>
> I'm surprised that such a thing doesn't exist already.
>
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> > --- a/doc/device-tree-bindings/regulator/regulator.txt
> > +++ b/doc/device-tree-bindings/regulator/regulator.txt
> > @@ -35,6 +35,7 @@ Optional properties:
> >  - regulator-max-microamp: a maximum allowed Current value
> >  - regulator-always-on: regulator should never be disabled
> >  - regulator-boot-on: enabled by bootloader/firmware
> > +- regulator-ramp-delay: ramp delay for regulator (in uV/us)
>
> I guess you mean s/V, not V/s; at least the code suggests so.

uV/uS. It is correct in the code:
int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
delay = (uV - uV) / (uV / uS) = uS

> But my main point is: is the required delay always a linear
> function of the voltage jump? Depending on the dampening and
> load on the rail this could be an overshoot and settle, no?
>
> So I suggest to make that an array with 2 elements: a fixed part
> and a time per voltage change. Does that make sense?

Just to make it clear - then we do not follow Linux kernel DT bindings.
The voltage change might have exponential characteristic and/or have
additional fixed delay time (see patch 7 here). We could re-use some
properties from Linux bindings for that purpose:
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L19
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L24

Best regards,
Krzysztof

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

* [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay
  2019-02-18 14:28     ` Krzysztof Kozlowski
@ 2019-02-18 15:26       ` Torsten Duwe
  2019-02-19 12:14         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Torsten Duwe @ 2019-02-18 15:26 UTC (permalink / raw)
  To: u-boot

On Mon, Feb 18, 2019 at 03:28:46PM +0100, Krzysztof Kozlowski wrote:
> On Mon, 18 Feb 2019 at 15:03, Torsten Duwe <duwe@lst.de> wrote:
> >
> > > --- a/doc/device-tree-bindings/regulator/regulator.txt
> > > +++ b/doc/device-tree-bindings/regulator/regulator.txt
> > > @@ -35,6 +35,7 @@ Optional properties:
> > >  - regulator-max-microamp: a maximum allowed Current value
> > >  - regulator-always-on: regulator should never be disabled
> > >  - regulator-boot-on: enabled by bootloader/firmware
> > > +- regulator-ramp-delay: ramp delay for regulator (in uV/us)
> >
> > I guess you mean s/V, not V/s; at least the code suggests so.
> 
> uV/uS. It is correct in the code:
> int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
> delay = (uV - uV) / (uV / uS) = uS

You're right. _divide_ by that value; somhow this has escaped me.
Sorry for the noise.

nit: "s" is for seconds, "S" is for conductance.

> > But my main point is: is the required delay always a linear
> > function of the voltage jump? Depending on the dampening and
> > load on the rail this could be an overshoot and settle, no?
> >
> > So I suggest to make that an array with 2 elements: a fixed part
> > and a time per voltage change. Does that make sense?
> 
> Just to make it clear - then we do not follow Linux kernel DT bindings.
> The voltage change might have exponential characteristic and/or have
> additional fixed delay time (see patch 7 here). We could re-use some
> properties from Linux bindings for that purpose:
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L19
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L24

I see. But then "static void regulator_set_value_delay(...)" should either
at least have a "ramp" somewhere in its name or it should discover the device
properties on its own, in order to be able to handle regulator-settling-time*
and regulator-enable-ramp-delay as well in the future. (i.e. pass it uc_pdata
instead of uc_pdata->ramp_delay and also let it handle the condition).

	Torsten

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

* [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay
  2019-02-18 15:26       ` Torsten Duwe
@ 2019-02-19 12:14         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-02-19 12:14 UTC (permalink / raw)
  To: u-boot

On Mon, 18 Feb 2019 at 16:27, Torsten Duwe <duwe@lst.de> wrote:

> > > But my main point is: is the required delay always a linear
> > > function of the voltage jump? Depending on the dampening and
> > > load on the rail this could be an overshoot and settle, no?
> > >
> > > So I suggest to make that an array with 2 elements: a fixed part
> > > and a time per voltage change. Does that make sense?
> >
> > Just to make it clear - then we do not follow Linux kernel DT bindings.
> > The voltage change might have exponential characteristic and/or have
> > additional fixed delay time (see patch 7 here). We could re-use some
> > properties from Linux bindings for that purpose:
> > https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L19
> > https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/regulator/regulator.txt#L24
>
> I see. But then "static void regulator_set_value_delay(...)" should either
> at least have a "ramp" somewhere in its name or it should discover the device
> properties on its own, in order to be able to handle regulator-settling-time*
> and regulator-enable-ramp-delay as well in the future. (i.e. pass it uc_pdata
> instead of uc_pdata->ramp_delay and also let it handle the condition).

Makes sense, so let me add the ramp keyword. I will also mention in
comment that other delays are not yet handled.

Thanks for feedback!

Best regards,
Krzysztof

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

* [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1
  2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
                   ` (8 preceding siblings ...)
  2019-02-16  9:45 ` [U-Boot] [PATCH v3 9/9] arm: dts: exynos: Add ramp delay property to LDO regulators " Krzysztof Kozlowski
@ 2019-02-24 12:55 ` Anand Moon
  2019-03-05 10:16   ` Minkyu Kang
  9 siblings, 1 reply; 18+ messages in thread
From: Anand Moon @ 2019-02-24 12:55 UTC (permalink / raw)
  To: u-boot

Hi Krzysztof,

Please add my. on Odroid HC1

Tested-by: Anand Moon <linux.amoon@gmail.com>

Best Regards
-Anand

On Sat, 16 Feb 2019 at 15:16, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> Hi,
>
> Changes since v2
> ================
> 1. Add Lukasz review tags.
> 2. Patch 7: Return on error, as suggested by Simon.
> 2. Patch 3: Use IS_ENABLED() to run revision detection only
>    once - either during late display board or misc_init_r.
>
> Changes since v1
> ================
> 1. Move fixes to beginning of patchset.
> 2. Patch 3: Rework the idea - split revision detection to be executed later.
> 3. Patch 4: New patch.
> 4. Patch 6: Apply Simon's comments.
> 5. Patch 6: Do not delay when changing voltage if regulator is disabled.
> 6. Patch 6: Do not delay when disabling the regulator.
>
>
> Description
> ===========
> Odroid HC1 does not reboot properly (at least from SD card but
> I do not expect difference on eMMC), if LDO4/VDD_ADC was turned
> off by Linux kernel.  This condition happens so far always, because
> Linux kernel did not enable ADC on Odroid HC1, therefore the
> VDD_ADC regulator was turned off as unused.
>
> The issue is in detection of revision which later is used to load
> proper DTB.
>
> The revision is obtained by ADC read of a voltage depending on VDD_ADC.
> Therefore:
> 1. VDD_ADC has to be turned on (but board detection happens before
>    power is initialized),
> 2. Turning VDD_ADC should wait with ramp delay,
> 3. Reading the value from ADC should wait for it to stabilize.
>
> Tested on Odroid XU3-Lite and Odroid HC1.
>
> Commends and testing are welcomed.
>
> Best regards,
> Krzysztof
>
> Krzysztof Kozlowski (9):
>   adc: exynos-adc: Fix wrong bit operation used to stop the ADC
>   power: regulator: s2mps11: Fix step for LDO27 and LDO35
>   arm: exynos: Detect revision later, when all resources are ready
>   arm: exynos: odroid-xu3: Display info late to have proper type
>   arm: exynos: Wait till ADC stabilizes before checking Odroid HC1
>     revision
>   regulator: Add support for ramp delay
>   power: regulator: s2mps11: Add enable delay
>   arm: dts: exynos: Add supply for ADC block to Odroid XU3 family
>   arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid
>     XU3 family
>
>  arch/arm/dts/exynos5422-odroidxu3.dts         | 20 +++++++
>  board/samsung/common/board.c                  | 24 ++++++++-
>  board/samsung/common/exynos5-dt-types.c       | 54 +++++++++++++++++--
>  board/samsung/odroid/odroid.c                 |  8 +++
>  configs/odroid-xu3_defconfig                  |  2 +
>  .../regulator/regulator.txt                   |  2 +
>  drivers/adc/exynos-adc.c                      |  2 +-
>  drivers/power/regulator/regulator-uclass.c    | 47 +++++++++++++++-
>  drivers/power/regulator/s2mps11_regulator.c   | 15 +++++-
>  include/power/regulator.h                     |  2 +
>  include/samsung/misc.h                        |  1 +
>  11 files changed, 167 insertions(+), 10 deletions(-)
>
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1
  2019-02-24 12:55 ` [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Anand Moon
@ 2019-03-05 10:16   ` Minkyu Kang
  2019-03-05 19:54     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Minkyu Kang @ 2019-03-05 10:16 UTC (permalink / raw)
  To: u-boot

Dear Krzysztof Kozlowski,

On 24/02/2019 21:55, Anand Moon wrote:
> Hi Krzysztof,
> 
> Please add my. on Odroid HC1
> 
> Tested-by: Anand Moon <linux.amoon@gmail.com>
> 
> Best Regards
> -Anand
> 
> On Sat, 16 Feb 2019 at 15:16, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> Hi,
>>
>> Changes since v2
>> ================
>> 1. Add Lukasz review tags.
>> 2. Patch 7: Return on error, as suggested by Simon.
>> 2. Patch 3: Use IS_ENABLED() to run revision detection only
>>    once - either during late display board or misc_init_r.
>>
>> Changes since v1
>> ================
>> 1. Move fixes to beginning of patchset.
>> 2. Patch 3: Rework the idea - split revision detection to be executed later.
>> 3. Patch 4: New patch.
>> 4. Patch 6: Apply Simon's comments.
>> 5. Patch 6: Do not delay when changing voltage if regulator is disabled.
>> 6. Patch 6: Do not delay when disabling the regulator.
>>
>>
>> Description
>> ===========
>> Odroid HC1 does not reboot properly (at least from SD card but
>> I do not expect difference on eMMC), if LDO4/VDD_ADC was turned
>> off by Linux kernel.  This condition happens so far always, because
>> Linux kernel did not enable ADC on Odroid HC1, therefore the
>> VDD_ADC regulator was turned off as unused.
>>
>> The issue is in detection of revision which later is used to load
>> proper DTB.
>>
>> The revision is obtained by ADC read of a voltage depending on VDD_ADC.
>> Therefore:
>> 1. VDD_ADC has to be turned on (but board detection happens before
>>    power is initialized),
>> 2. Turning VDD_ADC should wait with ramp delay,
>> 3. Reading the value from ADC should wait for it to stabilize.
>>
>> Tested on Odroid XU3-Lite and Odroid HC1.
>>
>> Commends and testing are welcomed.
>>
>> Best regards,
>> Krzysztof
>>
>> Krzysztof Kozlowski (9):
>>   adc: exynos-adc: Fix wrong bit operation used to stop the ADC
>>   power: regulator: s2mps11: Fix step for LDO27 and LDO35
>>   arm: exynos: Detect revision later, when all resources are ready
>>   arm: exynos: odroid-xu3: Display info late to have proper type
>>   arm: exynos: Wait till ADC stabilizes before checking Odroid HC1
>>     revision
>>   regulator: Add support for ramp delay
>>   power: regulator: s2mps11: Add enable delay
>>   arm: dts: exynos: Add supply for ADC block to Odroid XU3 family
>>   arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid
>>     XU3 family
>>
>>  arch/arm/dts/exynos5422-odroidxu3.dts         | 20 +++++++
>>  board/samsung/common/board.c                  | 24 ++++++++-
>>  board/samsung/common/exynos5-dt-types.c       | 54 +++++++++++++++++--
>>  board/samsung/odroid/odroid.c                 |  8 +++
>>  configs/odroid-xu3_defconfig                  |  2 +
>>  .../regulator/regulator.txt                   |  2 +
>>  drivers/adc/exynos-adc.c                      |  2 +-
>>  drivers/power/regulator/regulator-uclass.c    | 47 +++++++++++++++-
>>  drivers/power/regulator/s2mps11_regulator.c   | 15 +++++-
>>  include/power/regulator.h                     |  2 +
>>  include/samsung/misc.h                        |  1 +
>>  11 files changed, 167 insertions(+), 10 deletions(-)
>>
>> --
>> 2.17.1
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
> 
> 

Patch looks good but could you please rebase on latest u-boot-samsung tree?

Applying: adc: exynos-adc: Fix wrong bit operation used to stop the ADC
Applying: power: regulator: s2mps11: Fix step for LDO27 and LDO35
Applying: arm: exynos: Detect revision later, when all resources are ready
Applying: arm: exynos: odroid-xu3: Display info late to have proper type
error: patch failed: configs/odroid-xu3_defconfig:11
error: configs/odroid-xu3_defconfig: patch does not apply
Patch failed at 0004 arm: exynos: odroid-xu3: Display info late to have proper type

Thanks,
Minkyu Kang.

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

* [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1
  2019-03-05 10:16   ` Minkyu Kang
@ 2019-03-05 19:54     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2019-03-05 19:54 UTC (permalink / raw)
  To: u-boot

On Tue, 5 Mar 2019 at 11:16, Minkyu Kang <mk7.kang@samsung.com> wrote:
>
> Dear Krzysztof Kozlowski,
>
> On 24/02/2019 21:55, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > Please add my. on Odroid HC1
> >
> > Tested-by: Anand Moon <linux.amoon@gmail.com>
> >
> > Best Regards
> > -Anand
> >
> > On Sat, 16 Feb 2019 at 15:16, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> Hi,
> >>
> >> Changes since v2
> >> ================
> >> 1. Add Lukasz review tags.
> >> 2. Patch 7: Return on error, as suggested by Simon.
> >> 2. Patch 3: Use IS_ENABLED() to run revision detection only
> >>    once - either during late display board or misc_init_r.
> >>
> >> Changes since v1
> >> ================
> >> 1. Move fixes to beginning of patchset.
> >> 2. Patch 3: Rework the idea - split revision detection to be executed later.
> >> 3. Patch 4: New patch.
> >> 4. Patch 6: Apply Simon's comments.
> >> 5. Patch 6: Do not delay when changing voltage if regulator is disabled.
> >> 6. Patch 6: Do not delay when disabling the regulator.
> >>
> >>
> >> Description
> >> ===========
> >> Odroid HC1 does not reboot properly (at least from SD card but
> >> I do not expect difference on eMMC), if LDO4/VDD_ADC was turned
> >> off by Linux kernel.  This condition happens so far always, because
> >> Linux kernel did not enable ADC on Odroid HC1, therefore the
> >> VDD_ADC regulator was turned off as unused.
> >>
> >> The issue is in detection of revision which later is used to load
> >> proper DTB.
> >>
> >> The revision is obtained by ADC read of a voltage depending on VDD_ADC.
> >> Therefore:
> >> 1. VDD_ADC has to be turned on (but board detection happens before
> >>    power is initialized),
> >> 2. Turning VDD_ADC should wait with ramp delay,
> >> 3. Reading the value from ADC should wait for it to stabilize.
> >>
> >> Tested on Odroid XU3-Lite and Odroid HC1.
> >>
> >> Commends and testing are welcomed.
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >> Krzysztof Kozlowski (9):
> >>   adc: exynos-adc: Fix wrong bit operation used to stop the ADC
> >>   power: regulator: s2mps11: Fix step for LDO27 and LDO35
> >>   arm: exynos: Detect revision later, when all resources are ready
> >>   arm: exynos: odroid-xu3: Display info late to have proper type
> >>   arm: exynos: Wait till ADC stabilizes before checking Odroid HC1
> >>     revision
> >>   regulator: Add support for ramp delay
> >>   power: regulator: s2mps11: Add enable delay
> >>   arm: dts: exynos: Add supply for ADC block to Odroid XU3 family
> >>   arm: dts: exynos: Add ramp delay property to LDO regulators to Odroid
> >>     XU3 family
> >>
> >>  arch/arm/dts/exynos5422-odroidxu3.dts         | 20 +++++++
> >>  board/samsung/common/board.c                  | 24 ++++++++-
> >>  board/samsung/common/exynos5-dt-types.c       | 54 +++++++++++++++++--
> >>  board/samsung/odroid/odroid.c                 |  8 +++
> >>  configs/odroid-xu3_defconfig                  |  2 +
> >>  .../regulator/regulator.txt                   |  2 +
> >>  drivers/adc/exynos-adc.c                      |  2 +-
> >>  drivers/power/regulator/regulator-uclass.c    | 47 +++++++++++++++-
> >>  drivers/power/regulator/s2mps11_regulator.c   | 15 +++++-
> >>  include/power/regulator.h                     |  2 +
> >>  include/samsung/misc.h                        |  1 +
> >>  11 files changed, 167 insertions(+), 10 deletions(-)
> >>
> >> --
> >> 2.17.1
> >>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot at lists.denx.de
> >> https://lists.denx.de/listinfo/u-boot
> >
> >
>
> Patch looks good but could you please rebase on latest u-boot-samsung tree?
>
> Applying: adc: exynos-adc: Fix wrong bit operation used to stop the ADC
> Applying: power: regulator: s2mps11: Fix step for LDO27 and LDO35
> Applying: arm: exynos: Detect revision later, when all resources are ready
> Applying: arm: exynos: odroid-xu3: Display info late to have proper type
> error: patch failed: configs/odroid-xu3_defconfig:11
> error: configs/odroid-xu3_defconfig: patch does not apply
> Patch failed at 0004 arm: exynos: odroid-xu3: Display info late to have proper type

Sure, let me rebase.

Best regards,
Krzysztof

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

end of thread, other threads:[~2019-03-05 19:54 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-16  9:45 [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Krzysztof Kozlowski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 1/9] adc: exynos-adc: Fix wrong bit operation used to stop the ADC Krzysztof Kozlowski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 2/9] power: regulator: s2mps11: Fix step for LDO27 and LDO35 Krzysztof Kozlowski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 3/9] arm: exynos: Detect revision later, when all resources are ready Krzysztof Kozlowski
2019-02-17 22:34   ` Lukasz Majewski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 4/9] arm: exynos: odroid-xu3: Display info late to have proper type Krzysztof Kozlowski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 5/9] arm: exynos: Wait till ADC stabilizes before checking Odroid HC1 revision Krzysztof Kozlowski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 6/9] regulator: Add support for ramp delay Krzysztof Kozlowski
2019-02-18 14:03   ` Torsten Duwe
2019-02-18 14:28     ` Krzysztof Kozlowski
2019-02-18 15:26       ` Torsten Duwe
2019-02-19 12:14         ` Krzysztof Kozlowski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 7/9] power: regulator: s2mps11: Add enable delay Krzysztof Kozlowski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 8/9] arm: dts: exynos: Add supply for ADC block to Odroid XU3 family Krzysztof Kozlowski
2019-02-16  9:45 ` [U-Boot] [PATCH v3 9/9] arm: dts: exynos: Add ramp delay property to LDO regulators " Krzysztof Kozlowski
2019-02-24 12:55 ` [U-Boot] [PATCH v3 0/9] arm: exynos: Fix reboot on Odroid HC1 Anand Moon
2019-03-05 10:16   ` Minkyu Kang
2019-03-05 19:54     ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.