On 01/25/2011 09:28 AM, Julia Lawall wrote: >> Julia is correct. Some architectures can return NULL from clk_get, but I >> didn't check the at91 before posting :-/. If we can't return NULL from >> clk_get then we shouldn't bother checking for it. I do think we should >> drop the !IS_ERR(clk_get(dev, func)) check though. > > It seems a bit subtle, because the clk manipulated by clk_get in the call > of clk_get(dev, func) is not necessarily the same as the one in > clock_associate. But perhaps this is the only possibility in practice? Not sure I follow. The at91 clk_get does not modify the clk. In at91_clock_associate we have: clk->function = func; clk->dev = dev; and in clk_get we have: if (clk->function && (dev == clk->dev) && strcmp(id, clk->function) == 0) return clk; So at91_clock_associate sets the function for a clock, and clk_get returns clocks based on the function association if the name lookup fails. The only caveat to this is that the the clock function name (clk->function) is not the same as any others clock's clk->name. The !IS_ERR(clk_get(dev, func)) check in at91_clock_associate just appears to check that the clock is not already associated with the given function. We don't really need this check because we are just making the same assignment anyway. ~Ryan -- Bluewater Systems Ltd - ARM Technology Solution Centre Ryan Mallon 5 Amuri Park, 404 Barbadoes St ryan@xxxxxxxxxxxxxxxx PO Box 13 889, Christchurch 8013 http://www.bluewatersys.com New Zealand Phone: +64 3 3779127 Freecall: Australia 1800 148 751 Fax: +64 3 3779135 USA 1800 261 2934 -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html