By bailing out early if chan->output is false for the IIO_CHAN_INFO_RAW,
indentation can be decreased by a tab and code readability improved.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: William Breathitt Gray <william.gray@linaro.org>
---
 drivers/iio/addac/stx104.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/addac/stx104.c b/drivers/iio/addac/stx104.c
index 8730b79e921c..9cc467469dde 100644
--- a/drivers/iio/addac/stx104.c
+++ b/drivers/iio/addac/stx104.c
@@ -180,20 +180,20 @@ static int stx104_write_raw(struct iio_dev *indio_dev,
 
 		return 0;
 	case IIO_CHAN_INFO_RAW:
-		if (chan->output) {
-			/* DAC can only accept up to a 16-bit value */
-			if ((unsigned int)val > 65535)
-				return -EINVAL;
+		if (!chan->output)
+			return -EINVAL;
 
-			mutex_lock(&priv->lock);
+		/* DAC can only accept up to a 16-bit value */
+		if ((unsigned int)val > 65535)
+			return -EINVAL;
 
-			priv->chan_out_states[chan->channel] = val;
-			iowrite16(val, &priv->reg->dac[chan->channel]);
+		mutex_lock(&priv->lock);
 
-			mutex_unlock(&priv->lock);
-			return 0;
-		}
-		return -EINVAL;
+		priv->chan_out_states[chan->channel] = val;
+		iowrite16(val, &priv->reg->dac[chan->channel]);
+
+		mutex_unlock(&priv->lock);
+		return 0;
 	}
 
 	return -EINVAL;
-- 
2.39.2

On Tue, Apr 04, 2023 at 10:12:00AM -0400, William Breathitt Gray wrote:
> By bailing out early if chan->output is false for the IIO_CHAN_INFO_RAW,
> indentation can be decreased by a tab and code readability improved.

...

> +		/* DAC can only accept up to a 16-bit value */
> +		if ((unsigned int)val > 65535)
> +			return -EINVAL;

While the patch is good per se, I don't like two things (which are also in the
original code):
- explicit casting (can it be avoided?)
- would be good to have U16_MAX or ?.. instead of hard coded number

Can it be addressed with (additional) patches?

-- 
With Best Regards,
Andy Shevchenko

On Wed, Apr 05, 2023 at 11:50:08AM +0300, Andy Shevchenko wrote:
> On Tue, Apr 04, 2023 at 10:12:00AM -0400, William Breathitt Gray wrote:
> > By bailing out early if chan->output is false for the IIO_CHAN_INFO_RAW,
> > indentation can be decreased by a tab and code readability improved.
> 
> ...
> 
> > +		/* DAC can only accept up to a 16-bit value */
> > +		if ((unsigned int)val > 65535)
> > +			return -EINVAL;
> 
> While the patch is good per se, I don't like two things (which are also in the
> original code):
> - explicit casting (can it be avoided?)

We could explicitly check for negative instead:

    if (val < 0 || val > 65535)

Would that be better?

> - would be good to have U16_MAX or ?.. instead of hard coded number

Fair point, I'll use U16_MAX then.

> Can it be addressed with (additional) patches?

Sure, I'll submit a separate patch to address this.

William Breathitt Gray