Re: [PATCH v3 1/2] HID: HID Sensor: Custom and Generic sensor support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 10 Apr 2015, Srinivas Pandruvada wrote:

> > > +	/* 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.

Ah, indeed, I've missed the fact that !CONFIG_DEBUG_LOCK_ALLOC will hide 
this issue.

> So this is a bug, I will resubmit this.

Thanks,

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux