Re: [PATCH -next] drm/tegra: fix return value check

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

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-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
>> 
>> 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-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org> --- 
>> 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.