Re: [PATCH v2 1/4] iio: cros_ec_sensors_core: Add common functions for the ChromeOS EC Sensor Hub.

2016-07-26 Thread Jonathan Cameron
On 25/07/16 09:38, Enric Balletbo Serra wrote:
> Hi Jonathan,
> 
> Many thanks for the review, some answers below.
> 
> 2016-07-24 15:51 GMT+02:00 Jonathan Cameron :
>> On 20/07/16 10:22, Enric Balletbo i Serra wrote:
>>> Add the core functions to be able to support the sensors attached behind
>>> the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
>>> drivers.
>>>
>>> The cros_ec_sensor_core driver matches with current driver in ChromeOS
>>> 4.4 tree, so it includes all the fixes at the moment. The support for
>>> this driver was made by Gwendal Grignou. The original patch and all the
>>> fixes has been squashed and rebased on top of mainline.
>>>
>>> Signed-off-by: Gwendal Grignou 
>>> Signed-off-by: Guenter Roeck 
>>> [eballetbo: split, squash and rebase on top of mainline the patches
>>> found in ChromeOS tree]
>>> Signed-off-by: Enric Balletbo i Serra 
>> Few minor bits inline.  I'm happy enough with the ABI (except that I would
>> like a justification for why userspace needs the id one)
>> We should see if anyone else has comments on that.
>>
>> The location one may well become more general in future (to cover other
>> devices).
>>
>> Jonathan
>>> ---
>>>
>>> Changes since v1:
>>>   - Check kernel-doc documentation.
>>>   - Bring this in as an when you need it in the rest of the series.
>>>   - Fix some spelling mistakes.
>>>   - Include ABI documentation.
>>>   - Be more careful with buffer sizes (sprintf -> snprintf)
>>>   - Add cros_ec_sensors prefix to all function.
>>>   - Check return values on some functions.
>>>
>>>  Documentation/ABI/testing/sysfs-bus-iio-cros-ec|  25 +
>>>  drivers/iio/common/Kconfig |   1 +
>>>  drivers/iio/common/Makefile|   1 +
>>>  drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
>>>  drivers/iio/common/cros_ec_sensors/Makefile|   5 +
>>>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 505 
>>> +
>>>  .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 177 
>>>  include/linux/mfd/cros_ec.h|   9 +
>>>  include/linux/mfd/cros_ec_commands.h   |  99 +++-
>>>  9 files changed, 831 insertions(+), 5 deletions(-)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
>>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
>>>  create mode 100644 
>>> drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>>>  create mode 100644 
>>> drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec 
>>> b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>>> new file mode 100644
>>> index 000..3e626b6
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>>> @@ -0,0 +1,25 @@
>>> +What:/sys/bus/iio/devices/iio:deviceX/calibrate
>>> +Date:July 2015
>>> +KernelVersion:   4.7
>>> +Contact: linux-...@vger.kernel.org
>>> +Description:
>>> + Writing '1' will perform a FOC (Fast Online Calibration). The
>>> +corresponding calibration offsets can be read from 
>>> *_calibbias
>>> +entries.
>> This one is fine as far as I'm concerned.
>>> +
>>> +What:/sys/bus/iio/devices/iio:deviceX/id
>>> +Date:July 2015
>>> +KernelVersion:   4.7
>>> +Contact: linux-...@vger.kernel.org
>>> +Description:
>>> +The id attribute holds an unique number for the motion 
>>> sensor
>>> +identification, as reported by the ChromeOS Embedded 
>>> Controller.
>> Mostly with these, we've just spat the out to the kernel logs as dev_info
>> during startup.
>>
>> Any reason this needs to be more explictly exposed to userspace?
> 
> After think a bit more about this, and to be honest, I'm not sure if
> this is really useful to be exposed to userspace. The only use case
> that I'm thinking is that some userspace app uses it to uniquely
> identify the sensors that hang from the ChromeOS Embedded Controller,
> but I'm not sure about this exist. I'll investigate a bit more for if
> I find a good justification, otherwise I'll remove this.
> 
> Maybe someone from chromium can give us more opinions?
> 
>>> +
>>> +What:/sys/bus/iio/devices/iio:deviceX/location
>>> +Date:July 2015
>>> +KernelVersion:   4.7
>>> +Contact: linux-...@vger.kernel.org
>>> +Description:
>>> + This attribute returns a string with the physical location 
>>> where
>>> +the motion sensor is placed. For example, in a laptop a 
>>> motion
>>> +sensor can be located on the base or on the lid.
>> Interesting...
>>
>> We'll probably want to ultimately use this one more generally.  This probably
>> 

