On Thu, 21 Sep 2023 09:43:55 -0500
David Lechner <dlechner@xxxxxxxxxxxx> wrote:
> This combines the ad2s1210_update_frequency_control_word() and
> ad2s1210_soft_reset() functions into a single function since they
> both have to be called together.
>
> Also clean up a few things while touching this:
> - move AD2S1210_DEF_EXCIT macro with similar macros
> - remove unnecessary dev_err() calls
>
> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
Probably worth calling out that this reset doesn't touch config
registers (they normally do!) but instead resets the tracking logic.
Jonathan
> ---
> drivers/staging/iio/resolver/ad2s1210.c | 63 ++++++++++++-------------
> 1 file changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index fe413759deb9..f1ffee34ebbc 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -53,12 +53,11 @@
> #define AD2S1210_MIN_CLKIN 6144000
> #define AD2S1210_MAX_CLKIN 10240000
> #define AD2S1210_MIN_EXCIT 2000
> +#define AD2S1210_DEF_EXCIT 10000
> #define AD2S1210_MAX_EXCIT 20000
> #define AD2S1210_MIN_FCW 0x4
> #define AD2S1210_MAX_FCW 0x50
>
> -#define AD2S1210_DEF_EXCIT 10000
> -
> enum ad2s1210_mode {
> MOD_POS = 0b00,
> MOD_VEL = 0b01,
> @@ -184,18 +183,29 @@ static int ad2s1210_regmap_reg_read(void *context, unsigned int reg,
> return 0;
> }
>
> -static inline
> -int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
> +/*
> + * Sets the excitation frequency and performs software reset.
> + *
> + * Must be called with lock held.
> + */
> +static int ad2s1210_set_excitation_frequency(struct ad2s1210_state *st,
> + u16 fexcit)
> {
> - unsigned char fcw;
> + int ret;
> + u8 fcw;
>
> - fcw = (unsigned char)(st->fexcit * (1 << 15) / st->fclkin);
> - if (fcw < AD2S1210_MIN_FCW || fcw > AD2S1210_MAX_FCW) {
> - dev_err(&st->sdev->dev, "ad2s1210: FCW out of range\n");
> + fcw = fexcit * (1 << 15) / st->fclkin;
> + if (fcw < AD2S1210_MIN_FCW || fcw > AD2S1210_MAX_FCW)
> return -ERANGE;
> - }
>
> - return regmap_write(st->regmap, AD2S1210_REG_EXCIT_FREQ, fcw);
> + ret = regmap_write(st->regmap, AD2S1210_REG_EXCIT_FREQ, fcw);
> + if (ret < 0)
> + return ret;
> +
> + st->fexcit = fexcit;
> +
> + /* software reset reinitializes the excitation frequency output */
> + return regmap_write(st->regmap, AD2S1210_REG_SOFT_RESET, 0);
> }
>
> static int ad2s1210_set_resolution_gpios(struct ad2s1210_state *st,
> @@ -210,11 +220,6 @@ static int ad2s1210_set_resolution_gpios(struct ad2s1210_state *st,
> bitmap);
> }
>
> -static inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
> -{
> - return regmap_write(st->regmap, AD2S1210_REG_SOFT_RESET, 0);
> -}
> -
> static ssize_t ad2s1210_show_fexcit(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -229,27 +234,24 @@ static ssize_t ad2s1210_store_fexcit(struct device *dev,
> const char *buf, size_t len)
> {
> struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
> - unsigned int fexcit;
> + u16 fexcit;
> int ret;
>
> - ret = kstrtouint(buf, 10, &fexcit);
> - if (ret < 0)
> - return ret;
> - if (fexcit < AD2S1210_MIN_EXCIT || fexcit > AD2S1210_MAX_EXCIT) {
> - dev_err(dev,
> - "ad2s1210: excitation frequency out of range\n");
> + ret = kstrtou16(buf, 10, &fexcit);
> + if (ret < 0 || fexcit < AD2S1210_MIN_EXCIT || fexcit > AD2S1210_MAX_EXCIT)
> return -EINVAL;
> - }
> +
> mutex_lock(&st->lock);
> - st->fexcit = fexcit;
> - ret = ad2s1210_update_frequency_control_word(st);
> + ret = ad2s1210_set_excitation_frequency(st, fexcit);
> if (ret < 0)
> goto error_ret;
> - ret = ad2s1210_soft_reset(st);
> +
> + ret = len;
> +
> error_ret:
> mutex_unlock(&st->lock);
>
> - return ret < 0 ? ret : len;
> + return ret;
> }
>
> static ssize_t ad2s1210_show_resolution(struct device *dev,
> @@ -624,10 +626,8 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
> if (ret < 0)
> goto error_ret;
>
> - ret = ad2s1210_update_frequency_control_word(st);
> - if (ret < 0)
> - goto error_ret;
> - ret = ad2s1210_soft_reset(st);
> + ret = ad2s1210_set_excitation_frequency(st, AD2S1210_DEF_EXCIT);
> +
> error_ret:
> mutex_unlock(&st->lock);
> return ret;
> @@ -773,7 +773,6 @@ static int ad2s1210_probe(struct spi_device *spi)
> mutex_init(&st->lock);
> st->sdev = spi;
> st->resolution = 12;
> - st->fexcit = AD2S1210_DEF_EXCIT;
>
> ret = ad2s1210_setup_clocks(st);
> if (ret < 0)
