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/
