Good point about the timer! > > > > - err = lm3554_gpio_init(client); > > - if (err) { > > + ret = lm3554_gpio_init(client); > > + if (ret) { > > dev_err(&client->dev, "gpio request/direction_output fail"); > > - goto fail2; > > + goto err_cleanup_entity; > > } > > return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); > > If atomisp_register_i2c_module() fails then we need to call > lm3554_gpio_uninit(client) and do other cleanup. I'm probably showing my ignorance here, but I can't see what cleanup we need. Inside lm3554_gpio_init we have: ret = gpiod_direction_output(pdata->gpio_reset, 0); if (ret < 0) return ret; dev_info(&client->dev, "flash led reset successfully\n"); if (!pdata->gpio_strobe) return -EINVAL; ret = gpiod_direction_output(pdata->gpio_strobe, 0); if (ret < 0) return ret; I'm not sure how you "undo" a call to gpiod_direction_output, but I'm thinking you won't need to do anything because it should be ok to leave a gpio to output 0... right? Alex > > > -fail2: > > + > > +err_cleanup_entity: > > media_entity_cleanup(&flash->sd.entity); > > +err_free_ctrl_handler: > > v4l2_ctrl_handler_free(&flash->ctrl_handler); > > regards, > dan carpenter >