On 18/06/15 09:09, Breana, Tiberiu A wrote: >> -----Original Message----- >> From: Peter Meerwald [mailto:pmeerw@xxxxxxxxxx] >> Sent: Wednesday, June 17, 2015 4:18 PM >> To: Breana, Tiberiu A >> Cc: linux-iio@xxxxxxxxxxxxxxx >> Subject: Re: [PATCH] iio: light: STK3310: un-invert proximity values >> >> >>> In accordance with the recent ABI changes, STK3310's proximity >>> readings should be un-inversed in order to return low values for >>> far-away objects and high values for close ones. >> >> the patch does a lot more than it advertises in the text above; I think it needs >> to be split up >> >> thanks, p. >> > > Peter, how would you see this patch split? A patch that deals with the raw readings and max values and another that switches the event types? > Or should I rather just write a more detailed commit message? I think a small change to the commit message to detail that both the event directions and the actual returned values need to be flipped should do the job. > > Thanks, > Tiberiu > >>> Signed-off-by: Tiberiu Breana <tiberiu.a.breana@xxxxxxxxx> >>> --- >>> drivers/iio/light/stk3310.c | 53 >>> +++++++++++++++------------------------------ >>> 1 file changed, 17 insertions(+), 36 deletions(-) >>> >>> diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c >>> index fee4297..c1a2182 100644 >>> --- a/drivers/iio/light/stk3310.c >>> +++ b/drivers/iio/light/stk3310.c >>> @@ -43,7 +43,6 @@ >>> #define STK3311_CHIP_ID_VAL 0x1D >>> #define STK3310_PSINT_EN 0x01 >>> #define STK3310_PS_MAX_VAL 0xFFFF >>> -#define STK3310_THRESH_MAX 0xFFFF >>> >>> #define STK3310_DRIVER_NAME "stk3310" >>> #define STK3310_REGMAP_NAME "stk3310_regmap" >>> @@ -84,15 +83,13 @@ static const struct reg_field >> stk3310_reg_field_flag_psint = >>> REG_FIELD(STK3310_REG_FLAG, 4, 4); static >> const struct reg_field >>> stk3310_reg_field_flag_nf = >>> REG_FIELD(STK3310_REG_FLAG, 0, 0); >>> -/* >>> - * Maximum PS values with regard to scale. Used to export the 'inverse' >>> - * PS value (high values for far objects, low values for near objects). >>> - */ >>> + >>> +/* Estimate maximum proximity values with regard to measurement >>> +scale. */ >>> static const int stk3310_ps_max[4] = { >>> - STK3310_PS_MAX_VAL / 64, >>> - STK3310_PS_MAX_VAL / 16, >>> - STK3310_PS_MAX_VAL / 4, >>> - STK3310_PS_MAX_VAL, >>> + STK3310_PS_MAX_VAL / 640, >>> + STK3310_PS_MAX_VAL / 160, >>> + STK3310_PS_MAX_VAL / 40, >>> + STK3310_PS_MAX_VAL / 10 >>> }; >>> >>> static const int stk3310_scale_table[][2] = { @@ -128,14 +125,14 @@ >>> static const struct iio_event_spec stk3310_events[] = { >>> /* Proximity event */ >>> { >>> .type = IIO_EV_TYPE_THRESH, >>> - .dir = IIO_EV_DIR_FALLING, >>> + .dir = IIO_EV_DIR_RISING, >>> .mask_separate = BIT(IIO_EV_INFO_VALUE) | >>> BIT(IIO_EV_INFO_ENABLE), >>> }, >>> /* Out-of-proximity event */ >>> { >>> .type = IIO_EV_TYPE_THRESH, >>> - .dir = IIO_EV_DIR_RISING, >>> + .dir = IIO_EV_DIR_FALLING, >>> .mask_separate = BIT(IIO_EV_INFO_VALUE) | >>> BIT(IIO_EV_INFO_ENABLE), >>> }, >>> @@ -205,23 +202,16 @@ static int stk3310_read_event(struct iio_dev >> *indio_dev, >>> u8 reg; >>> u16 buf; >>> int ret; >>> - unsigned int index; >>> struct stk3310_data *data = iio_priv(indio_dev); >>> >>> if (info != IIO_EV_INFO_VALUE) >>> return -EINVAL; >>> >>> - /* >>> - * Only proximity interrupts are implemented at the moment. >>> - * Since we're inverting proximity values, the sensor's 'high' >>> - * threshold will become our 'low' threshold, associated with >>> - * 'near' events. Similarly, the sensor's 'low' threshold will >>> - * be our 'high' threshold, associated with 'far' events. >>> - */ >>> + /* Only proximity interrupts are implemented at the moment. */ >>> if (dir == IIO_EV_DIR_RISING) >>> - reg = STK3310_REG_THDL_PS; >>> - else if (dir == IIO_EV_DIR_FALLING) >>> reg = STK3310_REG_THDH_PS; >>> + else if (dir == IIO_EV_DIR_FALLING) >>> + reg = STK3310_REG_THDL_PS; >>> else >>> return -EINVAL; >>> >>> @@ -232,8 +222,7 @@ static int stk3310_read_event(struct iio_dev >> *indio_dev, >>> dev_err(&data->client->dev, "register read failed\n"); >>> return ret; >>> } >>> - regmap_field_read(data->reg_ps_gain, &index); >>> - *val = swab16(stk3310_ps_max[index] - buf); >>> + *val = swab16(buf); >>> >>> return IIO_VAL_INT; >>> } >>> @@ -257,13 +246,13 @@ static int stk3310_write_event(struct iio_dev >> *indio_dev, >>> return -EINVAL; >>> >>> if (dir == IIO_EV_DIR_RISING) >>> - reg = STK3310_REG_THDL_PS; >>> - else if (dir == IIO_EV_DIR_FALLING) >>> reg = STK3310_REG_THDH_PS; >>> + else if (dir == IIO_EV_DIR_FALLING) >>> + reg = STK3310_REG_THDL_PS; >>> else >>> return -EINVAL; >>> >>> - buf = swab16(stk3310_ps_max[index] - val); >>> + buf = swab16(val); >>> ret = regmap_bulk_write(data->regmap, reg, &buf, 2); >>> if (ret < 0) >>> dev_err(&client->dev, "failed to set PS threshold!\n"); @@ - >> 334,14 >>> +323,6 @@ static int stk3310_read_raw(struct iio_dev *indio_dev, >>> return ret; >>> } >>> *val = swab16(buf); >>> - if (chan->type == IIO_PROXIMITY) { >>> - /* >>> - * Invert the proximity data so we return low values >>> - * for close objects and high values for far ones. >>> - */ >>> - regmap_field_read(data->reg_ps_gain, &index); >>> - *val = stk3310_ps_max[index] - *val; >>> - } >>> mutex_unlock(&data->lock); >>> return IIO_VAL_INT; >>> case IIO_CHAN_INFO_INT_TIME: >>> @@ -581,8 +562,8 @@ static irqreturn_t stk3310_irq_event_handler(int >> irq, void *private) >>> } >>> event = IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 1, >>> IIO_EV_TYPE_THRESH, >>> - (dir ? IIO_EV_DIR_RISING : >>> - IIO_EV_DIR_FALLING)); >>> + (dir ? IIO_EV_DIR_FALLING : >>> + IIO_EV_DIR_RISING)); >>> iio_push_event(indio_dev, event, data->timestamp); >>> >>> /* Reset the interrupt flag */ >>> >> >> -- >> >> Peter Meerwald >> +43-664-2444418 (mobile) > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in
