[PATCH 1/3] gpio: bcm-kona: Fix return value of bcm_kona_gpio_probe()

[PATCH 1/3] gpio: bcm-kona: Fix return value of bcm_kona_gpio_probe()

[Tiezhu Yang]

When call function devm_platform_ioremap_resource(), we should use IS_ERR()
to check the return value and return PTR_ERR() if failed.

Fixes: 72d8cb715477 ("drivers: gpio: bcm-kona: use devm_platform_ioremap_resource()")
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/gpio/gpio-bcm-kona.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
index baee8c3..cf3687a 100644
--- a/drivers/gpio/gpio-bcm-kona.c
+++ b/drivers/gpio/gpio-bcm-kona.c
@@ -625,7 +625,7 @@ static int bcm_kona_gpio_probe(struct platform_device *pdev)
 
 	kona_gpio->reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(kona_gpio->reg_base)) {
-		ret = -ENXIO;
+		ret = PTR_ERR(kona_gpio->reg_base);
 		goto err_irq_domain;
 	}
 
-- 
2.1.0

[PATCH 2/3] gpio: pxa: Fix return value of pxa_gpio_probe()

[Tiezhu Yang]

When call function devm_platform_ioremap_resource(), we should use IS_ERR()
to check the return value and return PTR_ERR() if failed.

Fixes: 542c25b7a209 ("drivers: gpio: pxa: use devm_platform_ioremap_resource()")
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/gpio/gpio-pxa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
index 1361270..0cb6600 100644
--- a/drivers/gpio/gpio-pxa.c
+++ b/drivers/gpio/gpio-pxa.c
@@ -660,8 +660,8 @@ static int pxa_gpio_probe(struct platform_device *pdev)
 	pchip->irq1 = irq1;
 
 	gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
-	if (!gpio_reg_base)
-		return -EINVAL;
+	if (IS_ERR(gpio_reg_base))
+		return PTR_ERR(gpio_reg_base);
 
 	clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
-- 
2.1.0

[PATCH 3/3] gpio: pxa: Add COMPILE_TEST support

[Tiezhu Yang]

Add COMPILE_TEST support to the PXA GPIO driver for better compile
testing coverage.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/gpio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 03c01f4..5e90aad 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -439,7 +439,7 @@ config GPIO_PMIC_EIC_SPRD
 
 config GPIO_PXA
 	bool "PXA GPIO support"
-	depends on ARCH_PXA || ARCH_MMP
+	depends on ARCH_PXA || ARCH_MMP || COMPILE_TEST
 	help
 	  Say yes here to support the PXA GPIO device
 
-- 
2.1.0

Re: [PATCH 2/3] gpio: pxa: Fix return value of pxa_gpio_probe()

[Robert Jarzmik]

Tiezhu Yang <yangtiezhu@loongson.cn> writes:

> When call function devm_platform_ioremap_resource(), we should use IS_ERR()
> to check the return value and return PTR_ERR() if failed.
>
> Fixes: 542c25b7a209 ("drivers: gpio: pxa: use devm_platform_ioremap_resource()")
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  drivers/gpio/gpio-pxa.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
> index 1361270..0cb6600 100644
> --- a/drivers/gpio/gpio-pxa.c
> +++ b/drivers/gpio/gpio-pxa.c
> @@ -660,8 +660,8 @@ static int pxa_gpio_probe(struct platform_device *pdev)
>  	pchip->irq1 = irq1;
>  
>  	gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
> -	if (!gpio_reg_base)
> -		return -EINVAL;
> +	if (IS_ERR(gpio_reg_base))
> +		return PTR_ERR(gpio_reg_base);
As far as I know, devm_platform_ioremap_resource() could return NULL which is
not handled by this test (unless __devm_ioremap() semantics changed since I had
a look).

Therefore, this patch is incorrect, or rather incomplete.

Cheers.

-- 
Robert

Re: [PATCH 2/3] gpio: pxa: Fix return value of pxa_gpio_probe()

[Tiezhu Yang]

On 05/23/2020 03:07 AM, Robert Jarzmik wrote:
> Tiezhu Yang <yangtiezhu@loongson.cn> writes:
>
>> When call function devm_platform_ioremap_resource(), we should use IS_ERR()
>> to check the return value and return PTR_ERR() if failed.
>>
>> Fixes: 542c25b7a209 ("drivers: gpio: pxa: use devm_platform_ioremap_resource()")
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>   drivers/gpio/gpio-pxa.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
>> index 1361270..0cb6600 100644
>> --- a/drivers/gpio/gpio-pxa.c
>> +++ b/drivers/gpio/gpio-pxa.c
>> @@ -660,8 +660,8 @@ static int pxa_gpio_probe(struct platform_device *pdev)
>>   	pchip->irq1 = irq1;
>>   
>>   	gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
>> -	if (!gpio_reg_base)
>> -		return -EINVAL;
>> +	if (IS_ERR(gpio_reg_base))
>> +		return PTR_ERR(gpio_reg_base);
> As far as I know, devm_platform_ioremap_resource() could return NULL which is
> not handled by this test (unless __devm_ioremap() semantics changed since I had
> a look).

