* [PATCH] gpio: using devm functions for timberdale gpio memory allocation This eases memory allocation and provides appropriate logging
@ 2014-05-13 1:21 abdoulaye berthe
2014-05-13 7:55 ` Alexandre Courbot
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: abdoulaye berthe @ 2014-05-13 1:21 UTC (permalink / raw)
To: berthe.ab, linus.walleij, gnurou, linux-gpio, linux-kernel
Signed-off-by: abdoulaye berthe <berthe.ab@gmail.com>
---
drivers/gpio/gpio-timberdale.c | 47 ++++++++++++++----------------------------
1 file changed, 16 insertions(+), 31 deletions(-)
diff --git a/drivers/gpio/gpio-timberdale.c b/drivers/gpio/gpio-timberdale.c
index f9a8fbd..efc7c12 100644
--- a/drivers/gpio/gpio-timberdale.c
+++ b/drivers/gpio/gpio-timberdale.c
@@ -224,6 +224,7 @@ static struct irq_chip timbgpio_irqchip = {
static int timbgpio_probe(struct platform_device *pdev)
{
int err, i;
+ struct device *dev = &pdev->dev;
struct gpio_chip *gc;
struct timbgpio *tgpio;
struct resource *iomem;
@@ -231,35 +232,35 @@ static int timbgpio_probe(struct platform_device *pdev)
int irq = platform_get_irq(pdev, 0);
if (!pdata || pdata->nr_pins > 32) {
- err = -EINVAL;
- goto err_mem;
+ dev_err(dev, "Invalid platform data\n");
+ return -EINVAL;
}
iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!iomem) {
- err = -EINVAL;
- goto err_mem;
+ dev_err(dev, "Unable to get resource\n");
+ return -EINVAL;
}
- tgpio = kzalloc(sizeof(*tgpio), GFP_KERNEL);
+ tgpio = devm_kzalloc(dev, sizeof(struct timbgpio), GFP_KERNEL);
if (!tgpio) {
- err = -EINVAL;
- goto err_mem;
+ dev_err(dev, "Memory alloc failed\n");
+ return -EINVAL;
}
tgpio->irq_base = pdata->irq_base;
spin_lock_init(&tgpio->lock);
- if (!request_mem_region(iomem->start, resource_size(iomem),
- DRIVER_NAME)) {
- err = -EBUSY;
- goto err_request;
+ if (!devm_request_mem_region(dev, iomem->start, resource_size(iomem),
+ DRIVER_NAME)) {
+ dev_err(dev, "Region already claimed\n");
+ return -EBUSY;
}
- tgpio->membase = ioremap(iomem->start, resource_size(iomem));
+ tgpio->membase = devm_ioremap(dev, iomem->start, resource_size(iomem));
if (!tgpio->membase) {
- err = -ENOMEM;
- goto err_ioremap;
+ dev_err(dev, "Cannot ioremap\n");
+ return -ENOMEM;
}
gc = &tgpio->gpio;
@@ -279,7 +280,7 @@ static int timbgpio_probe(struct platform_device *pdev)
err = gpiochip_add(gc);
if (err)
- goto err_chipadd;
+ return err;
platform_set_drvdata(pdev, tgpio);
@@ -302,17 +303,6 @@ static int timbgpio_probe(struct platform_device *pdev)
irq_set_chained_handler(irq, timbgpio_irq);
return 0;
-
-err_chipadd:
- iounmap(tgpio->membase);
-err_ioremap:
- release_mem_region(iomem->start, resource_size(iomem));
-err_request:
- kfree(tgpio);
-err_mem:
- printk(KERN_ERR DRIVER_NAME": Failed to register GPIOs: %d\n", err);
-
- return err;
}
static int timbgpio_remove(struct platform_device *pdev)
@@ -320,7 +310,6 @@ static int timbgpio_remove(struct platform_device *pdev)
int err;
struct timbgpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
struct timbgpio *tgpio = platform_get_drvdata(pdev);
- struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
int irq = platform_get_irq(pdev, 0);
if (irq >= 0 && tgpio->irq_base > 0) {
@@ -338,10 +327,6 @@ static int timbgpio_remove(struct platform_device *pdev)
if (err)
printk(KERN_ERR DRIVER_NAME": failed to remove gpio_chip\n");
- iounmap(tgpio->membase);
- release_mem_region(iomem->start, resource_size(iomem));
- kfree(tgpio);
-
return 0;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: using devm functions for timberdale gpio memory allocation This eases memory allocation and provides appropriate logging
2014-05-13 1:21 [PATCH] gpio: using devm functions for timberdale gpio memory allocation This eases memory allocation and provides appropriate logging abdoulaye berthe
@ 2014-05-13 7:55 ` Alexandre Courbot
2014-05-13 9:42 ` Linus Walleij
2014-05-13 10:03 ` Thierry Reding
2 siblings, 0 replies; 4+ messages in thread
From: Alexandre Courbot @ 2014-05-13 7:55 UTC (permalink / raw)
To: abdoulaye berthe
Cc: Linus Walleij, linux-gpio@vger.kernel.org,
Linux Kernel Mailing List
A welcome cleanup, but the subject line of your patch is waaay too
long. As a rule of thumb, try to limit it to 50 characters ([PATCH]
tag not included), with slight overlapping tolerated.
Also to better spot which driver is changed, please prefix it with
"gpio: timberdale: "
For this patch, "gpio: timberdale: use devm functions" looks good to me.
The rest of your subject line should be the actual commit message.
Thanks,
Alex.
On Tue, May 13, 2014 at 10:21 AM, abdoulaye berthe <berthe.ab@gmail.com> wrote:
> Signed-off-by: abdoulaye berthe <berthe.ab@gmail.com>
> ---
> drivers/gpio/gpio-timberdale.c | 47 ++++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpio/gpio-timberdale.c b/drivers/gpio/gpio-timberdale.c
> index f9a8fbd..efc7c12 100644
> --- a/drivers/gpio/gpio-timberdale.c
> +++ b/drivers/gpio/gpio-timberdale.c
> @@ -224,6 +224,7 @@ static struct irq_chip timbgpio_irqchip = {
> static int timbgpio_probe(struct platform_device *pdev)
> {
> int err, i;
> + struct device *dev = &pdev->dev;
> struct gpio_chip *gc;
> struct timbgpio *tgpio;
> struct resource *iomem;
> @@ -231,35 +232,35 @@ static int timbgpio_probe(struct platform_device *pdev)
> int irq = platform_get_irq(pdev, 0);
>
> if (!pdata || pdata->nr_pins > 32) {
> - err = -EINVAL;
> - goto err_mem;
> + dev_err(dev, "Invalid platform data\n");
> + return -EINVAL;
> }
>
> iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!iomem) {
> - err = -EINVAL;
> - goto err_mem;
> + dev_err(dev, "Unable to get resource\n");
> + return -EINVAL;
> }
>
> - tgpio = kzalloc(sizeof(*tgpio), GFP_KERNEL);
> + tgpio = devm_kzalloc(dev, sizeof(struct timbgpio), GFP_KERNEL);
> if (!tgpio) {
> - err = -EINVAL;
> - goto err_mem;
> + dev_err(dev, "Memory alloc failed\n");
> + return -EINVAL;
> }
> tgpio->irq_base = pdata->irq_base;
>
> spin_lock_init(&tgpio->lock);
>
> - if (!request_mem_region(iomem->start, resource_size(iomem),
> - DRIVER_NAME)) {
> - err = -EBUSY;
> - goto err_request;
> + if (!devm_request_mem_region(dev, iomem->start, resource_size(iomem),
> + DRIVER_NAME)) {
> + dev_err(dev, "Region already claimed\n");
> + return -EBUSY;
> }
>
> - tgpio->membase = ioremap(iomem->start, resource_size(iomem));
> + tgpio->membase = devm_ioremap(dev, iomem->start, resource_size(iomem));
> if (!tgpio->membase) {
> - err = -ENOMEM;
> - goto err_ioremap;
> + dev_err(dev, "Cannot ioremap\n");
> + return -ENOMEM;
> }
>
> gc = &tgpio->gpio;
> @@ -279,7 +280,7 @@ static int timbgpio_probe(struct platform_device *pdev)
>
> err = gpiochip_add(gc);
> if (err)
> - goto err_chipadd;
> + return err;
>
> platform_set_drvdata(pdev, tgpio);
>
> @@ -302,17 +303,6 @@ static int timbgpio_probe(struct platform_device *pdev)
> irq_set_chained_handler(irq, timbgpio_irq);
>
> return 0;
> -
> -err_chipadd:
> - iounmap(tgpio->membase);
> -err_ioremap:
> - release_mem_region(iomem->start, resource_size(iomem));
> -err_request:
> - kfree(tgpio);
> -err_mem:
> - printk(KERN_ERR DRIVER_NAME": Failed to register GPIOs: %d\n", err);
> -
> - return err;
> }
>
> static int timbgpio_remove(struct platform_device *pdev)
> @@ -320,7 +310,6 @@ static int timbgpio_remove(struct platform_device *pdev)
> int err;
> struct timbgpio_platform_data *pdata = dev_get_platdata(&pdev->dev);
> struct timbgpio *tgpio = platform_get_drvdata(pdev);
> - struct resource *iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> int irq = platform_get_irq(pdev, 0);
>
> if (irq >= 0 && tgpio->irq_base > 0) {
> @@ -338,10 +327,6 @@ static int timbgpio_remove(struct platform_device *pdev)
> if (err)
> printk(KERN_ERR DRIVER_NAME": failed to remove gpio_chip\n");
>
> - iounmap(tgpio->membase);
> - release_mem_region(iomem->start, resource_size(iomem));
> - kfree(tgpio);
> -
> return 0;
> }
>
> --
> 1.8.3.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: using devm functions for timberdale gpio memory allocation This eases memory allocation and provides appropriate logging
2014-05-13 1:21 [PATCH] gpio: using devm functions for timberdale gpio memory allocation This eases memory allocation and provides appropriate logging abdoulaye berthe
2014-05-13 7:55 ` Alexandre Courbot
@ 2014-05-13 9:42 ` Linus Walleij
2014-05-13 10:03 ` Thierry Reding
2 siblings, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2014-05-13 9:42 UTC (permalink / raw)
To: abdoulaye berthe
Cc: Alexandre Courbot, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org
On Tue, May 13, 2014 at 3:21 AM, abdoulaye berthe <berthe.ab@gmail.com> wrote:
> Signed-off-by: abdoulaye berthe <berthe.ab@gmail.com>
Good patch, so I fixed up the commit message and applied it.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gpio: using devm functions for timberdale gpio memory allocation This eases memory allocation and provides appropriate logging
2014-05-13 1:21 [PATCH] gpio: using devm functions for timberdale gpio memory allocation This eases memory allocation and provides appropriate logging abdoulaye berthe
2014-05-13 7:55 ` Alexandre Courbot
2014-05-13 9:42 ` Linus Walleij
@ 2014-05-13 10:03 ` Thierry Reding
2 siblings, 0 replies; 4+ messages in thread
From: Thierry Reding @ 2014-05-13 10:03 UTC (permalink / raw)
To: abdoulaye berthe; +Cc: linus.walleij, gnurou, linux-gpio, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]
On Tue, May 13, 2014 at 03:21:42AM +0200, abdoulaye berthe wrote:
[...]
> diff --git a/drivers/gpio/gpio-timberdale.c b/drivers/gpio/gpio-timberdale.c
[...]
> iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!iomem) {
> - err = -EINVAL;
> - goto err_mem;
> + dev_err(dev, "Unable to get resource\n");
> + return -EINVAL;
> }
[...]
> - if (!request_mem_region(iomem->start, resource_size(iomem),
> - DRIVER_NAME)) {
> - err = -EBUSY;
> - goto err_request;
> + if (!devm_request_mem_region(dev, iomem->start, resource_size(iomem),
> + DRIVER_NAME)) {
> + dev_err(dev, "Region already claimed\n");
> + return -EBUSY;
> }
>
> - tgpio->membase = ioremap(iomem->start, resource_size(iomem));
> + tgpio->membase = devm_ioremap(dev, iomem->start, resource_size(iomem));
> if (!tgpio->membase) {
> - err = -ENOMEM;
> - goto err_ioremap;
> + dev_err(dev, "Cannot ioremap\n");
> + return -ENOMEM;
> }
The above could be further simplified to:
iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
tgpio->membase = devm_ioremap_resource(&pdev->dev, iomem);
if (IS_ERR(tgpio->membase))
return PTR_ERR(tgpio->membase);
Where devm_ioremap_resource() already provides error messages as
appropriate.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-13 10:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-13 1:21 [PATCH] gpio: using devm functions for timberdale gpio memory allocation This eases memory allocation and provides appropriate logging abdoulaye berthe
2014-05-13 7:55 ` Alexandre Courbot
2014-05-13 9:42 ` Linus Walleij
2014-05-13 10:03 ` Thierry Reding
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).