Re: [PATCH v2 1/4] iio: cros_ec_sensors_core: Add common functions for the ChromeOS EC Sensor Hub.

2016-07-26 Thread Jonathan Cameron
On 25/07/16 09:38, Enric Balletbo Serra wrote:
> Hi Jonathan,
> 
> Many thanks for the review, some answers below.
> 
> 2016-07-24 15:51 GMT+02:00 Jonathan Cameron :
>> On 20/07/16 10:22, Enric Balletbo i Serra wrote:
>>> Add the core functions to be able to support the sensors attached behind
>>> the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
>>> drivers.
>>>
>>> The cros_ec_sensor_core driver matches with current driver in ChromeOS
>>> 4.4 tree, so it includes all the fixes at the moment. The support for
>>> this driver was made by Gwendal Grignou. The original patch and all the
>>> fixes has been squashed and rebased on top of mainline.
>>>
>>> Signed-off-by: Gwendal Grignou 
>>> Signed-off-by: Guenter Roeck 
>>> [eballetbo: split, squash and rebase on top of mainline the patches
>>> found in ChromeOS tree]
>>> Signed-off-by: Enric Balletbo i Serra 
>> Few minor bits inline.  I'm happy enough with the ABI (except that I would
>> like a justification for why userspace needs the id one)
>> We should see if anyone else has comments on that.
>>
>> The location one may well become more general in future (to cover other
>> devices).
>>
>> Jonathan
>>> ---
>>>
>>> Changes since v1:
>>>   - Check kernel-doc documentation.
>>>   - Bring this in as an when you need it in the rest of the series.
>>>   - Fix some spelling mistakes.
>>>   - Include ABI documentation.
>>>   - Be more careful with buffer sizes (sprintf -> snprintf)
>>>   - Add cros_ec_sensors prefix to all function.
>>>   - Check return values on some functions.
>>>
>>>  Documentation/ABI/testing/sysfs-bus-iio-cros-ec|  25 +
>>>  drivers/iio/common/Kconfig |   1 +
>>>  drivers/iio/common/Makefile|   1 +
>>>  drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
>>>  drivers/iio/common/cros_ec_sensors/Makefile|   5 +
>>>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 505 
>>> +
>>>  .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 177 
>>>  include/linux/mfd/cros_ec.h|   9 +
>>>  include/linux/mfd/cros_ec_commands.h   |  99 +++-
>>>  9 files changed, 831 insertions(+), 5 deletions(-)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
>>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
>>>  create mode 100644 
>>> drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>>>  create mode 100644 
>>> drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec 
>>> b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>>> new file mode 100644
>>> index 000..3e626b6
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>>> @@ -0,0 +1,25 @@
>>> +What:/sys/bus/iio/devices/iio:deviceX/calibrate
>>> +Date:July 2015
>>> +KernelVersion:   4.7
>>> +Contact: linux-...@vger.kernel.org
>>> +Description:
>>> + Writing '1' will perform a FOC (Fast Online Calibration). The
>>> +corresponding calibration offsets can be read from 
>>> *_calibbias
>>> +entries.
>> This one is fine as far as I'm concerned.
>>> +
>>> +What:/sys/bus/iio/devices/iio:deviceX/id
>>> +Date:July 2015
>>> +KernelVersion:   4.7
>>> +Contact: linux-...@vger.kernel.org
>>> +Description:
>>> +The id attribute holds an unique number for the motion 
>>> sensor
>>> +identification, as reported by the ChromeOS Embedded 
>>> Controller.
>> Mostly with these, we've just spat the out to the kernel logs as dev_info
>> during startup.
>>
>> Any reason this needs to be more explictly exposed to userspace?
> 
> After think a bit more about this, and to be honest, I'm not sure if
> this is really useful to be exposed to userspace. The only use case
> that I'm thinking is that some userspace app uses it to uniquely
> identify the sensors that hang from the ChromeOS Embedded Controller,
> but I'm not sure about this exist. I'll investigate a bit more for if
> I find a good justification, otherwise I'll remove this.
> 
> Maybe someone from chromium can give us more opinions?
> 
>>> +
>>> +What:/sys/bus/iio/devices/iio:deviceX/location
>>> +Date:July 2015
>>> +KernelVersion:   4.7
>>> +Contact: linux-...@vger.kernel.org
>>> +Description:
>>> + This attribute returns a string with the physical location 
>>> where
>>> +the motion sensor is placed. For example, in a laptop a 
>>> motion
>>> +sensor can be located on the base or on the lid.
>> Interesting...
>>
>> We'll probably want to ultimately use this one more generally.  This probably
>> won't be the last sensor where we have some means of finding this out!
>>
> Ok, I will 

