diff mbox series

[v1,3/4] usb: dwc3: exynos: Use devm_regulator_bulk_get_enable() helper function

Message ID 20240301193831.3346-4-linux.amoon@gmail.com
State New
Headers
Series [v1,1/4] usb: ehci-exynos: Use devm_clk_get_enabled() helpers |

Commit Message

Anand Moon March 1, 2024, 7:38 p.m. UTC 
  Use devm_regulator_bulk_get_enable() instead of open coded
'devm_regulator_get(), regulator_enable(), regulator_disable().

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/usb/dwc3/dwc3-exynos.c | 49 +++-------------------------------
 1 file changed, 4 insertions(+), 45 deletions(-)

Comments

Christophe JAILLET March 2, 2024, 3:50 p.m. UTC | #1 
Le 01/03/2024 à 20:38, Anand Moon a écrit :
> Use devm_regulator_bulk_get_enable() instead of open coded
> 'devm_regulator_get(), regulator_enable(), regulator_disable().
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>   drivers/usb/dwc3/dwc3-exynos.c | 49 +++-------------------------------
>   1 file changed, 4 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index 5d365ca51771..7c77f3c69825 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -32,9 +32,6 @@ struct dwc3_exynos {
>   	struct clk		*clks[DWC3_EXYNOS_MAX_CLOCKS];
>   	int			num_clks;
>   	int			suspend_clk_idx;
> -
> -	struct regulator	*vdd33;
> -	struct regulator	*vdd10;
>   };
>   
>   static int dwc3_exynos_probe(struct platform_device *pdev)
> @@ -44,6 +41,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>   	struct device_node	*node = dev->of_node;
>   	const struct dwc3_exynos_driverdata *driver_data;
>   	int			i, ret;
> +	static const char * const regulators[] = { "vdd33", "vdd10" };
>   
>   	exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
>   	if (!exynos)
> @@ -78,27 +76,9 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>   	if (exynos->suspend_clk_idx >= 0)
>   		clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
>   
> -	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> -	if (IS_ERR(exynos->vdd33)) {
> -		ret = PTR_ERR(exynos->vdd33);
> -		goto vdd33_err;
> -	}
> -	ret = regulator_enable(exynos->vdd33);
> -	if (ret) {
> -		dev_err(dev, "Failed to enable VDD33 supply\n");
> -		goto vdd33_err;
> -	}
> -
> -	exynos->vdd10 = devm_regulator_get(dev, "vdd10");
> -	if (IS_ERR(exynos->vdd10)) {
> -		ret = PTR_ERR(exynos->vdd10);
> -		goto vdd10_err;
> -	}
> -	ret = regulator_enable(exynos->vdd10);
> -	if (ret) {
> -		dev_err(dev, "Failed to enable VDD10 supply\n");
> -		goto vdd10_err;
> -	}
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), regulators);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
>   
>   	if (node) {
>   		ret = of_platform_populate(node, NULL, NULL, dev);
> @@ -115,10 +95,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>   	return 0;
>   
>   populate_err:
> -	regulator_disable(exynos->vdd10);
> -vdd10_err:
> -	regulator_disable(exynos->vdd33);
> -vdd33_err:
>   	for (i = exynos->num_clks - 1; i >= 0; i--)
>   		clk_disable_unprepare(exynos->clks[i]);
>   
> @@ -140,9 +116,6 @@ static void dwc3_exynos_remove(struct platform_device *pdev)
>   
>   	if (exynos->suspend_clk_idx >= 0)
>   		clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
> -
> -	regulator_disable(exynos->vdd33);
> -	regulator_disable(exynos->vdd10);
>   }
>   
>   static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
> @@ -196,9 +169,6 @@ static int dwc3_exynos_suspend(struct device *dev)
>   	for (i = exynos->num_clks - 1; i >= 0; i--)
>   		clk_disable_unprepare(exynos->clks[i]);
>   
> -	regulator_disable(exynos->vdd33);
> -	regulator_disable(exynos->vdd10);

Hi,

Same here, I don't think that removing regulator_[en|dis]able from the 
suspend and resume function is correct.

The goal is to stop some hardware when the system is suspended, in order 
to save some power.

Why did you removed it?

CJ

> -
>   	return 0;
>   }
>   
> @@ -207,17 +177,6 @@ static int dwc3_exynos_resume(struct device *dev)
>   	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>   	int i, ret;
>   
> -	ret = regulator_enable(exynos->vdd33);
> -	if (ret) {
> -		dev_err(dev, "Failed to enable VDD33 supply\n");
> -		return ret;
> -	}
> -	ret = regulator_enable(exynos->vdd10);
> -	if (ret) {
> -		dev_err(dev, "Failed to enable VDD10 supply\n");
> -		return ret;
> -	}
> -
>   	for (i = 0; i < exynos->num_clks; i++) {
>   		ret = clk_prepare_enable(exynos->clks[i]);
>   		if (ret) {
Anand Moon March 2, 2024, 4:48 p.m. UTC | #2 
Hi Christophe,

On Sat, 2 Mar 2024 at 21:20, Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 01/03/2024 à 20:38, Anand Moon a écrit :
> > Use devm_regulator_bulk_get_enable() instead of open coded
> > 'devm_regulator_get(), regulator_enable(), regulator_disable().
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >   drivers/usb/dwc3/dwc3-exynos.c | 49 +++-------------------------------
> >   1 file changed, 4 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> > index 5d365ca51771..7c77f3c69825 100644
> > --- a/drivers/usb/dwc3/dwc3-exynos.c
> > +++ b/drivers/usb/dwc3/dwc3-exynos.c
> > @@ -32,9 +32,6 @@ struct dwc3_exynos {
> >       struct clk              *clks[DWC3_EXYNOS_MAX_CLOCKS];
> >       int                     num_clks;
> >       int                     suspend_clk_idx;
> > -
> > -     struct regulator        *vdd33;
> > -     struct regulator        *vdd10;
> >   };
> >
> >   static int dwc3_exynos_probe(struct platform_device *pdev)
> > @@ -44,6 +41,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >       struct device_node      *node = dev->of_node;
> >       const struct dwc3_exynos_driverdata *driver_data;
> >       int                     i, ret;
> > +     static const char * const regulators[] = { "vdd33", "vdd10" };
> >
> >       exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
> >       if (!exynos)
> > @@ -78,27 +76,9 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >       if (exynos->suspend_clk_idx >= 0)
> >               clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
> >
> > -     exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> > -     if (IS_ERR(exynos->vdd33)) {
> > -             ret = PTR_ERR(exynos->vdd33);
> > -             goto vdd33_err;
> > -     }
> > -     ret = regulator_enable(exynos->vdd33);
> > -     if (ret) {
> > -             dev_err(dev, "Failed to enable VDD33 supply\n");
> > -             goto vdd33_err;
> > -     }
> > -
> > -     exynos->vdd10 = devm_regulator_get(dev, "vdd10");
> > -     if (IS_ERR(exynos->vdd10)) {
> > -             ret = PTR_ERR(exynos->vdd10);
> > -             goto vdd10_err;
> > -     }
> > -     ret = regulator_enable(exynos->vdd10);
> > -     if (ret) {
> > -             dev_err(dev, "Failed to enable VDD10 supply\n");
> > -             goto vdd10_err;
> > -     }
> > +     ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), regulators);
> > +     if (ret)
> > +             return dev_err_probe(dev, ret, "Failed to enable regulators\n");
> >
> >       if (node) {
> >               ret = of_platform_populate(node, NULL, NULL, dev);
> > @@ -115,10 +95,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >       return 0;
> >
> >   populate_err:
> > -     regulator_disable(exynos->vdd10);
> > -vdd10_err:
> > -     regulator_disable(exynos->vdd33);
> > -vdd33_err:
> >       for (i = exynos->num_clks - 1; i >= 0; i--)
> >               clk_disable_unprepare(exynos->clks[i]);
> >
> > @@ -140,9 +116,6 @@ static void dwc3_exynos_remove(struct platform_device *pdev)
> >
> >       if (exynos->suspend_clk_idx >= 0)
> >               clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
> > -
> > -     regulator_disable(exynos->vdd33);
> > -     regulator_disable(exynos->vdd10);
> >   }
> >
> >   static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
> > @@ -196,9 +169,6 @@ static int dwc3_exynos_suspend(struct device *dev)
> >       for (i = exynos->num_clks - 1; i >= 0; i--)
> >               clk_disable_unprepare(exynos->clks[i]);
> >
> > -     regulator_disable(exynos->vdd33);
> > -     regulator_disable(exynos->vdd10);
>
> Hi,
>
> Same here, I don't think that removing regulator_[en|dis]able from the
> suspend and resume function is correct.
>
> The goal is to stop some hardware when the system is suspended, in order
> to save some power.
Ok,
>
> Why did you removed it?