Hi Robert,

In the function __devm_ioremap_resource(), if __devm_ioremap returns NULL,
it will return IOMEM_ERR_PTR(-ENOMEM).

devm_platform_ioremap_resource()
         devm_ioremap_resource()
                 __devm_ioremap_resource()
                        __devm_ioremap()

static void __iomem *
__devm_ioremap_resource(struct device *dev, const struct resource *res,
             enum devm_ioremap_type type)
{
...
     dest_ptr = __devm_ioremap(dev, res->start, size, type);
     if (!dest_ptr) {
         dev_err(dev, "ioremap failed for resource %pR\n", res);
         devm_release_mem_region(dev, res->start, size);
         dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
     }

     return dest_ptr;
}

And also, we can see the comment of devm_ioremap_resource():

Usage example:

         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
         base = devm_ioremap_resource(&pdev->dev, res);
         if (IS_ERR(base))
                 return PTR_ERR(base);

>
> Therefore, this patch is incorrect, or rather incomplete.

So I think this patch is correct, do I miss something?

Thanks,
Tiezhu Yang

>
> Cheers.
>

Re: [PATCH 2/3] gpio: pxa: Fix return value of pxa_gpio_probe()

[Bartosz Golaszewski]

sob., 23 maj 2020 o 05:25 Tiezhu Yang <yangtiezhu@loongson.cn> napisał(a):
>
> On 05/23/2020 03:07 AM, Robert Jarzmik wrote:
> > Tiezhu Yang <yangtiezhu@loongson.cn> writes:
> >
> >> When call function devm_platform_ioremap_resource(), we should use IS_ERR()
> >> to check the return value and return PTR_ERR() if failed.
> >>
> >> Fixes: 542c25b7a209 ("drivers: gpio: pxa: use devm_platform_ioremap_resource()")
> >> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> >> ---
> >>   drivers/gpio/gpio-pxa.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
> >> index 1361270..0cb6600 100644
> >> --- a/drivers/gpio/gpio-pxa.c
> >> +++ b/drivers/gpio/gpio-pxa.c
> >> @@ -660,8 +660,8 @@ static int pxa_gpio_probe(struct platform_device *pdev)
> >>      pchip->irq1 = irq1;
> >>
> >>      gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
> >> -    if (!gpio_reg_base)
> >> -            return -EINVAL;
> >> +    if (IS_ERR(gpio_reg_base))
> >> +            return PTR_ERR(gpio_reg_base);
> > As far as I know, devm_platform_ioremap_resource() could return NULL which is
> > not handled by this test (unless __devm_ioremap() semantics changed since I had
> > a look).
>
> Hi Robert,
>
> In the function __devm_ioremap_resource(), if __devm_ioremap returns NULL,
> it will return IOMEM_ERR_PTR(-ENOMEM).
>
> devm_platform_ioremap_resource()
>          devm_ioremap_resource()
>                  __devm_ioremap_resource()
>                         __devm_ioremap()
>
> static void __iomem *
> __devm_ioremap_resource(struct device *dev, const struct resource *res,
>              enum devm_ioremap_type type)
> {
> ...
>      dest_ptr = __devm_ioremap(dev, res->start, size, type);
>      if (!dest_ptr) {
>          dev_err(dev, "ioremap failed for resource %pR\n", res);
>          devm_release_mem_region(dev, res->start, size);
>          dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
>      }
>
>      return dest_ptr;
> }
>
> And also, we can see the comment of devm_ioremap_resource():
>
> Usage example:
>
>          res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>          base = devm_ioremap_resource(&pdev->dev, res);
>          if (IS_ERR(base))
>                  return PTR_ERR(base);
>
> >
> > Therefore, this patch is incorrect, or rather incomplete.
>
> So I think this patch is correct, do I miss something?
>
>

Yes, this patch is correct. Applied for fixes.

Bart

Re: [PATCH 3/3] gpio: pxa: Add COMPILE_TEST support

[Bartosz Golaszewski]

