Hi Lukasz On 1/25/20 9:00 AM, Lukasz Majewski wrote: > The commit e8e9715df2d4 ("regulator: fixed: Modify enable-active-high > behavior") > fixed the regulator driver behavior when 'enable-active-high' is defined. > Unfortunately, this patch used dm_regulator_platdata()'s "boot_on" member > to set GPIOD_IS_OUT_ACTIVE flag and enable the regulator. > > The issue here is that regulator_common_ofdata_to_platdata() is called > _before_ regulator_pre_probe() function in which the 'regulator-boot-on' > property is asserted. > > As a result the GPIOD_IS_OUT_ACTIVE flag is not set and gpio_request_by_name() > called in the former function is not enabling the regulator. > This is problematic for e.g. i.MX ethernet driver, which then tries to > perform initialization without power (and fails). > > The solution here is to explicitly enable regulator in regulator_pre_probe() > callback only when 'regulator-boot-on' property is present in device tree. > The GPIOD_IS_OUT_ACTIVE flag is not set at all, but relevant gpio is > requested. > > Signed-off-by: Lukasz Majewski <lu...@denx.de> > --- > > drivers/power/regulator/regulator-uclass.c | 3 +++ > drivers/power/regulator/regulator_common.c | 5 ----- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/power/regulator/regulator-uclass.c > b/drivers/power/regulator/regulator-uclass.c > index 90961de95c..c9d26344d7 100644 > --- a/drivers/power/regulator/regulator-uclass.c > +++ b/drivers/power/regulator/regulator-uclass.c > @@ -464,6 +464,9 @@ static int regulator_pre_probe(struct udevice *dev) > (uc_pdata->min_uA == uc_pdata->max_uA)) > uc_pdata->flags |= REGULATOR_FLAG_AUTOSET_UA; > > + if (uc_pdata->boot_on) > + regulator_set_enable(dev, uc_pdata->boot_on); > + > return 0; > } > > diff --git a/drivers/power/regulator/regulator_common.c > b/drivers/power/regulator/regulator_common.c > index 939efb2c0d..33b73b7c2f 100644 > --- a/drivers/power/regulator/regulator_common.c > +++ b/drivers/power/regulator/regulator_common.c > @@ -12,16 +12,11 @@ int regulator_common_ofdata_to_platdata(struct udevice > *dev, > struct regulator_common_platdata *dev_pdata, const char > *enable_gpio_name) > { > struct gpio_desc *gpio; > - struct dm_regulator_uclass_platdata *uc_pdata; > int flags = GPIOD_IS_OUT; > int ret; > > - uc_pdata = dev_get_uclass_platdata(dev); > - > if (!dev_read_bool(dev, "enable-active-high")) > flags |= GPIOD_ACTIVE_LOW; > - if (uc_pdata->boot_on) > - flags |= GPIOD_IS_OUT_ACTIVE; > > /* Get optional enable GPIO desc */ > gpio = &dev_pdata->gpio;
Reviewed-by: Patrice Chotard <patrice.chot...@st.com> Tested-by: Patrice Chotard <patrice.chot...@st.com> Tested on STM32MP1-ev1. Thanks Patrice