[PATCH -next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[PATCH -next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[weiyj_lk at 163.com]

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

In case of error, the function of_platform_device_create() returns
NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
check should be replaced with NULL test.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/iommu/exynos-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 33dcc29..9b23059 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
 		exynos_iommu_init();
 
 	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
+	if (!pdev)
+		return -ENOMEM;
 
 	/*
 	 * use the first registered sysmmu device for performing

[PATCH -next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[Dennis Chen]

On Wed, Jul 06, 2016 at 12:15:54PM +0000, weiyj_lk at 163.com wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> In case of error, the function of_platform_device_create() returns
> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>  drivers/iommu/exynos-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 33dcc29..9b23059 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
>               exynos_iommu_init();
>
>       pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> -     if (IS_ERR(pdev))
> -             return PTR_ERR(pdev);
> +     if (!pdev)
> +             return -ENOMEM;
>
Indeed!If of_platform_device_create only returns NULL in case of failure, then IS_ERR() will be
false, so
Reviewed-by: Dennis Chen <dennis.chen@arm.com>

Thanks,
Dennis
>
>       /*
>        * use the first registered sysmmu device for performing
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[PATCH -next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[Krzysztof Kozlowski]

On 07/06/2016 02:15 PM, weiyj_lk at 163.com wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> In case of error, the function of_platform_device_create() returns
> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>  drivers/iommu/exynos-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 33dcc29..9b23059 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
>  		exynos_iommu_init();
>  
>  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> -	if (IS_ERR(pdev))
> -		return PTR_ERR(pdev);
> +	if (!pdev)
> +		return -ENOMEM;
>  
>  	/*
>  	 * use the first registered sysmmu device for performing
> 

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>


Which in case of first sysmmu device and of_platform_device_create()
failure will lead to NULL pointer exception, so I think it is
appropriate to:
Fixes: 8ed55c812fa8 ("iommu/exynos: Init from dt-specific callback
instead of initcall")
Cc: <stable@vger.kernel.org>

Best regards,
Krzysztof

[PATCH -next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[Andi Shyti]

Hi,

just a question,

> >  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> > -	if (IS_ERR(pdev))
> > -		return PTR_ERR(pdev);
> > +	if (!pdev)
> > +		return -ENOMEM;
> >  
> >  	/*
> >  	 * use the first registered sysmmu device for performing
> > 
> 

is ENOMEM the right return value? of_platform_device_create()
doesn't fail only in case of malloc failure.

Shouldn't it be ENODEV instead, which might also mean with some
imagination that there is no memory available for that device?

Thanks,
Andi

> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

[PATCH -next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[Wei Yongjun]

Hi,

> just a question,
>
>>>  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
>>> -	if (IS_ERR(pdev))
>>> -		return PTR_ERR(pdev);
>>> +	if (!pdev)
>>> +		return -ENOMEM;
>>>  
>>>  	/*
>>>  	 * use the first registered sysmmu device for performing
>>>
> is ENOMEM the right return value? of_platform_device_create()
> doesn't fail only in case of malloc failure.
>
> Shouldn't it be ENODEV instead, which might also mean with some
> imagination that there is no memory available for that device?
>
Your are right, I will change the error to -ENODEV and resend the patch.

Thanks,
Wei Yongjun

[PATCH -next v2] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[weiyj_lk at 163.com]

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

In case of error, the function of_platform_device_create() returns
NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
check should be replaced with NULL test.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
v1 -> v2: chenge the error code to -ENODEV
---
 drivers/iommu/exynos-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 33dcc29..9b23059 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
 		exynos_iommu_init();
 
 	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
+	if (!pdev)
+		return -ENODEV;
 
 	/*
 	 * use the first registered sysmmu device for performing

[PATCH -next v2] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[Andi Shyti]

Hi Wei,

On Thu, Jul 07, 2016 at 02:59:32AM +0000, weiyj_lk at 163.com wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> In case of error, the function of_platform_device_create() returns
> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Reviewed-by: Andi Shyti <andi.shyti@samsung.com>

Thanks,
Andi

> ---
> v1 -> v2: chenge the error code to -ENODEV
> ---
>  drivers/iommu/exynos-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 33dcc29..9b23059 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
>  		exynos_iommu_init();
>  
>  	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
> -	if (IS_ERR(pdev))
> -		return PTR_ERR(pdev);
> +	if (!pdev)
> +		return -ENODEV;
>  
>  	/*
>  	 * use the first registered sysmmu device for performing
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH -next v2] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[Krzysztof Kozlowski]

On 07/07/2016 04:59 AM, weiyj_lk at 163.com wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> In case of error, the function of_platform_device_create() returns
> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> v1 -> v2: chenge the error code to -ENODEV
> ---
>  drivers/iommu/exynos-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Why you didn't include mine and Dennis reviews? Why you ignored my
comments - you don't agree with them?

Best regards,
Krzysztof

[PATCH -next v2] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[Wei Yongjun]

On 07/07/2016 05:43 PM, Krzysztof Kozlowski wrote:
> On 07/07/2016 04:59 AM, weiyj_lk at 163.com wrote:
>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> In case of error, the function of_platform_device_create() returns
>> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
>> check should be replaced with NULL test.
>>
>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>> ---
>> v1 -> v2: chenge the error code to -ENODEV
>> ---
>>   drivers/iommu/exynos-iommu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
> Why you didn't include mine and Dennis reviews? Why you ignored my
> comments - you don't agree with them?
>
Sorry, I missed your last mail. I will change patch as your advise.

Regards,
Yongjun Wei

[PATCH -next v3] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[weiyj_lk at 163.com]

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

In case of error, the function of_platform_device_create() returns
NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
check should be replaced with NULL test.

Fixes: 8ed55c812fa8 ("iommu/exynos: Init from dt-specific callback
instead of initcall")
Cc: stable at vger.kernel.org
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Reviewed-by: Andi Shyti <andi.shyti@samsung.com>
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Reviewed-by: Dennis Chen <dennis.chen@arm.com>
---
v2 -> v3: added fixes and Reviewed-by.
---
 drivers/iommu/exynos-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 33dcc29..9b23059 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
 		exynos_iommu_init();
 
 	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
+	if (!pdev)
+		return -ENODEV;
 
 	/*
 	 * use the first registered sysmmu device for performing

[PATCH -next] iommu/exynos: Fix return value check in exynos_iommu_of_setup()

[Marek Szyprowski]

Hello,


On 2016-07-06 15:09, Krzysztof Kozlowski wrote:
> On 07/06/2016 02:15 PM, weiyj_lk at 163.com wrote:
>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> In case of error, the function of_platform_device_create() returns
>> NULL pointer not ERR_PTR(). The IS_ERR() test in the return value
>> check should be replaced with NULL test.
>>
>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>> ---
>>   drivers/iommu/exynos-iommu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
>> index 33dcc29..9b23059 100644
>> --- a/drivers/iommu/exynos-iommu.c
>> +++ b/drivers/iommu/exynos-iommu.c
>> @@ -1345,8 +1345,8 @@ static int __init exynos_iommu_of_setup(struct device_node *np)
>>   		exynos_iommu_init();
>>   
>>   	pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
>> -	if (IS_ERR(pdev))
>> -		return PTR_ERR(pdev);
>> +	if (!pdev)
>> +		return -ENOMEM;
>>   
>>   	/*
>>   	 * use the first registered sysmmu device for performing
>>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>
>
> Which in case of first sysmmu device and of_platform_device_create()
> failure will lead to NULL pointer exception, so I think it is
> appropriate to:
> Fixes: 8ed55c812fa8 ("iommu/exynos: Init from dt-specific callback
> instead of initcall")
> Cc: <stable@vger.kernel.org>

Thanks for spotting this issue.

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland