* [PATCH v2] iio: sx9310: Fix semtech,avg-pos-strength setting when > 16
@ 2020-11-20 18:29 Stephen Boyd
2020-11-20 18:31 ` Doug Anderson
2020-11-21 14:58 ` Jonathan Cameron
0 siblings, 2 replies; 5+ messages in thread
From: Stephen Boyd @ 2020-11-20 18:29 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-kernel, linux-iio, Daniel Campello, Lars-Peter Clausen,
Peter Meerwald-Stadler, Douglas Anderson, Gwendal Grignou,
Evan Green
This DT property can be 0, 16, and then 64, but not 32. The math here
doesn't recognize this slight bump in the power of 2 numbers and
translates a DT property of 64 into the register value '3' when it
really should be '2'. Fix it by subtracting one more if the number being
translated is larger than 16.
Cc: Daniel Campello <campello@chromium.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Gwendal Grignou <gwendal@chromium.org>
Cc: Evan Green <evgreen@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
Changes from v1 (https://lore.kernel.org/r/20201120073842.3232458-1-swboyd@chromium.org):
* Changed ternary to consider 17 to 31 as the same as 16
drivers/iio/proximity/sx9310.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index a2f820997afc..ee1b4ff05a37 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -1305,7 +1305,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
if (ret)
break;
- pos = min(max(ilog2(pos), 3), 10) - 3;
+ pos = min(max(ilog2(pos), 3), 11) - (pos >= 32 ? 4 : 3);
reg_def->def &= ~SX9310_REG_PROX_CTRL7_AVGPOSFILT_MASK;
reg_def->def |= FIELD_PREP(SX9310_REG_PROX_CTRL7_AVGPOSFILT_MASK,
pos);
base-commit: 5b19ca2c78a0838976064c0347e46a2c859b541d
--
https://chromeos.dev
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] iio: sx9310: Fix semtech,avg-pos-strength setting when > 16
2020-11-20 18:29 [PATCH v2] iio: sx9310: Fix semtech,avg-pos-strength setting when > 16 Stephen Boyd
@ 2020-11-20 18:31 ` Doug Anderson
2020-11-21 14:58 ` Jonathan Cameron
1 sibling, 0 replies; 5+ messages in thread
From: Doug Anderson @ 2020-11-20 18:31 UTC (permalink / raw)
To: Stephen Boyd
Cc: Jonathan Cameron, LKML, linux-iio, Daniel Campello,
Lars-Peter Clausen, Peter Meerwald-Stadler, Gwendal Grignou,
Evan Green
Hi,
On Fri, Nov 20, 2020 at 10:29 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> This DT property can be 0, 16, and then 64, but not 32. The math here
> doesn't recognize this slight bump in the power of 2 numbers and
> translates a DT property of 64 into the register value '3' when it
> really should be '2'. Fix it by subtracting one more if the number being
> translated is larger than 16.
>
> Cc: Daniel Campello <campello@chromium.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Gwendal Grignou <gwendal@chromium.org>
> Cc: Evan Green <evgreen@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> Changes from v1 (https://lore.kernel.org/r/20201120073842.3232458-1-swboyd@chromium.org):
> * Changed ternary to consider 17 to 31 as the same as 16
>
> drivers/iio/proximity/sx9310.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] iio: sx9310: Fix semtech,avg-pos-strength setting when > 16
2020-11-20 18:29 [PATCH v2] iio: sx9310: Fix semtech,avg-pos-strength setting when > 16 Stephen Boyd
2020-11-20 18:31 ` Doug Anderson
@ 2020-11-21 14:58 ` Jonathan Cameron
2020-11-21 15:02 ` Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2020-11-21 14:58 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-kernel, linux-iio, Daniel Campello, Lars-Peter Clausen,
Peter Meerwald-Stadler, Douglas Anderson, Gwendal Grignou,
Evan Green
On Fri, 20 Nov 2020 10:29:44 -0800
Stephen Boyd <swboyd@chromium.org> wrote:
> This DT property can be 0, 16, and then 64, but not 32. The math here
> doesn't recognize this slight bump in the power of 2 numbers and
> translates a DT property of 64 into the register value '3' when it
> really should be '2'. Fix it by subtracting one more if the number being
> translated is larger than 16.
>
> Cc: Daniel Campello <campello@chromium.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Gwendal Grignou <gwendal@chromium.org>
> Cc: Evan Green <evgreen@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> Changes from v1 (https://lore.kernel.org/r/20201120073842.3232458-1-swboyd@chromium.org):
> * Changed ternary to consider 17 to 31 as the same as 16
>
> drivers/iio/proximity/sx9310.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> index a2f820997afc..ee1b4ff05a37 100644
> --- a/drivers/iio/proximity/sx9310.c
> +++ b/drivers/iio/proximity/sx9310.c
> @@ -1305,7 +1305,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
> if (ret)
> break;
>
> - pos = min(max(ilog2(pos), 3), 10) - 3;
> + pos = min(max(ilog2(pos), 3), 11) - (pos >= 32 ? 4 : 3);
Given this is now rather tricky to read, I'd just do an explicit lookup of
the allowed values. Probably just a switch statement.
Thanks,
Jonathan
> reg_def->def &= ~SX9310_REG_PROX_CTRL7_AVGPOSFILT_MASK;
> reg_def->def |= FIELD_PREP(SX9310_REG_PROX_CTRL7_AVGPOSFILT_MASK,
> pos);
>
> base-commit: 5b19ca2c78a0838976064c0347e46a2c859b541d
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] iio: sx9310: Fix semtech,avg-pos-strength setting when > 16
2020-11-21 14:58 ` Jonathan Cameron
@ 2020-11-21 15:02 ` Jonathan Cameron
2020-12-02 19:32 ` Stephen Boyd
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2020-11-21 15:02 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-kernel, linux-iio, Daniel Campello, Lars-Peter Clausen,
Peter Meerwald-Stadler, Douglas Anderson, Gwendal Grignou,
Evan Green
On Sat, 21 Nov 2020 14:58:49 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 20 Nov 2020 10:29:44 -0800
> Stephen Boyd <swboyd@chromium.org> wrote:
>
> > This DT property can be 0, 16, and then 64, but not 32. The math here
> > doesn't recognize this slight bump in the power of 2 numbers and
> > translates a DT property of 64 into the register value '3' when it
> > really should be '2'. Fix it by subtracting one more if the number being
> > translated is larger than 16.
> >
> > Cc: Daniel Campello <campello@chromium.org>
> > Cc: Lars-Peter Clausen <lars@metafoo.de>
> > Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Gwendal Grignou <gwendal@chromium.org>
> > Cc: Evan Green <evgreen@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >
> > Changes from v1 (https://lore.kernel.org/r/20201120073842.3232458-1-swboyd@chromium.org):
> > * Changed ternary to consider 17 to 31 as the same as 16
> >
> > drivers/iio/proximity/sx9310.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> > index a2f820997afc..ee1b4ff05a37 100644
> > --- a/drivers/iio/proximity/sx9310.c
> > +++ b/drivers/iio/proximity/sx9310.c
> > @@ -1305,7 +1305,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
> > if (ret)
> > break;
> >
> > - pos = min(max(ilog2(pos), 3), 10) - 3;
> > + pos = min(max(ilog2(pos), 3), 11) - (pos >= 32 ? 4 : 3);
>
> Given this is now rather tricky to read, I'd just do an explicit lookup of
> the allowed values. Probably just a switch statement.
As an alternative, a comment would help saying that it's powers of 2 except
for a gap at 64.
Thanks,
J
>
> Thanks,
>
> Jonathan
>
> > reg_def->def &= ~SX9310_REG_PROX_CTRL7_AVGPOSFILT_MASK;
> > reg_def->def |= FIELD_PREP(SX9310_REG_PROX_CTRL7_AVGPOSFILT_MASK,
> > pos);
> >
> > base-commit: 5b19ca2c78a0838976064c0347e46a2c859b541d
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] iio: sx9310: Fix semtech,avg-pos-strength setting when > 16
2020-11-21 15:02 ` Jonathan Cameron
@ 2020-12-02 19:32 ` Stephen Boyd
0 siblings, 0 replies; 5+ messages in thread
From: Stephen Boyd @ 2020-12-02 19:32 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-kernel, linux-iio, Daniel Campello, Lars-Peter Clausen,
Peter Meerwald-Stadler, Douglas Anderson, Gwendal Grignou,
Evan Green
Quoting Jonathan Cameron (2020-11-21 07:02:45)
> On Sat, 21 Nov 2020 14:58:49 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Fri, 20 Nov 2020 10:29:44 -0800
> > Stephen Boyd <swboyd@chromium.org> wrote:
> > > diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
> > > index a2f820997afc..ee1b4ff05a37 100644
> > > --- a/drivers/iio/proximity/sx9310.c
> > > +++ b/drivers/iio/proximity/sx9310.c
> > > @@ -1305,7 +1305,7 @@ sx9310_get_default_reg(struct sx9310_data *data, int i,
> > > if (ret)
> > > break;
> > >
> > > - pos = min(max(ilog2(pos), 3), 10) - 3;
> > > + pos = min(max(ilog2(pos), 3), 11) - (pos >= 32 ? 4 : 3);
> >
> > Given this is now rather tricky to read, I'd just do an explicit lookup of
> > the allowed values. Probably just a switch statement.
> As an alternative, a comment would help saying that it's powers of 2 except
> for a gap at 64.
>
Ok. I'll update with a comment.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-02 19:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 18:29 [PATCH v2] iio: sx9310: Fix semtech,avg-pos-strength setting when > 16 Stephen Boyd
2020-11-20 18:31 ` Doug Anderson
2020-11-21 14:58 ` Jonathan Cameron
2020-11-21 15:02 ` Jonathan Cameron
2020-12-02 19:32 ` Stephen Boyd
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).