Hi Lothar
On 06/21/2017 09:56 AM, Lothar Waßmann wrote:
> Hi,
>
> On Tue, 20 Jun 2017 12:56:48 +0000 Patrice CHOTARD wrote:
>> Hi Lothar
>>
>> On 06/20/2017 02:06 PM, Lothar Waßmann wrote:
>>> Hi,
>>>
>>> On Tue, 20 Jun 2017 11:59:09 +0200 patrice.chot...@st.com wrote:
>>>> From: Patrice Chotard <patrice.chot...@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.chot...@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
>>
> But as long as you are working with some resource (whether it's a
> clock, a gpio, a 'reset' or whatever), you should keep it requested and
> free it only after you relinquished using it.
> Otherwise the object you are using may be destroyed underneath your
> feet.
Ah yes, my bad, i focused on the clk_free() inside the previous statement.
I will fix it
Thanks
>
>
> Lothar Waßmann
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