Re: [PATCH v2 1/4] iio: cros_ec_sensors_core: Add common functions for the ChromeOS EC Sensor Hub.

2016-07-25 Thread Enric Balletbo Serra
Hi Jonathan,

Many thanks for the review, some answers below.

2016-07-24 15:51 GMT+02:00 Jonathan Cameron :
> On 20/07/16 10:22, Enric Balletbo i Serra wrote:
>> Add the core functions to be able to support the sensors attached behind
>> the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
>> drivers.
>>
>> The cros_ec_sensor_core driver matches with current driver in ChromeOS
>> 4.4 tree, so it includes all the fixes at the moment. The support for
>> this driver was made by Gwendal Grignou. The original patch and all the
>> fixes has been squashed and rebased on top of mainline.
>>
>> Signed-off-by: Gwendal Grignou 
>> Signed-off-by: Guenter Roeck 
>> [eballetbo: split, squash and rebase on top of mainline the patches
>> found in ChromeOS tree]
>> Signed-off-by: Enric Balletbo i Serra 
> Few minor bits inline.  I'm happy enough with the ABI (except that I would
> like a justification for why userspace needs the id one)
> We should see if anyone else has comments on that.
>
> The location one may well become more general in future (to cover other
> devices).
>
> Jonathan
>> ---
>>
>> Changes since v1:
>>   - Check kernel-doc documentation.
>>   - Bring this in as an when you need it in the rest of the series.
>>   - Fix some spelling mistakes.
>>   - Include ABI documentation.
>>   - Be more careful with buffer sizes (sprintf -> snprintf)
>>   - Add cros_ec_sensors prefix to all function.
>>   - Check return values on some functions.
>>
>>  Documentation/ABI/testing/sysfs-bus-iio-cros-ec|  25 +
>>  drivers/iio/common/Kconfig |   1 +
>>  drivers/iio/common/Makefile|   1 +
>>  drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
>>  drivers/iio/common/cros_ec_sensors/Makefile|   5 +
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 505 
>> +
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 177 
>>  include/linux/mfd/cros_ec.h|   9 +
>>  include/linux/mfd/cros_ec_commands.h   |  99 +++-
>>  9 files changed, 831 insertions(+), 5 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec 
>> b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>> new file mode 100644
>> index 000..3e626b6
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>> @@ -0,0 +1,25 @@
>> +What:/sys/bus/iio/devices/iio:deviceX/calibrate
>> +Date:July 2015
>> +KernelVersion:   4.7
>> +Contact: linux-...@vger.kernel.org
>> +Description:
>> + Writing '1' will perform a FOC (Fast Online Calibration). The
>> +corresponding calibration offsets can be read from 
>> *_calibbias
>> +entries.
> This one is fine as far as I'm concerned.
>> +
>> +What:/sys/bus/iio/devices/iio:deviceX/id
>> +Date:July 2015
>> +KernelVersion:   4.7
>> +Contact: linux-...@vger.kernel.org
>> +Description:
>> +The id attribute holds an unique number for the motion 
>> sensor
>> +identification, as reported by the ChromeOS Embedded 
>> Controller.
> Mostly with these, we've just spat the out to the kernel logs as dev_info
> during startup.
>
> Any reason this needs to be more explictly exposed to userspace?

After think a bit more about this, and to be honest, I'm not sure if
this is really useful to be exposed to userspace. The only use case
that I'm thinking is that some userspace app uses it to uniquely
identify the sensors that hang from the ChromeOS Embedded Controller,
but I'm not sure about this exist. I'll investigate a bit more for if
I find a good justification, otherwise I'll remove this.

Maybe someone from chromium can give us more opinions?

>> +
>> +What:/sys/bus/iio/devices/iio:deviceX/location
>> +Date:July 2015
>> +KernelVersion:   4.7
>> +Contact: linux-...@vger.kernel.org
>> +Description:
>> + This attribute returns a string with the physical location 
>> where
>> +the motion sensor is placed. For example, in a laptop a 
>> motion
>> +sensor can be located on the base or on the lid.
> Interesting...
>
> We'll probably want to ultimately use this one more generally.  This probably
> won't be the last sensor where we have some means of finding this out!
>
Ok, I will rewrite the sentence to be more generic

> Please include a list of possible values in the 

