hwmon: (sht15) check GPIO directions

Without this patch, the SHT15 driver may fail silently with a
non-bidirectional data line and/or an input-only clock line.

This patch checks the return value of gpio_direction_* function calls
and returns the error code (if any) to the caller. If an error occurs in
the read work function (work_funct_t), we wake the queue up directly
without updating the data->state flag, to notice the waiter of the I/O
error.

The patch also makes minor cleanups: s/error_ret/unlock for some labels
and uses devm_gpio_request_one() for the clock line.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>

diff --git a/drivers/hwmon/sht15.c b/drivers/hwmon/sht15.c
index 9a594e6533a9c6877f3f087f123b918e50e2eda2..bfe326e896dfd6d277ed22aec18ff9431ae8b8c1 100644 (file)
--- a/drivers/hwmon/sht15.c
+++ b/drivers/hwmon/sht15.c
@@ -212,11 +212,13 @@ static u8 sht15_crc8(struct sht15_data *data,
  *
  * This implements section 3.4 of the data sheet
  */
-static void sht15_connection_reset(struct sht15_data *data)
+static int sht15_connection_reset(struct sht15_data *data)
 {
-       int i;
+       int i, err;
 
-       gpio_direction_output(data->pdata->gpio_data, 1);
+       err = gpio_direction_output(data->pdata->gpio_data, 1);
+       if (err)
+               return err;
        ndelay(SHT15_TSCKL);
        gpio_set_value(data->pdata->gpio_sck, 0);
        ndelay(SHT15_TSCKL);
@@ -226,6 +228,7 @@ static void sht15_connection_reset(struct sht15_data *data)
                gpio_set_value(data->pdata->gpio_sck, 0);
                ndelay(SHT15_TSCKL);
        }
+       return 0;
 }
 
 /**
@@ -251,10 +254,14 @@ static inline void sht15_send_bit(struct sht15_data *data, int val)
  * conservative ones used in implementation. This implements
  * figure 12 on the data sheet.
  */
-static void sht15_transmission_start(struct sht15_data *data)
+static int sht15_transmission_start(struct sht15_data *data)
 {
+       int err;
+
        /* ensure data is high and output */
-       gpio_direction_output(data->pdata->gpio_data, 1);
+       err = gpio_direction_output(data->pdata->gpio_data, 1);
+       if (err)
+               return err;
        ndelay(SHT15_TSU);
        gpio_set_value(data->pdata->gpio_sck, 0);
        ndelay(SHT15_TSCKL);
@@ -270,6 +277,7 @@ static void sht15_transmission_start(struct sht15_data *data)
        ndelay(SHT15_TSU);
        gpio_set_value(data->pdata->gpio_sck, 0);
        ndelay(SHT15_TSCKL);
+       return 0;
 }
 
 /**
@@ -293,13 +301,19 @@ static void sht15_send_byte(struct sht15_data *data, u8 byte)
  */
 static int sht15_wait_for_response(struct sht15_data *data)
 {
-       gpio_direction_input(data->pdata->gpio_data);
+       int err;
+
+       err = gpio_direction_input(data->pdata->gpio_data);
+       if (err)
+               return err;
        gpio_set_value(data->pdata->gpio_sck, 1);
        ndelay(SHT15_TSCKH);
        if (gpio_get_value(data->pdata->gpio_data)) {
                gpio_set_value(data->pdata->gpio_sck, 0);
                dev_err(data->dev, "Command not acknowledged\n");
-               sht15_connection_reset(data);
+               err = sht15_connection_reset(data);
+               if (err)
+                       return err;
                return -EIO;
        }
        gpio_set_value(data->pdata->gpio_sck, 0);
@@ -317,12 +331,13 @@ static int sht15_wait_for_response(struct sht15_data *data)
  */
 static int sht15_send_cmd(struct sht15_data *data, u8 cmd)
 {
-       int ret = 0;
+       int err;
 
-       sht15_transmission_start(data);
+       err = sht15_transmission_start(data);
+       if (err)
+               return err;
        sht15_send_byte(data, cmd);
-       ret = sht15_wait_for_response(data);
-       return ret;
+       return sht15_wait_for_response(data);
 }
 
 /**
@@ -352,9 +367,13 @@ static int sht15_soft_reset(struct sht15_data *data)
  * Each byte of data is acknowledged by pulling the data line
  * low for one clock pulse.
  */
-static void sht15_ack(struct sht15_data *data)
+static int sht15_ack(struct sht15_data *data)
 {
-       gpio_direction_output(data->pdata->gpio_data, 0);
+       int err;
+
+       err = gpio_direction_output(data->pdata->gpio_data, 0);
+       if (err)
+               return err;
        ndelay(SHT15_TSU);
        gpio_set_value(data->pdata->gpio_sck, 1);
        ndelay(SHT15_TSU);
@@ -362,7 +381,7 @@ static void sht15_ack(struct sht15_data *data)
        ndelay(SHT15_TSU);
        gpio_set_value(data->pdata->gpio_data, 1);
 
-       gpio_direction_input(data->pdata->gpio_data);
+       return gpio_direction_input(data->pdata->gpio_data);
 }
 
 /**
@@ -371,14 +390,19 @@ static void sht15_ack(struct sht15_data *data)
  *
  * This is basically a NAK (single clock pulse, data high).
  */
-static void sht15_end_transmission(struct sht15_data *data)
+static int sht15_end_transmission(struct sht15_data *data)
 {
-       gpio_direction_output(data->pdata->gpio_data, 1);
+       int err;
+
+       err = gpio_direction_output(data->pdata->gpio_data, 1);
+       if (err)
+               return err;
        ndelay(SHT15_TSU);
        gpio_set_value(data->pdata->gpio_sck, 1);
        ndelay(SHT15_TSCKH);
        gpio_set_value(data->pdata->gpio_sck, 0);
        ndelay(SHT15_TSCKL);
+       return 0;
 }
 
 /**
@@ -410,17 +434,19 @@ static u8 sht15_read_byte(struct sht15_data *data)
  */
 static int sht15_send_status(struct sht15_data *data, u8 status)
 {
-       int ret;
-
-       ret = sht15_send_cmd(data, SHT15_WRITE_STATUS);
-       if (ret)
-               return ret;
-       gpio_direction_output(data->pdata->gpio_data, 1);
+       int err;
+
+       err = sht15_send_cmd(data, SHT15_WRITE_STATUS);
+       if (err)
+               return err;
+       err = gpio_direction_output(data->pdata->gpio_data, 1);
+       if (err)
+               return err;
        ndelay(SHT15_TSU);
        sht15_send_byte(data, status);
-       ret = sht15_wait_for_response(data);
-       if (ret)
-               return ret;
+       err = sht15_wait_for_response(data);
+       if (err)
+               return err;
 
        data->val_status = status;
        return 0;
@@ -446,7 +472,7 @@ static int sht15_update_status(struct sht15_data *data)
                        || !data->status_valid) {
                ret = sht15_send_cmd(data, SHT15_READ_STATUS);
                if (ret)
-                       goto error_ret;
+                       goto unlock;
                status = sht15_read_byte(data);
 
                if (data->checksumming) {
@@ -458,7 +484,9 @@ static int sht15_update_status(struct sht15_data *data)
                                        == dev_checksum);
                }
 
-               sht15_end_transmission(data);
+               ret = sht15_end_transmission(data);
+               if (ret)
+                       goto unlock;
 
                /*
                 * Perform checksum validation on the received data.
@@ -469,27 +497,27 @@ static int sht15_update_status(struct sht15_data *data)
                        previous_config = data->val_status & 0x07;
                        ret = sht15_soft_reset(data);
                        if (ret)
-                               goto error_ret;
+                               goto unlock;
                        if (previous_config) {
                                ret = sht15_send_status(data, previous_config);
                                if (ret) {
                                        dev_err(data->dev,
                                                "CRC validation failed, unable "
                                                "to restore device settings\n");
-                                       goto error_ret;
+                                       goto unlock;
                                }
                        }
                        ret = -EAGAIN;
-                       goto error_ret;
+                       goto unlock;
                }
 
                data->val_status = status;
                data->status_valid = true;
                data->last_status = jiffies;
        }
-error_ret:
-       mutex_unlock(&data->read_lock);
 
+unlock:
+       mutex_unlock(&data->read_lock);
        return ret;
 }
 
@@ -511,7 +539,9 @@ static int sht15_measurement(struct sht15_data *data,
        if (ret)
                return ret;
 
-       gpio_direction_input(data->pdata->gpio_data);
+       ret = gpio_direction_input(data->pdata->gpio_data);
+       if (ret)
+               return ret;
        atomic_set(&data->interrupt_handled, 0);
 
        enable_irq(gpio_to_irq(data->pdata->gpio_data));
@@ -524,9 +554,14 @@ static int sht15_measurement(struct sht15_data *data,
        ret = wait_event_timeout(data->wait_queue,
                                 (data->state == SHT15_READING_NOTHING),
                                 msecs_to_jiffies(timeout_msecs));
-       if (ret == 0) {/* timeout occurred */
+       if (data->state != SHT15_READING_NOTHING) { /* I/O error occurred */
+               data->state = SHT15_READING_NOTHING;
+               return -EIO;
+       } else if (ret == 0) { /* timeout occurred */
                disable_irq_nosync(gpio_to_irq(data->pdata->gpio_data));
-               sht15_connection_reset(data);
+               ret = sht15_connection_reset(data);
+               if (ret)
+                       return ret;
                return -ETIME;
        }
 
@@ -570,17 +605,17 @@ static int sht15_update_measurements(struct sht15_data *data)
                data->state = SHT15_READING_HUMID;
                ret = sht15_measurement(data, SHT15_MEASURE_RH, 160);
                if (ret)
-                       goto error_ret;
+                       goto unlock;
                data->state = SHT15_READING_TEMP;
                ret = sht15_measurement(data, SHT15_MEASURE_TEMP, 400);
                if (ret)
-                       goto error_ret;
+                       goto unlock;
                data->measurements_valid = true;
                data->last_measurement = jiffies;
        }
