Message ID | 20210406151424.110477-1-nuno.sa@analog.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | iio: adis16480: support burst read function | expand |
On 4/6/21 5:14 PM, Nuno Sa wrote: > Some supported devices support burst read function. This provides a method > for reading a batch of data (status, temperature, gyroscopes, > accelerometers, time stamp/data counter, and CRC code), which does not > require a stall time between each 16-bit segment and only requires one > command on the DIN line to initiate. Devices supporting this mode > are: > > * adis16495-1 > * adis16495-2 > * adis16495-3 > * adis16497-1 > * adis16497-2 > * adis16497-3 > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> Looks good to me, just some thoughts on CRC and endiness conversion. > --- > drivers/iio/imu/adis16480.c | 157 +++++++++++++++++++++++++++++++++--- > 1 file changed, 144 insertions(+), 13 deletions(-) > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > index f81b86690b76..341945f8339e 100644 > --- a/drivers/iio/imu/adis16480.c > +++ b/drivers/iio/imu/adis16480.c > @@ -5,6 +5,7 @@ > * Copyright 2012 Analog Devices Inc. > */ > > [...] > +static bool adis16480_validate_crc(const u16 *buf, const u8 n_elem, const u32 crc) const __be16 *buf > +{ > + u32 crc_calc; > + u16 crc_buf[15]; > + int j; > + > + for (j = 0; j < n_elem; j++) > + crc_buf[j] = swab16(buf[j]); be16_to_cpu(buf[j]) > + > + crc_calc = crc32(~0, crc_buf, n_elem * 2); > + crc_calc ^= ~0; > + > + return (crc == crc_calc); > +} > + > +static irqreturn_t adis16480_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct adis16480 *st = iio_priv(indio_dev); > + struct adis *adis = &st->adis; > + int ret, bit, offset, i = 0; > + __be16 *buffer; > + u32 crc; > + bool valid; > + const u32 cached_spi_speed_hz = adis->spi->max_speed_hz; > + > + adis_dev_lock(adis); > + if (adis->current_page != 0) { > + adis->tx[0] = ADIS_WRITE_REG(ADIS_REG_PAGE_ID); > + adis->tx[1] = 0; > + ret = spi_write(adis->spi, adis->tx, 2); > + if (ret) { > + dev_err(&adis->spi->dev, "Failed to change device page: %d\n", ret); > + adis_dev_unlock(adis); > + return ret; > + } > + } > + > + adis->spi->max_speed_hz = ADIS16495_BURST_MAX_SPEED; > + > + ret = spi_sync(adis->spi, &adis->msg); > + if (ret) { > + dev_err(&adis->spi->dev, "Failed to read data: %d\n", ret); > + adis_dev_unlock(adis); > + return ret; > + } > + > + adis->spi->max_speed_hz = cached_spi_speed_hz; > + adis->current_page = 0; > + adis_dev_unlock(adis); > + > + /* > + * After making the burst request, the response can have one or two > + * 16-bit responses containing the BURST_ID depending on the sclk. If > + * clk > 3.6MHz, then we will have two BURST_ID in a row. If clk < 3MHZ, > + * we have only one. To manage that variation, we use the transition from the > + * BURST_ID to the SYS_E_FLAG register, which will not be equal to 0xA5A5. If > + * we not find this variation in the first 4 segments, then the data should > + * not be valid. We don't return right away since the crc validation should > + * fail anyways... > + */ > + buffer = adis->buffer; > + for (offset = 0; offset < 4; offset++) { > + u16 curr = be16_to_cpu(buffer[offset]); > + u16 next = be16_to_cpu(buffer[offset + 1]); > + > + if (curr == ADIS16495_BURST_ID && next != ADIS16495_BURST_ID) { > + offset++; > + break; > + } > + } I think offset can be up to 4 here, in which case offset + 16 is out-of-bounds for buffer. Maybe return right away if offset is 4. > + > + crc = be16_to_cpu(buffer[offset + 16]) << 16 | be16_to_cpu(buffer[offset + 15]); > + valid = adis16480_validate_crc((u16 *)&buffer[offset], 15, crc); > + if (!valid) { > + dev_err(&adis->spi->dev, "Invalid crc\n"); > + goto irq_done; > + } > + > [...]
> From: Lars-Peter Clausen <lars@metafoo.de> > Sent: Wednesday, April 7, 2021 10:25 AM > To: Sa, Nuno <Nuno.Sa@analog.com>; linux-iio@vger.kernel.org > Cc: Jonathan Cameron <jic23@kernel.org>; Hennerich, Michael > <Michael.Hennerich@analog.com> > Subject: Re: [PATCH] iio: adis16480: support burst read function > > [External] > > On 4/6/21 5:14 PM, Nuno Sa wrote: > > Some supported devices support burst read function. This provides a > method > > for reading a batch of data (status, temperature, gyroscopes, > > accelerometers, time stamp/data counter, and CRC code), which > does not > > require a stall time between each 16-bit segment and only requires > one > > command on the DIN line to initiate. Devices supporting this mode > > are: > > > > * adis16495-1 > > * adis16495-2 > > * adis16495-3 > > * adis16497-1 > > * adis16497-2 > > * adis16497-3 > > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > > Looks good to me, just some thoughts on CRC and endiness > conversion. > > > --- > > drivers/iio/imu/adis16480.c | 157 > +++++++++++++++++++++++++++++++++--- > > 1 file changed, 144 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c > > index f81b86690b76..341945f8339e 100644 > > --- a/drivers/iio/imu/adis16480.c > > +++ b/drivers/iio/imu/adis16480.c > > @@ -5,6 +5,7 @@ > > * Copyright 2012 Analog Devices Inc. > > */ > > > > [...] > > +static bool adis16480_validate_crc(const u16 *buf, const u8 n_elem, > const u32 crc) > const __be16 *buf > > +{ > > + u32 crc_calc; > > + u16 crc_buf[15]; > > + int j; > > + > > + for (j = 0; j < n_elem; j++) > > + crc_buf[j] = swab16(buf[j]); > be16_to_cpu(buf[j]) I don't think this would work on BE machines. AFAIU, the crc computation must be done in little endian ("order of bytes low-high"). In BE machines, crc_buf would be left in BE order which would lead to a wrong crc... The ' swab16()' was really intentional here (naturally can still be wrong :D). > > + > > + crc_calc = crc32(~0, crc_buf, n_elem * 2); > > + crc_calc ^= ~0; > > + > > + return (crc == crc_calc); > > +} > > + > > +static irqreturn_t adis16480_trigger_handler(int irq, void *p) > > +{ > > + struct iio_poll_func *pf = p; > > + struct iio_dev *indio_dev = pf->indio_dev; > > + struct adis16480 *st = iio_priv(indio_dev); > > + struct adis *adis = &st->adis; > > + int ret, bit, offset, i = 0; > > + __be16 *buffer; > > + u32 crc; > > + bool valid; > > + const u32 cached_spi_speed_hz = adis->spi->max_speed_hz; > > + > > + adis_dev_lock(adis); > > + if (adis->current_page != 0) { > > + adis->tx[0] = ADIS_WRITE_REG(ADIS_REG_PAGE_ID); > > + adis->tx[1] = 0; > > + ret = spi_write(adis->spi, adis->tx, 2); > > + if (ret) { > > + dev_err(&adis->spi->dev, "Failed to change > device page: %d\n", ret); > > + adis_dev_unlock(adis); > > + return ret; > > + } > > + } > > + > > + adis->spi->max_speed_hz = ADIS16495_BURST_MAX_SPEED; > > + > > + ret = spi_sync(adis->spi, &adis->msg); > > + if (ret) { > > + dev_err(&adis->spi->dev, "Failed to read data: %d\n", > ret); > > + adis_dev_unlock(adis); > > + return ret; > > + } > > + > > + adis->spi->max_speed_hz = cached_spi_speed_hz; > > + adis->current_page = 0; > > + adis_dev_unlock(adis); > > + > > + /* > > + * After making the burst request, the response can have one > or two > > + * 16-bit responses containing the BURST_ID depending on the > sclk. If > > + * clk > 3.6MHz, then we will have two BURST_ID in a row. If clk > < 3MHZ, > > + * we have only one. To manage that variation, we use the > transition from the > > + * BURST_ID to the SYS_E_FLAG register, which will not be > equal to 0xA5A5. If > > + * we not find this variation in the first 4 segments, then the > data should > > + * not be valid. We don't return right away since the crc > validation should > > + * fail anyways... > > + */ > > + buffer = adis->buffer; > > + for (offset = 0; offset < 4; offset++) { > > + u16 curr = be16_to_cpu(buffer[offset]); > > + u16 next = be16_to_cpu(buffer[offset + 1]); > > + > > + if (curr == ADIS16495_BURST_ID && next != > ADIS16495_BURST_ID) { > > + offset++; > > + break; > > + } > > + } > > I think offset can be up to 4 here, in which case offset + 16 is > out-of-bounds for buffer. Maybe return right away if offset is 4. > Auch, good catch! - Nuno Sá
On 4/7/21 1:31 PM, Sa, Nuno wrote: >> From: Lars-Peter Clausen <lars@metafoo.de> >> Sent: Wednesday, April 7, 2021 10:25 AM >> To: Sa, Nuno <Nuno.Sa@analog.com>; linux-iio@vger.kernel.org >> Cc: Jonathan Cameron <jic23@kernel.org>; Hennerich, Michael >> <Michael.Hennerich@analog.com> >> Subject: Re: [PATCH] iio: adis16480: support burst read function >> >> [External] >> >> On 4/6/21 5:14 PM, Nuno Sa wrote: >>> Some supported devices support burst read function. This provides a >> method >>> for reading a batch of data (status, temperature, gyroscopes, >>> accelerometers, time stamp/data counter, and CRC code), which >> does not >>> require a stall time between each 16-bit segment and only requires >> one >>> command on the DIN line to initiate. Devices supporting this mode >>> are: >>> >>> * adis16495-1 >>> * adis16495-2 >>> * adis16495-3 >>> * adis16497-1 >>> * adis16497-2 >>> * adis16497-3 >>> >>> Signed-off-by: Nuno Sa <nuno.sa@analog.com> >> Looks good to me, just some thoughts on CRC and endiness >> conversion. >> >>> --- >>> drivers/iio/imu/adis16480.c | 157 >> +++++++++++++++++++++++++++++++++--- >>> 1 file changed, 144 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c >>> index f81b86690b76..341945f8339e 100644 >>> --- a/drivers/iio/imu/adis16480.c >>> +++ b/drivers/iio/imu/adis16480.c >>> @@ -5,6 +5,7 @@ >>> * Copyright 2012 Analog Devices Inc. >>> */ >>> >>> [...] >>> +static bool adis16480_validate_crc(const u16 *buf, const u8 n_elem, >> const u32 crc) >> const __be16 *buf >>> +{ >>> + u32 crc_calc; >>> + u16 crc_buf[15]; >>> + int j; >>> + >>> + for (j = 0; j < n_elem; j++) >>> + crc_buf[j] = swab16(buf[j]); >> be16_to_cpu(buf[j]) > I don't think this would work on BE machines. AFAIU, the crc computation > must be done in little endian ("order of bytes low-high"). In BE machines, > crc_buf would be left in BE order which would lead to a wrong crc... > The ' swab16()' was really intentional here (naturally can still be wrong :D) Yea, that makes sense. crc32() processes byte wise and in the byte buffer we have MSB,LSB,MSB,LSB... and we want to process LSB,MSB,LSB,MSB....
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c index f81b86690b76..341945f8339e 100644 --- a/drivers/iio/imu/adis16480.c +++ b/drivers/iio/imu/adis16480.c @@ -5,6 +5,7 @@ * Copyright 2012 Analog Devices Inc. */ +#include <linux/crc32.h> #include <linux/clk.h> #include <linux/bitfield.h> #include <linux/of_irq.h> @@ -19,11 +20,14 @@ #include <linux/sysfs.h> #include <linux/module.h> #include <linux/lcm.h> +#include <linux/swab.h> #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> #include <linux/iio/buffer.h> #include <linux/iio/imu/adis.h> +#include <linux/iio/triggered_buffer.h> +#include <linux/iio/trigger_consumer.h> #include <linux/debugfs.h> @@ -103,6 +107,12 @@ * Available only for ADIS1649x devices */ #define ADIS16495_REG_SYNC_SCALE ADIS16480_REG(0x03, 0x10) +#define ADIS16495_REG_BURST_CMD ADIS16480_REG(0x00, 0x7C) +#define ADIS16495_BURST_ID 0xA5A5 +/* total number of segments in burst */ +#define ADIS16495_BURST_MAX_DATA 20 +/* spi max speed in burst mode */ +#define ADIS16495_BURST_MAX_SPEED 6000000 #define ADIS16480_REG_SERIAL_NUM ADIS16480_REG(0x04, 0x20) @@ -163,6 +173,8 @@ struct adis16480 { struct clk *ext_clk; enum adis16480_clock_mode clk_mode; unsigned int clk_freq; + /* Alignment needed for the timestamp */ + __be16 data[ADIS16495_BURST_MAX_DATA] __aligned(8); }; static const char * const adis16480_int_pin_names[4] = { @@ -863,7 +875,7 @@ static const char * const adis16480_status_error_msgs[] = { static int adis16480_enable_irq(struct adis *adis, bool enable); -#define ADIS16480_DATA(_prod_id, _timeouts) \ +#define ADIS16480_DATA(_prod_id, _timeouts, _burst_len) \ { \ .diag_stat_reg = ADIS16480_REG_DIAG_STS, \ .glob_cmd_reg = ADIS16480_REG_GLOB_CMD, \ @@ -887,6 +899,8 @@ static int adis16480_enable_irq(struct adis *adis, bool enable); BIT(ADIS16480_DIAG_STAT_BARO_FAIL), \ .enable_irq = adis16480_enable_irq, \ .timeouts = (_timeouts), \ + .burst_reg_cmd = ADIS16495_REG_BURST_CMD, \ + .burst_len = (_burst_len), \ } static const struct adis_timeout adis16485_timeouts = { @@ -931,7 +945,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .int_clk = 2460000, .max_dec_rate = 2048, .filter_freqs = adis16480_def_filter_freqs, - .adis_data = ADIS16480_DATA(16375, &adis16485_timeouts), + .adis_data = ADIS16480_DATA(16375, &adis16485_timeouts, 0), }, [ADIS16480] = { .channels = adis16480_channels, @@ -944,7 +958,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .int_clk = 2460000, .max_dec_rate = 2048, .filter_freqs = adis16480_def_filter_freqs, - .adis_data = ADIS16480_DATA(16480, &adis16480_timeouts), + .adis_data = ADIS16480_DATA(16480, &adis16480_timeouts, 0), }, [ADIS16485] = { .channels = adis16485_channels, @@ -957,7 +971,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .int_clk = 2460000, .max_dec_rate = 2048, .filter_freqs = adis16480_def_filter_freqs, - .adis_data = ADIS16480_DATA(16485, &adis16485_timeouts), + .adis_data = ADIS16480_DATA(16485, &adis16485_timeouts, 0), }, [ADIS16488] = { .channels = adis16480_channels, @@ -970,7 +984,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .int_clk = 2460000, .max_dec_rate = 2048, .filter_freqs = adis16480_def_filter_freqs, - .adis_data = ADIS16480_DATA(16488, &adis16485_timeouts), + .adis_data = ADIS16480_DATA(16488, &adis16485_timeouts, 0), }, [ADIS16490] = { .channels = adis16485_channels, @@ -984,7 +998,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .max_dec_rate = 4250, .filter_freqs = adis16495_def_filter_freqs, .has_pps_clk_mode = true, - .adis_data = ADIS16480_DATA(16490, &adis16495_timeouts), + .adis_data = ADIS16480_DATA(16490, &adis16495_timeouts, 0), }, [ADIS16495_1] = { .channels = adis16485_channels, @@ -998,7 +1012,9 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .max_dec_rate = 4250, .filter_freqs = adis16495_def_filter_freqs, .has_pps_clk_mode = true, - .adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts), + /* 20 elements of 16bits */ + .adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts, + ADIS16495_BURST_MAX_DATA * 2), }, [ADIS16495_2] = { .channels = adis16485_channels, @@ -1012,7 +1028,9 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .max_dec_rate = 4250, .filter_freqs = adis16495_def_filter_freqs, .has_pps_clk_mode = true, - .adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts), + /* 20 elements of 16bits */ + .adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts, + ADIS16495_BURST_MAX_DATA * 2), }, [ADIS16495_3] = { .channels = adis16485_channels, @@ -1026,7 +1044,9 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .max_dec_rate = 4250, .filter_freqs = adis16495_def_filter_freqs, .has_pps_clk_mode = true, - .adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts), + /* 20 elements of 16bits */ + .adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts, + ADIS16495_BURST_MAX_DATA * 2), }, [ADIS16497_1] = { .channels = adis16485_channels, @@ -1040,7 +1060,9 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .max_dec_rate = 4250, .filter_freqs = adis16495_def_filter_freqs, .has_pps_clk_mode = true, - .adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts), + /* 20 elements of 16bits */ + .adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts, + ADIS16495_BURST_MAX_DATA * 2), }, [ADIS16497_2] = { .channels = adis16485_channels, @@ -1054,7 +1076,9 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .max_dec_rate = 4250, .filter_freqs = adis16495_def_filter_freqs, .has_pps_clk_mode = true, - .adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts), + /* 20 elements of 16bits */ + .adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts, + ADIS16495_BURST_MAX_DATA * 2), }, [ADIS16497_3] = { .channels = adis16485_channels, @@ -1068,10 +1092,117 @@ static const struct adis16480_chip_info adis16480_chip_info[] = { .max_dec_rate = 4250, .filter_freqs = adis16495_def_filter_freqs, .has_pps_clk_mode = true, - .adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts), + /* 20 elements of 16bits */ + .adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts, + ADIS16495_BURST_MAX_DATA * 2), }, }; +static bool adis16480_validate_crc(const u16 *buf, const u8 n_elem, const u32 crc) +{ + u32 crc_calc; + u16 crc_buf[15]; + int j; + + for (j = 0; j < n_elem; j++) + crc_buf[j] = swab16(buf[j]); + + crc_calc = crc32(~0, crc_buf, n_elem * 2); + crc_calc ^= ~0; + + return (crc == crc_calc); +} + +static irqreturn_t adis16480_trigger_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct adis16480 *st = iio_priv(indio_dev); + struct adis *adis = &st->adis; + int ret, bit, offset, i = 0; + __be16 *buffer; + u32 crc; + bool valid; + const u32 cached_spi_speed_hz = adis->spi->max_speed_hz; + + adis_dev_lock(adis); + if (adis->current_page != 0) { + adis->tx[0] = ADIS_WRITE_REG(ADIS_REG_PAGE_ID); + adis->tx[1] = 0; + ret = spi_write(adis->spi, adis->tx, 2); + if (ret) { + dev_err(&adis->spi->dev, "Failed to change device page: %d\n", ret); + adis_dev_unlock(adis); + return ret; + } + } + + adis->spi->max_speed_hz = ADIS16495_BURST_MAX_SPEED; + + ret = spi_sync(adis->spi, &adis->msg); + if (ret) { + dev_err(&adis->spi->dev, "Failed to read data: %d\n", ret); + adis_dev_unlock(adis); + return ret; + } + + adis->spi->max_speed_hz = cached_spi_speed_hz; + adis->current_page = 0; + adis_dev_unlock(adis); + + /* + * After making the burst request, the response can have one or two + * 16-bit responses containing the BURST_ID depending on the sclk. If + * clk > 3.6MHz, then we will have two BURST_ID in a row. If clk < 3MHZ, + * we have only one. To manage that variation, we use the transition from the + * BURST_ID to the SYS_E_FLAG register, which will not be equal to 0xA5A5. If + * we not find this variation in the first 4 segments, then the data should + * not be valid. We don't return right away since the crc validation should + * fail anyways... + */ + buffer = adis->buffer; + for (offset = 0; offset < 4; offset++) { + u16 curr = be16_to_cpu(buffer[offset]); + u16 next = be16_to_cpu(buffer[offset + 1]); + + if (curr == ADIS16495_BURST_ID && next != ADIS16495_BURST_ID) { + offset++; + break; + } + } + + crc = be16_to_cpu(buffer[offset + 16]) << 16 | be16_to_cpu(buffer[offset + 15]); + valid = adis16480_validate_crc((u16 *)&buffer[offset], 15, crc); + if (!valid) { + dev_err(&adis->spi->dev, "Invalid crc\n"); + goto irq_done; + } + + for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) { + /* + * When burst mode is used, temperature is the first data + * channel in the sequence, but the temperature scan index + * is 10. + */ + switch (bit) { + case ADIS16480_SCAN_TEMP: + st->data[i++] = buffer[offset + 1]; + break; + case ADIS16480_SCAN_GYRO_X ... ADIS16480_SCAN_ACCEL_Z: + /* The lower register data is sequenced first */ + st->data[i++] = buffer[2 * bit + offset + 3]; + st->data[i++] = buffer[2 * bit + offset + 2]; + break; + } + } + + iio_push_to_buffers_with_timestamp(indio_dev, st->data, pf->timestamp); +irq_done: + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + static const struct iio_info adis16480_info = { .read_raw = &adis16480_read_raw, .write_raw = &adis16480_write_raw, @@ -1341,7 +1472,7 @@ static int adis16480_probe(struct spi_device *spi) st->clk_freq = st->chip_info->int_clk; } - ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL); + ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, adis16480_trigger_handler); if (ret) return ret;
Some supported devices support burst read function. This provides a method for reading a batch of data (status, temperature, gyroscopes, accelerometers, time stamp/data counter, and CRC code), which does not require a stall time between each 16-bit segment and only requires one command on the DIN line to initiate. Devices supporting this mode are: * adis16495-1 * adis16495-2 * adis16495-3 * adis16497-1 * adis16497-2 * adis16497-3 Signed-off-by: Nuno Sa <nuno.sa@analog.com> --- drivers/iio/imu/adis16480.c | 157 +++++++++++++++++++++++++++++++++--- 1 file changed, 144 insertions(+), 13 deletions(-)