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 <patrice.chot...@st.com>
>>
>> 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 <patrice.chot...@st.com>
> 
> 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 <dm.h>
>>   #include "ehci.h"
>>   
>> +#include <dm/ofnode.h>
>> +
>>   /*
>>    * 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, &clk);
>> -            if (ret < 0)
>> -                    break;
>> -            if (clk_enable(&clk))
>> -                    error("failed to enable clock %d\n", i);
>> -            clk_free(&clk);
>> -    }
>> +    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, &priv->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, &reset);
>> -            if (ret < 0)
>> -                    break;
>> -            if (reset_deassert(&reset))
>> -                    error("failed to deassert reset %d\n", i);
>> -            reset_free(&reset);
>> +                    if (clk_enable(&priv->clocks[i])) {
>> +                            error("failed to enable clock %d\n", i);
>> +                            clk_free(&priv->clocks[i]);
>> +                            goto clk_err;
>> +                    }
>> +                    clk_free(&priv->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, &priv->resets[i]);
>> +                    if (err < 0)
>> +                            break;
>> +
>> +                    priv->reset_count++;
>> +
>> +                    if (reset_deassert(&priv->resets[i])) {
>> +                            error("failed to deassert reset %d\n", i);
>> +                            reset_free(&priv->resets[i]);
>> +                            goto reset_err;
>> +                    }
>> +                    reset_free(&priv->resets[i]);
>> +            }
>> +    } else {
>> +            if (reset_nb != -ENOENT) {
>> +                    error("failed to get reset phandle(%d)\n", reset_nb);
>> +                    goto clk_err;
>> +            }
>> +    }
> 
> Newline

ok

> 
>>      hccr = map_physmem(devfdt_get_addr(dev), 0x100, MAP_NOCACHE);
>>      hcor = (struct ehci_hcor *)((uintptr_t)hccr +
>>                                  HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
>>   
>> -    return ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST);
>> +    err = ehci_register(dev, hccr, hcor, NULL, 0, USB_INIT_HOST);
>> +    if (!err)
>> +            return err;
>> +
>> +reset_err:
>> +    ret = reset_assert_all(priv->resets, priv->reset_count);
>> +    if (ret)
>> +            return ret;
>> +clk_err:
>> +    ret = clk_disable_all(priv->clocks, priv->clock_count);
>> +    if (ret)
>> +            return ret;
>> +
>> +    return err;
>> +}
>> +
>> +static int ehci_usb_remove(struct udevice *dev)
>> +{
>> +    struct generic_ehci *priv = dev_get_priv(dev);
>> +    int ret;
>> +
>> +    ret = ehci_deregister(dev);
>> +    if (ret)
>> +            return ret;
>> +
>> +    ret =  reset_assert_all(priv->resets, priv->reset_count);
>> +    if (ret)
>> +            return ret;
>> +
>> +    return clk_disable_all(priv->clocks, priv->clock_count);
>>   }
>>   
>>   static const struct udevice_id ehci_usb_ids[] = {
>> @@ -67,7 +145,7 @@ U_BOOT_DRIVER(ehci_generic) = {
>>      .id     = UCLASS_USB,
>>      .of_match = ehci_usb_ids,
>>      .probe = ehci_usb_probe,
>> -    .remove = ehci_deregister,
>> +    .remove = ehci_usb_remove,
>>      .ops    = &ehci_usb_ops,
>>      .priv_auto_alloc_size = sizeof(struct generic_ehci),
>>      .flags  = DM_FLAG_ALLOC_PRIV_DMA,
>>
> 
> 
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to