Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 9 Mar 2020 20:26:53 +0530
From:   Syed Nayyar Waris <syednwaris@gmail.com>
To:     vilhelm.gray@gmail.com
Cc:     jic23@kernel.org, linux-iio@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: [PATCH] counter: 104-quad-8: Add lock protection
Message-ID: <20200309145653.GA3329@syed.domain.name>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Add lock protection from race conditions in the 104-quad-8 counter
driver. There is no IRQ handling so spin_lock calls are used for
protection.

Signed-off-by: Syed Nayyar Waris <syednwaris@gmail.com>
---
 drivers/counter/104-quad-8.c | 138 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 136 insertions(+), 2 deletions(-)

diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c
index 9dab190..431c754 100644
--- a/drivers/counter/104-quad-8.c
+++ b/drivers/counter/104-quad-8.c
@@ -45,6 +45,7 @@ MODULE_PARM_DESC(base, "ACCES 104-QUAD-8 base addresses");
  */
 struct quad8_iio {
 	struct counter_device counter;
+	spinlock_t lock;
 	unsigned int fck_prescaler[QUAD8_NUM_COUNTERS];
 	unsigned int preset[QUAD8_NUM_COUNTERS];
 	unsigned int count_mode[QUAD8_NUM_COUNTERS];
@@ -111,15 +112,24 @@ static int quad8_read_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		if (chan->type == IIO_INDEX) {
+			spin_lock(&priv->lock);
 			*val = !!(inb(priv->base + QUAD8_REG_INDEX_INPUT_LEVELS)
 				& BIT(chan->channel));
+			spin_unlock(&priv->lock);
 			return IIO_VAL_INT;
 		}
 
+		spin_lock(&priv->lock);
+
 		flags = inb(base_offset + 1);
+
+		spin_unlock(&priv->lock);
+
 		borrow = flags & QUAD8_FLAG_BT;
 		carry = !!(flags & QUAD8_FLAG_CT);
 
+		spin_lock(&priv->lock);
+
 		/* Borrow XOR Carry effectively doubles count range */
 		*val = (borrow ^ carry) << 24;
 
@@ -130,6 +140,8 @@ static int quad8_read_raw(struct iio_dev *indio_dev,
 		for (i = 0; i < 3; i++)
 			*val |= (unsigned int)inb(base_offset) << (8 * i);
 
+		spin_unlock(&priv->lock);
+
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_ENABLE:
 		*val = priv->ab_enable[chan->channel];
@@ -160,6 +172,8 @@ static int quad8_write_raw(struct iio_dev *indio_dev,
 		if ((unsigned int)val > 0xFFFFFF)
 			return -EINVAL;
 
+		spin_lock(&priv->lock);
+
 		/* Reset Byte Pointer */
 		outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_BP, base_offset + 1);
 
@@ -183,12 +197,16 @@ static int quad8_write_raw(struct iio_dev *indio_dev,
 		/* Reset Error flag */
 		outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_E, base_offset + 1);
 
+		spin_unlock(&priv->lock);
+
 		return 0;
 	case IIO_CHAN_INFO_ENABLE:
 		/* only boolean values accepted */
 		if (val < 0 || val > 1)
 			return -EINVAL;
 
+		spin_lock(&priv->lock);
+
 		priv->ab_enable[chan->channel] = val;
 
 		ior_cfg = val | priv->preset_enable[chan->channel] << 1;
@@ -196,6 +214,8 @@ static int quad8_write_raw(struct iio_dev *indio_dev,
 		/* Load I/O control configuration */
 		outb(QUAD8_CTR_IOR | ior_cfg, base_offset + 1);
 
+		spin_unlock(&priv->lock);
+
 		return 0;
 	case IIO_CHAN_INFO_SCALE:
 		/* Quadrature scaling only available in quadrature mode */
@@ -255,6 +275,8 @@ static ssize_t quad8_write_preset(struct iio_dev *indio_dev, uintptr_t private,
 	if (preset > 0xFFFFFF)
 		return -EINVAL;
 
+	spin_lock(&priv->lock);
+
 	priv->preset[chan->channel] = preset;
 
 	/* Reset Byte Pointer */
@@ -264,6 +286,8 @@ static ssize_t quad8_write_preset(struct iio_dev *indio_dev, uintptr_t private,
 	for (i = 0; i < 3; i++)
 		outb(preset >> (8 * i), base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return len;
 }
 
@@ -293,6 +317,8 @@ static ssize_t quad8_write_set_to_preset_on_index(struct iio_dev *indio_dev,
 	/* Preset enable is active low in Input/Output Control register */
 	preset_enable = !preset_enable;
 
+	spin_lock(&priv->lock);
+
 	priv->preset_enable[chan->channel] = preset_enable;
 
 	ior_cfg = priv->ab_enable[chan->channel] |
@@ -301,6 +327,8 @@ static ssize_t quad8_write_set_to_preset_on_index(struct iio_dev *indio_dev,
 	/* Load I/O control configuration to Input / Output Control Register */
 	outb(QUAD8_CTR_IOR | ior_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return len;
 }
 
@@ -314,8 +342,15 @@ static int quad8_get_noise_error(struct iio_dev *indio_dev,
 {
 	struct quad8_iio *const priv = iio_priv(indio_dev);
 	const int base_offset = priv->base + 2 * chan->channel + 1;
+	int quad8_noise_error;
 
-	return !!(inb(base_offset) & QUAD8_FLAG_E);
+	spin_lock(&priv->lock);
+
+	quad8_noise_error = !!(inb(base_offset) & QUAD8_FLAG_E);
+
+	spin_unlock(&priv->lock);
+
+	return quad8_noise_error;
 }
 
 static const struct iio_enum quad8_noise_error_enum = {
@@ -334,8 +369,15 @@ static int quad8_get_count_direction(struct iio_dev *indio_dev,
 {
 	struct quad8_iio *const priv = iio_priv(indio_dev);
 	const int base_offset = priv->base + 2 * chan->channel + 1;
+	int quad8_count_direction;
+
+	spin_lock(&priv->lock);
 
-	return !!(inb(base_offset) & QUAD8_FLAG_UD);
+	quad8_count_direction = !!(inb(base_offset) & QUAD8_FLAG_UD);
+
+	spin_unlock(&priv->lock);
+
+	return quad8_count_direction;
 }
 
 static const struct iio_enum quad8_count_direction_enum = {
@@ -358,6 +400,8 @@ static int quad8_set_count_mode(struct iio_dev *indio_dev,
 	unsigned int mode_cfg = cnt_mode << 1;
 	const int base_offset = priv->base + 2 * chan->channel + 1;
 
+	spin_lock(&priv->lock);
+
 	priv->count_mode[chan->channel] = cnt_mode;
 
 	/* Add quadrature mode configuration */
@@ -367,6 +411,8 @@ static int quad8_set_count_mode(struct iio_dev *indio_dev,
 	/* Load mode configuration to Counter Mode Register */
 	outb(QUAD8_CTR_CMR | mode_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -402,11 +448,15 @@ static int quad8_set_synchronous_mode(struct iio_dev *indio_dev,
 	if (synchronous_mode && !priv->quadrature_mode[chan->channel])
 		return -EINVAL;
 
+	spin_lock(&priv->lock);
+
 	priv->synchronous_mode[chan->channel] = synchronous_mode;
 
 	/* Load Index Control configuration to Index Control Register */
 	outb(QUAD8_CTR_IDR | idr_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -448,11 +498,15 @@ static int quad8_set_quadrature_mode(struct iio_dev *indio_dev,
 			quad8_set_synchronous_mode(indio_dev, chan, 0);
 	}
 
+	spin_lock(&priv->lock);
+
 	priv->quadrature_mode[chan->channel] = quadrature_mode;
 
 	/* Load mode configuration to Counter Mode Register */
 	outb(QUAD8_CTR_CMR | mode_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -484,11 +538,15 @@ static int quad8_set_index_polarity(struct iio_dev *indio_dev,
 		index_polarity << 1;
 	const int base_offset = priv->base + 2 * chan->channel + 1;
 
+	spin_lock(&priv->lock);
+
 	priv->index_polarity[chan->channel] = index_polarity;
 
 	/* Load Index Control configuration to Index Control Register */
 	outb(QUAD8_CTR_IDR | idr_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -578,9 +636,13 @@ static int quad8_signal_read(struct counter_device *counter,
 	if (signal->id < 16)
 		return -EINVAL;
 
+	spin_lock(&((struct quad8_iio *)priv)->lock);
+
 	state = inb(priv->base + QUAD8_REG_INDEX_INPUT_LEVELS)
 		& BIT(signal->id - 16);
 
+	spin_unlock(&((struct quad8_iio *)priv)->lock);
+
 	*val = (state) ? COUNTER_SIGNAL_HIGH : COUNTER_SIGNAL_LOW;
 
 	return 0;
@@ -596,10 +658,17 @@ static int quad8_count_read(struct counter_device *counter,
 	unsigned int carry;
 	int i;
 
+	spin_lock(&((struct quad8_iio *)priv)->lock);
+
 	flags = inb(base_offset + 1);
+
+	spin_unlock(&((struct quad8_iio *)priv)->lock);
+
 	borrow = flags & QUAD8_FLAG_BT;
 	carry = !!(flags & QUAD8_FLAG_CT);
 
+	spin_lock(&((struct quad8_iio *)priv)->lock);
+
 	/* Borrow XOR Carry effectively doubles count range */
 	*val = (unsigned long)(borrow ^ carry) << 24;
 
@@ -610,6 +679,8 @@ static int quad8_count_read(struct counter_device *counter,
 	for (i = 0; i < 3; i++)
 		*val |= (unsigned long)inb(base_offset) << (8 * i);
 
+	spin_unlock(&((struct quad8_iio *)priv)->lock);
+
 	return 0;
 }
 
@@ -624,6 +695,8 @@ static int quad8_count_write(struct counter_device *counter,
 	if (val > 0xFFFFFF)
 		return -EINVAL;
 
+	spin_lock(&((struct quad8_iio *)priv)->lock);
+
 	/* Reset Byte Pointer */
 	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_BP, base_offset + 1);
 
@@ -647,6 +720,8 @@ static int quad8_count_write(struct counter_device *counter,
 	/* Reset Error flag */
 	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_E, base_offset + 1);
 
+	spin_unlock(&((struct quad8_iio *)priv)->lock);
+
 	return 0;
 }
 
@@ -711,8 +786,11 @@ static int quad8_function_set(struct counter_device *counter,
 		/* Synchronous function not supported in non-quadrature mode */
 		if (*synchronous_mode) {
 			*synchronous_mode = 0;
+			spin_lock(&priv->lock);
 			/* Disable synchronous function mode */
 			outb(QUAD8_CTR_IDR | idr_cfg, base_offset);
+			spin_unlock(&priv->lock);
+
 		}
 	} else {
 		*quadrature_mode = 1;
@@ -733,9 +811,13 @@ static int quad8_function_set(struct counter_device *counter,
 		}
 	}
 
+	spin_lock(&priv->lock);
+
 	/* Load mode configuration to Counter Mode Register */
 	outb(QUAD8_CTR_CMR | mode_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -746,9 +828,13 @@ static void quad8_direction_get(struct counter_device *counter,
 	unsigned int ud_flag;
 	const unsigned int flag_addr = priv->base + 2 * count->id + 1;
 
+	spin_lock(&((struct quad8_iio *)priv)->lock);
+
 	/* U/D flag: nonzero = up, zero = down */
 	ud_flag = inb(flag_addr) & QUAD8_FLAG_UD;
 
+	spin_unlock(&((struct quad8_iio *)priv)->lock);
+
 	*direction = (ud_flag) ? COUNTER_COUNT_DIRECTION_FORWARD :
 		COUNTER_COUNT_DIRECTION_BACKWARD;
 }
@@ -856,11 +942,15 @@ static int quad8_index_polarity_set(struct counter_device *counter,
 		index_polarity << 1;
 	const int base_offset = priv->base + 2 * channel_id + 1;
 
+	spin_lock(&priv->lock);
+
 	priv->index_polarity[channel_id] = index_polarity;
 
 	/* Load Index Control configuration to Index Control Register */
 	outb(QUAD8_CTR_IDR | idr_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -895,11 +985,15 @@ static int quad8_synchronous_mode_set(struct counter_device *counter,
 	if (synchronous_mode && !priv->quadrature_mode[channel_id])
 		return -EINVAL;
 
+	spin_lock(&priv->lock);
+
 	priv->synchronous_mode[channel_id] = synchronous_mode;
 
 	/* Load Index Control configuration to Index Control Register */
 	outb(QUAD8_CTR_IDR | idr_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -964,6 +1058,8 @@ static int quad8_count_mode_set(struct counter_device *counter,
 		break;
 	}
 
+	spin_lock(&priv->lock);
+
 	priv->count_mode[count->id] = cnt_mode;
 
 	/* Set count mode configuration value */
@@ -976,6 +1072,8 @@ static int quad8_count_mode_set(struct counter_device *counter,
 	/* Load mode configuration to Counter Mode Register */
 	outb(QUAD8_CTR_CMR | mode_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return 0;
 }
 
@@ -1017,6 +1115,8 @@ static ssize_t quad8_count_enable_write(struct counter_device *counter,
 	if (err)
 		return err;
 
+	spin_lock(&priv->lock);
+
 	priv->ab_enable[count->id] = ab_enable;
 
 	ior_cfg = ab_enable | priv->preset_enable[count->id] << 1;
@@ -1024,6 +1124,8 @@ static ssize_t quad8_count_enable_write(struct counter_device *counter,
 	/* Load I/O control configuration */
 	outb(QUAD8_CTR_IOR | ior_cfg, base_offset + 1);
 
+	spin_unlock(&priv->lock);
+
 	return len;
 }
 
@@ -1033,8 +1135,12 @@ static int quad8_error_noise_get(struct counter_device *counter,
 	const struct quad8_iio *const priv = counter->priv;
 	const int base_offset = priv->base + 2 * count->id + 1;
 
+	spin_lock(&((struct quad8_iio *)priv)->lock);
+
 	*noise_error = !!(inb(base_offset) & QUAD8_FLAG_E);
 
+	spin_unlock(&((struct quad8_iio *)priv)->lock);
+
 	return 0;
 }
 
@@ -1069,6 +1175,8 @@ static ssize_t quad8_count_preset_write(struct counter_device *counter,
 	if (preset > 0xFFFFFF)
 		return -EINVAL;
 
+	spin_lock(&priv->lock);
+
 	priv->preset[count->id] = preset;
 
 	/* Reset Byte Pointer */
@@ -1078,6 +1186,8 @@ static ssize_t quad8_count_preset_write(struct counter_device *counter,
 	for (i = 0; i < 3; i++)
 		outb(preset >> (8 * i), base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return len;
 }
 
@@ -1137,6 +1247,8 @@ static ssize_t quad8_count_preset_enable_write(struct counter_device *counter,
 	/* Preset enable is active low in Input/Output Control register */
 	preset_enable = !preset_enable;
 
+	spin_lock(&priv->lock);
+
 	priv->preset_enable[count->id] = preset_enable;
 
 	ior_cfg = priv->ab_enable[count->id] | (unsigned int)preset_enable << 1;
@@ -1144,6 +1256,8 @@ static ssize_t quad8_count_preset_enable_write(struct counter_device *counter,
 	/* Load I/O control configuration to Input / Output Control Register */
 	outb(QUAD8_CTR_IOR | ior_cfg, base_offset);
 
+	spin_unlock(&priv->lock);
+
 	return len;
 }
 
@@ -1160,9 +1274,13 @@ static ssize_t quad8_signal_cable_fault_read(struct counter_device *counter,
 	if (disabled)
 		return -EINVAL;
 
+	spin_lock(&((struct quad8_iio *)priv)->lock);
+
 	/* Logic 0 = cable fault */
 	status = inb(priv->base + QUAD8_DIFF_ENCODER_CABLE_STATUS);
 
+	spin_unlock(&((struct quad8_iio *)priv)->lock);
+
 	/* Mask respective channel and invert logic */
 	fault = !(status & BIT(channel_id));
 
@@ -1194,6 +1312,8 @@ static ssize_t quad8_signal_cable_fault_enable_write(
 	if (ret)
 		return ret;
 
+	spin_lock(&priv->lock);
+
 	if (enable)
 		priv->cable_fault_enable |= BIT(channel_id);
 	else
@@ -1204,6 +1324,8 @@ static ssize_t quad8_signal_cable_fault_enable_write(
 
 	outb(cable_fault_enable, priv->base + QUAD8_DIFF_ENCODER_CABLE_STATUS);
 
+	spin_unlock(&priv->lock);
+
 	return len;
 }
 
@@ -1230,6 +1352,8 @@ static ssize_t quad8_signal_fck_prescaler_write(struct counter_device *counter,
 	if (ret)
 		return ret;
 
+	spin_lock(&priv->lock);
+
 	priv->fck_prescaler[channel_id] = prescaler;
 
 	/* Reset Byte Pointer */
@@ -1240,6 +1364,8 @@ static ssize_t quad8_signal_fck_prescaler_write(struct counter_device *counter,
 	outb(QUAD8_CTR_RLD | QUAD8_RLD_RESET_BP | QUAD8_RLD_PRESET_PSC,
 	     base_offset + 1);
 
+	spin_unlock(&priv->lock);
+
 	return len;
 }
 
@@ -1429,6 +1555,11 @@ static int quad8_probe(struct device *dev, unsigned int id)
 	quad8iio->counter.priv = quad8iio;
 	quad8iio->base = base[id];
 
+	/* Initialize spin lock */
+	spin_lock_init(&quad8iio->lock);
+
+	spin_lock(&quad8iio->lock);
+
 	/* Reset all counters and disable interrupt function */
 	outb(QUAD8_CHAN_OP_RESET_COUNTERS, base[id] + QUAD8_REG_CHAN_OP);
 	/* Set initial configuration for all counters */
@@ -1456,11 +1587,14 @@ static int quad8_probe(struct device *dev, unsigned int id)
 		/* Disable index function; negative index polarity */
 		outb(QUAD8_CTR_IDR, base_offset + 1);
 	}
+
 	/* Disable Differential Encoder Cable Status for all channels */
 	outb(0xFF, base[id] + QUAD8_DIFF_ENCODER_CABLE_STATUS);
 	/* Enable all counters */
 	outb(QUAD8_CHAN_OP_ENABLE_COUNTERS, base[id] + QUAD8_REG_CHAN_OP);
 
+	spin_unlock(&quad8iio->lock);
+
 	/* Register IIO device */
 	err = devm_iio_device_register(dev, indio_dev);
 	if (err)
-- 
2.7.4