Hi Pavel, You have missed some things. See inline comments below. On Fri, Jan 10, 2014 at 01:18:05PM +0100, Pavel Machek wrote: > diff --git a/drivers/bluetooth/nokia_core.c b/drivers/bluetooth/nokia_core.c > index 5c7acad..d6b0701 100644 > --- a/drivers/bluetooth/nokia_core.c > v+++ b/drivers/bluetooth/nokia_core.c > @@ -39,6 +39,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -1079,7 +1080,7 @@ static int hci_h4p_probe(struct platform_device *pdev) > int err; > > dev_info(&pdev->dev, "Registering HCI H4P device\n"); > - info = kzalloc(sizeof(struct hci_h4p_info), GFP_KERNEL); > + info = devm_kzalloc(&pdev->dev, sizeof(struct hci_h4p_info), GFP_KERNEL); > if (!info) > return -ENOMEM; > > @@ -1092,7 +1093,6 @@ static int hci_h4p_probe(struct platform_device *pdev) > > if (pdev->dev.platform_data == NULL) { > dev_err(&pdev->dev, "Could not get Bluetooth config data\n"); > - kfree(info); > return -ENODATA; > } > > @@ -1113,67 +1113,59 @@ static int hci_h4p_probe(struct platform_device *pdev) > complete_all(&info->test_completion); > > if (!info->reset_gpio_shared) { > - err = gpio_request(info->reset_gpio, "bt_reset"); > + err = devm_gpio_request_one(&pdev->dev, info->reset_gpio, > + GPIOF_OUT_INIT_LOW, "bt_reset"); > if (err < 0) { > dev_err(&pdev->dev, "Cannot get GPIO line %d\n", > info->reset_gpio); > - goto cleanup_setup; > + return err; > } > } > > - err = gpio_request(info->bt_wakeup_gpio, "bt_wakeup"); > + err = devm_gpio_request_one(&pdev->dev, info->bt_wakeup_gpio, > + GPIOF_OUT_INIT_LOW, "bt_wakeup"); > + > if (err < 0) { > dev_err(info->dev, "Cannot get GPIO line 0x%d", > info->bt_wakeup_gpio); > - if (!info->reset_gpio_shared) > - gpio_free(info->reset_gpio); > - goto cleanup_setup; > + return err; > } > > - err = gpio_request(info->host_wakeup_gpio, "host_wakeup"); > + err = devm_gpio_request_one(&pdev->dev, info->host_wakeup_gpio, > + GPIOF_DIR_IN, "host_wakeup"); > if (err < 0) { > dev_err(info->dev, "Cannot get GPIO line %d", > info->host_wakeup_gpio); > - if (!info->reset_gpio_shared) > - gpio_free(info->reset_gpio); > - gpio_free(info->bt_wakeup_gpio); > - goto cleanup_setup; > + return err; > } > > - gpio_direction_output(info->reset_gpio, 0); > - gpio_direction_output(info->bt_wakeup_gpio, 0); > - gpio_direction_input(info->host_wakeup_gpio); > - > info->irq = bt_plat_data->uart_irq; > - info->uart_base = ioremap(bt_plat_data->uart_base, SZ_2K); > - info->uart_iclk = clk_get(NULL, bt_plat_data->uart_iclk); > - info->uart_fclk = clk_get(NULL, bt_plat_data->uart_fclk); > + info->uart_base = devm_ioremap(&pdev->dev, bt_plat_data->uart_base, SZ_2K); > + info->uart_iclk = devm_clk_get(&pdev->dev, bt_plat_data->uart_iclk); > + info->uart_fclk = devm_clk_get(&pdev->dev, bt_plat_data->uart_fclk); > > err = request_irq(info->irq, hci_h4p_interrupt, IRQF_DISABLED, "hci_h4p", > info); This one should also use devm_request_irq(). Especially since you removed the cleanup in the probe function already ;) > if (err < 0) { > dev_err(info->dev, "hci_h4p: unable to get IRQ %d\n", info->irq); > - goto cleanup; > + return err; > } > > - err = request_irq(gpio_to_irq(info->host_wakeup_gpio), > + err = devm_request_irq(&pdev->dev, gpio_to_irq(info->host_wakeup_gpio), > hci_h4p_wakeup_interrupt, IRQF_TRIGGER_FALLING | > IRQF_TRIGGER_RISING | IRQF_DISABLED, > "hci_h4p_wkup", info); > if (err < 0) { > dev_err(info->dev, "hci_h4p: unable to get wakeup IRQ %d\n", > gpio_to_irq(info->host_wakeup_gpio)); > - free_irq(info->irq, info); > - goto cleanup; > + return err; > } > > err = irq_set_irq_wake(gpio_to_irq(info->host_wakeup_gpio), 1); > if (err < 0) { > dev_err(info->dev, "hci_h4p: unable to set wakeup for IRQ %d\n", > gpio_to_irq(info->host_wakeup_gpio)); > - free_irq(info->irq, info); > - free_irq(gpio_to_irq(info->host_wakeup_gpio), info); > - goto cleanup; > + return err; > } > > init_timer_deferrable(&info->lazy_release); > @@ -1182,7 +1174,7 @@ static int hci_h4p_probe(struct platform_device *pdev) > hci_h4p_set_clk(info, &info->tx_clocks_en, 1); > err = hci_h4p_reset_uart(info); > if (err < 0) > - goto cleanup_irq; > + return err; > gpio_set_value(info->reset_gpio, 0); > hci_h4p_set_clk(info, &info->tx_clocks_en, 0); > > @@ -1190,23 +1182,10 @@ static int hci_h4p_probe(struct platform_device *pdev) > > if (hci_h4p_register_hdev(info) < 0) { > dev_err(info->dev, "failed to register hci_h4p hci device\n"); > - goto cleanup_irq; > + return -EINVAL; > } > > return 0; > - > -cleanup_irq: > - free_irq(info->irq, (void *)info); > - free_irq(gpio_to_irq(info->host_wakeup_gpio), info); > -cleanup: > - gpio_set_value(info->reset_gpio, 0); > - if (!info->reset_gpio_shared) > - gpio_free(info->reset_gpio); > - gpio_free(info->bt_wakeup_gpio); > - gpio_free(info->host_wakeup_gpio); > -cleanup_setup: > - kfree(info); > - return err; > } > > static int hci_h4p_remove(struct platform_device *pdev) > @@ -1217,15 +1196,11 @@ static int hci_h4p_remove(struct platform_device *pdev) > > hci_h4p_sysfs_remove_files(info->dev); > hci_h4p_hci_close(info->hdev); > - free_irq(gpio_to_irq(info->host_wakeup_gpio), info); > hci_unregister_dev(info->hdev); > hci_free_dev(info->hdev); > - if (!info->reset_gpio_shared) > - gpio_free(info->reset_gpio); > gpio_free(info->bt_wakeup_gpio); > gpio_free(info->host_wakeup_gpio); those two gpios are also managed. No need to free them manually. > free_irq(info->irq, (void *) info); Once this irq is also requested using managed resources you can also drop this line. > - kfree(info); > > return 0; > } > [...] -- Sebastian