As per the description of the function  devm_regulator_bulk_get_enable

* This helper function allows drivers to get several regulator
 * consumers in one operation with management, the regulators will
 * automatically be freed when the device is unbound.  If any of the
 * regulators cannot be acquired then any regulators that were
 * allocated will be freed before returning to the caller.

[0] https://elixir.bootlin.com/linux/latest/source/drivers/regulator/devresc#L330

I have tested with rtc suspend resume and did not find any issue with
this.patch.

>
> CJ
>
Thanks
-Anand
Christophe JAILLET March 2, 2024, 6:37 p.m. UTC | #3 
Le 02/03/2024 à 17:48, Anand Moon a écrit :
> Hi Christophe,
> 
> On Sat, 2 Mar 2024 at 21:20, Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:
>>
>> Le 01/03/2024 à 20:38, Anand Moon a écrit :
>>> Use devm_regulator_bulk_get_enable() instead of open coded
>>> 'devm_regulator_get(), regulator_enable(), regulator_disable().
>>>
>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>> ---
>>>    drivers/usb/dwc3/dwc3-exynos.c | 49 +++-------------------------------
>>>    1 file changed, 4 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
>>> index 5d365ca51771..7c77f3c69825 100644
>>> --- a/drivers/usb/dwc3/dwc3-exynos.c
>>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
>>> @@ -32,9 +32,6 @@ struct dwc3_exynos {
>>>        struct clk              *clks[DWC3_EXYNOS_MAX_CLOCKS];
>>>        int                     num_clks;
>>>        int                     suspend_clk_idx;
>>> -
>>> -     struct regulator        *vdd33;
>>> -     struct regulator        *vdd10;
>>>    };
>>>
>>>    static int dwc3_exynos_probe(struct platform_device *pdev)
>>> @@ -44,6 +41,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>>        struct device_node      *node = dev->of_node;
>>>        const struct dwc3_exynos_driverdata *driver_data;
>>>        int                     i, ret;
>>> +     static const char * const regulators[] = { "vdd33", "vdd10" };
>>>
>>>        exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
>>>        if (!exynos)
>>> @@ -78,27 +76,9 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>>        if (exynos->suspend_clk_idx >= 0)
>>>                clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
>>>
>>> -     exynos->vdd33 = devm_regulator_get(dev, "vdd33");
>>> -     if (IS_ERR(exynos->vdd33)) {
>>> -             ret = PTR_ERR(exynos->vdd33);
>>> -             goto vdd33_err;
>>> -     }
>>> -     ret = regulator_enable(exynos->vdd33);
>>> -     if (ret) {
>>> -             dev_err(dev, "Failed to enable VDD33 supply\n");
>>> -             goto vdd33_err;
>>> -     }
>>> -
>>> -     exynos->vdd10 = devm_regulator_get(dev, "vdd10");
>>> -     if (IS_ERR(exynos->vdd10)) {
>>> -             ret = PTR_ERR(exynos->vdd10);
>>> -             goto vdd10_err;
>>> -     }
>>> -     ret = regulator_enable(exynos->vdd10);
>>> -     if (ret) {
>>> -             dev_err(dev, "Failed to enable VDD10 supply\n");
>>> -             goto vdd10_err;
>>> -     }
>>> +     ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), regulators);
>>> +     if (ret)
>>> +             return dev_err_probe(dev, ret, "Failed to enable regulators\n");
>>>
>>>        if (node) {
>>>                ret = of_platform_populate(node, NULL, NULL, dev);
>>> @@ -115,10 +95,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>>        return 0;
>>>
>>>    populate_err:
>>> -     regulator_disable(exynos->vdd10);
>>> -vdd10_err:
>>> -     regulator_disable(exynos->vdd33);
>>> -vdd33_err:
>>>        for (i = exynos->num_clks - 1; i >= 0; i--)
>>>                clk_disable_unprepare(exynos->clks[i]);
>>>
>>> @@ -140,9 +116,6 @@ static void dwc3_exynos_remove(struct platform_device *pdev)
>>>
>>>        if (exynos->suspend_clk_idx >= 0)
>>>                clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
>>> -
>>> -     regulator_disable(exynos->vdd33);
>>> -     regulator_disable(exynos->vdd10);
>>>    }
>>>
>>>    static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
>>> @@ -196,9 +169,6 @@ static int dwc3_exynos_suspend(struct device *dev)
>>>        for (i = exynos->num_clks - 1; i >= 0; i--)
>>>                clk_disable_unprepare(exynos->clks[i]);
>>>
>>> -     regulator_disable(exynos->vdd33);
>>> -     regulator_disable(exynos->vdd10);
>>
>> Hi,
>>
>> Same here, I don't think that removing regulator_[en|dis]able from the
>> suspend and resume function is correct.
>>
>> The goal is to stop some hardware when the system is suspended, in order
>> to save some power.
> Ok,
>>
>> Why did you removed it?
> 
> As per the description of the function  devm_regulator_bulk_get_enable
> 
> * This helper function allows drivers to get several regulator
>   * consumers in one operation with management, the regulators will
>   * automatically be freed when the device is unbound.  If any of the
>   * regulators cannot be acquired then any regulators that were
>   * allocated will be freed before returning to the caller.

