On Fri, 2015-04-10 at 12:03 +0200, Jiri Kosina wrote: > On Fri, 20 Mar 2015, Srinivas Pandruvada wrote: > B > > HID Sensor Spec defines two usage ids for custom sensors > > HID_USAGE_SENSOR_TYPE_OTHER_CUSTOM (0x09, 0xE1) > > HID_USAGE_SENSOR_TYPE_OTHER_GENERIC(0x09, 0xE2) > > In addition the standard also defines usage ids for custom fields. > > The purpose of these sensors is to extend the functionality or provide a > > way to obfuscate the data being communicated by a sensor. Without knowing the > > mapping between the data and its encapsulated form, it is difficult for > > an driver to determine what data is being communicated by the sensor. > > This allows some differentiating use cases, where vendor can provide applications. > > Since these can't be represented by standard sensor interfaces like IIO, > > we present these as fields with > > - type (input/output) > > - units > > - min/max > > - get/set value > > In addition an dev interface to transfer report events. Details about this > > interface is described in /Documentation/hid/hid-sensor.txt. > > Manufacturers should not use these ids for any standard sensors, otherwise > > the the product/vendor id can be added to black list. > > > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> > [ ... snip ... ] > > +static int hid_sensor_custom_add_attributes(struct hid_sensor_custom > > + *sensor_inst) > > +{ > > + struct hid_sensor_hub_device *hsdev = sensor_inst->hsdev; > > + struct hid_device *hdev = hsdev->hdev; > > + int ret = -1; > > + int i, j; > > + > > + for (j = 0; j < HID_REPORT_TYPES; ++j) { > > + if (j == HID_OUTPUT_REPORT) > > + continue; > > + > > + ret = hid_sensor_custom_add_fields(sensor_inst, > > + &hdev->report_enum[j], j); > > + if (ret) > > + return ret; > > + > > + } > > + > > + /* Create sysfs attributes */ > > + for (i = 0; i < sensor_inst->sensor_field_count; ++i) { > > + j = 0; > > + while (j < HID_CUSTOM_TOTAL_ATTRS && > > + hid_custom_attrs[j].name) { > > + struct device_attribute *device_attr; > > + > > + device_attr = &sensor_inst->fields[i].sd_attrs[j]; > > + > > + snprintf((char *)&sensor_inst->fields[i].attr_name[j], > > + HID_CUSTOM_NAME_LENGTH, "%s-%s", > > + sensor_inst->fields[i].group_name, > > + hid_custom_attrs[j].name); > > + sysfs_attr_init(device_attr->dev_attr.attr); > > This is wrong -- struct device_attribute doesn't have 'dev_attr' field. > > The fix would be trivial, but given the fact that this means that your > patch hasn't been even compile-tested, I'd like to ask for proper > resubmission of code you've actually tested. You found a bug. But it doesn't mean that this is not tested (I submit 100+ patches each cycle related to power and thermal, so this is just a honest mistake). This is even tested by ST Micro developer. The issue is sysfs_attr_init() is ignored unless CONFIG_DEBUG_LOCK_ALLOC is defined. I use Fedora, Ubuntu and Android builds, none of which defined this symbol. So it slipped away. So this is a bug, I will resubmit this. Thanks, Srinivas > > Thanks, > > -- > Jiri Kosina > SUSE Labs -- 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