Re: [PATCH v2 2/2] iio: accel: Add driver for Murata SCA3300 accelerometer
From: Tomas Melin
Date: Mon Apr 19 2021 - 07:36:28 EST
Hi,
On 4/19/21 2:14 PM, Andy Shevchenko wrote:
On Mon, Apr 19, 2021 at 1:29 PM Tomas Melin <tomas.melin@xxxxxxxxxxx> wrote:
On 4/17/21 3:39 PM, Andy Shevchenko wrote:
On Fri, Apr 16, 2021 at 5:21 PM Tomas Melin <tomas.melin@xxxxxxxxxxx> wrote:
Add initial support for Murata SCA3300 3-axis industrial
accelerometer with digital SPI interface. This device also
provides a temperature measurement.
...
+ ret = spi_sync_transfer(sca_data->spi, xfers, ARRAY_SIZE(xfers));
+ if (ret < 0) {
+ dev_err(&sca_data->spi->dev,
+ "transfer error, error: %d\n", ret);
+ return -EIO;
Why shadowing error code?
Returning EIO here to have full control over the return value from this
function. As return value of this is used for testing
Care to show what kind of testing requires this?
Also why can't it be refactored to accept all error codes?
I was referring to this:
+static int sca3300_read_reg(struct sca3300_data *sca_data, u8 reg, int *val)
+{
+ int ret;
+
+ mutex_lock(&sca_data->lock);
+ sca_data->txbuf[0] = 0x0 | (reg << 2);
+ ret = sca3300_transfer(sca_data, val);
+ mutex_unlock(&sca_data->lock);
+ if (ret == -EINVAL)
+ ret = sca3300_error_handler(sca_data);
+
+ return ret;
+}
+
+static int sca3300_write_reg(struct sca3300_data *sca_data, u8 reg, int val)
+{
+ int reg_val = 0;
+ int ret;
+
+ mutex_lock(&sca_data->lock);
+ sca_data->txbuf[0] = BIT(7) | (reg << 2);
+ put_unaligned_be16(val, &sca_data->txbuf[1]);
+ ret = sca3300_transfer(sca_data, ®_val);
+ mutex_unlock(&sca_data->lock);
+ if (ret == -EINVAL)
+ ret = sca3300_error_handler(sca_data);
+
+ return ret;
+}
So this goes into error handling only when transfer indicates EINVAL
(which happens when
transfer otherwise is good, but device return status has error flags set
i message).
Thanks,
Tomas
for possible status error (EINVAL), feels more confident to have it like
this to avoid any confusion. And atleast spi_sync_transfer() return value
would be visible in error message.
+ }