The code in suspend/resume is not about freeing some resources. It is 
about enabling/disabling some hardware to save some power.

Think to the probe/remove functions as the software in the kernel that 
knows how to handle some hardawre, and the suspend/resume as the on/off 
button to power-on and off the electrical chips.

When the system is suspended, the software is still around. But some 
hardware can be set in a low consumption mode to save some power.

IMHO, part of the code you removed changed this behaviour and increase 
the power consumption when the system is suspended.

CJ

> 
> [0] https://elixir.bootlin.com/linux/latest/source/drivers/regulator/devres.c#L330
> 
> I have tested with rtc suspend resume and did not find any issue with
> this.patch.
> 
>>
>> CJ
>>
> Thanks
> -Anand
> 
>
Anand Moon March 4, 2024, 11:46 a.m. UTC | #4 
Hi Christophe,

On Sun, 3 Mar 2024 at 00:07, Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 02/03/2024 à 17:48, Anand Moon a écrit :
> > Hi Christophe,
> >
> > On Sat, 2 Mar 2024 at 21:20, Christophe JAILLET
> > <christophe.jaillet@wanadoo.fr> wrote:
> >>
> >> Le 01/03/2024 à 20:38, Anand Moon a écrit :
> >>> Use devm_regulator_bulk_get_enable() instead of open coded
> >>> 'devm_regulator_get(), regulator_enable(), regulator_disable().
> >>>
> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> >>> ---
> >>>    drivers/usb/dwc3/dwc3-exynos.c | 49 +++-------------------------------
> >>>    1 file changed, 4 insertions(+), 45 deletions(-)
> >>>
> >>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> >>> index 5d365ca51771..7c77f3c69825 100644
> >>> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >>> @@ -32,9 +32,6 @@ struct dwc3_exynos {
> >>>        struct clk              *clks[DWC3_EXYNOS_MAX_CLOCKS];
> >>>        int                     num_clks;
> >>>        int                     suspend_clk_idx;
> >>> -
> >>> -     struct regulator        *vdd33;
> >>> -     struct regulator        *vdd10;
> >>>    };
> >>>
> >>>    static int dwc3_exynos_probe(struct platform_device *pdev)
> >>> @@ -44,6 +41,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>        struct device_node      *node = dev->of_node;
> >>>        const struct dwc3_exynos_driverdata *driver_data;
> >>>        int                     i, ret;
> >>> +     static const char * const regulators[] = { "vdd33", "vdd10" };
> >>>
> >>>        exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
> >>>        if (!exynos)
> >>> @@ -78,27 +76,9 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>        if (exynos->suspend_clk_idx >= 0)
> >>>                clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
> >>>
> >>> -     exynos->vdd33 = devm_regulator_get(dev, "vdd33");
> >>> -     if (IS_ERR(exynos->vdd33)) {
> >>> -             ret = PTR_ERR(exynos->vdd33);
> >>> -             goto vdd33_err;
> >>> -     }
> >>> -     ret = regulator_enable(exynos->vdd33);
> >>> -     if (ret) {
> >>> -             dev_err(dev, "Failed to enable VDD33 supply\n");
> >>> -             goto vdd33_err;
> >>> -     }
> >>> -
> >>> -     exynos->vdd10 = devm_regulator_get(dev, "vdd10");
> >>> -     if (IS_ERR(exynos->vdd10)) {
> >>> -             ret = PTR_ERR(exynos->vdd10);
> >>> -             goto vdd10_err;
> >>> -     }
> >>> -     ret = regulator_enable(exynos->vdd10);
> >>> -     if (ret) {
> >>> -             dev_err(dev, "Failed to enable VDD10 supply\n");
> >>> -             goto vdd10_err;
> >>> -     }
> >>> +     ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), regulators);
> >>> +     if (ret)
> >>> +             return dev_err_probe(dev, ret, "Failed to enable regulators\n");
> >>>
> >>>        if (node) {
> >>>                ret = of_platform_populate(node, NULL, NULL, dev);
> >>> @@ -115,10 +95,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>>        return 0;
> >>>
> >>>    populate_err:
> >>> -     regulator_disable(exynos->vdd10);
> >>> -vdd10_err:
> >>> -     regulator_disable(exynos->vdd33);
> >>> -vdd33_err:
> >>>        for (i = exynos->num_clks - 1; i >= 0; i--)
> >>>                clk_disable_unprepare(exynos->clks[i]);
> >>>
> >>> @@ -140,9 +116,6 @@ static void dwc3_exynos_remove(struct platform_device *pdev)
> >>>
> >>>        if (exynos->suspend_clk_idx >= 0)
> >>>                clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
> >>> -
> >>> -     regulator_disable(exynos->vdd33);
> >>> -     regulator_disable(exynos->vdd10);
> >>>    }
> >>>
> >>>    static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
> >>> @@ -196,9 +169,6 @@ static int dwc3_exynos_suspend(struct device *dev)
> >>>        for (i = exynos->num_clks - 1; i >= 0; i--)
> >>>                clk_disable_unprepare(exynos->clks[i]);
> >>>
> >>> -     regulator_disable(exynos->vdd33);
> >>> -     regulator_disable(exynos->vdd10);
> >>
> >> Hi,
> >>
> >> Same here, I don't think that removing regulator_[en|dis]able from the
> >> suspend and resume function is correct.
> >>
> >> The goal is to stop some hardware when the system is suspended, in order
> >> to save some power.
> > Ok,
> >>
> >> Why did you removed it?
> >
> > As per the description of the function  devm_regulator_bulk_get_enable
> >
> > * This helper function allows drivers to get several regulator
> >   * consumers in one operation with management, the regulators will
> >   * automatically be freed when the device is unbound.  If any of the
> >   * regulators cannot be acquired then any regulators that were
> >   * allocated will be freed before returning to the caller.
>
> The code in suspend/resume is not about freeing some resources. It is
> about enabling/disabling some hardware to save some power.
>
> Think to the probe/remove functions as the software in the kernel that
> knows how to handle some hardawre, and the suspend/resume as the on/off
> button to power-on and off the electrical chips.
>
> When the system is suspended, the software is still around. But some
> hardware can be set in a low consumption mode to save some power.
>
> IMHO, part of the code you removed changed this behaviour and increase
> the power consumption when the system is suspended.
>

