On 19/02/15 23:54, Srinivas Pandruvada wrote:
> On Wed, 2015-01-21 at 21:53 +0000, Jonathan Cameron wrote:
>> 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!
> I will submitted RFCs, once I see in production releases.
Cool
>
> Currently planning to use misc driver for FW update.
> What do you think about using dev/iio:deviceX for such case to download
> FW for each sensor? We also need mechanism to initiate and reset after
> FW download.
I think using the main iio device is pushing things a little. We could add
the ability to get another anonymous cdev like we do for events I suppose.
That way, at least it will be tied to the normal iio device.
Sort of feels like there should be a generic / standard way of doing
this sort of firmware update, irrespective of what the device does.
I guess, maybe it's always non generic enough that this isn't
possible.
>
> Thanks,
> Srinivas
>
>>> 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 <srinivas.pandruvada@xxxxxxxxxxxxxxx>
>>>
>>
>> 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-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