Re: [PATCH v2 1/4] iio: cros_ec_sensors_core: Add common functions for the ChromeOS EC Sensor Hub.

2016-07-25 Thread Enric Balletbo Serra
Hi Jonathan,

Many thanks for the review, some answers below.

2016-07-24 15:51 GMT+02:00 Jonathan Cameron :
> On 20/07/16 10:22, Enric Balletbo i Serra wrote:
>> Add the core functions to be able to support the sensors attached behind
>> the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
>> drivers.
>>
>> The cros_ec_sensor_core driver matches with current driver in ChromeOS
>> 4.4 tree, so it includes all the fixes at the moment. The support for
>> this driver was made by Gwendal Grignou. The original patch and all the
>> fixes has been squashed and rebased on top of mainline.
>>
>> Signed-off-by: Gwendal Grignou 
>> Signed-off-by: Guenter Roeck 
>> [eballetbo: split, squash and rebase on top of mainline the patches
>> found in ChromeOS tree]
>> Signed-off-by: Enric Balletbo i Serra 
> Few minor bits inline.  I'm happy enough with the ABI (except that I would
> like a justification for why userspace needs the id one)
> We should see if anyone else has comments on that.
>
> The location one may well become more general in future (to cover other
> devices).
>
> Jonathan
>> ---
>>
>> Changes since v1:
>>   - Check kernel-doc documentation.
>>   - Bring this in as an when you need it in the rest of the series.
>>   - Fix some spelling mistakes.
>>   - Include ABI documentation.
>>   - Be more careful with buffer sizes (sprintf -> snprintf)
>>   - Add cros_ec_sensors prefix to all function.
>>   - Check return values on some functions.
>>
>>  Documentation/ABI/testing/sysfs-bus-iio-cros-ec|  25 +
>>  drivers/iio/common/Kconfig |   1 +
>>  drivers/iio/common/Makefile|   1 +
>>  drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
>>  drivers/iio/common/cros_ec_sensors/Makefile|   5 +
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 505 
>> +
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 177 
>>  include/linux/mfd/cros_ec.h|   9 +
>>  include/linux/mfd/cros_ec_commands.h   |  99 +++-
>>  9 files changed, 831 insertions(+), 5 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec 
>> b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>> new file mode 100644
>> index 000..3e626b6
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>> @@ -0,0 +1,25 @@
>> +What:/sys/bus/iio/devices/iio:deviceX/calibrate
>> +Date:July 2015
>> +KernelVersion:   4.7
>> +Contact: linux-...@vger.kernel.org
>> +Description:
>> + Writing '1' will perform a FOC (Fast Online Calibration). The
>> +corresponding calibration offsets can be read from 
>> *_calibbias
>> +entries.
> This one is fine as far as I'm concerned.
>> +
>> +What:/sys/bus/iio/devices/iio:deviceX/id
>> +Date:July 2015
>> +KernelVersion:   4.7
>> +Contact: linux-...@vger.kernel.org
>> +Description:
>> +The id attribute holds an unique number for the motion 
>> sensor
>> +identification, as reported by the ChromeOS Embedded 
>> Controller.
> Mostly with these, we've just spat the out to the kernel logs as dev_info
> during startup.
>
> Any reason this needs to be more explictly exposed to userspace?

After think a bit more about this, and to be honest, I'm not sure if
this is really useful to be exposed to userspace. The only use case
that I'm thinking is that some userspace app uses it to uniquely
identify the sensors that hang from the ChromeOS Embedded Controller,
but I'm not sure about this exist. I'll investigate a bit more for if
I find a good justification, otherwise I'll remove this.

Maybe someone from chromium can give us more opinions?

>> +
>> +What:/sys/bus/iio/devices/iio:deviceX/location
>> +Date:July 2015
>> +KernelVersion:   4.7
>> +Contact: linux-...@vger.kernel.org
>> +Description:
>> + This attribute returns a string with the physical location 
>> where
>> +the motion sensor is placed. For example, in a laptop a 
>> motion
>> +sensor can be located on the base or on the lid.
> Interesting...
>
> We'll probably want to ultimately use this one more generally.  This probably
> won't be the last sensor where we have some means of finding this out!
>
Ok, I will rewrite the sentence to be more generic

> Please include a list of possible values in the description.

For now lid and base are the values used, is that ok?

>> \ No newline at end of file
> 

Re: [PATCH v2 1/4] iio: cros_ec_sensors_core: Add common functions for the ChromeOS EC Sensor Hub.

