Re: [U-Boot] [PATCH v7 06/10] usb: host: ehci-generic: add error path and .remove callback

[Patrice CHOTARD]

Hi Lothar

On 06/20/2017 01:32 PM, Lothar Waßmann wrote:
> Hi,
> 
> On Tue, 20 Jun 2017 11:59:07 +0200 patrice.chot...@st.com wrote:
>> From: Patrice Chotard 
>>
>> Use an array to save enabled clocks reference and deasserted resets
>> in order to respectively disabled and asserted them in case of error
>> during probe() or during driver removal.
>>
>> Signed-off-by: Patrice Chotard 
>> ---
>>
>> v7:  _ replace clk_count() and reset_count() by 
>> ofnode_count_phandle_with_args()
>>
>> v6:  _ none
>>
>> v5:  _ none
>>
>> v4:  _ update the memory allocation for deasserted resets and enabled
>>clocks reference list. Replace lists by arrays.
>>  _ usage of new RESET and CLOCK methods clk_count(), reset_count(),
>>reset_assert_all() and clk_disable_all().
>>
>> v3:  _ keep enabled clocks and deasserted resets reference in list in order 
>> to
>>disable clock or assert resets in error path or in .remove callback
>>  _ use struct generic_ehci * instead of struct udevice * as parameter for
>>ehci_release_resets() and ehci_release_clocks()
>>
>>   drivers/usb/host/ehci-generic.c | 125 
>> 
>>   1 file changed, 101 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-generic.c 
>> b/drivers/usb/host/ehci-generic.c
>> index 2116ae1..388b242 100644
>> --- a/drivers/usb/host/ehci-generic.c
>> +++ b/drivers/usb/host/ehci-generic.c
>> @@ -11,6 +11,8 @@
>>   #include 
>>   #include "ehci.h"
>>   
>> +#include 
>> +
>>   /*
>>* Even though here we don't explicitly use "struct ehci_ctrl"
>>* ehci_register() expects it to be the first thing that resides in
>> @@ -18,43 +20,119 @@
>>*/
>>   struct generic_ehci {
>>  struct ehci_ctrl ctrl;
>> +struct clk *clocks;
>> +struct reset_ctl *resets;
>> +int clock_count;
>> +int reset_count;
>>   };
>>   
>>   static int ehci_usb_probe(struct udevice *dev)
>>   {
>> +struct generic_ehci *priv = dev_get_priv(dev);
>>  struct ehci_hccr *hccr;
>>  struct ehci_hcor *hcor;
>> -int i;
>> -
>> -for (i = 0; ; i++) {
>> -struct clk clk;
>> -int ret;
>> -
>> -ret = clk_get_by_index(dev, i, );
>> -if (ret < 0)
>> -break;
>> -if (clk_enable())
>> -error("failed to enable clock %d\n", i);
>> -clk_free();
>> -}
>> +int i, err, ret, clock_nb, reset_nb;
>> +
>> +err = 0;
>> +priv->clock_count = 0;
>> +clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks",
>> +  "#clock-cells");
>> +if (clock_nb > 0) {
>> +priv->clocks = devm_kmalloc(dev, sizeof(struct clk) * clock_nb,
>> +GFP_KERNEL);
>> +if (!priv->clocks) {
>> +error("Can't allocate resource\n");
>> +return -ENOMEM;
>> +}
>> +
>> +for (i = 0; i < clock_nb; i++) {
>> +err = clk_get_by_index(dev, i, >clocks[i]);
>> +
>> +if (err < 0)
>> +break;
>>   
>> -for (i = 0; ; i++) {
>> -struct reset_ctl reset;
>> -int ret;
>> +priv->clock_count++;
>>   
>> -ret = reset_get_by_index(dev, i, );
>> -if (ret < 0)
>> -break;
>> -if (reset_deassert())
>> -error("failed to deassert reset %d\n", i);
>> -reset_free();
>> +if (clk_enable(>clocks[i])) {
>> +error("failed to enable clock %d\n", i);
>> +clk_free(>clocks[i]);
>>
> You free the clock here and again with clk_disable_all() in the error
> path. Also you do not have a valid error code in 'err' which is
> returned at the end of the function!

Good catch, to fix it, i will move the priv->clock_count++ after the 
clk_enable statement. In this case, in the error path, only clocks which 
have been previously correctly enabled will be requested/disabled/freed

Ok i will catch the error code in err and use it as return value.


> 
>> +goto clk_err;
>> +}
>> +clk_free(>clocks[i]);
>> +}
>> +} else {
>> +if (clock_nb != -ENOENT) {
>> +error("failed to get clock phandle(%d)\n", clock_nb);
>> +return clock_nb;
>> +}
>>  }
>>   
>> +priv->reset_count = 0;
>> +reset_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "resets",
>> +  "#reset-cells");
>> +if (reset_nb > 0) {
>> +priv->resets = devm_kmalloc(dev,
>> +sizeof(struct reset_ctl) * reset_nb,
>> +   

Re: [U-Boot] [PATCH v7 06/10] usb: host: ehci-generic: add error path and .remove callback

[Patrice CHOTARD]

Hi Marek

On 06/20/2017 12:09 PM, Marek Vasut wrote:
> On 06/20/2017 11:59 AM, patrice.chot...@st.com wrote:
>> From: Patrice Chotard 
>>
>> Use an array to save enabled clocks reference and deasserted resets
>> in order to respectively disabled and asserted them in case of error
>> during probe() or during driver removal.
>>
>> Signed-off-by: Patrice Chotard 
> 
> Nitpicks below
> 
>> ---
>>
>> v7:  _ replace clk_count() and reset_count() by 
>> ofnode_count_phandle_with_args()
>>
>> v6:  _ none
>>
>> v5:  _ none
>>
>> v4:  _ update the memory allocation for deasserted resets and enabled
>>clocks reference list. Replace lists by arrays.
>>  _ usage of new RESET and CLOCK methods clk_count(), reset_count(),
>>reset_assert_all() and clk_disable_all().
>>
>> v3:  _ keep enabled clocks and deasserted resets reference in list in order 
>> to
>>disable clock or assert resets in error path or in .remove callback
>>  _ use struct generic_ehci * instead of struct udevice * as parameter for
>>ehci_release_resets() and ehci_release_clocks()
>>
>>   drivers/usb/host/ehci-generic.c | 125 
>> 
>>   1 file changed, 101 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-generic.c 
>> b/drivers/usb/host/ehci-generic.c
>> index 2116ae1..388b242 100644
>> --- a/drivers/usb/host/ehci-generic.c
>> +++ b/drivers/usb/host/ehci-generic.c
>> @@ -11,6 +11,8 @@
>>   #include 
>>   #include "ehci.h"
>>   
>> +#include 
>> +
>>   /*
>>* Even though here we don't explicitly use "struct ehci_ctrl"
>>* ehci_register() expects it to be the first thing that resides in
>> @@ -18,43 +20,119 @@
>>*/
>>   struct generic_ehci {
>>  struct ehci_ctrl ctrl;
>> +struct clk *clocks;
>> +struct reset_ctl *resets;
>> +int clock_count;
>> +int reset_count;
>>   };
>>   
>>   static int ehci_usb_probe(struct udevice *dev)
>>   {
>> +struct generic_ehci *priv = dev_get_priv(dev);
>>  struct ehci_hccr *hccr;
>>  struct ehci_hcor *hcor;
>> -int i;
>> -
>> -for (i = 0; ; i++) {
>> -struct clk clk;
>> -int ret;
>> -
>> -ret = clk_get_by_index(dev, i, );
>> -if (ret < 0)
>> -break;
>> -if (clk_enable())
>> -error("failed to enable clock %d\n", i);
>> -clk_free();
>> -}
>> +int i, err, ret, clock_nb, reset_nb;
>> +
>> +err = 0;
>> +priv->clock_count = 0;
>> +clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks",
>> +  "#clock-cells");
>> +if (clock_nb > 0) {
>> +priv->clocks = devm_kmalloc(dev, sizeof(struct clk) * clock_nb,
>> +GFP_KERNEL);
> 
> devm_kzalloc()

Ok

> 
>> +if (!priv->clocks) {
>> +error("Can't allocate resource\n");
> 
> If you run out of memory, you're screwed anyway, drop this print.

Ok, i will remove it

> 
>> +return -ENOMEM;
>> +}
>> +
>> +for (i = 0; i < clock_nb; i++) {
>> +err = clk_get_by_index(dev, i, >clocks[i]);
>> +
>> +if (err < 0)
>> +break;
>>   
>> -for (i = 0; ; i++) {
>> -struct reset_ctl reset;
>> -int ret;
>> +priv->clock_count++;
>>   
>> -ret = reset_get_by_index(dev, i, );
>> -if (ret < 0)
>> -break;
>> -if (reset_deassert())
>> -error("failed to deassert reset %d\n", i);
>> -reset_free();
>> +if (clk_enable(>clocks[i])) {
>> +error("failed to enable clock %d\n", i);
>> +clk_free(>clocks[i]);
>> +goto clk_err;
>> +}
>> +clk_free(>clocks[i]);
>> +}
>> +} else {
>> +if (clock_nb != -ENOENT) {
>> +error("failed to get clock phandle(%d)\n", clock_nb);
>> +return clock_nb;
>> +}
>>  }
>>   
>> +priv->reset_count = 0;
>> +reset_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "resets",
>> +  "#reset-cells");
>> +if (reset_nb > 0) {
>> +priv->resets = devm_kmalloc(dev,
>> +sizeof(struct reset_ctl) * reset_nb,
>> +GFP_KERNEL);
>> +if (!priv->resets) {
>> +error("Can't allocate resource\n");
>> +return -ENOMEM;
>> +}
>> +
>> +for (i = 0; i < reset_nb; i++) {
>> +err = reset_get_by_index(dev, i, >resets[i]);
>> +if (err < 0)
>> 

Re: [U-Boot] [PATCH v7 06/10] usb: host: ehci-generic: add error path and .remove callback

[Lothar Waßmann]

Hi,

On Tue, 20 Jun 2017 11:59:07 +0200 patrice.chot...@st.com wrote:
> From: Patrice Chotard 
> 
> Use an array to save enabled clocks reference and deasserted resets
> in order to respectively disabled and asserted them in case of error
> during probe() or during driver removal.
> 
> Signed-off-by: Patrice Chotard 
> ---
> 
> v7:   _ replace clk_count() and reset_count() by 
> ofnode_count_phandle_with_args()
> 
> v6:   _ none
> 
> v5:   _ none
> 
> v4:   _ update the memory allocation for deasserted resets and enabled
> clocks reference list. Replace lists by arrays.
>   _ usage of new RESET and CLOCK methods clk_count(), reset_count(),
> reset_assert_all() and clk_disable_all().
> 
> v3:   _ keep enabled clocks and deasserted resets reference in list in order 
> to
> disable clock or assert resets in error path or in .remove callback
>   _ use struct generic_ehci * instead of struct udevice * as parameter for
> ehci_release_resets() and ehci_release_clocks()
> 
>  drivers/usb/host/ehci-generic.c | 125 
> 
>  1 file changed, 101 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c
> index 2116ae1..388b242 100644
> --- a/drivers/usb/host/ehci-generic.c
> +++ b/drivers/usb/host/ehci-generic.c
> @@ -11,6 +11,8 @@
>  #include 
>  #include "ehci.h"
>  
> +#include 
> +
>  /*
>   * Even though here we don't explicitly use "struct ehci_ctrl"
>   * ehci_register() expects it to be the first thing that resides in
> @@ -18,43 +20,119 @@
>   */
>  struct generic_ehci {
>   struct ehci_ctrl ctrl;
> + struct clk *clocks;
> + struct reset_ctl *resets;
> + int clock_count;
> + int reset_count;
>  };
>  
>  static int ehci_usb_probe(struct udevice *dev)
>  {
> + struct generic_ehci *priv = dev_get_priv(dev);
>   struct ehci_hccr *hccr;
>   struct ehci_hcor *hcor;
> - int i;
> -
> - for (i = 0; ; i++) {
> - struct clk clk;
> - int ret;
> -
> - ret = clk_get_by_index(dev, i, );
> - if (ret < 0)
> - break;
> - if (clk_enable())
> - error("failed to enable clock %d\n", i);
> - clk_free();
> - }
> + int i, err, ret, clock_nb, reset_nb;
> +
> + err = 0;
> + priv->clock_count = 0;
> + clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks",
> +   "#clock-cells");
> + if (clock_nb > 0) {
> + priv->clocks = devm_kmalloc(dev, sizeof(struct clk) * clock_nb,
> + GFP_KERNEL);
> + if (!priv->clocks) {
> + error("Can't allocate resource\n");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < clock_nb; i++) {
> + err = clk_get_by_index(dev, i, >clocks[i]);
> +
> + if (err < 0)
> + break;
>  
> - for (i = 0; ; i++) {
> - struct reset_ctl reset;
> - int ret;
> + priv->clock_count++;
>  
> - ret = reset_get_by_index(dev, i, );
> - if (ret < 0)
> - break;
> - if (reset_deassert())
> - error("failed to deassert reset %d\n", i);
> - reset_free();
> + if (clk_enable(>clocks[i])) {
> + error("failed to enable clock %d\n", i);
> + clk_free(>clocks[i]);
>
You free the clock here and again with clk_disable_all() in the error
path. Also you do not have a valid error code in 'err' which is
returned at the end of the function!

> + goto clk_err;
> + }
> + clk_free(>clocks[i]);
> + }
> + } else {
> + if (clock_nb != -ENOENT) {
> + error("failed to get clock phandle(%d)\n", clock_nb);
> + return clock_nb;
> + }
>   }
>  
> + priv->reset_count = 0;
> + reset_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "resets",
> +   "#reset-cells");
> + if (reset_nb > 0) {
> + priv->resets = devm_kmalloc(dev,
> + sizeof(struct reset_ctl) * reset_nb,
> + GFP_KERNEL);
> + if (!priv->resets) {
> + error("Can't allocate resource\n");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < reset_nb; i++) {
> + err = reset_get_by_index(dev, i, >resets[i]);
> + if (err < 0)
> + break;
> +
> + 

Re: [U-Boot] [PATCH v7 06/10] usb: host: ehci-generic: add error path and .remove callback

[Lothar Waßmann]

Hi,

On Tue, 20 Jun 2017 12:09:22 +0200 Marek Vasut wrote:
> On 06/20/2017 11:59 AM, patrice.chot...@st.com wrote:
> > From: Patrice Chotard 
> > 
> > Use an array to save enabled clocks reference and deasserted resets
> > in order to respectively disabled and asserted them in case of error
> > during probe() or during driver removal.
> > 
> > Signed-off-by: Patrice Chotard 
> 
> Nitpicks below
> 
> > ---
> > 
> > v7: _ replace clk_count() and reset_count() by 
> > ofnode_count_phandle_with_args()
> > 
> > v6: _ none
> > 
> > v5: _ none
> > 
> > v4: _ update the memory allocation for deasserted resets and enabled
> >   clocks reference list. Replace lists by arrays.
> > _ usage of new RESET and CLOCK methods clk_count(), reset_count(),
> >   reset_assert_all() and clk_disable_all().
> > 
> > v3: _ keep enabled clocks and deasserted resets reference in list in order 
> > to
> >   disable clock or assert resets in error path or in .remove callback
> > _ use struct generic_ehci * instead of struct udevice * as parameter for
> >   ehci_release_resets() and ehci_release_clocks()
> > 
> >  drivers/usb/host/ehci-generic.c | 125 
> > 
> >  1 file changed, 101 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/usb/host/ehci-generic.c 
> > b/drivers/usb/host/ehci-generic.c
> > index 2116ae1..388b242 100644
> > --- a/drivers/usb/host/ehci-generic.c
> > +++ b/drivers/usb/host/ehci-generic.c
> > @@ -11,6 +11,8 @@
> >  #include 
> >  #include "ehci.h"
> >  
> > +#include 
> > +
> >  /*
> >   * Even though here we don't explicitly use "struct ehci_ctrl"
> >   * ehci_register() expects it to be the first thing that resides in
> > @@ -18,43 +20,119 @@
> >   */
> >  struct generic_ehci {
> > struct ehci_ctrl ctrl;
> > +   struct clk *clocks;
> > +   struct reset_ctl *resets;
> > +   int clock_count;
> > +   int reset_count;
> >  };
> >  
> >  static int ehci_usb_probe(struct udevice *dev)
> >  {
> > +   struct generic_ehci *priv = dev_get_priv(dev);
> > struct ehci_hccr *hccr;
> > struct ehci_hcor *hcor;
> > -   int i;
> > -
> > -   for (i = 0; ; i++) {
> > -   struct clk clk;
> > -   int ret;
> > -
> > -   ret = clk_get_by_index(dev, i, );
> > -   if (ret < 0)
> > -   break;
> > -   if (clk_enable())
> > -   error("failed to enable clock %d\n", i);
> > -   clk_free();
> > -   }
> > +   int i, err, ret, clock_nb, reset_nb;
> > +
> > +   err = 0;
> > +   priv->clock_count = 0;
> > +   clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks",
> > + "#clock-cells");
> > +   if (clock_nb > 0) {
> > +   priv->clocks = devm_kmalloc(dev, sizeof(struct clk) * clock_nb,
> > +   GFP_KERNEL);
> 
> devm_kzalloc()
>
devm_kcalloc(dev, clock_nb, sizeof()...)


Lothar Waßmann
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v7 06/10] usb: host: ehci-generic: add error path and .remove callback

[Marek Vasut]

On 06/20/2017 11:59 AM, patrice.chot...@st.com wrote:
> From: Patrice Chotard 
> 
> Use an array to save enabled clocks reference and deasserted resets
> in order to respectively disabled and asserted them in case of error
> during probe() or during driver removal.
> 
> Signed-off-by: Patrice Chotard 

Nitpicks below

> ---
> 
> v7:   _ replace clk_count() and reset_count() by 
> ofnode_count_phandle_with_args()
> 
> v6:   _ none
> 
> v5:   _ none
> 
> v4:   _ update the memory allocation for deasserted resets and enabled
> clocks reference list. Replace lists by arrays.
>   _ usage of new RESET and CLOCK methods clk_count(), reset_count(),
> reset_assert_all() and clk_disable_all().
> 
> v3:   _ keep enabled clocks and deasserted resets reference in list in order 
> to
> disable clock or assert resets in error path or in .remove callback
>   _ use struct generic_ehci * instead of struct udevice * as parameter for
> ehci_release_resets() and ehci_release_clocks()
> 
>  drivers/usb/host/ehci-generic.c | 125 
> 
>  1 file changed, 101 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c
> index 2116ae1..388b242 100644
> --- a/drivers/usb/host/ehci-generic.c
> +++ b/drivers/usb/host/ehci-generic.c
> @@ -11,6 +11,8 @@
>  #include 
>  #include "ehci.h"
>  
> +#include 
> +
>  /*
>   * Even though here we don't explicitly use "struct ehci_ctrl"
>   * ehci_register() expects it to be the first thing that resides in
> @@ -18,43 +20,119 @@
>   */
>  struct generic_ehci {
>   struct ehci_ctrl ctrl;
> + struct clk *clocks;
> + struct reset_ctl *resets;
> + int clock_count;
> + int reset_count;
>  };
>  
>  static int ehci_usb_probe(struct udevice *dev)
>  {
> + struct generic_ehci *priv = dev_get_priv(dev);
>   struct ehci_hccr *hccr;
>   struct ehci_hcor *hcor;
> - int i;
> -
> - for (i = 0; ; i++) {
> - struct clk clk;
> - int ret;
> -
> - ret = clk_get_by_index(dev, i, );
> - if (ret < 0)
> - break;
> - if (clk_enable())
> - error("failed to enable clock %d\n", i);
> - clk_free();
> - }
> + int i, err, ret, clock_nb, reset_nb;
> +
> + err = 0;
> + priv->clock_count = 0;
> + clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks",
> +   "#clock-cells");
> + if (clock_nb > 0) {
> + priv->clocks = devm_kmalloc(dev, sizeof(struct clk) * clock_nb,
> + GFP_KERNEL);

devm_kzalloc()

> + if (!priv->clocks) {
> + error("Can't allocate resource\n");

If you run out of memory, you're screwed anyway, drop this print.

> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < clock_nb; i++) {
> + err = clk_get_by_index(dev, i, >clocks[i]);
> +
> + if (err < 0)
> + break;
>  
> - for (i = 0; ; i++) {
> - struct reset_ctl reset;
> - int ret;
> + priv->clock_count++;
>  
> - ret = reset_get_by_index(dev, i, );
> - if (ret < 0)
> - break;
> - if (reset_deassert())
> - error("failed to deassert reset %d\n", i);
> - reset_free();
> + if (clk_enable(>clocks[i])) {
> + error("failed to enable clock %d\n", i);
> + clk_free(>clocks[i]);
> + goto clk_err;
> + }
> + clk_free(>clocks[i]);
> + }
> + } else {
> + if (clock_nb != -ENOENT) {
> + error("failed to get clock phandle(%d)\n", clock_nb);
> + return clock_nb;
> + }
>   }
>  
> + priv->reset_count = 0;
> + reset_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "resets",
> +   "#reset-cells");
> + if (reset_nb > 0) {
> + priv->resets = devm_kmalloc(dev,
> + sizeof(struct reset_ctl) * reset_nb,
> + GFP_KERNEL);
> + if (!priv->resets) {
> + error("Can't allocate resource\n");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < reset_nb; i++) {
> + err = reset_get_by_index(dev, i, >resets[i]);
> + if (err < 0)
> + break;
> +
> + priv->reset_count++;
> +
> + if (reset_deassert(>resets[i])) {
> +

[U-Boot] [PATCH v7 06/10] usb: host: ehci-generic: add error path and .remove callback

[patrice.chotard]

From: Patrice Chotard 

Use an array to save enabled clocks reference and deasserted resets
in order to respectively disabled and asserted them in case of error
during probe() or during driver removal.

Signed-off-by: Patrice Chotard 
---

v7: _ replace clk_count() and reset_count() by 
ofnode_count_phandle_with_args()

v6: _ none

v5: _ none

v4: _ update the memory allocation for deasserted resets and enabled
  clocks reference list. Replace lists by arrays.
_ usage of new RESET and CLOCK methods clk_count(), reset_count(),
  reset_assert_all() and clk_disable_all().

v3: _ keep enabled clocks and deasserted resets reference in list in order 
to
  disable clock or assert resets in error path or in .remove callback
_ use struct generic_ehci * instead of struct udevice * as parameter for
  ehci_release_resets() and ehci_release_clocks()

 drivers/usb/host/ehci-generic.c | 125 
 1 file changed, 101 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c
index 2116ae1..388b242 100644
--- a/drivers/usb/host/ehci-generic.c
+++ b/drivers/usb/host/ehci-generic.c
@@ -11,6 +11,8 @@
 #include 
 #include "ehci.h"
 
+#include 
+
 /*
  * Even though here we don't explicitly use "struct ehci_ctrl"
  * ehci_register() expects it to be the first thing that resides in
@@ -18,43 +20,119 @@
  */
 struct generic_ehci {
struct ehci_ctrl ctrl;
+   struct clk *clocks;
+   struct reset_ctl *resets;
+   int clock_count;
+   int reset_count;
 };
 
 static int ehci_usb_probe(struct udevice *dev)
 {
+   struct generic_ehci *priv = dev_get_priv(dev);
struct ehci_hccr *hccr;
struct ehci_hcor *hcor;
-   int i;
-
-   for (i = 0; ; i++) {
-   struct clk clk;
-   int ret;
-
-   ret = clk_get_by_index(dev, i, );
-   if (ret < 0)
-   break;
-   if (clk_enable())
-   error("failed to enable clock %d\n", i);
-   clk_free();
-   }
+   int i, err, ret, clock_nb, reset_nb;
+
+   err = 0;
+   priv->clock_count = 0;
+   clock_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "clocks",
+ "#clock-cells");
+   if (clock_nb > 0) {
+   priv->clocks = devm_kmalloc(dev, sizeof(struct clk) * clock_nb,
+   GFP_KERNEL);
+   if (!priv->clocks) {
+   error("Can't allocate resource\n");
+   return -ENOMEM;
+   }
+
+   for (i = 0; i < clock_nb; i++) {
+   err = clk_get_by_index(dev, i, >clocks[i]);
+
+   if (err < 0)
+   break;
 
-   for (i = 0; ; i++) {
-   struct reset_ctl reset;
-   int ret;
+   priv->clock_count++;
 
-   ret = reset_get_by_index(dev, i, );
-   if (ret < 0)
-   break;
-   if (reset_deassert())
-   error("failed to deassert reset %d\n", i);
-   reset_free();
+   if (clk_enable(>clocks[i])) {
+   error("failed to enable clock %d\n", i);
+   clk_free(>clocks[i]);
+   goto clk_err;
+   }
+   clk_free(>clocks[i]);
+   }
+   } else {
+   if (clock_nb != -ENOENT) {
+   error("failed to get clock phandle(%d)\n", clock_nb);
+   return clock_nb;
+   }
}
 
+   priv->reset_count = 0;
+   reset_nb = ofnode_count_phandle_with_args(dev_ofnode(dev), "resets",
+ "#reset-cells");
+   if (reset_nb > 0) {
+   priv->resets = devm_kmalloc(dev,
+   sizeof(struct reset_ctl) * reset_nb,
+   GFP_KERNEL);
+   if (!priv->resets) {
+   error("Can't allocate resource\n");
+   return -ENOMEM;
+   }
+
+   for (i = 0; i < reset_nb; i++) {
+   err = reset_get_by_index(dev, i, >resets[i]);
+   if (err < 0)
+   break;
+
+   priv->reset_count++;
+
+   if (reset_deassert(>resets[i])) {
+   error("failed to deassert reset %d\n", i);
+   reset_free(>resets[i]);
+   goto reset_err;
+   }
+   reset_free(>resets[i]);
+   }
+