[PATCH 1/3] iio: light: tcs3472: use devm for resource management
Aldo Conte <[email protected]>
| Newsgroups | dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Found by code inspection. Convert the driver to use device-managed resource allocation: - Add tcs3472_powerdown_action() and register it with devm_add_action_or_reset() to ensure the device is powered down on cleanup. Before this patch, the chip remained powered if probe failed after enabling it. - Replace iio_triggered_buffer_setup() with devm_iio_triggered_buffer_setup(). - Replace request_threaded_irq() with devm_request_threaded_irq(). - Replace iio_device_register() with devm_iio_device_register(). - Remove tcs3472_remove() as all cleanup is now handled by devm. Compiled with `make drivers/iio/light/tcs3472.o W=1`. Tested on Raspberry Pi 3B with TCS3472 (Adafruit breakout): Verified that RGBC channel reads return valid data and that ENABLE register is cleared to 0x00 on device removal. Signed-off-by: Aldo Conte <[email protected]> --- drivers/iio/light/tcs3472.c | 85 ++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 49 deletions(-) diff --git a/drivers/iio/light/tcs3472.c b/drivers/iio/light/tcs3472.c index 12429a3261b3..6d37a473c420 100644 --- a/drivers/iio/light/tcs3472.c +++ b/drivers/iio/light/tcs3472.c @@ -440,6 +440,28 @@ static const struct iio_info tcs3472_info = { .attrs = &tcs3472_attribute_group, }; +static int tcs3472_powerdown(struct tcs3472_data *data) +{ + int ret; + u8 enable_mask = TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON; + + mutex_lock(&data->lock); + + ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE, + data->enable & ~enable_mask); + if (!ret) + data->enable &= ~enable_mask; + + mutex_unlock(&data->lock); + + return ret; +} + +static void tcs3472_powerdown_action(void *data) +{ + tcs3472_powerdown(data); +} + static int tcs3472_probe(struct i2c_client *client) { struct tcs3472_data *data; @@ -510,61 +532,27 @@ static int tcs3472_probe(struct i2c_client *client) if (ret < 0) return ret; - ret = iio_triggered_buffer_setup(indio_dev, NULL, - tcs3472_trigger_handler, NULL); + ret = devm_add_action_or_reset(&client->dev, + tcs3472_powerdown_action, data); + if (ret) + return ret; + + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL, + tcs3472_trigger_handler, NULL); if (ret < 0) return ret; if (client->irq) { - ret = request_threaded_irq(client->irq, NULL, - tcs3472_event_handler, - IRQF_TRIGGER_FALLING | IRQF_SHARED | - IRQF_ONESHOT, - client->name, indio_dev); + ret = devm_request_threaded_irq(&client->dev, client->irq, + NULL, tcs3472_event_handler, + IRQF_TRIGGER_FALLING | IRQF_SHARED | + IRQF_ONESHOT, + client->name, indio_dev); if (ret) - goto buffer_cleanup; + return ret; } - ret = iio_device_register(indio_dev); - if (ret < 0) - goto free_irq; - - return 0; - -free_irq: - if (client->irq) - free_irq(client->irq, indio_dev); -buffer_cleanup: - iio_triggered_buffer_cleanup(indio_dev); - return ret; -} - -static int tcs3472_powerdown(struct tcs3472_data *data) -{ - int ret; - u8 enable_mask = TCS3472_ENABLE_AEN | TCS3472_ENABLE_PON; - - mutex_lock(&data->lock); - - ret = i2c_smbus_write_byte_data(data->client, TCS3472_ENABLE, - data->enable & ~enable_mask); - if (!ret) - data->enable &= ~enable_mask; - - mutex_unlock(&data->lock); - - return ret; -} - -static void tcs3472_remove(struct i2c_client *client) -{ - struct iio_dev *indio_dev = i2c_get_clientdata(client); - - iio_device_unregister(indio_dev); - if (client->irq) - free_irq(client->irq, indio_dev); - iio_triggered_buffer_cleanup(indio_dev); - tcs3472_powerdown(iio_priv(indio_dev)); + return devm_iio_device_register(&client->dev, indio_dev); } static int tcs3472_suspend(struct device *dev) @@ -608,7 +596,6 @@ static struct i2c_driver tcs3472_driver = { .pm = pm_sleep_ptr(&tcs3472_pm_ops), }, .probe = tcs3472_probe, - .remove = tcs3472_remove, .id_table = tcs3472_id, }; module_i2c_driver(tcs3472_driver); -- 2.54.0