2016-07-24 Thread Jonathan Cameron
On 20/07/16 10:22, Enric Balletbo i Serra wrote:
> Add the core functions to be able to support the sensors attached behind
> the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
> drivers.
> 
> The cros_ec_sensor_core driver matches with current driver in ChromeOS
> 4.4 tree, so it includes all the fixes at the moment. The support for
> this driver was made by Gwendal Grignou. The original patch and all the
> fixes has been squashed and rebased on top of mainline.
> 
> Signed-off-by: Gwendal Grignou 
> Signed-off-by: Guenter Roeck 
> [eballetbo: split, squash and rebase on top of mainline the patches
> found in ChromeOS tree]
> Signed-off-by: Enric Balletbo i Serra 
Few minor bits inline.  I'm happy enough with the ABI (except that I would
like a justification for why userspace needs the id one)
We should see if anyone else has comments on that.

The location one may well become more general in future (to cover other
devices).

Jonathan
> ---
> 
> Changes since v1:
>   - Check kernel-doc documentation.
>   - Bring this in as an when you need it in the rest of the series.
>   - Fix some spelling mistakes.
>   - Include ABI documentation.
>   - Be more careful with buffer sizes (sprintf -> snprintf)
>   - Add cros_ec_sensors prefix to all function.
>   - Check return values on some functions.
> 
>  Documentation/ABI/testing/sysfs-bus-iio-cros-ec|  25 +
>  drivers/iio/common/Kconfig |   1 +
>  drivers/iio/common/Makefile|   1 +
>  drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   5 +
>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 505 
> +
>  .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 177 
>  include/linux/mfd/cros_ec.h|   9 +
>  include/linux/mfd/cros_ec_commands.h   |  99 +++-
>  9 files changed, 831 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>  create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
>  create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec 
> b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
> new file mode 100644
> index 000..3e626b6
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
> @@ -0,0 +1,25 @@
> +What:/sys/bus/iio/devices/iio:deviceX/calibrate
> +Date:July 2015
> +KernelVersion:   4.7
> +Contact: linux-...@vger.kernel.org
> +Description:
> + Writing '1' will perform a FOC (Fast Online Calibration). The
> +corresponding calibration offsets can be read from 
> *_calibbias
> +entries.
This one is fine as far as I'm concerned.
> +
> +What:/sys/bus/iio/devices/iio:deviceX/id
> +Date:July 2015
> +KernelVersion:   4.7
> +Contact: linux-...@vger.kernel.org
> +Description:
> +The id attribute holds an unique number for the motion sensor
> +identification, as reported by the ChromeOS Embedded 
> Controller.
Mostly with these, we've just spat the out to the kernel logs as dev_info
during startup.

Any reason this needs to be more explictly exposed to userspace?
> +
> +What:/sys/bus/iio/devices/iio:deviceX/location
> +Date:July 2015
> +KernelVersion:   4.7
> +Contact: linux-...@vger.kernel.org
> +Description:
> + This attribute returns a string with the physical location where
> +the motion sensor is placed. For example, in a laptop a 
> motion
> +sensor can be located on the base or on the lid.
Interesting...

We'll probably want to ultimately use this one more generally.  This probably
won't be the last sensor where we have some means of finding this out!

Please include a list of possible values in the description.
> \ No newline at end of file
Fix that ;)
> diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> index 26a6026..e108996 100644
> --- a/drivers/iio/common/Kconfig
> +++ b/drivers/iio/common/Kconfig
> @@ -2,6 +2,7 @@
>  # IIO common modules
>  #
>  
> +source "drivers/iio/common/cros_ec_sensors/Kconfig"
>  source "drivers/iio/common/hid-sensors/Kconfig"
>  source "drivers/iio/common/ms_sensors/Kconfig"
>  source "drivers/iio/common/ssp_sensors/Kconfig"
> diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
> index 585da6a..6fa760e 100644
> --- a/drivers/iio/common/Makefile
> +++ b/drivers/iio/common/Makefile
> @@ -7,6 +7,7 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> 

Re: [PATCH v2 1/4] iio: cros_ec_sensors_core: Add common functions for the ChromeOS EC Sensor Hub.

2016-07-24 Thread Jonathan Cameron
On 20/07/16 10:22, Enric Balletbo i Serra wrote:
> Add the core functions to be able to support the sensors attached behind
> the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
> drivers.
> 
> The cros_ec_sensor_core driver matches with current driver in ChromeOS
> 4.4 tree, so it includes all the fixes at the moment. The support for
> this driver was made by Gwendal Grignou. The original patch and all the
> fixes has been squashed and rebased on top of mainline.
> 
> Signed-off-by: Gwendal Grignou 
> Signed-off-by: Guenter Roeck 
> [eballetbo: split, squash and rebase on top of mainline the patches
> found in ChromeOS tree]
> Signed-off-by: Enric Balletbo i Serra 
Few minor bits inline.  I'm happy enough with the ABI (except that I would
like a justification for why userspace needs the id one)
We should see if anyone else has comments on that.