You are correct, I have changed the regulator API from
devm_regulator_get_enable to devm_regulator_bulk_get_enable
which changes this behavior.
I will fix it in the next version.

> CJ

Thanks
-Anand
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 5d365ca51771..7c77f3c69825 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -32,9 +32,6 @@  struct dwc3_exynos {
 	struct clk		*clks[DWC3_EXYNOS_MAX_CLOCKS];
 	int			num_clks;
 	int			suspend_clk_idx;
-
-	struct regulator	*vdd33;
-	struct regulator	*vdd10;
 };
 
 static int dwc3_exynos_probe(struct platform_device *pdev)
@@ -44,6 +41,7 @@  static int dwc3_exynos_probe(struct platform_device *pdev)
 	struct device_node	*node = dev->of_node;
 	const struct dwc3_exynos_driverdata *driver_data;
 	int			i, ret;
+	static const char * const regulators[] = { "vdd33", "vdd10" };
 
 	exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
 	if (!exynos)
@@ -78,27 +76,9 @@  static int dwc3_exynos_probe(struct platform_device *pdev)
 	if (exynos->suspend_clk_idx >= 0)
 		clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
 
-	exynos->vdd33 = devm_regulator_get(dev, "vdd33");
-	if (IS_ERR(exynos->vdd33)) {
-		ret = PTR_ERR(exynos->vdd33);
-		goto vdd33_err;
-	}
-	ret = regulator_enable(exynos->vdd33);
-	if (ret) {
-		dev_err(dev, "Failed to enable VDD33 supply\n");
-		goto vdd33_err;
-	}
-
-	exynos->vdd10 = devm_regulator_get(dev, "vdd10");
-	if (IS_ERR(exynos->vdd10)) {
-		ret = PTR_ERR(exynos->vdd10);
-		goto vdd10_err;
-	}
-	ret = regulator_enable(exynos->vdd10);
-	if (ret) {
-		dev_err(dev, "Failed to enable VDD10 supply\n");
-		goto vdd10_err;
-	}
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), regulators);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
 
 	if (node) {
 		ret = of_platform_populate(node, NULL, NULL, dev);
@@ -115,10 +95,6 @@  static int dwc3_exynos_probe(struct platform_device *pdev)
 	return 0;
 
 populate_err:
-	regulator_disable(exynos->vdd10);
-vdd10_err:
-	regulator_disable(exynos->vdd33);
-vdd33_err:
 	for (i = exynos->num_clks - 1; i >= 0; i--)
 		clk_disable_unprepare(exynos->clks[i]);
 
@@ -140,9 +116,6 @@  static void dwc3_exynos_remove(struct platform_device *pdev)
 
 	if (exynos->suspend_clk_idx >= 0)
 		clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
-
-	regulator_disable(exynos->vdd33);
-	regulator_disable(exynos->vdd10);
 }
 
 static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
@@ -196,9 +169,6 @@  static int dwc3_exynos_suspend(struct device *dev)
 	for (i = exynos->num_clks - 1; i >= 0; i--)
 		clk_disable_unprepare(exynos->clks[i]);
 
-	regulator_disable(exynos->vdd33);
-	regulator_disable(exynos->vdd10);
-
 	return 0;
 }
 
@@ -207,17 +177,6 @@  static int dwc3_exynos_resume(struct device *dev)
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
 	int i, ret;
 
-	ret = regulator_enable(exynos->vdd33);
-	if (ret) {
-		dev_err(dev, "Failed to enable VDD33 supply\n");
-		return ret;
-	}
-	ret = regulator_enable(exynos->vdd10);
-	if (ret) {
-		dev_err(dev, "Failed to enable VDD10 supply\n");
-		return ret;
-	}
-
 	for (i = 0; i < exynos->num_clks; i++) {
 		ret = clk_prepare_enable(exynos->clks[i]);
 		if (ret) {