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

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

On Mon, Oct 28, 2013 at 03:51:32PM -0600, Stephen Warren wrote:
> 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:

The patch didn't check the return value clk_get() but clk_get_parent().
Here's the implementation:

	struct clk *__clk_get_parent(struct clk *clk)
	{
		return !clk ? NULL : clk->parent;
	}

Note that clk_get_parent() in simply a locked version of the above. That
will obviously only return ERR_PTR() if clk->parent happens to be set to
one such value, which I don't think will ever happen.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]