On 7/11/2018 10:01 AM, Sudeep Holla wrote:

On 11/07/18 17:48, Ray Jui wrote:

quoted

On 7/11/2018 5:34 AM, Wei Yongjun wrote:

quoted

platform_get_resource() may fail and return NULL, so we should
better check it's return value to avoid a NULL pointer dereference
a bit later in the code.

This is detected by Coccinelle semantic patch.

@@
expression pdev, res, n, t, e, e1, e2;
@@

res = platform_get_resource(pdev, t, n);
+ if (!res)
+?? return -EINVAL;
... when != res == NULL
e = devm_ioremap_nocache(e1, res->start, e2);

Signed-off-by: Wei Yongjun <redacted>
---

Reviewed-by: Ray Jui <redacted>

Change looks good to me, although the check could have been avoided if
'devm_ioremap_resource' is used on the next line instead of
'devm_ioremap_nocache', where validation of resource pointer is done.

But there's probably a reason why 'devm_ioremap_nocache' was used in
this code here.

I am not sure about that. Both ARM and ARM64 has same definition as
ioremp. However, arch/arm/include/asm/io.h do mention:
"ioremap_nocache() is the same as ioremap() as there are too many device

  drivers using this for device registers, and documentation which tells

  people to use it for such for this to be any different."

You could technically use devm_ioremap_resource if you want.

I did not mean the difference on _nocache, which I'm aware it's the same 
on ARM/ARM64 based platforms.

I meant there's a reason why xxx_resource was not used, which is most 
likely due to some resource conflict with another driver on NSP.

Ray