The location one may well become more general in future (to cover other
devices).

Jonathan
> ---
> 
> Changes since v1:
>   - Check kernel-doc documentation.
>   - Bring this in as an when you need it in the rest of the series.
>   - Fix some spelling mistakes.
>   - Include ABI documentation.
>   - Be more careful with buffer sizes (sprintf -> snprintf)
>   - Add cros_ec_sensors prefix to all function.
>   - Check return values on some functions.
> 
>  Documentation/ABI/testing/sysfs-bus-iio-cros-ec|  25 +
>  drivers/iio/common/Kconfig |   1 +
>  drivers/iio/common/Makefile|   1 +
>  drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
>  drivers/iio/common/cros_ec_sensors/Makefile|   5 +
>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 505 
> +
>  .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 177 
>  include/linux/mfd/cros_ec.h|   9 +
>  include/linux/mfd/cros_ec_commands.h   |  99 +++-
>  9 files changed, 831 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cros-ec
>  create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
>  create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>  create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec 
> b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
> new file mode 100644
> index 000..3e626b6
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
> @@ -0,0 +1,25 @@
> +What:/sys/bus/iio/devices/iio:deviceX/calibrate
> +Date:July 2015
> +KernelVersion:   4.7
> +Contact: linux-...@vger.kernel.org
> +Description:
> + Writing '1' will perform a FOC (Fast Online Calibration). The
> +corresponding calibration offsets can be read from 
> *_calibbias
> +entries.
This one is fine as far as I'm concerned.
> +
> +What:/sys/bus/iio/devices/iio:deviceX/id
> +Date:July 2015
> +KernelVersion:   4.7
> +Contact: linux-...@vger.kernel.org
> +Description:
> +The id attribute holds an unique number for the motion sensor
> +identification, as reported by the ChromeOS Embedded 
> Controller.
Mostly with these, we've just spat the out to the kernel logs as dev_info
during startup.

Any reason this needs to be more explictly exposed to userspace?
> +
> +What:/sys/bus/iio/devices/iio:deviceX/location
> +Date:July 2015
> +KernelVersion:   4.7
> +Contact: linux-...@vger.kernel.org
> +Description:
> + This attribute returns a string with the physical location where
> +the motion sensor is placed. For example, in a laptop a 
> motion
> +sensor can be located on the base or on the lid.
Interesting...

We'll probably want to ultimately use this one more generally.  This probably
won't be the last sensor where we have some means of finding this out!

Please include a list of possible values in the description.
> \ No newline at end of file
Fix that ;)
> diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> index 26a6026..e108996 100644
> --- a/drivers/iio/common/Kconfig
> +++ b/drivers/iio/common/Kconfig
> @@ -2,6 +2,7 @@
>  # IIO common modules
>  #
>  
> +source "drivers/iio/common/cros_ec_sensors/Kconfig"
>  source "drivers/iio/common/hid-sensors/Kconfig"
>  source "drivers/iio/common/ms_sensors/Kconfig"
>  source "drivers/iio/common/ssp_sensors/Kconfig"
> diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
> index 585da6a..6fa760e 100644
> --- a/drivers/iio/common/Makefile
> +++ b/drivers/iio/common/Makefile
> @@ -7,6 +7,7 @@
>  #
>  
>  # When adding new entries keep the list in alphabetical order
> +obj-y += cros_ec_sensors/
>  obj-y += hid-sensors/
>  obj-y += ms_sensors/
>  

[PATCH v2 1/4] iio: cros_ec_sensors_core: Add common functions for the ChromeOS EC Sensor Hub.

2016-07-20 Thread Enric Balletbo i Serra
Add the core functions to be able to support the sensors attached behind
the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
drivers.

The cros_ec_sensor_core driver matches with current driver in ChromeOS
4.4 tree, so it includes all the fixes at the moment. The support for
this driver was made by Gwendal Grignou. The original patch and all the
fixes has been squashed and rebased on top of mainline.

Signed-off-by: Gwendal Grignou 
Signed-off-by: Guenter Roeck 
[eballetbo: split, squash and rebase on top of mainline the patches
found in ChromeOS tree]
Signed-off-by: Enric Balletbo i Serra 
---

