Hi Manish,
Thanks for the patch. I have few comments inline, though.
On Sunday 15 of September 2013 01:22:23 Manish Badarkhe wrote:
> Update the code to use devm_* API so that driver core will manage
> resources.
>
> Signed-off-by: Manish Badarkhe <badarkhe.manish@xxxxxxxxx>
> ---
>
> :100644 100644 440ce32... b4db721... M drivers/input/keyboard/gpio_keys.c
>
> drivers/input/keyboard/gpio_keys.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c
> b/drivers/input/keyboard/gpio_keys.c index 440ce32..b4db721 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -588,7 +588,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
> goto err_out;
> }
>
> - pdata = kzalloc(sizeof(*pdata) + nbuttons * (sizeof *button),
> + pdata = devm_kzalloc(dev, sizeof(*pdata) + nbuttons *
> (sizeof(*button)), GFP_KERNEL);
> if (!pdata) {
> error = -ENOMEM;
> @@ -618,7 +618,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
> dev_err(dev,
> "Failed to get gpio flags, error: %d\n",
> error);
> - goto err_free_pdata;
> + goto err_out;
Since the only thing done after err_out label is returning the error, you
can simply return the error here.
> }
>
> button = &pdata->buttons[i++];
> @@ -630,7 +630,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
> dev_err(dev, "Button without keycode: 0x%x\n",
> button->gpio);
> error = -EINVAL;
> - goto err_free_pdata;
> + goto err_out;
Ditto.
> }
>
> button->desc = of_get_property(pp, "label", NULL);
> @@ -647,13 +647,11 @@ gpio_keys_get_devtree_pdata(struct device *dev)
>
> if (pdata->nbuttons == 0) {
> error = -EINVAL;
> - goto err_free_pdata;
> + goto err_out;
Ditto.
> }
>
> return pdata;
>
> -err_free_pdata:
> - kfree(pdata);
> err_out:
> return ERR_PTR(error);
Then the whole error path here can be dropped.
> }
> @@ -699,10 +697,10 @@ static int gpio_keys_probe(struct platform_device
> *pdev) return PTR_ERR(pdata);
> }
>
> - ddata = kzalloc(sizeof(struct gpio_keys_drvdata) +
> + ddata = devm_kzalloc(&pdev->dev, sizeof(struct gpio_keys_drvdata) +
> pdata->nbuttons * sizeof(struct gpio_button_data),
> GFP_KERNEL);
> - input = input_allocate_device();
> + input = devm_input_allocate_device(&pdev->dev);
> if (!ddata || !input) {
> dev_err(dev, "failed to allocate state\n");
> error = -ENOMEM;
> @@ -768,8 +766,6 @@ static int gpio_keys_probe(struct platform_device
> *pdev) gpio_remove_key(&ddata->data[i]);
>
> fail1:
> - input_free_device(input);
> - kfree(ddata);
> /* If we have no platform data, we allocated pdata dynamically. */
> if (!dev_get_platdata(&pdev->dev))
> kfree(pdata);
This is incorrect and unnecessary. Since pdata was allocated using devm_
helper it will be freed automatically. As a side note, if you want to
explicitly free memory allocated using devm_ helpers, you need to do it
using their devm_ free counterparts, not directly.
> @@ -796,8 +792,6 @@ static int gpio_keys_remove(struct platform_device
> *pdev) if (!dev_get_platdata(&pdev->dev))
> kfree(ddata->pdata);
Same here.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
