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.


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

Reply via email to