Changes since v1:
  - Check kernel-doc documentation.
  - Bring this in as an when you need it in the rest of the series.
  - Fix some spelling mistakes.
  - Include ABI documentation.
  - Be more careful with buffer sizes (sprintf -> snprintf)
  - Add cros_ec_sensors prefix to all function.
  - Check return values on some functions.

 Documentation/ABI/testing/sysfs-bus-iio-cros-ec|  25 +
 drivers/iio/common/Kconfig |   1 +
 drivers/iio/common/Makefile|   1 +
 drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
 drivers/iio/common/cros_ec_sensors/Makefile|   5 +
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 505 +
 .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 177 
 include/linux/mfd/cros_ec.h|   9 +
 include/linux/mfd/cros_ec_commands.h   |  99 +++-
 9 files changed, 831 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cros-ec
 create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
 create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
 create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
 create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec 
b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
new file mode 100644
index 000..3e626b6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
@@ -0,0 +1,25 @@
+What:  /sys/bus/iio/devices/iio:deviceX/calibrate
+Date:  July 2015
+KernelVersion: 4.7
+Contact:   linux-...@vger.kernel.org
+Description:
+   Writing '1' will perform a FOC (Fast Online Calibration). The
+corresponding calibration offsets can be read from *_calibbias
+entries.
+
+What:  /sys/bus/iio/devices/iio:deviceX/id
+Date:  July 2015
+KernelVersion: 4.7
+Contact:   linux-...@vger.kernel.org
+Description:
+The id attribute holds an unique number for the motion sensor
+identification, as reported by the ChromeOS Embedded 
Controller.
+
+What:  /sys/bus/iio/devices/iio:deviceX/location
+Date:  July 2015
+KernelVersion: 4.7
+Contact:   linux-...@vger.kernel.org
+Description:
+   This attribute returns a string with the physical location where
+the motion sensor is placed. For example, in a laptop a motion
+sensor can be located on the base or on the lid.
\ No newline at end of file
diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
index 26a6026..e108996 100644
--- a/drivers/iio/common/Kconfig
+++ b/drivers/iio/common/Kconfig
@@ -2,6 +2,7 @@
 # IIO common modules
 #
 
+source "drivers/iio/common/cros_ec_sensors/Kconfig"
 source "drivers/iio/common/hid-sensors/Kconfig"
 source "drivers/iio/common/ms_sensors/Kconfig"
 source "drivers/iio/common/ssp_sensors/Kconfig"
diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
index 585da6a..6fa760e 100644
--- a/drivers/iio/common/Makefile
+++ b/drivers/iio/common/Makefile
@@ -7,6 +7,7 @@
 #
 
 # When adding new entries keep the list in alphabetical order
+obj-y += cros_ec_sensors/
 obj-y += hid-sensors/
 obj-y += ms_sensors/
 obj-y += ssp_sensors/
diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
b/drivers/iio/common/cros_ec_sensors/Kconfig
new file mode 100644
index 000..24743be
--- /dev/null
+++ b/drivers/iio/common/cros_ec_sensors/Kconfig
@@ -0,0 +1,14 @@
+#
+# Chrome OS Embedded Controller managed sensors library
+#
+config IIO_CROS_EC_SENSORS_CORE
+   tristate "ChromeOS EC Sensors Core"
+   depends on SYSFS && MFD_CROS_EC
+   select IIO_BUFFER
+   select IIO_TRIGGERED_BUFFER
+   help
+ Base module for the ChromeOS EC Sensors module.
+ Contains core functions used by other IIO CrosEC sensor
+ drivers.
+ Define common attributes and sysfs interrupt handler.
+
diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
b/drivers/iio/common/cros_ec_sensors/Makefile
new file mode 100644
index 000..95b6901
--- /dev/null
+++ b/drivers/iio/common/cros_ec_sensors/Makefile
@@ -0,0 +1,5 @@

[PATCH v2 1/4] iio: cros_ec_sensors_core: Add common functions for the ChromeOS EC Sensor Hub.

2016-07-20 Thread Enric Balletbo i Serra
Add the core functions to be able to support the sensors attached behind
the ChromeOS Embedded Controller and used by other IIO cros-ec sensor
drivers.

The cros_ec_sensor_core driver matches with current driver in ChromeOS
4.4 tree, so it includes all the fixes at the moment. The support for
this driver was made by Gwendal Grignou. The original patch and all the
fixes has been squashed and rebased on top of mainline.

