On Wed, May 29, 2019 at 4:44 AM Lee Jones <lee.jo...@linaro.org> wrote:
>
> On Tue, 28 May 2019, Gwendal Grignou wrote:
>
> > On Mon, Apr 1, 2019 at 8:46 PM Lee Jones <lee.jo...@linaro.org> wrote:
> > >
> > > On Wed, 27 Feb 2019, Gwendal Grignou wrote:
> > >
> > > > From: Enric Balletbo i Serra <enric.balle...@collabora.com>
> > > >
> > > > With this patch, the cros_ec_ctl driver will register the legacy
> > > > accelerometer driver (named cros_ec_accel_legacy) if it fails to
> > > > register sensors through the usual path cros_ec_sensors_register().
> > > > This legacy device is present on Chromebook devices with older EC
> > > > firmware only supporting deprecated EC commands (Glimmer based devices).
> > > >
> > > > Tested-by: Gwendal Grignou <gwen...@chromium.org>
> > > > Signed-off-by: Enric Balletbo i Serra <enric.balle...@collabora.com>
> > > > Reviewed-by: Gwendal Grignou <gwen...@chromium.org>
> > > > Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com>
> > > > ---
> > > > Changes in v5:
> > > > - Remove unnecessary white lines.
> > > >
> > > > Changes in v4:
> > > > - [5/8] Nit: EC -> ECs (Lee Jones)
> > > > - [5/8] Statically define cros_ec_accel_legacy_cells (Lee Jones)
> > > >
> > > > Changes in v3:
> > > > - [5/8] Add the Reviewed-by Andy Shevchenko.
> > > >
> > > > Changes in v2:
> > > > - [5/8] Add the Reviewed-by Gwendal.
> > > >
> > > > drivers/mfd/cros_ec_dev.c | 66 +++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 66 insertions(+)
> > > >
> > > > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> > > > index d275deaecb12..64567bd0a081 100644
> > > > --- a/drivers/mfd/cros_ec_dev.c
> > > > +++ b/drivers/mfd/cros_ec_dev.c
> > > > @@ -376,6 +376,69 @@ static void cros_ec_sensors_register(struct
> > > > cros_ec_dev *ec)
> > > > kfree(msg);
> > > > }
> > > >
> > > > +static struct cros_ec_sensor_platform sensor_platforms[] = {
> > > > + { .sensor_num = 0 },
> > > > + { .sensor_num = 1 }
> > > > +};
> > >
> > > I'm still very uncomfortable with this struct.
> > >
> > > Other than these indices, the sensors have no other distinguishing
> > > features, thus there should be no need to identify or distinguish
> > > between them in this way.
> > When initializing the sensors, the IIO driver expect to find in the
> > data structure pointed by dev_get_platdata(dev), in field sensor_num
> > is stored the index assigned by the embedded controller to talk to a
> > given sensor.
> > cros_ec_sensors_register() use the same mechanism; in that function,
> > the sensor_num field is populated from the output of an EC command
> > MOTIONSENSE_CMD_INFO. In case of legacy mode, that command may not be
> > available and in any case we know the EC has only either 2
> > accelerometers present or nothing.
> >
> > For instance, let's compare a legacy device with a more recent one:
> >
> > legacy:
> > type | id | sensor_num | device name
> > accelerometer | 0 | 0 | cros-ec-accel.0
> > accelerometer | 1 | 1 | cros-ec-accel.1
> >
> > Modern:
> > type | id | sensor_num | device name
> > accelerometer | 0 | 0 | cros-ec-accel.0
> > accelerometer | 1 | 1 | cros-ec-accel.1
> > gyroscope | 0 | 2 | cros-ec-gyro.0
> > magnetometer | 0 | 3 | cros-ec-mag.0
> > light | 0 | 4 |
> > cros-ec-light.0
> > ...
>
> Why can't these numbers be assigned at runtime?
I assume you want to know why IIO drivers need to know "sensor_num"
ahead of time. It is because each IIO driver is independent from the
other.
Let assume there was 2 light sensors in the device:
type | id | sensor_num | device name
light | 0 | 4 | cros-ec-light.0
light | 1 | 5 | cros-ec-light.1
In case of sensors of the same type without sensor_num, cros-ec-light
driver has no information at probe time if it should bind to sensors
named by the EC 4 or 5.
We could get away with cros-ec-accel, as EC always presents
accelerometers with sensor_num 0 and 1, but I don't want to rely on
this property in the general case.
Only cros_ec_dev MFD driver has the global view of all sensors available.
>
> --
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
