Re: [lm-sensors] HWMON: S3C24XX series ADC driver

[Ben Dooks <ben@simtec.co.uk>]

Jean Delvare wrote:
> Hi Ben,
> 

[snip]

>> +	struct device		*hwmon_dev;
>> +
>> +	struct sensor_device_attribute *in_attr[8];
> 
> What is the benefit of allocating these attributes dynamically? You are
> likely to fragment your memory and require ugly code to make it work.

I suppose if we're only looking at producing a single in
or in and label attribute then it is only 24+name bytes
for each attribute.

[snip]

>> +static int s3c_hwmon_create_attr(struct s3c_hwmon *hwmon,
>> +				 struct s3c_hwmon_pdata *pdata,
>> +				 int channel)
>> +{
>> +	struct s3c_hwmon_chcfg *cfg = pdata->in[channel];
>> +	struct sensor_device_attribute *attr;
>> +	const char *name;
>> +
>> +	if (!cfg)
>> +		return 0;
>> +
>> +	attr = kzalloc(sizeof(*attr) + MAX_LABEL, GFP_KERNEL);
>> +	if (attr = NULL)
>> +		return -ENOMEM;
>> +
>> +	if (cfg->name)
>> +		name = cfg->name;
>> +	else {
>> +		name = (char *)(attr+1);
>> +		snprintf((char *)(attr+1), MAX_LABEL, "in_%d", channel);
> 
> This name doesn't match what is described in
> Documentation/hwmon/sysfs-interface, which means that your driver won't
> work with libsensors. This is a blocker.

Ah, having re-read it, it seems the name should go in in%d_label sysfs 
field as well as changing in_%d to in%d_input.

Ben Dooks, Software Engineer, Simtec Electronics

http://www.simtec.co.uk/

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors