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@xxxxxxxxx> --- 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