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>

1 files changed, 88 insertions, 47 deletions

diff --git a/drivers/hwmon/sht15.c b/drivers/hwmon/sht15.c
index 9a594e6533a9..bfe326e896df 100644
--- 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;