On 10/28/2013 04:38 PM, Thierry Reding wrote:
> 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@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:
>
> 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.
Ah. That looks like a bug in __clk_get_parent() then, since !clk
doesn't sound like the correct error case for it to be checking.
Shouldn't it return IS_ERR(clk) ? clk : clk->parent? Either that, or
clk_get() shouldn't return an error value if the rest of the clock
code doesn NULL checks.
--
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
