lkml.org 
[lkml]   [2021]   [Apr]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/2] iio: accel: Add driver for Murata SCA3300 accelerometer
From
Date
Hi Andy,

Thanks for the comments. One reply below, otherwise implementing these
improvements

as suggested to next version.

Thanks.

Tomas


On 4/17/21 3:39 PM, Andy Shevchenko wrote:
> On Fri, Apr 16, 2021 at 5:21 PM Tomas Melin <tomas.melin@vaisala.com> wrote:
>> Add initial support for Murata SCA3300 3-axis industrial
>> accelerometer with digital SPI interface. This device also
>> provides a temperature measurement.
>>
>> Device product page including datasheet can be found at:
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.murata.com%2Fen-global%2Fproducts%2Fsensor%2Faccel%2Fsca3300&amp;data=04%7C01%7Ctomas.melin%40vaisala.com%7C7b536921ec37446568bf08d9019dd944%7C6d7393e041f54c2e9b124c2be5da5c57%7C0%7C0%7C637542599730610545%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=VulPc9WnmQMJJZpYZfHmAuFPeDhclwt%2F5uPw7iYx8fU%3D&amp;reserved=0
> ...
>> +static int sca3300_transfer(struct sca3300_data *sca_data, int *val)
>> +{
>> + struct spi_delay delay = {.value = 10, .unit = SPI_DELAY_UNIT_USECS};
>> + int32_t ret;
>> + int rs;
>> + u8 crc;
>> + struct spi_transfer xfers[2] = {
>> + {
>> + .tx_buf = sca_data->txbuf,
>> + .rx_buf = NULL,
>> + .len = ARRAY_SIZE(sca_data->txbuf),
>> + .delay = delay,
>> + .cs_change = 1,
>> + },
>> + {
>> + .tx_buf = NULL,
>> + .rx_buf = sca_data->rxbuf,
>> + .len = ARRAY_SIZE(sca_data->rxbuf),
>> + .delay = delay,
>> + .cs_change = 0,
>> + }
>> + };
>> +
>> + /* inverted crc value as described in device data sheet */
>> + crc = ~crc8(sca3300_crc_table, &sca_data->txbuf[0], 3, CRC8_INIT_VALUE);
>> + sca_data->txbuf[3] = crc;
>> +
>> + 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

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.


>
>> + }
>> +
>> + crc = ~crc8(sca3300_crc_table, &sca_data->rxbuf[0], 3, CRC8_INIT_VALUE);
>> + if (sca_data->rxbuf[3] != crc) {
>> + dev_err(&sca_data->spi->dev, "CRC checksum mismatch");
>> + return -EIO;
>> + }
>> +
>> + /* get return status */
>

\
 
 \ /
  Last update: 2021-04-19 12:32    [W:0.084 / U:0.932 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site