qcom-adc: fix up the ads7924 driver
Two main fixes:
1. Use '_cansleep' variant for the gpio functions to handle the cases where
the reset gpio might be on an expander. Also, actually reset the chip
when we request the GPIO and then pull it out of reset if we're
sucessful getting the gpio. This was the original intent of the reset
gpio, but now requires the GPIO_ACTIVE_LOW to be set in the device-tree
for the reset-gpios.
2. Use '_sync' variant to safely clean up the sample timer and workqueue.
Also, update the adc averaging logic to use devm_ functions so that I
don't need to worry about those leaking.
Change-Id: I2f0d95cc30018c18c4c2a12b3516e76f3087492a
diff --git a/qcom-adc/ads7924-driver.c b/qcom-adc/ads7924-driver.c
index 356b23a..c62199a 100644
--- a/qcom-adc/ads7924-driver.c
+++ b/qcom-adc/ads7924-driver.c
@@ -63,24 +63,16 @@
};
-void ads7924_timer_irq(unsigned long arg) {
+void ads7924_timer_irq(unsigned long arg)
+{
struct ads7924_data *data = (struct ads7924_data *)arg;
+
if (data->enabled) {
mod_timer(&data->sample_timer, jiffies + msecs_to_jiffies(data->update_interval));
schedule_work(&data->workq);
}
}
-void ads7924_start_sampler(struct ads7924_data *data)
-{
- setup_timer(&data->sample_timer, ads7924_timer_irq, (unsigned long) data);
- mod_timer(&data->sample_timer, jiffies + msecs_to_jiffies(data->update_interval));
-}
-void ads7924_stop_sampler(struct ads7924_data *data)
-{
- del_timer(&data->sample_timer);
-}
-
static uint16_t ads7924_sample_channel(struct i2c_client *client, uint8_t channel)
{
int err;
@@ -152,7 +144,7 @@
data->sample_length = length;
for (i = 0; i < ADS7924_NCH; i++) {
free_adc_channel(data->channel[i]);
- data->channel[i] = create_adc_channel(data->sample_length);
+ data->channel[i] = create_adc_channel(dev, data->sample_length);
if (!data->channel[i]) {
dev_warn(&client->dev, "Failed to resize the channel buffer\n");
/*TODO: If this happens, we're pretty screwed. Like, we should unload ourselves. */
@@ -183,12 +175,13 @@
return err;
mutex_lock(&data->update_lock);
- if (enabled)
- data->enabled = 1;
- else
- data->enabled = 0;
+ if (enabled != data->enabled) {
+ data->enabled = enabled;
+ if (enabled)
+ mod_timer(&data->sample_timer, 0);
+ }
mutex_unlock(&data->update_lock);
- ads7924_timer_irq((unsigned long) data);
+
return length;
}
static ssize_t enable_show(struct device *dev, struct device_attribute *da, char *buf)
@@ -291,19 +284,21 @@
struct ads7924_data *data = i2c_get_clientdata(client);
int i;
- ads7924_stop_sampler(data);
- mutex_lock(&data->update_lock);
- for (i = 0; i < ADS7924_NCH; i++)
- {
- if (data->channel[i]) {
- free_adc_channel(data->channel[i]);
- }
- data->channel[i] = NULL;
- }
-
hwmon_device_unregister(data->hwmon_dev);
+
sysfs_remove_group(&client->dev.kobj, &ads7924_group);
+
+ mutex_lock(&data->update_lock);
+ data->enabled = 0;
+ for (i = 0; i < ADS7924_NCH; i++)
+ free_adc_channel(data->channel[i]);
+
mutex_unlock(&data->update_lock);
+ del_timer_sync(&data->sample_timer);
+ cancel_work_sync(&data->workq);
+
+ mutex_destroy(&data->update_lock);
+
return 0;
}
@@ -324,28 +319,31 @@
data->sample_length = ADS7924_DEFAULT_BUFFER_SIZE;
data->enabled = ADS7924_ENABLE_DEFAULT;
data->client = client;
- data->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
- if (IS_ERR(data->reset)){
+ data->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH); /* Grab reset GPIO if available and assert reset */
+ if (IS_ERR(data->reset)) {
dev_warn(&client->dev, "Failed to request the reset gpio, error=%ld\n", PTR_ERR(data->reset));
data->reset = NULL;
} else {
- dev_info(&client->dev, "Successfully claimed the reset gpio (gpio %d), value is %d\n",desc_to_gpio(data->reset), gpiod_get_value(data->reset));
+ dev_info(&client->dev, "Successfully claimed the reset gpio (gpio %d), value is %d\n", desc_to_gpio(data->reset), gpiod_get_value_cansleep(data->reset));
+ mdelay(5);
+ gpiod_set_value_cansleep(data->reset, 0); /* Deassert reset */
+ mdelay(10);
}
- mdelay(10);
smbus_err = i2c_smbus_read_byte_data(client, ADS7924_REG_RESET);
- if (smbus_err < 0)
+ if (smbus_err < 0) {
dev_warn(&client->dev, "failed to read device id register 0x%02X, err=%d\n", ADS7924_REG_RESET, smbus_err);
-
- dev_id = (uint8_t) (smbus_err & 0xFF);
+ return smbus_err;
+ }
+
+ dev_id = (uint8_t) smbus_err;
if (dev_id != 0x19) { /* This is my device's ID, at least. */
dev_warn(&client->dev, "Device id of 0x%02x doesn't match the expected 0x19\n", dev_id);
- err = -ENODEV;
- goto err_free;
+ return -ENODEV;
}
for (i = 0; i < ADS7924_NCH; i++) {
- data->channel[i] = create_adc_channel(data->sample_length);
+ data->channel[i] = create_adc_channel(&client->dev, data->sample_length);
if (!data->channel[i]) {
dev_warn(&client->dev, "Failed to allocate memeory for adc_channel %d\n", i);
return -ENOMEM;
@@ -368,14 +366,13 @@
dev_err(&client->dev, "Failed to register with hwmon, error=%d\n", err);
goto err_remove;
}
- ads7924_start_sampler(data);
+ setup_timer(&data->sample_timer, ads7924_timer_irq, (unsigned long) data);
return 0;
err_remove:
+ mutex_destroy(&data->update_lock);
sysfs_remove_group(&client->dev.kobj, &ads7924_group);
-err_free:
- devm_kfree(&client->dev, data);
return err;
}