-error_ret:
-       mutex_unlock(&data->read_lock);
 
+unlock:
+       mutex_unlock(&data->read_lock);
        return ret;
 }
 
@@ -818,7 +853,8 @@ static void sht15_bh_read_data(struct work_struct *work_s)
        /* Read the data back from the device */
        val = sht15_read_byte(data);
        val <<= 8;
-       sht15_ack(data);
+       if (sht15_ack(data))
+               goto wakeup;
        val |= sht15_read_byte(data);
 
        if (data->checksumming) {
@@ -826,7 +862,8 @@ static void sht15_bh_read_data(struct work_struct *work_s)
                 * Ask the device for a checksum and read it back.
                 * Note: the device sends the checksum byte reversed.
                 */
-               sht15_ack(data);
+               if (sht15_ack(data))
+                       goto wakeup;
                dev_checksum = sht15_reverse(sht15_read_byte(data));
                checksum_vals[0] = (data->state == SHT15_READING_TEMP) ?
                        SHT15_MEASURE_TEMP : SHT15_MEASURE_RH;
@@ -837,7 +874,8 @@ static void sht15_bh_read_data(struct work_struct *work_s)
        }
 
        /* Tell the device we are done */
-       sht15_end_transmission(data);
+       if (sht15_end_transmission(data))
+               goto wakeup;
 
        switch (data->state) {
        case SHT15_READING_TEMP:
@@ -851,6 +889,7 @@ static void sht15_bh_read_data(struct work_struct *work_s)
        }
 
        data->state = SHT15_READING_NOTHING;
+wakeup:
        wake_up(&data->wait_queue);
 }
 
@@ -942,17 +981,17 @@ static int sht15_probe(struct platform_device *pdev)
        }
 
        /* Try requesting the GPIOs */
-       ret = devm_gpio_request(&pdev->dev, data->pdata->gpio_sck, "SHT15 sck");
+       ret = devm_gpio_request_one(&pdev->dev, data->pdata->gpio_sck,
+                       GPIOF_OUT_INIT_LOW, "SHT15 sck");
        if (ret) {
-               dev_err(&pdev->dev, "gpio request failed\n");
+               dev_err(&pdev->dev, "clock line GPIO request failed\n");
                goto err_release_reg;
        }
-       gpio_direction_output(data->pdata->gpio_sck, 0);
 
        ret = devm_gpio_request(&pdev->dev, data->pdata->gpio_data,
                                "SHT15 data");
        if (ret) {
-               dev_err(&pdev->dev, "gpio request failed\n");
+               dev_err(&pdev->dev, "data line GPIO request failed\n");
                goto err_release_reg;
        }
 
@@ -966,7 +1005,9 @@ static int sht15_probe(struct platform_device *pdev)
                goto err_release_reg;
        }
        disable_irq_nosync(gpio_to_irq(data->pdata->gpio_data));
-       sht15_connection_reset(data);
+       ret = sht15_connection_reset(data);
+       if (ret)
+               goto err_release_reg;
        ret = sht15_soft_reset(data);
        if (ret)
                goto err_release_reg;