From: David Lechner <[email protected]>
From: David Lechner <[email protected]>
- Remove "adi," prefix from gpio names.
- Sample gpio is now expected to be active low.
- Convert A0 and A1 gpios to "mode-gpios" gpio array.
- Convert RES0 and RES1 gpios to "resolution-gpios" gpio array.
- Remove extraneous lookup tables.
- Remove unused mode field from state struct.
- Swap argument order of ad2s1210_set_mode() while we are touching this.
Signed-off-by: David Lechner <[email protected]>
---
v3 changes:
* Fixed multiline comment style.
drivers/staging/iio/resolver/ad2s1210.c | 164 +++++++++++++++++---------------
1 file changed, 85 insertions(+), 79 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
index 2b9377447f6a..0ec3598b600a 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -58,39 +58,21 @@
#define AD2S1210_DEF_EXCIT 10000
enum ad2s1210_mode {
- MOD_POS = 0,
- MOD_VEL,
- MOD_CONFIG,
- MOD_RESERVED,
+ MOD_POS = 0b00,
+ MOD_VEL = 0b01,
+ MOD_RESERVED = 0b10,
+ MOD_CONFIG = 0b11,
};
-enum ad2s1210_gpios {
- AD2S1210_SAMPLE,
- AD2S1210_A0,
- AD2S1210_A1,
- AD2S1210_RES0,
- AD2S1210_RES1,
-};
-
-struct ad2s1210_gpio {
- const char *name;
- unsigned long flags;
-};
-
-static const struct ad2s1210_gpio gpios[] = {
- [AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_OUT_LOW },
- [AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_OUT_LOW },
- [AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_OUT_LOW },
- [AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_OUT_LOW },
- [AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_OUT_LOW },
-};
-
-static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
-
struct ad2s1210_state {
struct mutex lock;
struct spi_device *sdev;
- struct gpio_desc *gpios[5];
+ /** GPIO pin connected to SAMPLE line. */
+ 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. */
@@ -98,7 +80,6 @@ struct ad2s1210_state {
unsigned int fexcit;
bool hysteresis;
u8 resolution;
- enum ad2s1210_mode mode;
/** For reading raw sample value via SPI. */
__be16 sample __aligned(IIO_DMA_MINALIGN);
/** SPI transmit buffer. */
@@ -107,18 +88,15 @@ struct ad2s1210_state {
u8 tx[2];
};
-static const int ad2s1210_mode_vals[4][2] = {
- [MOD_POS] = { 0, 0 },
- [MOD_VEL] = { 0, 1 },
- [MOD_CONFIG] = { 1, 1 },
-};
-
-static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
- struct ad2s1210_state *st)
+static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
{
- gpiod_set_value(st->gpios[AD2S1210_A0], ad2s1210_mode_vals[mode][0]);
- gpiod_set_value(st->gpios[AD2S1210_A1], ad2s1210_mode_vals[mode][1]);
- st->mode = mode;
+ struct gpio_descs *gpios = st->mode_gpios;
+ DECLARE_BITMAP(bitmap, 2);
+
+ bitmap[0] = mode;
+
+ return gpiod_set_array_value(gpios->ndescs, gpios->desc, gpios->info,
+ bitmap);
}
/*
@@ -143,6 +121,7 @@ static int ad2s1210_regmap_reg_write(void *context, unsigned int reg,
.tx_buf = &st->tx[1],
},
};
+ int ret;
/* values can only be 7 bits, the MSB indicates an address */
if (val & ~0x7F)
@@ -151,7 +130,9 @@ static int ad2s1210_regmap_reg_write(void *context, unsigned int reg,
st->tx[0] = reg;
st->tx[1] = val;
- ad2s1210_set_mode(MOD_CONFIG, st);
+ ret = ad2s1210_set_mode(st, MOD_CONFIG);
+ if (ret < 0)
+ return ret;
return spi_sync_transfer(st->sdev, xfers, ARRAY_SIZE(xfers));
}
@@ -180,7 +161,10 @@ static int ad2s1210_regmap_reg_read(void *context, unsigned int reg,
};
int ret;
- ad2s1210_set_mode(MOD_CONFIG, st);
+ ret = ad2s1210_set_mode(st, MOD_CONFIG);
+ if (ret < 0)
+ return ret;
+
st->tx[0] = reg;
/*
* Must be valid register address here otherwise this could write data.
@@ -219,16 +203,16 @@ int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
return regmap_write(st->regmap, AD2S1210_REG_EXCIT_FREQ, fcw);
}
-static const int ad2s1210_res_pins[4][2] = {
- { 0, 0 }, {0, 1}, {1, 0}, {1, 1}
-};
-
-static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
+static int ad2s1210_set_resolution_gpios(struct ad2s1210_state *st,
+ u8 resolution)
{
- gpiod_set_value(st->gpios[AD2S1210_RES0],
- ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
- gpiod_set_value(st->gpios[AD2S1210_RES1],
- ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
+ 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 inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
@@ -305,10 +289,13 @@ static ssize_t ad2s1210_store_resolution(struct device *dev,
if (ret < 0)
goto error_ret;
- st->resolution =
- ad2s1210_resolution_value[data & AD2S1210_SET_RES];
- ad2s1210_set_resolution_pin(st);
+ ret = ad2s1210_set_resolution_gpios(st, udata);
+ if (ret < 0)
+ goto error_ret;
+
+ st->resolution = udata;
ret = len;
+
error_ret:
mutex_unlock(&st->lock);
return ret;
@@ -339,15 +326,19 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
int ret;
mutex_lock(&st->lock);
- gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
+
+ gpiod_set_value(st->sample_gpio, 1);
/* delay (2 * tck + 20) nano seconds */
udelay(1);
- gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
+ gpiod_set_value(st->sample_gpio, 0);
+
ret = regmap_read(st->regmap, AD2S1210_REG_FAULT, &value);
if (ret < 0)
goto error_ret;
- gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
- gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
+
+ gpiod_set_value(st->sample_gpio, 1);
+ gpiod_set_value(st->sample_gpio, 0);
+
error_ret:
mutex_unlock(&st->lock);
@@ -393,19 +384,19 @@ static int ad2s1210_single_conversion(struct ad2s1210_state *st,
struct iio_chan_spec const *chan,
int *val)
{
- int ret = 0;
+ int ret;
mutex_lock(&st->lock);
- gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
+ gpiod_set_value(st->sample_gpio, 1);
/* delay (6 * tck + 20) nano seconds */
udelay(1);
switch (chan->type) {
case IIO_ANGL:
- ad2s1210_set_mode(MOD_POS, st);
+ ret = ad2s1210_set_mode(st, MOD_POS);
break;
case IIO_ANGL_VEL:
- ad2s1210_set_mode(MOD_VEL, st);
+ ret = ad2s1210_set_mode(st, MOD_VEL);
break;
default:
ret = -EINVAL;
@@ -432,7 +423,7 @@ static int ad2s1210_single_conversion(struct ad2s1210_state *st,
}
error_ret:
- gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
+ gpiod_set_value(st->sample_gpio, 0);
/* delay (2 * tck + 20) nano seconds */
udelay(1);
mutex_unlock(&st->lock);
@@ -546,7 +537,9 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
int ret;
mutex_lock(&st->lock);
- ad2s1210_set_resolution_pin(st);
+ ret = ad2s1210_set_resolution_gpios(st, st->resolution);
+ if (ret < 0)
+ return ret;
/* Use default config register value plus resolution from devicetree. */
data = FIELD_PREP(AD2S1210_PHASE_LOCK_RANGE_44, 1);
@@ -612,20 +605,34 @@ static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
{
- struct spi_device *spi = st->sdev;
- int i, ret;
-
- for (i = 0; i < ARRAY_SIZE(gpios); i++) {
- st->gpios[i] = devm_gpiod_get(&spi->dev, gpios[i].name,
- gpios[i].flags);
- if (IS_ERR(st->gpios[i])) {
- ret = PTR_ERR(st->gpios[i]);
- dev_err(&spi->dev,
- "ad2s1210: failed to request %s GPIO: %d\n",
- gpios[i].name, ret);
- return ret;
- }
- }
+ struct device *dev = &st->sdev->dev;
+
+ /* should not be sampling on startup */
+ st->sample_gpio = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
+ if (IS_ERR(st->sample_gpio))
+ return dev_err_probe(dev, PTR_ERR(st->sample_gpio),
+ "failed to request sample GPIO\n");
+
+ /* both pins high means that we start in config mode */
+ st->mode_gpios = devm_gpiod_get_array(dev, "mode", GPIOD_OUT_HIGH);
+ if (IS_ERR(st->mode_gpios))
+ return dev_err_probe(dev, PTR_ERR(st->mode_gpios),
+ "failed to request mode GPIOs\n");
+
+ if (st->mode_gpios->ndescs != 2)
+ 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),
+ "failed to request resolution GPIOs\n");
+
+ if (st->resolution_gpios->ndescs != 2)
+ return dev_err_probe(dev, -EINVAL,
+ "requires exactly 2 resolution-gpios\n");
return 0;
}
@@ -690,7 +697,6 @@ static int ad2s1210_probe(struct spi_device *spi)
mutex_init(&st->lock);
st->sdev = spi;
st->hysteresis = true;
- st->mode = MOD_CONFIG;
st->resolution = 12;
st->fexcit = AD2S1210_DEF_EXCIT;
--
2.42.0
On Fri, 29 Sep 2023 12:23:18 -0500
David Lechner <[email protected]> wrote:
> From: David Lechner <[email protected]>
>
> From: David Lechner <[email protected]>
>
> - Remove "adi," prefix from gpio names.
> - Sample gpio is now expected to be active low.
> - Convert A0 and A1 gpios to "mode-gpios" gpio array.
> - Convert RES0 and RES1 gpios to "resolution-gpios" gpio array.
> - Remove extraneous lookup tables.
> - Remove unused mode field from state struct.
> - Swap argument order of ad2s1210_set_mode() while we are touching this.
>
> Signed-off-by: David Lechner <[email protected]>
Applied,
Thanks,
Jonathan
> ---
>
> v3 changes:
> * Fixed multiline comment style.
>
> drivers/staging/iio/resolver/ad2s1210.c | 164 +++++++++++++++++---------------
> 1 file changed, 85 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index 2b9377447f6a..0ec3598b600a 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -58,39 +58,21 @@
> #define AD2S1210_DEF_EXCIT 10000
>
> enum ad2s1210_mode {
> - MOD_POS = 0,
> - MOD_VEL,
> - MOD_CONFIG,
> - MOD_RESERVED,
> + MOD_POS = 0b00,
> + MOD_VEL = 0b01,
> + MOD_RESERVED = 0b10,
> + MOD_CONFIG = 0b11,
> };
>
> -enum ad2s1210_gpios {
> - AD2S1210_SAMPLE,
> - AD2S1210_A0,
> - AD2S1210_A1,
> - AD2S1210_RES0,
> - AD2S1210_RES1,
> -};
> -
> -struct ad2s1210_gpio {
> - const char *name;
> - unsigned long flags;
> -};
> -
> -static const struct ad2s1210_gpio gpios[] = {
> - [AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_OUT_LOW },
> - [AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_OUT_LOW },
> - [AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_OUT_LOW },
> - [AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_OUT_LOW },
> - [AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_OUT_LOW },
> -};
> -
> -static const unsigned int ad2s1210_resolution_value[] = { 10, 12, 14, 16 };
> -
> struct ad2s1210_state {
> struct mutex lock;
> struct spi_device *sdev;
> - struct gpio_desc *gpios[5];
> + /** GPIO pin connected to SAMPLE line. */
> + 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. */
> @@ -98,7 +80,6 @@ struct ad2s1210_state {
> unsigned int fexcit;
> bool hysteresis;
> u8 resolution;
> - enum ad2s1210_mode mode;
> /** For reading raw sample value via SPI. */
> __be16 sample __aligned(IIO_DMA_MINALIGN);
> /** SPI transmit buffer. */
> @@ -107,18 +88,15 @@ struct ad2s1210_state {
> u8 tx[2];
> };
>
> -static const int ad2s1210_mode_vals[4][2] = {
> - [MOD_POS] = { 0, 0 },
> - [MOD_VEL] = { 0, 1 },
> - [MOD_CONFIG] = { 1, 1 },
> -};
> -
> -static inline void ad2s1210_set_mode(enum ad2s1210_mode mode,
> - struct ad2s1210_state *st)
> +static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
> {
> - gpiod_set_value(st->gpios[AD2S1210_A0], ad2s1210_mode_vals[mode][0]);
> - gpiod_set_value(st->gpios[AD2S1210_A1], ad2s1210_mode_vals[mode][1]);
> - st->mode = mode;
> + struct gpio_descs *gpios = st->mode_gpios;
> + DECLARE_BITMAP(bitmap, 2);
> +
> + bitmap[0] = mode;
> +
> + return gpiod_set_array_value(gpios->ndescs, gpios->desc, gpios->info,
> + bitmap);
> }
>
> /*
> @@ -143,6 +121,7 @@ static int ad2s1210_regmap_reg_write(void *context, unsigned int reg,
> .tx_buf = &st->tx[1],
> },
> };
> + int ret;
>
> /* values can only be 7 bits, the MSB indicates an address */
> if (val & ~0x7F)
> @@ -151,7 +130,9 @@ static int ad2s1210_regmap_reg_write(void *context, unsigned int reg,
> st->tx[0] = reg;
> st->tx[1] = val;
>
> - ad2s1210_set_mode(MOD_CONFIG, st);
> + ret = ad2s1210_set_mode(st, MOD_CONFIG);
> + if (ret < 0)
> + return ret;
>
> return spi_sync_transfer(st->sdev, xfers, ARRAY_SIZE(xfers));
> }
> @@ -180,7 +161,10 @@ static int ad2s1210_regmap_reg_read(void *context, unsigned int reg,
> };
> int ret;
>
> - ad2s1210_set_mode(MOD_CONFIG, st);
> + ret = ad2s1210_set_mode(st, MOD_CONFIG);
> + if (ret < 0)
> + return ret;
> +
> st->tx[0] = reg;
> /*
> * Must be valid register address here otherwise this could write data.
> @@ -219,16 +203,16 @@ int ad2s1210_update_frequency_control_word(struct ad2s1210_state *st)
> return regmap_write(st->regmap, AD2S1210_REG_EXCIT_FREQ, fcw);
> }
>
> -static const int ad2s1210_res_pins[4][2] = {
> - { 0, 0 }, {0, 1}, {1, 0}, {1, 1}
> -};
> -
> -static inline void ad2s1210_set_resolution_pin(struct ad2s1210_state *st)
> +static int ad2s1210_set_resolution_gpios(struct ad2s1210_state *st,
> + u8 resolution)
> {
> - gpiod_set_value(st->gpios[AD2S1210_RES0],
> - ad2s1210_res_pins[(st->resolution - 10) / 2][0]);
> - gpiod_set_value(st->gpios[AD2S1210_RES1],
> - ad2s1210_res_pins[(st->resolution - 10) / 2][1]);
> + 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 inline int ad2s1210_soft_reset(struct ad2s1210_state *st)
> @@ -305,10 +289,13 @@ static ssize_t ad2s1210_store_resolution(struct device *dev,
> if (ret < 0)
> goto error_ret;
>
> - st->resolution =
> - ad2s1210_resolution_value[data & AD2S1210_SET_RES];
> - ad2s1210_set_resolution_pin(st);
> + ret = ad2s1210_set_resolution_gpios(st, udata);
> + if (ret < 0)
> + goto error_ret;
> +
> + st->resolution = udata;
> ret = len;
> +
> error_ret:
> mutex_unlock(&st->lock);
> return ret;
> @@ -339,15 +326,19 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
> int ret;
>
> mutex_lock(&st->lock);
> - gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
> +
> + gpiod_set_value(st->sample_gpio, 1);
> /* delay (2 * tck + 20) nano seconds */
> udelay(1);
> - gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
> + gpiod_set_value(st->sample_gpio, 0);
> +
> ret = regmap_read(st->regmap, AD2S1210_REG_FAULT, &value);
> if (ret < 0)
> goto error_ret;
> - gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
> - gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
> +
> + gpiod_set_value(st->sample_gpio, 1);
> + gpiod_set_value(st->sample_gpio, 0);
> +
> error_ret:
> mutex_unlock(&st->lock);
>
> @@ -393,19 +384,19 @@ static int ad2s1210_single_conversion(struct ad2s1210_state *st,
> struct iio_chan_spec const *chan,
> int *val)
> {
> - int ret = 0;
> + int ret;
>
> mutex_lock(&st->lock);
> - gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 0);
> + gpiod_set_value(st->sample_gpio, 1);
> /* delay (6 * tck + 20) nano seconds */
> udelay(1);
>
> switch (chan->type) {
> case IIO_ANGL:
> - ad2s1210_set_mode(MOD_POS, st);
> + ret = ad2s1210_set_mode(st, MOD_POS);
> break;
> case IIO_ANGL_VEL:
> - ad2s1210_set_mode(MOD_VEL, st);
> + ret = ad2s1210_set_mode(st, MOD_VEL);
> break;
> default:
> ret = -EINVAL;
> @@ -432,7 +423,7 @@ static int ad2s1210_single_conversion(struct ad2s1210_state *st,
> }
>
> error_ret:
> - gpiod_set_value(st->gpios[AD2S1210_SAMPLE], 1);
> + gpiod_set_value(st->sample_gpio, 0);
> /* delay (2 * tck + 20) nano seconds */
> udelay(1);
> mutex_unlock(&st->lock);
> @@ -546,7 +537,9 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
> int ret;
>
> mutex_lock(&st->lock);
> - ad2s1210_set_resolution_pin(st);
> + ret = ad2s1210_set_resolution_gpios(st, st->resolution);
> + if (ret < 0)
> + return ret;
>
> /* Use default config register value plus resolution from devicetree. */
> data = FIELD_PREP(AD2S1210_PHASE_LOCK_RANGE_44, 1);
> @@ -612,20 +605,34 @@ static int ad2s1210_setup_clocks(struct ad2s1210_state *st)
>
> static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
> {
> - struct spi_device *spi = st->sdev;
> - int i, ret;
> -
> - for (i = 0; i < ARRAY_SIZE(gpios); i++) {
> - st->gpios[i] = devm_gpiod_get(&spi->dev, gpios[i].name,
> - gpios[i].flags);
> - if (IS_ERR(st->gpios[i])) {
> - ret = PTR_ERR(st->gpios[i]);
> - dev_err(&spi->dev,
> - "ad2s1210: failed to request %s GPIO: %d\n",
> - gpios[i].name, ret);
> - return ret;
> - }
> - }
> + struct device *dev = &st->sdev->dev;
> +
> + /* should not be sampling on startup */
> + st->sample_gpio = devm_gpiod_get(dev, "sample", GPIOD_OUT_LOW);
> + if (IS_ERR(st->sample_gpio))
> + return dev_err_probe(dev, PTR_ERR(st->sample_gpio),
> + "failed to request sample GPIO\n");
> +
> + /* both pins high means that we start in config mode */
> + st->mode_gpios = devm_gpiod_get_array(dev, "mode", GPIOD_OUT_HIGH);
> + if (IS_ERR(st->mode_gpios))
> + return dev_err_probe(dev, PTR_ERR(st->mode_gpios),
> + "failed to request mode GPIOs\n");
> +
> + if (st->mode_gpios->ndescs != 2)
> + 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),
> + "failed to request resolution GPIOs\n");
> +
> + if (st->resolution_gpios->ndescs != 2)
> + return dev_err_probe(dev, -EINVAL,
> + "requires exactly 2 resolution-gpios\n");
>
> return 0;
> }
> @@ -690,7 +697,6 @@ static int ad2s1210_probe(struct spi_device *spi)
> mutex_init(&st->lock);
> st->sdev = spi;
> st->hysteresis = true;
> - st->mode = MOD_CONFIG;
> st->resolution = 12;
> st->fexcit = AD2S1210_DEF_EXCIT;
>
>
On Sat, 30 Sep 2023 15:55:36 +0100
Jonathan Cameron <[email protected]> wrote:
> On Fri, 29 Sep 2023 12:23:18 -0500
> David Lechner <[email protected]> wrote:
>
> > From: David Lechner <[email protected]>
> >
> > From: David Lechner <[email protected]>
> >
> > - Remove "adi," prefix from gpio names.
> > - Sample gpio is now expected to be active low.
> > - Convert A0 and A1 gpios to "mode-gpios" gpio array.
> > - Convert RES0 and RES1 gpios to "resolution-gpios" gpio array.
> > - Remove extraneous lookup tables.
> > - Remove unused mode field from state struct.
> > - Swap argument order of ad2s1210_set_mode() while we are touching this.
> >
> > Signed-off-by: David Lechner <[email protected]>
> Applied,
0-day ran smatch on this and it picked up that a log isn't released in
an error path. I've fixed that up with a goto error_ret and will push out a
fresh testing branch for 0-day to take another look at.
...
> > @@ -546,7 +537,9 @@ static int ad2s1210_initial(struct ad2s1210_state *st)
> > int ret;
> >
> > mutex_lock(&st->lock);
> > - ad2s1210_set_resolution_pin(st);
> > + ret = ad2s1210_set_resolution_gpios(st, st->resolution);
> > + if (ret < 0)
Exiting with lock held. There is an error_ret label that releases the lock
so use that.
> > + return ret;
> >
> > /* Use default config register value plus resolution from devicetree. */
> > data = FIELD_PREP(AD2S1210_PHASE_LOCK_RANGE_44, 1);