Re: [PATCH 09/12] hwmon: vexpress: Use devm helper for hwmon device registration

From: Arnd Bergmann
Date: Tue Feb 11 2014 - 15:59:39 EST


On Tuesday 11 February 2014, Pawel Moll wrote:
> -#define VEXPRESS_HWMON_ATTRS(_name, _label_attr, _input_attr) \
> -struct attribute *vexpress_hwmon_attrs_##_name[] = { \
> - &dev_attr_name.attr, \
> - &dev_attr_##_label_attr.attr, \
> - &sensor_dev_attr_##_input_attr.dev_attr.attr, \
> - NULL \
> -}
> +#define VEXPRESS_HWMON_ATTR_GROUPS(_name, _label_attr, _input_attr) \
> +static struct attribute *vexpress_hwmon_##_name##_attrs[] = { \
> + &dev_attr_##_label_attr.attr, \
> + &sensor_dev_attr_##_input_attr.dev_attr.attr, \
> + NULL \
> +}; \
> +ATTRIBUTE_GROUPS(vexpress_hwmon_##_name)
>
> #if !defined(CONFIG_REGULATOR_VEXPRESS)
> static DEVICE_ATTR(in1_label, S_IRUGO, vexpress_hwmon_label_show, NULL);
> static SENSOR_DEVICE_ATTR(in1_input, S_IRUGO, vexpress_hwmon_u32_show,
> NULL, 1000);
> -static VEXPRESS_HWMON_ATTRS(volt, in1_label, in1_input);
> -static struct attribute_group vexpress_hwmon_group_volt = {
> - .attrs = vexpress_hwmon_attrs_volt,
> -};
> +VEXPRESS_HWMON_ATTR_GROUPS(volt, in1_label, in1_input);

Maybe it would be a good time to get rid of the obfuscation here and just
open-code the attribute groups rather than defining another macro.

I understand you want to save a few source lines, but it really helps
readability if you can search for the attribute groups when they
are referenced later. Just because the common driver model code uses
macros with string concatenation doesn't mean it's a good idea to
do the same in a device driver.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/