Re: [U-Boot] [PATCH v3 5/7] usb: host: ohci-generic: add CLOCK support

On 05/17/2017 03:34 PM, patrice.chotard@st.com wrote:

From: Patrice Chotard patrice.chotard@st.com

use list to save reference to enabled clocks in order to disabled them in case of error during probe() or during driver removal.

Signed-off-by: Patrice Chotard patrice.chotard@st.com

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 | 81 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-generic.c b/drivers/usb/host/ohci-generic.c index f3307f4..a6d89a8 100644 --- a/drivers/usb/host/ohci-generic.c +++ b/drivers/usb/host/ohci-generic.c @@ -5,6 +5,7 @@ */

#include <common.h> +#include <clk.h> #include <dm.h> #include "ohci.h"

@@ -12,20 +13,98 @@ # error "Generic OHCI driver requires CONFIG_USB_OHCI_NEW" #endif

+struct ohci_clock {

• struct clk *clk;
• struct list_head list;

+};

struct generic_ohci { ohci_t ohci;

• struct list_head clks;

};

+static int ohci_release_clocks(struct generic_ohci *priv) +{

• struct ohci_clock *ohci_clock, *tmp;
• struct clk *clk;
• int ret;
• list_for_each_entry_safe(ohci_clock, tmp, &priv->clks, list) {

• clk = ohci_clock->clk;
  

• clk_request(clk->dev, clk);
  

• if (ret)
  

• 	return ret;
  

• clk_disable(clk);
  

• ret = clk_free(clk);
  

• if (ret)
  

• 	return ret;
  

• list_del(&ohci_clock->list);
  

• }
• return 0;

+}

static int ohci_usb_probe(struct udevice *dev) { struct ohci_regs *regs = (struct ohci_regs *)dev_get_addr(dev);

• struct generic_ohci *priv = dev_get_priv(dev);
• int i, ret;
• INIT_LIST_HEAD(&priv->clks);
• for (i = 0; ; i++) {

• struct ohci_clock *ohci_clock;
  

• struct clk *clk;
  

• clk = devm_kmalloc(dev, sizeof(*clk), GFP_KERNEL);
  

Since you know how many entries the clock phandle has, you can allocate an array and drop this while list handling and this per-element kmalloc, which fragments the allocator pool.

• if (!clk) {
  

• 	error("Can't allocate resource\n");
  

• 	goto clk_err;
  

• }
  

• ret = clk_get_by_index(dev, i, clk);
  

• if (ret < 0)
  

• 	break;
  

• if (clk_enable(clk)) {
  

• 	error("failed to enable ohci_clock %d\n", i);
  

• 	clk_free(clk);
  

• 	goto clk_err;
  

• }
  

• clk_free(clk);
  

• /*
  

•  * add enabled clocks into clks list in order to be disabled
  

•  * later on ohci_usb_remove() call or in error path if needed
  

•  */
  

• ohci_clock = devm_kmalloc(dev, sizeof(*ohci_clock), GFP_KERNEL);
  

Can't you just embed one structure into the other ?

• if (!ohci_clock) {
  

• 	error("Can't allocate resource\n");
  

• 	goto clk_err;
  

• }
  

• ohci_clock->clk = clk;
  

• list_add(&ohci_clock->list, &priv->clks);
  

• }

  return ohci_register(dev, regs);

+clk_err:

• return ohci_release_clocks(priv);

}

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 ohci_release_clocks(priv);

}

static const struct udevice_id ohci_usb_ids[] = {