| Message ID | 1412681501-28072-1-git-send-email-pramod.gurav@smartplayin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2014-10-07 at 13:31:41 +0200, Pramod Gurav <pramod.gurav@smartplayin.com> wrote: > This change switch to managed resources to simplifies error handling > and module unloading and does away with platform_driver remove function. > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Cc: linux-input@vger.kernel.org > Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com> > --- > drivers/input/keyboard/opencores-kbd.c | 54 ++++++++------------------------ > 1 file changed, 13 insertions(+), 41 deletions(-) > > diff --git a/drivers/input/keyboard/opencores-kbd.c b/drivers/input/keyboard/opencores-kbd.c > index 7b9b441..ecacb87 100644 > --- a/drivers/input/keyboard/opencores-kbd.c > +++ b/drivers/input/keyboard/opencores-kbd.c > @@ -56,27 +56,27 @@ static int opencores_kbd_probe(struct platform_device *pdev) > return -EINVAL; > } > > - opencores_kbd = kzalloc(sizeof(*opencores_kbd), GFP_KERNEL); > - input = input_allocate_device(); > + opencores_kbd = devm_kzalloc(&pdev->dev, sizeof(*opencores_kbd), > + GFP_KERNEL); > + input = devm_input_allocate_device(&pdev->dev); > if (!opencores_kbd || !input) { > dev_err(&pdev->dev, "failed to allocate device structures\n"); > - error = -ENOMEM; > - goto err_free_mem; > + return -ENOMEM; > } > > opencores_kbd->addr_res = res; > - res = request_mem_region(res->start, resource_size(res), pdev->name); > + res = devm_request_mem_region(&pdev->dev, res->start, > + resource_size(res), pdev->name); > if (!res) { > dev_err(&pdev->dev, "failed to request I/O memory\n"); > - error = -EBUSY; > - goto err_free_mem; > + return -EBUSY; > } > > - opencores_kbd->addr = ioremap(res->start, resource_size(res)); > + opencores_kbd->addr = devm_ioremap(&pdev->dev, res->start, > + resource_size(res)); You could use devm_ioremap_resource() here and get rid of the call to devm_request_mem_region() and the check for !res above, i.e. something like: res = platform_get_resource(pdev, IORESOURCE_MEM, 0); [...] opencores_kbd->addr = devm_ioremap_resource(&pdev->dev, res); if (!opencores_kbd->addr) { dev_err(&pdev->dev, "failed to remap I/O memory\n"); return -ENXIO; } > if (!opencores_kbd->addr) { > dev_err(&pdev->dev, "failed to remap I/O memory\n"); > - error = -ENXIO; > - goto err_rel_mem; > + return -ENXIO; > } > > opencores_kbd->input = input; > @@ -109,54 +109,26 @@ static int opencores_kbd_probe(struct platform_device *pdev) > } > __clear_bit(KEY_RESERVED, input->keybit); > > - error = request_irq(irq, &opencores_kbd_isr, > + error = devm_request_irq(&pdev->dev, irq, &opencores_kbd_isr, > IRQF_TRIGGER_RISING, pdev->name, opencores_kbd); The second line should be aligned to the opening parenthesis: error = devm_request_irq(&pdev->dev, irq, &opencores_kbd_isr, IRQF_TRIGGER_RISING, pdev->name, opencores_kbd); -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 07, 2014 at 03:33:22PM +0200, Tobias Klauser wrote: > On 2014-10-07 at 13:31:41 +0200, Pramod Gurav <pramod.gurav@smartplayin.com> wrote: > > This change switch to managed resources to simplifies error handling > > and module unloading and does away with platform_driver remove function. > > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Cc: linux-input@vger.kernel.org > > Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com> > > --- > > drivers/input/keyboard/opencores-kbd.c | 54 ++++++++------------------------ > > 1 file changed, 13 insertions(+), 41 deletions(-) > > > > diff --git a/drivers/input/keyboard/opencores-kbd.c b/drivers/input/keyboard/opencores-kbd.c > > index 7b9b441..ecacb87 100644 > > --- a/drivers/input/keyboard/opencores-kbd.c > > +++ b/drivers/input/keyboard/opencores-kbd.c > > @@ -56,27 +56,27 @@ static int opencores_kbd_probe(struct platform_device *pdev) > > return -EINVAL; > > } > > > > - opencores_kbd = kzalloc(sizeof(*opencores_kbd), GFP_KERNEL); > > - input = input_allocate_device(); > > + opencores_kbd = devm_kzalloc(&pdev->dev, sizeof(*opencores_kbd), > > + GFP_KERNEL); > > + input = devm_input_allocate_device(&pdev->dev); > > if (!opencores_kbd || !input) { > > dev_err(&pdev->dev, "failed to allocate device structures\n"); > > - error = -ENOMEM; > > - goto err_free_mem; > > + return -ENOMEM; > > } > > > > opencores_kbd->addr_res = res; > > - res = request_mem_region(res->start, resource_size(res), pdev->name); > > + res = devm_request_mem_region(&pdev->dev, res->start, > > + resource_size(res), pdev->name); > > if (!res) { > > dev_err(&pdev->dev, "failed to request I/O memory\n"); > > - error = -EBUSY; > > - goto err_free_mem; > > + return -EBUSY; > > } > > > > - opencores_kbd->addr = ioremap(res->start, resource_size(res)); > > + opencores_kbd->addr = devm_ioremap(&pdev->dev, res->start, > > + resource_size(res)); > > You could use devm_ioremap_resource() here and get rid of the call to > devm_request_mem_region() and the check for !res above, i.e. something > like: > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > [...] > opencores_kbd->addr = devm_ioremap_resource(&pdev->dev, res); > if (!opencores_kbd->addr) { > dev_err(&pdev->dev, "failed to remap I/O memory\n"); > return -ENXIO; > } You need to be careful with devm_ioremap_resource() ocnversions as it returns ERR_PTR-encoded pointer on failures, not NULL. I adjusted the patch to use devm_ioremap_resource() and applied. Thanks.
On 2014-10-07 at 18:17:16 +0200, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Tue, Oct 07, 2014 at 03:33:22PM +0200, Tobias Klauser wrote: > > On 2014-10-07 at 13:31:41 +0200, Pramod Gurav <pramod.gurav@smartplayin.com> wrote: > > > This change switch to managed resources to simplifies error handling > > > and module unloading and does away with platform_driver remove function. > > > > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > Cc: linux-input@vger.kernel.org > > > Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com> > > > --- > > > drivers/input/keyboard/opencores-kbd.c | 54 ++++++++------------------------ > > > 1 file changed, 13 insertions(+), 41 deletions(-) > > > > > > diff --git a/drivers/input/keyboard/opencores-kbd.c b/drivers/input/keyboard/opencores-kbd.c > > > index 7b9b441..ecacb87 100644 > > > --- a/drivers/input/keyboard/opencores-kbd.c > > > +++ b/drivers/input/keyboard/opencores-kbd.c > > > @@ -56,27 +56,27 @@ static int opencores_kbd_probe(struct platform_device *pdev) > > > return -EINVAL; > > > } > > > > > > - opencores_kbd = kzalloc(sizeof(*opencores_kbd), GFP_KERNEL); > > > - input = input_allocate_device(); > > > + opencores_kbd = devm_kzalloc(&pdev->dev, sizeof(*opencores_kbd), > > > + GFP_KERNEL); > > > + input = devm_input_allocate_device(&pdev->dev); > > > if (!opencores_kbd || !input) { > > > dev_err(&pdev->dev, "failed to allocate device structures\n"); > > > - error = -ENOMEM; > > > - goto err_free_mem; > > > + return -ENOMEM; > > > } > > > > > > opencores_kbd->addr_res = res; > > > - res = request_mem_region(res->start, resource_size(res), pdev->name); > > > + res = devm_request_mem_region(&pdev->dev, res->start, > > > + resource_size(res), pdev->name); > > > if (!res) { > > > dev_err(&pdev->dev, "failed to request I/O memory\n"); > > > - error = -EBUSY; > > > - goto err_free_mem; > > > + return -EBUSY; > > > } > > > > > > - opencores_kbd->addr = ioremap(res->start, resource_size(res)); > > > + opencores_kbd->addr = devm_ioremap(&pdev->dev, res->start, > > > + resource_size(res)); > > > > You could use devm_ioremap_resource() here and get rid of the call to > > devm_request_mem_region() and the check for !res above, i.e. something > > like: > > > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > [...] > > opencores_kbd->addr = devm_ioremap_resource(&pdev->dev, res); > > if (!opencores_kbd->addr) { > > dev_err(&pdev->dev, "failed to remap I/O memory\n"); > > return -ENXIO; > > } > > You need to be careful with devm_ioremap_resource() ocnversions as it > returns ERR_PTR-encoded pointer on failures, not NULL. Ah yes of course, I was a bit too quick with my top-of-the-head conversion. Thanks for fixing it up. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/keyboard/opencores-kbd.c b/drivers/input/keyboard/opencores-kbd.c index 7b9b441..ecacb87 100644 --- a/drivers/input/keyboard/opencores-kbd.c +++ b/drivers/input/keyboard/opencores-kbd.c @@ -56,27 +56,27 @@ static int opencores_kbd_probe(struct platform_device *pdev) return -EINVAL; } - opencores_kbd = kzalloc(sizeof(*opencores_kbd), GFP_KERNEL); - input = input_allocate_device(); + opencores_kbd = devm_kzalloc(&pdev->dev, sizeof(*opencores_kbd), + GFP_KERNEL); + input = devm_input_allocate_device(&pdev->dev); if (!opencores_kbd || !input) { dev_err(&pdev->dev, "failed to allocate device structures\n"); - error = -ENOMEM; - goto err_free_mem; + return -ENOMEM; } opencores_kbd->addr_res = res; - res = request_mem_region(res->start, resource_size(res), pdev->name); + res = devm_request_mem_region(&pdev->dev, res->start, + resource_size(res), pdev->name); if (!res) { dev_err(&pdev->dev, "failed to request I/O memory\n"); - error = -EBUSY; - goto err_free_mem; + return -EBUSY; } - opencores_kbd->addr = ioremap(res->start, resource_size(res)); + opencores_kbd->addr = devm_ioremap(&pdev->dev, res->start, + resource_size(res)); if (!opencores_kbd->addr) { dev_err(&pdev->dev, "failed to remap I/O memory\n"); - error = -ENXIO; - goto err_rel_mem; + return -ENXIO; } opencores_kbd->input = input; @@ -109,54 +109,26 @@ static int opencores_kbd_probe(struct platform_device *pdev) } __clear_bit(KEY_RESERVED, input->keybit); - error = request_irq(irq, &opencores_kbd_isr, + error = devm_request_irq(&pdev->dev, irq, &opencores_kbd_isr, IRQF_TRIGGER_RISING, pdev->name, opencores_kbd); if (error) { dev_err(&pdev->dev, "unable to claim irq %d\n", irq); - goto err_unmap_mem; + return error; } error = input_register_device(input); if (error) { dev_err(&pdev->dev, "unable to register input device\n"); - goto err_free_irq; + return error; } platform_set_drvdata(pdev, opencores_kbd); return 0; - - err_free_irq: - free_irq(irq, opencores_kbd); - err_unmap_mem: - iounmap(opencores_kbd->addr); - err_rel_mem: - release_mem_region(res->start, resource_size(res)); - err_free_mem: - input_free_device(input); - kfree(opencores_kbd); - - return error; -} - -static int opencores_kbd_remove(struct platform_device *pdev) -{ - struct opencores_kbd *opencores_kbd = platform_get_drvdata(pdev); - - free_irq(opencores_kbd->irq, opencores_kbd); - - iounmap(opencores_kbd->addr); - release_mem_region(opencores_kbd->addr_res->start, - resource_size(opencores_kbd->addr_res)); - input_unregister_device(opencores_kbd->input); - kfree(opencores_kbd); - - return 0; } static struct platform_driver opencores_kbd_device_driver = { .probe = opencores_kbd_probe, - .remove = opencores_kbd_remove, .driver = { .name = "opencores-kbd", },
This change switch to managed resources to simplifies error handling and module unloading and does away with platform_driver remove function. Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: linux-input@vger.kernel.org Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com> --- drivers/input/keyboard/opencores-kbd.c | 54 ++++++++------------------------ 1 file changed, 13 insertions(+), 41 deletions(-)