[PATCH 2/4] arch/arm/mach-at91/clock.c: Add missing IS_ERR test
julia at diku.dk
Mon Jan 24 16:01:46 EST 2011
On Tue, 25 Jan 2011, Ryan Mallon wrote:
> 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.
Right, that was what I was worried about. That one would find the same
information already present but somewhere else. But perhaps it can't
happen, or it doesn't matter if it does?
> 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.
> Bluewater Systems Ltd - ARM Technology Solution Centre
> Ryan Mallon 5 Amuri Park, 404 Barbadoes St
> ryan at bluewatersys.com 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 at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
More information about the linux-arm-kernel