pt., 22 maj 2020 o 06:12 Tiezhu Yang <yangtiezhu@loongson.cn> napisał(a):
>
> Add COMPILE_TEST support to the PXA GPIO driver for better compile
> testing coverage.
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  drivers/gpio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 03c01f4..5e90aad 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -439,7 +439,7 @@ config GPIO_PMIC_EIC_SPRD
>
>  config GPIO_PXA
>         bool "PXA GPIO support"
> -       depends on ARCH_PXA || ARCH_MMP
> +       depends on ARCH_PXA || ARCH_MMP || COMPILE_TEST
>         help
>           Say yes here to support the PXA GPIO device
>
> --
> 2.1.0
>

Patch applied.

Bart

Re: [PATCH 1/3] gpio: bcm-kona: Fix return value of bcm_kona_gpio_probe()

[Bartosz Golaszewski]

pt., 22 maj 2020 o 06:12 Tiezhu Yang <yangtiezhu@loongson.cn> napisał(a):
>
> When call function devm_platform_ioremap_resource(), we should use IS_ERR()
> to check the return value and return PTR_ERR() if failed.
>
> Fixes: 72d8cb715477 ("drivers: gpio: bcm-kona: use devm_platform_ioremap_resource()")
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  drivers/gpio/gpio-bcm-kona.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
> index baee8c3..cf3687a 100644
> --- a/drivers/gpio/gpio-bcm-kona.c
> +++ b/drivers/gpio/gpio-bcm-kona.c
> @@ -625,7 +625,7 @@ static int bcm_kona_gpio_probe(struct platform_device *pdev)
>
>         kona_gpio->reg_base = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(kona_gpio->reg_base)) {
> -               ret = -ENXIO;
> +               ret = PTR_ERR(kona_gpio->reg_base);
>                 goto err_irq_domain;
>         }
>
> --
> 2.1.0
>

I fixed the commit message, since this patch addresses the error code
propagation, not the checking itself. Applied for fixes.

Bartosz

Re: [PATCH 2/3] gpio: pxa: Fix return value of pxa_gpio_probe()

[Robert Jarzmik]

Tiezhu Yang <yangtiezhu@loongson.cn> writes:

> On 05/23/2020 03:07 AM, Robert Jarzmik wrote:
>> Tiezhu Yang <yangtiezhu@loongson.cn> writes:
>>
>>> When call function devm_platform_ioremap_resource(), we should use IS_ERR()
>>> to check the return value and return PTR_ERR() if failed.
>>>
>>> Fixes: 542c25b7a209 ("drivers: gpio: pxa: use devm_platform_ioremap_resource()")
>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>>> ---
>>>   drivers/gpio/gpio-pxa.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
>>> index 1361270..0cb6600 100644
>>> --- a/drivers/gpio/gpio-pxa.c
>>> +++ b/drivers/gpio/gpio-pxa.c
>>> @@ -660,8 +660,8 @@ static int pxa_gpio_probe(struct platform_device *pdev)
>>>   	pchip->irq1 = irq1;
>>>     	gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
>>> -	if (!gpio_reg_base)
>>> -		return -EINVAL;
>>> +	if (IS_ERR(gpio_reg_base))
>>> +		return PTR_ERR(gpio_reg_base);
>> As far as I know, devm_platform_ioremap_resource() could return NULL which is
>> not handled by this test (unless __devm_ioremap() semantics changed since I had
>> a look).
>
> Hi Robert,
>
> In the function __devm_ioremap_resource(), if __devm_ioremap returns NULL,
> it will return IOMEM_ERR_PTR(-ENOMEM).
>
> devm_platform_ioremap_resource()
>         devm_ioremap_resource()
>                 __devm_ioremap_resource()
>                        __devm_ioremap()
>
> static void __iomem *
> __devm_ioremap_resource(struct device *dev, const struct resource *res,
>             enum devm_ioremap_type type)
> {
> ...
>     dest_ptr = __devm_ioremap(dev, res->start, size, type);
>     if (!dest_ptr) {
>         dev_err(dev, "ioremap failed for resource %pR\n", res);
>         devm_release_mem_region(dev, res->start, size);
>         dest_ptr = IOMEM_ERR_PTR(-ENOMEM);
>     }
>
>     return dest_ptr;
> }
>
> And also, we can see the comment of devm_ioremap_resource():
>
> Usage example:
>
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         base = devm_ioremap_resource(&pdev->dev, res);
>         if (IS_ERR(base))
>                 return PTR_ERR(base);
>
>>
>> Therefore, this patch is incorrect, or rather incomplete.
>
> So I think this patch is correct, do I miss something?
You're right, my bad, didn't see the test in __devm_ioremap_resource().

Cheers.

-- 
Robert