Signed-off-by: Gwendal Grignou 
Signed-off-by: Guenter Roeck 
[eballetbo: split, squash and rebase on top of mainline the patches
found in ChromeOS tree]
Signed-off-by: Enric Balletbo i Serra 
---

Changes since v1:
  - Check kernel-doc documentation.
  - Bring this in as an when you need it in the rest of the series.
  - Fix some spelling mistakes.
  - Include ABI documentation.
  - Be more careful with buffer sizes (sprintf -> snprintf)
  - Add cros_ec_sensors prefix to all function.
  - Check return values on some functions.

 Documentation/ABI/testing/sysfs-bus-iio-cros-ec|  25 +
 drivers/iio/common/Kconfig |   1 +
 drivers/iio/common/Makefile|   1 +
 drivers/iio/common/cros_ec_sensors/Kconfig |  14 +
 drivers/iio/common/cros_ec_sensors/Makefile|   5 +
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  | 505 +
 .../common/cros_ec_sensors/cros_ec_sensors_core.h  | 177 
 include/linux/mfd/cros_ec.h|   9 +
 include/linux/mfd/cros_ec_commands.h   |  99 +++-
 9 files changed, 831 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cros-ec
 create mode 100644 drivers/iio/common/cros_ec_sensors/Kconfig
 create mode 100644 drivers/iio/common/cros_ec_sensors/Makefile
 create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
 create mode 100644 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.h

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-cros-ec 
b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
new file mode 100644
index 000..3e626b6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-cros-ec
@@ -0,0 +1,25 @@
+What:  /sys/bus/iio/devices/iio:deviceX/calibrate
+Date:  July 2015
+KernelVersion: 4.7
+Contact:   linux-...@vger.kernel.org
+Description:
+   Writing '1' will perform a FOC (Fast Online Calibration). The
+corresponding calibration offsets can be read from *_calibbias
+entries.
+
+What:  /sys/bus/iio/devices/iio:deviceX/id
+Date:  July 2015
+KernelVersion: 4.7
+Contact:   linux-...@vger.kernel.org
+Description:
+The id attribute holds an unique number for the motion sensor
+identification, as reported by the ChromeOS Embedded 
Controller.
+
+What:  /sys/bus/iio/devices/iio:deviceX/location
+Date:  July 2015
+KernelVersion: 4.7
+Contact:   linux-...@vger.kernel.org
+Description:
+   This attribute returns a string with the physical location where
+the motion sensor is placed. For example, in a laptop a motion
+sensor can be located on the base or on the lid.
\ No newline at end of file
diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
index 26a6026..e108996 100644
--- a/drivers/iio/common/Kconfig
+++ b/drivers/iio/common/Kconfig
@@ -2,6 +2,7 @@
 # IIO common modules
 #
 
+source "drivers/iio/common/cros_ec_sensors/Kconfig"
 source "drivers/iio/common/hid-sensors/Kconfig"
 source "drivers/iio/common/ms_sensors/Kconfig"
 source "drivers/iio/common/ssp_sensors/Kconfig"
diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
index 585da6a..6fa760e 100644
--- a/drivers/iio/common/Makefile
+++ b/drivers/iio/common/Makefile
@@ -7,6 +7,7 @@
 #
 
 # When adding new entries keep the list in alphabetical order
+obj-y += cros_ec_sensors/
 obj-y += hid-sensors/
 obj-y += ms_sensors/
 obj-y += ssp_sensors/
diff --git a/drivers/iio/common/cros_ec_sensors/Kconfig 
b/drivers/iio/common/cros_ec_sensors/Kconfig
new file mode 100644
index 000..24743be
--- /dev/null
+++ b/drivers/iio/common/cros_ec_sensors/Kconfig
@@ -0,0 +1,14 @@
+#
+# Chrome OS Embedded Controller managed sensors library
+#
+config IIO_CROS_EC_SENSORS_CORE
+   tristate "ChromeOS EC Sensors Core"
+   depends on SYSFS && MFD_CROS_EC
+   select IIO_BUFFER
+   select IIO_TRIGGERED_BUFFER
+   help
+ Base module for the ChromeOS EC Sensors module.
+ Contains core functions used by other IIO CrosEC sensor
+ drivers.
+ Define common attributes and sysfs interrupt handler.
+
diff --git a/drivers/iio/common/cros_ec_sensors/Makefile 
b/drivers/iio/common/cros_ec_sensors/Makefile
new file mode 100644
index 000..95b6901
--- /dev/null
+++ b/drivers/iio/common/cros_ec_sensors/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for sensors seen through the ChromeOS EC sensor hub.
+#
+