On 10/28/2013 02:53 AM, Thierry Reding wrote: > On Mon, Oct 21, 2013 at 11:34:07AM +0800, Wei Yongjun wrote: >> From: Wei Yongjun <yongjun_wei@xxxxxxxxxxxxxxxxx> >> >> In case of error, the function clk_get_parent() and >> devm_ioremap_resource() returns ERR_PTR() and never returns NULL. >> The NULL test in the return value check should be replaced with >> IS_ERR(). >> >> Signed-off-by: Wei Yongjun <yongjun_wei@xxxxxxxxxxxxxxxxx> --- >> drivers/gpu/drm/tegra/dsi.c | 6 +++--- 1 file changed, 3 >> insertions(+), 3 deletions(-) > > I've applied this, but with the first hunk removed, since looking > at the implementation of clk_get_parent() it can actually return > NULL. In fact it seems like it will never return ERR_PTR(). > > I've also updated the commit message to reflect that. Hmm. The documentation for clk_get() says: /** * clk_get - lookup and obtain a reference to a clock producer. * @dev: device for clock "consumer" * @id: clock consumer ID * * Returns a struct clk corresponding to the clock producer, or * valid IS_ERR() condition containing errno. The implementation * uses @dev and @id to determine the clock consumer, and thereby * the clock producer. (IOW, @id may be identical strings, but * clk_get may return different clock producers depending on @dev.) If the implementation doesn't match that, then it's a bug, and a whole slew of drivers will need changing... On the surface, it looks like the hunk you dropped was correct. NULL may be a perfectly legal return value from a function that returns either valid data or an IS_ERR() value. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html