[U-Boot] [PATCH v7 08/10] usb: host: ohci-generic: add CLOCK support

[Patrice CHOTARD]

Hi Lothar

On 06/20/2017 02:06 PM, Lothar Waßmann wrote:
> Hi,
> 
> On Tue, 20 Jun 2017 11:59:09 +0200 patrice.chotard at st.com wrote:
>> From: Patrice Chotard <patrice.chotard at st.com>
>>
>> use array to save enabled clocks reference in order to
>> disabled them in case of error during probe() or during
>> driver removal.
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
>> ---
>> v7:	_ replace clk_count() by ofnode_count_phandle_with_args()
>>
>> v6:	_ none
>>
>> v5:	_ none
>>
>> v4:	_ use generic_phy_valid() before generic_phy_exit() call
>>
>> v3:	_ extract in this patch the CLOCK support add-on from previous patch 5
>> 	_ keep enabled clocks reference in list in order to
>> 	  disable clocks in error path or in .remove callback
>>
>> v2:	_ add error path management
>> 	_ add .remove callback
>>
>>   drivers/usb/host/ohci-generic.c | 59 +++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 57 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-generic.c b/drivers/usb/host/ohci-generic.c
>> index f85738f..f76a1c2 100644
>> --- a/drivers/usb/host/ohci-generic.c
>> +++ b/drivers/usb/host/ohci-generic.c
>> @@ -5,27 +5,83 @@
>>    */
>>   
>>   #include <common.h>
>> +#include <clk.h>
>>   #include <dm.h>
>>   #include "ohci.h"
>>   
>> +#include <dm/ofnode.h>
>> +
>>   #if !defined(CONFIG_USB_OHCI_NEW)
>>   # error "Generic OHCI driver requires CONFIG_USB_OHCI_NEW"
>>   #endif
>>   
>>   struct generic_ohci {
>>   	ohci_t ohci;
>> +	struct clk *clocks;
>> +	int clock_count;
>>   };
>>   
>>   static int ohci_usb_probe(struct udevice *dev)
>>   {
>>   	struct ohci_regs *regs = (struct ohci_regs *)devfdt_get_addr(dev);
>> +	struct generic_ohci *priv = dev_get_priv(dev);
>> +	int i, err, ret, clock_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, &priv->clocks[i]);
>> +			if (err < 0)
>> +				break;
>> +
>> +			priv->clock_count++;
>> +
>> +			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]);
>>
> You free the freshly allocated clock right away and use it lateron?

clk_free() didn't free clock resources, it's the opposite of 
clk_request(), see comments in include.clk.h

> 
>> +	} else {
>> +		if (clock_nb != -ENOENT) {
>> +			error("failed to get clock phandle(%d)\n", clock_nb);
>> +			return clock_nb;
>> +		}
>> +	}
>>   
>> -	return ohci_register(dev, regs);
>> +	err = ohci_register(dev, regs);
>> +	if (!err)
>> +		return err;
>> +
> 	if (err)
> 		goto clk_err;
> 	return 0;
> would be more easy to follow.

ok, i will fix it

Thanks

Patrice

> 
>> +clk_err:
>> +	ret = clk_disable_all(priv->clocks, priv->clock_count);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return err;
>>   }
>>   
>>   static int ohci_usb_remove(struct udevice *dev)
>>   {
>> -	return ohci_deregister(dev);
>> +	struct generic_ohci *priv = dev_get_priv(dev);
>> +	int ret;
>> +
>> +	ret = ohci_deregister(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return clk_disable_all(priv->clocks, priv->clock_count);
>>   }
>>   
>>   static const struct udevice_id ohci_usb_ids[] = {
> 
> 
> Lothar Waßmann
>