Messages in this thread | | | Subject | Re: [PATCH 2/3] gpio: pxa: Fix return value of pxa_gpio_probe() | From | Tiezhu Yang <> | Date | Sat, 23 May 2020 11:24:46 +0800 |
| |
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. >
| |