[OpenWrt-Devel] [PATCH 1/4] mvebu: add Turris Omnia LED support

[Klaus Kudielka]

On 13.12.18 09:50, Bjørn Mork wrote:
> Klaus Kudielka <klaus.kudielka at gmail.com> writes:
>
>> ++		err = led_classdev_register(&client->dev, &leds[i].led_cdev);
>> ++		if (err < 0)
>> ++			goto exit;
>> ++
>> ++		err = device_create_file(leds[i].led_cdev.dev,
>> ++						&dev_attr_autonomous);
>> ++		if (err < 0) {
>> ++			dev_err(leds[i].led_cdev.dev,
>> ++				"failed to create attribute autonomous\n");
>> ++			goto exit;
>> ++		}
>> ++
>> ++		err = device_create_file(leds[i].led_cdev.dev,
>> ++						&dev_attr_color);
>> ++		if (err < 0) {
>> ++			dev_err(leds[i].led_cdev.dev,
>> ++				"failed to create attribute color\n");
>> ++			goto exit;
>> ++		}
>> +
> I believe GregKH often complains about patterns like this because of the
> userspace race it creates.  Why not add the extra attributes to a group
> and point leds[i].led_cdev.groups to it instead?  Then you get automatic
> attribute creation in led_classdev_register. No race and much simpler
> error handling and cleanup.

I have implemented your suggestion [1], and it seems to work in the 
normal use case. I guess I'll wait some more time for other comments, 
before sending patch version 2.

[1] https://github.com/kkudielka/openwrt/commits/omnia-leds-dev

Klaus

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel