On Mon, 22 Jul 2019 16:53:41 +0200
Enric Balletbo i Serra <enric.balle...@collabora.com> wrote:
> Hi Jonathan,
>
> On 14/7/19 18:32, Jonathan Cameron wrote:
> > On Fri, 28 Jun 2019 12:17:08 -0700
> > Gwendal Grignou <gwen...@chromium.org> wrote:
> >
> >> To allow cros_ec iio core library to be used with legacy device, add a
> >> vector to rotate sensor data if necessary: legacy devices are not
> >> reporting data in HTML5/Android sensor referential.
> >>
> >> Check the data is not rotated on recent chromebooks that use the HTML5
> >> standard to present sensor data.
> >>
> >> Signed-off-by: Gwendal Grignou <gwen...@chromium.org>
> >> Reviewed-by: Douglas Anderson <diand...@chromium.org>
> > Acked-by: Jonathan Cameron <jonathan.came...@huawei.com>
> >
> > As I mentioned in one of the other series. I've lost track of whether
> > anyone wants me to apply any of these through IIO, so will just ack
> > them as appropriate and assume someone will shout if they do want
> > me to pick them up ;)
> >
>
> To try to give you a bit of light on this, all the required changes in
> chrome-platform are now in upstream so all the patches can go safely through
> your tree. The order to pick the patches is as follow:
>
>
> 1096491 [v4,1/1] iio: common: cros_ec_sensors: determine protocol version
>
> 1100922 [v6,1/4] iio: cros_ec: Add sign vector in core for backward
> compatibility
> 1100924 [v6,3/4] iio: cros_ec_accel_legacy: Use cros_ec_sensors_core
> 1100923 [v6,4/4] iio: cros_ec_accel_legacy: Add support for veyron-minnie
>
> 1100982 [v5,1/1] iio: common: cros_ec_sensors: Expose cros_ec_sensors
> frequency
> range via iio sysfs
>
> But if you try to apply latest versions from patchwork you'll get some trivial
> conflicts. So, I fixed the problems, rebased on top of your testing branch,
> added my Rb tag to all the patches and put together in this branch [1]
>
> All the patches have your Ack, so should be fine if you apply all of them just
> replacing your Ack for your Signed-off
Thanks! This is very helpful indeed.
I've done exactly - well sort of :)
Note I did get:
drivers/iio/light/cros_ec_light_prox.c:120:9: warning: ‘ret’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
120 | return ret;
| ^~~
Which looks like a bug, as there is one path under
case IIO_CHAN_INFO_CALIBBIAS:
that got caught by gcc.
Ah, it's my merge mess up on an earlier patch. I'll fix it and post
in reply to that patch.
Also I swapped in the v6 of the veyron_minnie patches and v5 of the sysfs one
as your branch predates those I think.
Thanks,
Jonathan
>
> I can also send a new patch series with those if you prefer this option.
>
> Hopefully is more clear now and sorry for that mess.
> ~ Enric
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=for-iio-next
>
>
> > Thanks,
> >
> > Jonathan
> >
> >> ---
> >> drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 4 ++++
> >> include/linux/iio/common/cros_ec_sensors_core.h | 1 +
> >> 2 files changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> >> b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> >> index 719a0df5aeeb..e8a4d78659c8 100644
> >> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> >> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> >> @@ -66,6 +66,9 @@ int cros_ec_sensors_core_init(struct platform_device
> >> *pdev,
> >> }
> >> state->type = state->resp->info.type;
> >> state->loc = state->resp->info.location;
> >> +
> >> + /* Set sign vector, only used for backward compatibility. */
> >> + memset(state->sign, 1, CROS_EC_SENSOR_MAX_AXIS);
> >> }
> >>
> >> return 0;
> >> @@ -254,6 +257,7 @@ static int cros_ec_sensors_read_data_unsafe(struct
> >> iio_dev *indio_dev,
> >> if (ret < 0)
> >> return ret;
> >>
> >> + *data *= st->sign[i];
> >> data++;
> >> }
> >>
> >> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h
> >> b/include/linux/iio/common/cros_ec_sensors_core.h
> >> index ce16445411ac..a1c85ad4df91 100644
> >> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> >> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> >> @@ -71,6 +71,7 @@ struct cros_ec_sensors_core_state {
> >> enum motionsensor_location loc;
> >>
> >> s16 calib[CROS_EC_SENSOR_MAX_AXIS];
> >> + s8 sign[CROS_EC_SENSOR_MAX_AXIS];
> >>
> >> u8 samples[CROS_EC_SAMPLE_SIZE];
> >>
> >
