On 07/01/15 18:24, Srinivas Pandruvada wrote:
> HID sensor hub exports several sensor which are fusion sensors, where
> data is interpreted from one or more sensors. Some of them can't be
> exported via IIO like sysfs as the user space download some firmware,
> which defines what sensor will look like. They can be a part of a
> collection.
This functionality is beginning to turn up on a number of devices.
I'd argue that where at all possible, we should try for standard
interfaces on the resulting 'fused sensors'.  Still you do indeed
always need a means of uploading the firmware!
> Creating a MFD cell for a collection to write a standalone driver
> to manage collections. Most of the time they will be propritery drivers
> may not be even upstreamed. This patch allows framework to have
> capability to write such drivers.
> 
> Signed-off-by: Srinivas Pandruvada <[email protected]>
>

A few comments inline...

How does userspace download a firmware?

 
 ---
>  drivers/hid/hid-sensor-hub.c   | 50 
> ++++++++++++++++++++++++++++++++++++++----
>  include/linux/hid-sensor-ids.h |  2 ++
>  2 files changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 865cd56..83b6e15 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -119,11 +119,12 @@ static struct hid_sensor_hub_callbacks 
> *sensor_hub_get_callback(
>  
>       spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
>       list_for_each_entry(callback, &pdata->dyn_callback_list, list)
> -             if (callback->usage_id == usage_id &&
> +             if (callback->usage_id == HID_USAGE_SENSOR_TYPE_COLLECTION ||
> +                     (callback->usage_id == usage_id &&
>                       (collection_index >=
>                               callback->hsdev->start_collection_index) &&
>                       (collection_index <
> -                             callback->hsdev->end_collection_index)) {
> +                             callback->hsdev->end_collection_index))) {
>                       *priv = callback->priv;
>                       *hsdev = callback->hsdev;
>                       spin_unlock_irqrestore(&pdata->dyn_callback_lock,
> @@ -159,7 +160,12 @@ int sensor_hub_register_callback(struct 
> hid_sensor_hub_device *hsdev,
>       callback->usage_callback = usage_callback;
>       callback->usage_id = usage_id;
>       callback->priv = NULL;
> -     list_add_tail(&callback->list, &pdata->dyn_callback_list);
> +     /* Give higher priority to collection device, so add to front */
Comment should probably state why we want to give them higher priorities.

> +     if (usage_id == HID_USAGE_SENSOR_TYPE_COLLECTION)
> +             list_add(&callback->list, &pdata->dyn_callback_list);
> +     else
> +             list_add_tail(&callback->list, &pdata->dyn_callback_list);
> +
>       spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
>  
>       return 0;
> @@ -547,6 +553,37 @@ static __u8 *sensor_hub_report_fixup(struct hid_device 
> *hdev, __u8 *rdesc,
>       return rdesc;
>  }
>  
> +static int sensor_hub_add_collection_device(struct hid_device *hdev,
> +                                         struct sensor_hub_data *sd)
> +{
> +     struct hid_sensor_hub_device *hsdev;
> +     char *name;
> +
> +     hsdev = devm_kzalloc(&hdev->dev, sizeof(*hsdev), GFP_KERNEL);
> +     if (!hsdev)
> +             return -ENOMEM;
> +
> +     hsdev->hdev = hdev;
> +     hsdev->vendor_id = hdev->vendor;
> +     hsdev->product_id = hdev->product;
> +     hsdev->usage = HID_USAGE_SENSOR_TYPE_COLLECTION;
> +     mutex_init(&hsdev->mutex);
> +     name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "HID-SENSOR-%x",
> +                           HID_USAGE_SENSOR_TYPE_COLLECTION);
> +     if (name == NULL) {
> +             hid_err(hdev, "Failed MFD device name\n");
> +             return -ENOMEM;
> +     }
> +     sd->hid_sensor_hub_client_devs[sd->hid_sensor_client_cnt].name = name;
> +     sd->hid_sensor_hub_client_devs[
> +                     sd->hid_sensor_client_cnt].platform_data = hsdev;
Odd breaking up of the line...  Perhaps break before .platform_data instead?
> +     sd->hid_sensor_hub_client_devs[
> +                     sd->hid_sensor_client_cnt].pdata_size = sizeof(*hsdev);
> +     sd->hid_sensor_client_cnt++;
> +
> +     return 0;
> +}
> +
>  static int sensor_hub_probe(struct hid_device *hdev,
>                               const struct hid_device_id *id)
>  {
> @@ -591,7 +628,8 @@ static int sensor_hub_probe(struct hid_device *hdev,
>               ret = -EINVAL;
>               goto err_stop_hw;
>       }
> -     sd->hid_sensor_hub_client_devs = devm_kzalloc(&hdev->dev, dev_cnt *
> +     sd->hid_sensor_hub_client_devs = devm_kzalloc(&hdev->dev,
> +                                                   (dev_cnt + 1) *
>                                                     sizeof(struct mfd_cell),
>                                                     GFP_KERNEL);
>       if (sd->hid_sensor_hub_client_devs == NULL) {
> @@ -645,6 +683,10 @@ static int sensor_hub_probe(struct hid_device *hdev,
>       if (last_hsdev)
>               last_hsdev->end_collection_index = i;
>  
> +     ret = sensor_hub_add_collection_device(hdev, sd);
> +     if (ret)
> +             goto err_stop_hw;
> +
>       ret = mfd_add_hotplug_devices(&hdev->dev,
>                       sd->hid_sensor_hub_client_devs,
>                       sd->hid_sensor_client_cnt);
> diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
> index 109f0e6..300ffea 100644
> --- a/include/linux/hid-sensor-ids.h
> +++ b/include/linux/hid-sensor-ids.h
> @@ -21,6 +21,8 @@
>  
>  #define HID_MAX_PHY_DEVICES                                  0xFF
>  
> +#define HID_USAGE_SENSOR_TYPE_COLLECTION                     0x200001
> +
>  /* Accel 3D (200073) */
>  #define HID_USAGE_SENSOR_ACCEL_3D                            0x200073
>  #define HID_USAGE_SENSOR_DATA_ACCELERATION                   0x200452
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to