* [PATCH] iio: si1133: read 24 signed integer for measurement
@ 2020-02-07 16:07 Maxime Roussin-Bélanger
2020-02-14 14:22 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Maxime Roussin-Bélanger @ 2020-02-07 16:07 UTC (permalink / raw)
To: linux-iio
Cc: jic23, knaack.h, lars, pmeerw, Maxime Roussin-Bélanger,
Guillaume Champagne
The chip is configured in 24 bit mode. The values read from it must
always be treated as is. This fixes the issue by replacing the previous
16 bits value by a 24 bits buffer.
This changes affects the value output by previous version of the driver,
since the least significant byte was missing. The upper half of 16
bit values previously output are now the upper half of a 24 bit value.
Co-authored-by: Guillaume Champagne <champagne.guillaume.c@gmail.com>
Signed-off-by: Maxime Roussin-Bélanger <maxime.roussinbelanger@gmail.com>
---
drivers/iio/light/si1133.c | 37 ++++++++++++++++++++++++-------------
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
index 777b1a0848c9..509af982e185 100644
--- a/drivers/iio/light/si1133.c
+++ b/drivers/iio/light/si1133.c
@@ -102,6 +102,9 @@
#define SI1133_INPUT_FRACTION_LOW 15
#define SI1133_LUX_OUTPUT_FRACTION 12
#define SI1133_LUX_BUFFER_SIZE 9
+#define SI1133_MEASURE_BUFFER_SIZE 3
+
+#define SI1133_SIGN_BIT_INDEX 23
static const int si1133_scale_available[] = {
1, 2, 4, 8, 16, 32, 64, 128};
@@ -234,13 +237,13 @@ static const struct si1133_lux_coeff lux_coeff = {
}
};
-static int si1133_calculate_polynomial_inner(u32 input, u8 fraction, u16 mag,
+static int si1133_calculate_polynomial_inner(s32 input, u8 fraction, u16 mag,
s8 shift)
{
return ((input << fraction) / mag) << shift;
}
-static int si1133_calculate_output(u32 x, u32 y, u8 x_order, u8 y_order,
+static int si1133_calculate_output(s32 x, s32 y, u8 x_order, u8 y_order,
u8 input_fraction, s8 sign,
const struct si1133_coeff *coeffs)
{
@@ -276,7 +279,7 @@ static int si1133_calculate_output(u32 x, u32 y, u8 x_order, u8 y_order,
* The algorithm is from:
* https://siliconlabs.github.io/Gecko_SDK_Doc/efm32zg/html/si1133_8c_source.html#l00716
*/
-static int si1133_calc_polynomial(u32 x, u32 y, u8 input_fraction, u8 num_coeff,
+static int si1133_calc_polynomial(s32 x, s32 y, u8 input_fraction, u8 num_coeff,
const struct si1133_coeff *coeffs)
{
u8 x_order, y_order;
@@ -614,23 +617,24 @@ static int si1133_measure(struct si1133_data *data,
{
int err;
- __be16 resp;
+ u8 buffer[SI1133_MEASURE_BUFFER_SIZE];
err = si1133_set_adcmux(data, 0, chan->channel);
if (err)
return err;
/* Deactivate lux measurements if they were active */
err = si1133_set_chlist(data, BIT(0));
if (err)
return err;
- err = si1133_bulk_read(data, SI1133_REG_HOSTOUT(0), sizeof(resp),
- (u8 *)&resp);
+ err = si1133_bulk_read(data, SI1133_REG_HOSTOUT(0), sizeof(buffer),
+ buffer);
if (err)
return err;
- *val = be16_to_cpu(resp);
+ *val = sign_extend32((buffer[0] << 16) | (buffer[1] << 8) | buffer[2],
+ SI1133_SIGN_BIT_INDEX);
return err;
}
@@ -704,9 +708,9 @@ static int si1133_get_lux(struct si1133_data *data, int *val)
{
int err;
int lux;
- u32 high_vis;
- u32 low_vis;
- u32 ir;
+ s32 high_vis;
+ s32 low_vis;
+ s32 ir;
u8 buffer[SI1133_LUX_BUFFER_SIZE];
/* Activate lux channels */
@@ -719,9 +723,16 @@ static int si1133_get_lux(struct si1133_data *data, int *val)
if (err)
return err;
- high_vis = (buffer[0] << 16) | (buffer[1] << 8) | buffer[2];
- low_vis = (buffer[3] << 16) | (buffer[4] << 8) | buffer[5];
- ir = (buffer[6] << 16) | (buffer[7] << 8) | buffer[8];
+ high_vis =
+ sign_extend32((buffer[0] << 16) | (buffer[1] << 8) | buffer[2],
+ SI1133_SIGN_BIT_INDEX);
+
+ low_vis =
+ sign_extend32((buffer[3] << 16) | (buffer[4] << 8) | buffer[5],
+ SI1133_SIGN_BIT_INDEX);
+
+ ir = sign_extend32((buffer[6] << 16) | (buffer[7] << 8) | buffer[8],
+ SI1133_SIGN_BIT_INDEX);
if (high_vis > SI1133_ADC_THRESHOLD || ir > SI1133_ADC_THRESHOLD)
lux = si1133_calc_polynomial(high_vis, ir,
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] iio: si1133: read 24 signed integer for measurement
2020-02-07 16:07 [PATCH] iio: si1133: read 24 signed integer for measurement Maxime Roussin-Bélanger
@ 2020-02-14 14:22 ` Jonathan Cameron
[not found] ` <CAE=T-s7E46Ym9yvxW4iDVtFqw3VmXQAHRA5kr_VV-uSBnuoXsA@mail.gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:22 UTC (permalink / raw)
To: Maxime Roussin-Bélanger
Cc: linux-iio, knaack.h, lars, pmeerw, Guillaume Champagne
On Fri, 7 Feb 2020 11:07:40 -0500
Maxime Roussin-Bélanger <maxime.roussinbelanger@gmail.com> wrote:
> The chip is configured in 24 bit mode. The values read from it must
> always be treated as is. This fixes the issue by replacing the previous
> 16 bits value by a 24 bits buffer.
>
> This changes affects the value output by previous version of the driver,
> since the least significant byte was missing. The upper half of 16
> bit values previously output are now the upper half of a 24 bit value.
>
> Co-authored-by: Guillaume Champagne <champagne.guillaume.c@gmail.com>
> Signed-off-by: Maxime Roussin-Bélanger <maxime.roussinbelanger@gmail.com>
Patch looks fine, so question is whether we treat this as an enhancement,
or a fix? If it's a fix please provide a suitable fixes tag.
Thanks,
Jonathan
> ---
> drivers/iio/light/si1133.c | 37 ++++++++++++++++++++++++-------------
> 1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iio/light/si1133.c b/drivers/iio/light/si1133.c
> index 777b1a0848c9..509af982e185 100644
> --- a/drivers/iio/light/si1133.c
> +++ b/drivers/iio/light/si1133.c
> @@ -102,6 +102,9 @@
> #define SI1133_INPUT_FRACTION_LOW 15
> #define SI1133_LUX_OUTPUT_FRACTION 12
> #define SI1133_LUX_BUFFER_SIZE 9
> +#define SI1133_MEASURE_BUFFER_SIZE 3
> +
> +#define SI1133_SIGN_BIT_INDEX 23
>
> static const int si1133_scale_available[] = {
> 1, 2, 4, 8, 16, 32, 64, 128};
> @@ -234,13 +237,13 @@ static const struct si1133_lux_coeff lux_coeff = {
> }
> };
>
> -static int si1133_calculate_polynomial_inner(u32 input, u8 fraction, u16 mag,
> +static int si1133_calculate_polynomial_inner(s32 input, u8 fraction, u16 mag,
> s8 shift)
> {
> return ((input << fraction) / mag) << shift;
> }
>
> -static int si1133_calculate_output(u32 x, u32 y, u8 x_order, u8 y_order,
> +static int si1133_calculate_output(s32 x, s32 y, u8 x_order, u8 y_order,
> u8 input_fraction, s8 sign,
> const struct si1133_coeff *coeffs)
> {
> @@ -276,7 +279,7 @@ static int si1133_calculate_output(u32 x, u32 y, u8 x_order, u8 y_order,
> * The algorithm is from:
> * https://siliconlabs.github.io/Gecko_SDK_Doc/efm32zg/html/si1133_8c_source.html#l00716
> */
> -static int si1133_calc_polynomial(u32 x, u32 y, u8 input_fraction, u8 num_coeff,
> +static int si1133_calc_polynomial(s32 x, s32 y, u8 input_fraction, u8 num_coeff,
> const struct si1133_coeff *coeffs)
> {
> u8 x_order, y_order;
> @@ -614,23 +617,24 @@ static int si1133_measure(struct si1133_data *data,
> {
> int err;
>
> - __be16 resp;
> + u8 buffer[SI1133_MEASURE_BUFFER_SIZE];
>
> err = si1133_set_adcmux(data, 0, chan->channel);
> if (err)
> return err;
>
> /* Deactivate lux measurements if they were active */
> err = si1133_set_chlist(data, BIT(0));
> if (err)
> return err;
>
> - err = si1133_bulk_read(data, SI1133_REG_HOSTOUT(0), sizeof(resp),
> - (u8 *)&resp);
> + err = si1133_bulk_read(data, SI1133_REG_HOSTOUT(0), sizeof(buffer),
> + buffer);
> if (err)
> return err;
>
> - *val = be16_to_cpu(resp);
> + *val = sign_extend32((buffer[0] << 16) | (buffer[1] << 8) | buffer[2],
> + SI1133_SIGN_BIT_INDEX);
>
> return err;
> }
> @@ -704,9 +708,9 @@ static int si1133_get_lux(struct si1133_data *data, int *val)
> {
> int err;
> int lux;
> - u32 high_vis;
> - u32 low_vis;
> - u32 ir;
> + s32 high_vis;
> + s32 low_vis;
> + s32 ir;
> u8 buffer[SI1133_LUX_BUFFER_SIZE];
>
> /* Activate lux channels */
> @@ -719,9 +723,16 @@ static int si1133_get_lux(struct si1133_data *data, int *val)
> if (err)
> return err;
>
> - high_vis = (buffer[0] << 16) | (buffer[1] << 8) | buffer[2];
> - low_vis = (buffer[3] << 16) | (buffer[4] << 8) | buffer[5];
> - ir = (buffer[6] << 16) | (buffer[7] << 8) | buffer[8];
> + high_vis =
> + sign_extend32((buffer[0] << 16) | (buffer[1] << 8) | buffer[2],
> + SI1133_SIGN_BIT_INDEX);
> +
> + low_vis =
> + sign_extend32((buffer[3] << 16) | (buffer[4] << 8) | buffer[5],
> + SI1133_SIGN_BIT_INDEX);
> +
> + ir = sign_extend32((buffer[6] << 16) | (buffer[7] << 8) | buffer[8],
> + SI1133_SIGN_BIT_INDEX);
>
> if (high_vis > SI1133_ADC_THRESHOLD || ir > SI1133_ADC_THRESHOLD)
> lux = si1133_calc_polynomial(high_vis, ir,
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-14 18:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-07 16:07 [PATCH] iio: si1133: read 24 signed integer for measurement Maxime Roussin-Bélanger
2020-02-14 14:22 ` Jonathan Cameron
[not found] ` <CAE=T-s7E46Ym9yvxW4iDVtFqw3VmXQAHRA5kr_VV-uSBnuoXsA@mail.gmail.com>
2020-02-14 15:53 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).