On 05/19/2017 01:51 PM, Marek Vasut wrote:
On 05/19/2017 01:27 PM, Patrice CHOTARD wrote:
Hi Marek
On 05/18/2017 11:29 AM, Marek Vasut wrote:
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.
I disagree, at this point we don't know how many entries the clock phandle has.
I know you can do it in less then 2 mallocs, in fact in exactly 1 , which is already better.
Can you elaborate ?
But, following your idea to avoid allocator pool fragmentation, in order to get the phandle number for array allocation, i can, for example add a new fdt API :
int fdtdec_get_phandle_nb(const void *blob, int src_node, const char *list_name, const char *cells_name, int cell_count, int phandle_nb);
Uh, why ?
Then, with phandle_nb,, we 'll be able to allocate the right array size for clocks and resets before populating them.
Just query the number of elements up front and then allocate the array ?
Can you indicate me with which API please ?
Thanks
Patrice
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[] = {