[PATCH v4 0/4] General regulator and pmic uclass improvements
This patchset derives from discussion of TPS65913 bringup and aims to cycle all regulator management inside uclass removing need of any board calls for regulators.
My hw setup is Tegra 3 LG Optimus Vu P895 (PMIC is MAX77663) with native spl u-boot build.
power: regulator: expand basic reference counter onto all uclass Commit is basically expansion of 4fcba5d ("regulator: implement basic reference counter") onto all regulators. My testing on hw has shown no issues so far with both pmic regulators and fixed regulators. Counting works as expected, dm test is set in test of regulator_set_enable_if_allowed.
power: regulator-uclass: perform regulator setup inside uclass All regulators with always-on or boot-on are set to probe after bind which ensures that essential regulators are set. In the post probe regulator autoset is called so that correct regulator state according to device tree is reached. DM test is set by checking regulators data without pre-configuring them manually.
--- Changes from v3: - no changes, re-sending
Changes from v2: - dropped changes related to pmic (pmic regulator probing on pmic post bind) - regulator uclass changes are dropped to minimum to remain compatibility: - always-on and boot-on check is moved to post bind - regulator_autoset is called on post_probe - adjusted dm tests to pass with this changes
Changes from v1: - adapted description of regulator_set_enable - remove uc_pdata->enable_count from post_probe - added tests from counter and regulators post_probe ---
Svyatoslav Ryhel (4): power: regulator: expand basic reference counter onto all uclass test: dm: regulator: test counter in set_enable_if_allowed test power: regulator: Perform regulator setup inside uclass test: dm: regulator: provide test of auto setup
drivers/power/regulator/regulator-uclass.c | 71 ++++++++++++++++++++-- drivers/power/regulator/regulator_common.c | 22 ------- drivers/power/regulator/regulator_common.h | 21 ------- include/power/regulator.h | 2 + test/dm/regulator.c | 27 +++++++- 5 files changed, 92 insertions(+), 51 deletions(-)
Commit is based on 4fcba5d ("regulator: implement basic reference counter") but expands the idea to all regulators instead of just fixed/gpio regulators.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- drivers/power/regulator/regulator-uclass.c | 41 ++++++++++++++++++++++ drivers/power/regulator/regulator_common.c | 22 ------------ drivers/power/regulator/regulator_common.h | 21 ----------- include/power/regulator.h | 2 ++ 4 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index d5a79d861c..4b2c6c2964 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -159,6 +159,25 @@ int regulator_get_enable(struct udevice *dev) return ops->get_enable(dev); }
+/* + * Enable or Disable a regulator + * + * This is a reentrant function and subsequent calls that enable will + * increase an internal counter, and disable calls will decrease the counter. + * The actual resource will be enabled when the counter gets to 1 coming from 0, + * and disabled when it reaches 0 coming from 1. + * + * @dev: regulator device + * @enable: bool indicating whether to enable or disable the regulator + * @return: + * 0 on Success + * -EBUSY if the regulator cannot be disabled because it's requested by + * another device + * -EALREADY if the regulator has already been enabled or has already been + * disabled + * -EACCES if there is no possibility to enable/disable the regulator + * -ve on different error situation + */ int regulator_set_enable(struct udevice *dev, bool enable) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); @@ -172,6 +191,23 @@ int regulator_set_enable(struct udevice *dev, bool enable) if (!enable && uc_pdata->always_on) return -EACCES;
+ /* If previously enabled, increase count */ + if (enable && uc_pdata->enable_count > 0) { + uc_pdata->enable_count++; + return -EALREADY; + } + + if (!enable) { + if (uc_pdata->enable_count > 1) { + /* If enabled multiple times, decrease count */ + uc_pdata->enable_count--; + return -EBUSY; + } else if (!uc_pdata->enable_count) { + /* If already disabled, do nothing */ + return -EALREADY; + } + } + if (uc_pdata->ramp_delay) old_enable = regulator_get_enable(dev);
@@ -187,6 +223,11 @@ int regulator_set_enable(struct udevice *dev, bool enable) } }
+ if (enable) + uc_pdata->enable_count++; + else + uc_pdata->enable_count--; + return ret; }
diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c index 0116fa01bb..62d06bf616 100644 --- a/drivers/power/regulator/regulator_common.c +++ b/drivers/power/regulator/regulator_common.c @@ -74,23 +74,6 @@ int regulator_common_set_enable(const struct udevice *dev, return 0; }
- /* If previously enabled, increase count */ - if (enable && plat->enable_count > 0) { - plat->enable_count++; - return -EALREADY; - } - - if (!enable) { - if (plat->enable_count > 1) { - /* If enabled multiple times, decrease count */ - plat->enable_count--; - return -EBUSY; - } else if (!plat->enable_count) { - /* If already disabled, do nothing */ - return -EALREADY; - } - } - ret = dm_gpio_set_value(&plat->gpio, enable); if (ret) { pr_err("Can't set regulator : %s gpio to: %d\n", dev->name, @@ -105,10 +88,5 @@ int regulator_common_set_enable(const struct udevice *dev, if (!enable && plat->off_on_delay_us) udelay(plat->off_on_delay_us);
- if (enable) - plat->enable_count++; - else - plat->enable_count--; - return 0; } diff --git a/drivers/power/regulator/regulator_common.h b/drivers/power/regulator/regulator_common.h index d4962899d8..15f1fa4c93 100644 --- a/drivers/power/regulator/regulator_common.h +++ b/drivers/power/regulator/regulator_common.h @@ -13,7 +13,6 @@ struct regulator_common_plat { struct gpio_desc gpio; /* GPIO for regulator enable control */ unsigned int startup_delay_us; unsigned int off_on_delay_us; - unsigned int enable_count; };
int regulator_common_of_to_plat(struct udevice *dev, @@ -21,26 +20,6 @@ int regulator_common_of_to_plat(struct udevice *dev, char *enable_gpio_name); int regulator_common_get_enable(const struct udevice *dev, struct regulator_common_plat *plat); -/* - * Enable or Disable a regulator - * - * This is a reentrant function and subsequent calls that enable will - * increase an internal counter, and disable calls will decrease the counter. - * The actual resource will be enabled when the counter gets to 1 coming from 0, - * and disabled when it reaches 0 coming from 1. - * - * @dev: regulator device - * @plat: Platform data - * @enable: bool indicating whether to enable or disable the regulator - * @return: - * 0 on Success - * -EBUSY if the regulator cannot be disabled because it's requested by - * another device - * -EALREADY if the regulator has already been enabled or has already been - * disabled - * -EACCES if there is no possibility to enable/disable the regulator - * -ve on different error situation - */ int regulator_common_set_enable(const struct udevice *dev, struct regulator_common_plat *plat, bool enable);
diff --git a/include/power/regulator.h b/include/power/regulator.h index 200652cb3d..1c7995bd5d 100644 --- a/include/power/regulator.h +++ b/include/power/regulator.h @@ -159,6 +159,7 @@ enum regulator_flag { * @name** - fdt regulator name - should be taken from the device tree * ctrl_reg: - Control register offset used to enable/disable regulator * volt_reg: - register offset for writing voltage vsel values + * enable_count - counter of enable calls for this regulator * * Note: * * - set automatically on device probe by the uclass's '.pre_probe' method. @@ -185,6 +186,7 @@ struct dm_regulator_uclass_plat { u8 volt_reg; bool suspend_on; u32 suspend_uV; + u32 enable_count; };
/* Regulator device operations */
On 2023-10-03 08:21, Svyatoslav Ryhel wrote:
Commit is based on 4fcba5d ("regulator: implement basic reference counter") but expands the idea to all regulators instead of just fixed/gpio regulators.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/power/regulator/regulator-uclass.c | 41 ++++++++++++++++++++++ drivers/power/regulator/regulator_common.c | 22 ------------ drivers/power/regulator/regulator_common.h | 21 ----------- include/power/regulator.h | 2 ++ 4 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index d5a79d861c..4b2c6c2964 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -159,6 +159,25 @@ int regulator_get_enable(struct udevice *dev) return ops->get_enable(dev); }
+/*
- Enable or Disable a regulator
- This is a reentrant function and subsequent calls that enable will
- increase an internal counter, and disable calls will decrease the counter.
- The actual resource will be enabled when the counter gets to 1 coming from 0,
- and disabled when it reaches 0 coming from 1.
- @dev: regulator device
- @enable: bool indicating whether to enable or disable the regulator
- @return:
- 0 on Success
- -EBUSY if the regulator cannot be disabled because it's requested by
another device
- -EALREADY if the regulator has already been enabled or has already been
disabled
- -EACCES if there is no possibility to enable/disable the regulator
- -ve on different error situation
- */
Maybe better to replace the existing comment for this function in include/power/regulator.h ?
Regards, Jonas
int regulator_set_enable(struct udevice *dev, bool enable) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); @@ -172,6 +191,23 @@ int regulator_set_enable(struct udevice *dev, bool enable) if (!enable && uc_pdata->always_on) return -EACCES;
- /* If previously enabled, increase count */
- if (enable && uc_pdata->enable_count > 0) {
uc_pdata->enable_count++;return -EALREADY;- }
- if (!enable) {
if (uc_pdata->enable_count > 1) {/* If enabled multiple times, decrease count */uc_pdata->enable_count--;return -EBUSY;} else if (!uc_pdata->enable_count) {/* If already disabled, do nothing */return -EALREADY;}- }
- if (uc_pdata->ramp_delay) old_enable = regulator_get_enable(dev);
@@ -187,6 +223,11 @@ int regulator_set_enable(struct udevice *dev, bool enable) } }
- if (enable)
uc_pdata->enable_count++;- else
uc_pdata->enable_count--;- return ret;
}
diff --git a/drivers/power/regulator/regulator_common.c b/drivers/power/regulator/regulator_common.c index 0116fa01bb..62d06bf616 100644 --- a/drivers/power/regulator/regulator_common.c +++ b/drivers/power/regulator/regulator_common.c @@ -74,23 +74,6 @@ int regulator_common_set_enable(const struct udevice *dev, return 0; }
- /* If previously enabled, increase count */
- if (enable && plat->enable_count > 0) {
plat->enable_count++;return -EALREADY;- }
- if (!enable) {
if (plat->enable_count > 1) {/* If enabled multiple times, decrease count */plat->enable_count--;return -EBUSY;} else if (!plat->enable_count) {/* If already disabled, do nothing */return -EALREADY;}- }
- ret = dm_gpio_set_value(&plat->gpio, enable); if (ret) { pr_err("Can't set regulator : %s gpio to: %d\n", dev->name,
@@ -105,10 +88,5 @@ int regulator_common_set_enable(const struct udevice *dev, if (!enable && plat->off_on_delay_us) udelay(plat->off_on_delay_us);
- if (enable)
plat->enable_count++;- else
plat->enable_count--;- return 0;
} diff --git a/drivers/power/regulator/regulator_common.h b/drivers/power/regulator/regulator_common.h index d4962899d8..15f1fa4c93 100644 --- a/drivers/power/regulator/regulator_common.h +++ b/drivers/power/regulator/regulator_common.h @@ -13,7 +13,6 @@ struct regulator_common_plat { struct gpio_desc gpio; /* GPIO for regulator enable control */ unsigned int startup_delay_us; unsigned int off_on_delay_us;
- unsigned int enable_count;
};
int regulator_common_of_to_plat(struct udevice *dev, @@ -21,26 +20,6 @@ int regulator_common_of_to_plat(struct udevice *dev, char *enable_gpio_name); int regulator_common_get_enable(const struct udevice *dev, struct regulator_common_plat *plat); -/*
- Enable or Disable a regulator
- This is a reentrant function and subsequent calls that enable will
- increase an internal counter, and disable calls will decrease the counter.
- The actual resource will be enabled when the counter gets to 1 coming from 0,
- and disabled when it reaches 0 coming from 1.
- @dev: regulator device
- @plat: Platform data
- @enable: bool indicating whether to enable or disable the regulator
- @return:
- 0 on Success
- -EBUSY if the regulator cannot be disabled because it's requested by
another device
- -EALREADY if the regulator has already been enabled or has already been
disabled
- -EACCES if there is no possibility to enable/disable the regulator
- -ve on different error situation
- */
int regulator_common_set_enable(const struct udevice *dev, struct regulator_common_plat *plat, bool enable);
diff --git a/include/power/regulator.h b/include/power/regulator.h index 200652cb3d..1c7995bd5d 100644 --- a/include/power/regulator.h +++ b/include/power/regulator.h @@ -159,6 +159,7 @@ enum regulator_flag {
- @name** - fdt regulator name - should be taken from the device tree
- ctrl_reg: - Control register offset used to enable/disable regulator
- volt_reg: - register offset for writing voltage vsel values
- enable_count - counter of enable calls for this regulator
- Note:
- set automatically on device probe by the uclass's '.pre_probe' method.
@@ -185,6 +186,7 @@ struct dm_regulator_uclass_plat { u8 volt_reg; bool suspend_on; u32 suspend_uV;
- u32 enable_count;
};
/* Regulator device operations */
Emulate use of the regulator by a few consumers with balanced on/off calls.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com --- test/dm/regulator.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/test/dm/regulator.c b/test/dm/regulator.c index 86f4862d9d..33115daaf5 100644 --- a/test/dm/regulator.c +++ b/test/dm/regulator.c @@ -194,6 +194,24 @@ int dm_test_power_regulator_set_enable_if_allowed(struct unit_test_state *uts) ut_assertok(regulator_set_enable_if_allowed(dev, val_set)); ut_asserteq(regulator_get_enable(dev), !val_set);
+ /* Set the Enable of LDO1 - default is disabled */ + platname = regulator_names[LDO1][PLATNAME]; + ut_assertok(regulator_get_by_platname(platname, &dev)); + ut_assertok(regulator_set_enable_if_allowed(dev, true)); + + /* Get the Enable state of LDO1 and compare it with the requested one */ + ut_asserteq(regulator_get_enable(dev), true); + + /* Emulate second consumer */ + ut_assertok(regulator_set_enable_if_allowed(dev, true)); + ut_asserteq(regulator_get_enable(dev), true); + + /* Emulate one of counsumers disable call */ + ut_assertok(regulator_set_enable_if_allowed(dev, false)); + + /* Regulator should still be on since counter indicates one consumer active */ + ut_asserteq(regulator_get_enable(dev), true); + return 0; } DM_TEST(dm_test_power_regulator_set_enable_if_allowed, UT_TESTF_SCAN_FDT);
Regulators initial setup was previously dependent on board call. To move from this behaviour next solution is proposed: on post_bind boot-on/always-on properties are checked, all regulators with such props will be probed just after binding which ensures that essential regulators are set, then in the post probe regulator autoset is called so that correct regulator state according to device tree is reached.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com [jonas@kwiboo.se: use autoset func, only probe with always/boot-on prop] Signed-off-by: Jonas Karlman jonas@kwiboo.se --- drivers/power/regulator/regulator-uclass.c | 30 ++++++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 4b2c6c2964..69d6d6428d 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -473,6 +473,7 @@ static int regulator_post_bind(struct udevice *dev) { struct dm_regulator_uclass_plat *uc_pdata; const char *property = "regulator-name"; + int ret;
uc_pdata = dev_get_uclass_plat(dev);
@@ -486,13 +487,20 @@ static int regulator_post_bind(struct udevice *dev) return -EINVAL; }
- if (regulator_name_is_unique(dev, uc_pdata->name)) - return 0; + ret = regulator_name_is_unique(dev, uc_pdata->name); + if (!ret) { + debug("'%s' of dev: '%s', has nonunique value: '%s'\n", + property, dev->name, uc_pdata->name); + return -EINVAL; + }
- debug("'%s' of dev: '%s', has nonunique value: '%s\n", - property, dev->name, uc_pdata->name); + uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on"); + uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
- return -EINVAL; + if (uc_pdata->always_on || uc_pdata->boot_on) + dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND); + + return 0; }
static int regulator_pre_probe(struct udevice *dev) @@ -546,6 +554,17 @@ static int regulator_pre_probe(struct udevice *dev) return 0; }
+static int regulator_post_probe(struct udevice *dev) +{ + int ret; + + ret = regulator_autoset(dev); + if (ret == -EMEDIUMTYPE || ret == -ENOSYS) + return 0; + + return ret; +} + int regulators_enable_boot_on(bool verbose) { struct udevice *dev; @@ -603,5 +622,6 @@ UCLASS_DRIVER(regulator) = { .name = "regulator", .post_bind = regulator_post_bind, .pre_probe = regulator_pre_probe, + .post_probe = regulator_post_probe, .per_device_plat_auto = sizeof(struct dm_regulator_uclass_plat), };
On 2023-10-03 08:21, Svyatoslav Ryhel wrote:
Regulators initial setup was previously dependent on board call. To move from this behaviour next solution is proposed: on post_bind boot-on/always-on properties are checked, all regulators with such props will be probed just after binding which ensures that essential regulators are set, then in the post probe regulator autoset is called so that correct regulator state according to device tree is reached.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com [jonas@kwiboo.se: use autoset func, only probe with always/boot-on prop] Signed-off-by: Jonas Karlman jonas@kwiboo.se
drivers/power/regulator/regulator-uclass.c | 30 ++++++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 4b2c6c2964..69d6d6428d 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -473,6 +473,7 @@ static int regulator_post_bind(struct udevice *dev) { struct dm_regulator_uclass_plat *uc_pdata; const char *property = "regulator-name";
int ret;
uc_pdata = dev_get_uclass_plat(dev);
@@ -486,13 +487,20 @@ static int regulator_post_bind(struct udevice *dev) return -EINVAL; }
- if (regulator_name_is_unique(dev, uc_pdata->name))
return 0;
- ret = regulator_name_is_unique(dev, uc_pdata->name);
- if (!ret) {
debug("'%s' of dev: '%s', has nonunique value: '%s'\n",property, dev->name, uc_pdata->name);return -EINVAL;- }
- debug("'%s' of dev: '%s', has nonunique value: '%s\n",
property, dev->name, uc_pdata->name);
- uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on");
- uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
- return -EINVAL;
- if (uc_pdata->always_on || uc_pdata->boot_on)
dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);- return 0;
}
static int regulator_pre_probe(struct udevice *dev) @@ -546,6 +554,17 @@ static int regulator_pre_probe(struct udevice *dev) return 0; }
+static int regulator_post_probe(struct udevice *dev) +{
- int ret;
- ret = regulator_autoset(dev);
- if (ret == -EMEDIUMTYPE || ret == -ENOSYS)
Would be good to also check for || ret == -EALREADY here.
Following pending patch extend regulator_autoset to also return -EALREADY when it has already been called for a regulator.
power: regulator: Only run autoset once for each regulator https://patchwork.ozlabs.org/patch/1823770/
Regards, Jonas
return 0;- return ret;
+}
int regulators_enable_boot_on(bool verbose) { struct udevice *dev; @@ -603,5 +622,6 @@ UCLASS_DRIVER(regulator) = { .name = "regulator", .post_bind = regulator_post_bind, .pre_probe = regulator_pre_probe,
- .post_probe = regulator_post_probe, .per_device_plat_auto = sizeof(struct dm_regulator_uclass_plat),
};
Hi Svyatoslav,
On Tue, 3 Oct 2023 at 00:21, Svyatoslav Ryhel clamor95@gmail.com wrote:
Regulators initial setup was previously dependent on board call. To move from this behaviour next solution is proposed: on post_bind boot-on/always-on properties are checked, all regulators with such props will be probed just after binding which ensures that essential regulators are set, then in the post probe regulator autoset is called so that correct regulator state according to device tree is reached.
We need a way to do this later, after all devices are probed.
i.e. we must not probe things as we go. There might be other dependencies not yet bound. It may also take some time. This is not following driver model design, sorry.
So please think of a way to do this properly.
One option would be to add a new phase in dm_init_and_scan() which probes devices that want to be probed early.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com [jonas@kwiboo.se: use autoset func, only probe with always/boot-on prop] Signed-off-by: Jonas Karlman jonas@kwiboo.se
drivers/power/regulator/regulator-uclass.c | 30 ++++++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-)
Regards, Simon
Adjust existing tests to pass with autosetup and so test if changes behave as expected.
Signed-off-by: Svyatoslav Ryhel clamor95@gmail.com --- test/dm/regulator.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/test/dm/regulator.c b/test/dm/regulator.c index 33115daaf5..fbbbddbf72 100644 --- a/test/dm/regulator.c +++ b/test/dm/regulator.c @@ -168,7 +168,8 @@ static int dm_test_power_regulator_set_get_enable(struct unit_test_state *uts) /* Set the Enable of LDO1 - default is disabled */ platname = regulator_names[LDO1][PLATNAME]; ut_assertok(regulator_get_by_platname(platname, &dev)); - ut_assertok(regulator_set_enable(dev, val_set)); + /* LDO1 has boot-on property so enable should return -EALREADY */ + ut_asserteq(regulator_set_enable(dev, val_set), -EALREADY);
/* Get the Enable state of LDO1 and compare it with the requested one */ ut_asserteq(regulator_get_enable(dev), val_set); @@ -187,7 +188,8 @@ int dm_test_power_regulator_set_enable_if_allowed(struct unit_test_state *uts)
/* Get BUCK1 - always on regulator */ platname = regulator_names[BUCK1][PLATNAME]; - ut_assertok(regulator_autoset_by_name(platname, &dev_autoset)); + /* BUCK1 has always-on property so autoset should return -EALREADY */ + ut_asserteq(regulator_autoset_by_name(platname, &dev_autoset), -EALREADY); ut_assertok(regulator_get_by_platname(platname, &dev));
/* Try disabling always-on regulator */ @@ -307,7 +309,8 @@ static int dm_test_power_regulator_autoset(struct unit_test_state *uts) * Expected output state: uV=1200000; uA=200000; output enabled */ platname = regulator_names[BUCK1][PLATNAME]; - ut_assertok(regulator_autoset_by_name(platname, &dev_autoset)); + /* BUCK1 has always-on property so autoset should return -EALREADY */ + ut_asserteq(regulator_autoset_by_name(platname, &dev_autoset), -EALREADY);
/* Check, that the returned device is proper */ ut_assertok(regulator_get_by_platname(platname, &dev));
participants (3)
-
Jonas Karlman -
Simon Glass -
Svyatoslav Ryhel