Selecting the resolution was implemented as the `bits` sysfs attribute. However, the selection of the resolution depends on how the hardware is wired and the specific application, so this is rather a job for devicetree to describe. A new devicetree property `assigned-resolution-bits` to specify the resolution required for each chip is added and the `bits` sysfs attribute is removed. Since the resolution is now supplied by a devicetree property, the resolution-gpios are now optional and we can allow for the case where the resolution pins on the AD2S1210 are hard-wired instead of requiring them to be connected to gpios. Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx> --- v4 changes: * Fixed devicetree property name in commit message. * Reworked handling of hysteresis_available to handle assigned-resolution-bits != 16. v3 changes: * Fixed multiline comment style. drivers/staging/iio/resolver/ad2s1210.c | 150 +++++++++++++++----------------- 1 file changed, 71 insertions(+), 79 deletions(-) diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c index 0c7772725330..66ef35fbb6fe 100644 --- a/drivers/staging/iio/resolver/ad2s1210.c +++ b/drivers/staging/iio/resolver/ad2s1210.c @@ -63,6 +63,13 @@ enum ad2s1210_mode { MOD_CONFIG = 0b11, }; +enum ad2s1210_resolution { + AD2S1210_RES_10 = 0b00, + AD2S1210_RES_12 = 0b01, + AD2S1210_RES_14 = 0b10, + AD2S1210_RES_16 = 0b11, +}; + struct ad2s1210_state { struct mutex lock; struct spi_device *sdev; @@ -70,15 +77,14 @@ struct ad2s1210_state { struct gpio_desc *sample_gpio; /** GPIO pins connected to A0 and A1 lines. */ struct gpio_descs *mode_gpios; - /** GPIO pins connected to RES0 and RES1 lines. */ - struct gpio_descs *resolution_gpios; /** Used to access config registers. */ struct regmap *regmap; /** The external oscillator frequency in Hz. */ unsigned long clkin_hz; /** Available raw hysteresis values based on resolution. */ int hysteresis_available[2]; - u8 resolution; + /** The selected resolution */ + enum ad2s1210_resolution resolution; /** For reading raw sample value via SPI. */ __be16 sample __aligned(IIO_DMA_MINALIGN); /** SPI transmit buffer. */ @@ -215,63 +221,6 @@ static int ad2s1210_reinit_excitation_frequency(struct ad2s1210_state *st, return regmap_write(st->regmap, AD2S1210_REG_SOFT_RESET, 0); } -static int ad2s1210_set_resolution_gpios(struct ad2s1210_state *st, - u8 resolution) -{ - struct gpio_descs *gpios = st->resolution_gpios; - DECLARE_BITMAP(bitmap, 2); - - bitmap[0] = (resolution - 10) >> 1; - - return gpiod_set_array_value(gpios->ndescs, gpios->desc, gpios->info, - bitmap); -} - -static ssize_t ad2s1210_show_resolution(struct device *dev, - struct device_attribute *attr, - char *buf) -{ - struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev)); - - return sprintf(buf, "%d\n", st->resolution); -} - -static ssize_t ad2s1210_store_resolution(struct device *dev, - struct device_attribute *attr, - const char *buf, size_t len) -{ - struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev)); - unsigned char data; - unsigned char udata; - int ret; - - ret = kstrtou8(buf, 10, &udata); - if (ret || udata < 10 || udata > 16) { - dev_err(dev, "ad2s1210: resolution out of range\n"); - return -EINVAL; - } - - data = (udata - 10) >> 1; - - mutex_lock(&st->lock); - ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL, - AD2S1210_SET_RES, data); - if (ret < 0) - goto error_ret; - - ret = ad2s1210_set_resolution_gpios(st, udata); - if (ret < 0) - goto error_ret; - - st->resolution = udata; - st->hysteresis_available[1] = 1 << (16 - st->resolution); - ret = len; - -error_ret: - mutex_unlock(&st->lock); - return ret; -} - /* read the fault register since last sample */ static ssize_t ad2s1210_show_fault(struct device *dev, struct device_attribute *attr, char *buf) @@ -413,7 +362,7 @@ static int ad2s1210_get_hysteresis(struct ad2s1210_state *st, int *val) if (ret < 0) return ret; - *val = ret << (16 - st->resolution); + *val = ret << (2 * (AD2S1210_RES_16 - st->resolution)); return IIO_VAL_INT; } @@ -577,8 +526,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev, } } -static IIO_DEVICE_ATTR(bits, 0644, - ad2s1210_show_resolution, ad2s1210_store_resolution, 0); static IIO_DEVICE_ATTR(fault, 0644, ad2s1210_show_fault, ad2s1210_clear_fault, 0); @@ -633,7 +580,6 @@ static const struct iio_chan_spec ad2s1210_channels[] = { }; static struct attribute *ad2s1210_attributes[] = { - &iio_dev_attr_bits.dev_attr.attr, &iio_dev_attr_fault.dev_attr.attr, &iio_dev_attr_los_thrd.dev_attr.attr, &iio_dev_attr_dos_ovr_thrd.dev_attr.attr, @@ -655,15 +601,12 @@ static int ad2s1210_initial(struct ad2s1210_state *st) int ret; mutex_lock(&st->lock); - ret = ad2s1210_set_resolution_gpios(st, st->resolution); - if (ret < 0) - goto error_ret; /* Use default config register value plus resolution from devicetree. */ data = FIELD_PREP(AD2S1210_PHASE_LOCK_RANGE_44, 1); data |= FIELD_PREP(AD2S1210_ENABLE_HYSTERESIS, 1); data |= FIELD_PREP(AD2S1210_SET_ENRES, 0x3); - data |= FIELD_PREP(AD2S1210_SET_RES, (st->resolution - 10) >> 1); + data |= FIELD_PREP(AD2S1210_SET_RES, st->resolution); ret = regmap_write(st->regmap, AD2S1210_REG_CONTROL, data); if (ret < 0) @@ -703,6 +646,35 @@ static const struct iio_info ad2s1210_info = { .debugfs_reg_access = &ad2s1210_debugfs_reg_access, }; +static int ad2s1210_setup_properties(struct ad2s1210_state *st) +{ + struct device *dev = &st->sdev->dev; + u32 val; + int ret; + + ret = device_property_read_u32(dev, "assigned-resolution-bits", &val); + if (ret < 0) + return dev_err_probe(dev, ret, + "failed to read assigned-resolution-bits property\n"); + + if (val < 10 || val > 16) + return dev_err_probe(dev, -EINVAL, + "resolution out of range: %u\n", val); + + st->resolution = (val - 10) >> 1; + /* + * These are values that correlate to the hysteresis bit in the Control + * register. 0 = disabled, 1 = enabled. When enabled, the actual + * hysteresis is +/- 1 LSB of the raw position value. Which bit is the + * LSB depends on the specified resolution. + */ + st->hysteresis_available[0] = 0; + st->hysteresis_available[1] = 1 << (2 * (AD2S1210_RES_16 - + st->resolution)); + + return 0; +} + static int ad2s1210_setup_clocks(struct ad2s1210_state *st) { struct device *dev = &st->sdev->dev; @@ -724,6 +696,9 @@ static int ad2s1210_setup_clocks(struct ad2s1210_state *st) static int ad2s1210_setup_gpios(struct ad2s1210_state *st) { struct device *dev = &st->sdev->dev; + struct gpio_descs *resolution_gpios; + DECLARE_BITMAP(bitmap, 2); + int ret; /* should not be sampling on startup */ st->sample_gpio = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW); @@ -741,16 +716,32 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st) return dev_err_probe(dev, -EINVAL, "requires exactly 2 mode-gpios\n"); - /* both pins high means that we start with 16-bit resolution */ - st->resolution_gpios = devm_gpiod_get_array(dev, "resolution", - GPIOD_OUT_HIGH); - if (IS_ERR(st->resolution_gpios)) - return dev_err_probe(dev, PTR_ERR(st->resolution_gpios), + /* + * If resolution gpios are provided, they get set to the required + * resolution, otherwise it is assumed the RES0 and RES1 pins are + * hard-wired to match the resolution indicated in the devicetree. + */ + resolution_gpios = devm_gpiod_get_array_optional(dev, "resolution", + GPIOD_ASIS); + if (IS_ERR(resolution_gpios)) + return dev_err_probe(dev, PTR_ERR(resolution_gpios), "failed to request resolution GPIOs\n"); - if (st->resolution_gpios->ndescs != 2) - return dev_err_probe(dev, -EINVAL, - "requires exactly 2 resolution-gpios\n"); + if (resolution_gpios) { + if (resolution_gpios->ndescs != 2) + return dev_err_probe(dev, -EINVAL, + "requires exactly 2 resolution-gpios\n"); + + bitmap[0] = st->resolution; + + ret = gpiod_set_array_value(resolution_gpios->ndescs, + resolution_gpios->desc, + resolution_gpios->info, + bitmap); + if (ret < 0) + return dev_err_probe(dev, ret, + "failed to set resolution gpios\n"); + } return 0; } @@ -814,9 +805,10 @@ static int ad2s1210_probe(struct spi_device *spi) mutex_init(&st->lock); st->sdev = spi; - st->resolution = 12; - st->hysteresis_available[0] = 0; - st->hysteresis_available[1] = 1 << (16 - st->resolution); + + ret = ad2s1210_setup_properties(st); + if (ret < 0) + return ret; ret = ad2s1210_setup_clocks(st); if (ret < 0) -- 2.42.0