On 12.08.17 13:56, Pavel Machek wrote: > Hi! > >>> On Sat 2017-08-12 12:09:35, Oleh Kravchenko wrote: >>>> This patch adds a LED class driver for the RGB LEDs found on >>>> the Crane Merchandising System CR0014114 LEDs board. >>> >>> What kind of hardware is this? >> >> It's from vending machines produced by Crane Merchandising System http://cranems.com/ >> >>>> Driver creates LED devices with name written using the following >>>> pattern "LABEL-{N}:{red,green,blue}:". >>> >>> Is the "-{N} suffix needed? >> >> It's number of RGB LED, board has 6 LEDs. > > Normally label differentiates them...? For example, possible names when two boards connected is: /sys/class/leds/board0-0:blue: /sys/class/leds/board0-0:green: /sys/class/leds/board0-0:red: /sys/class/leds/board0-1:blue: /sys/class/leds/board0-1:green: /sys/class/leds/board0-1:red: /sys/class/leds/board0-2:blue: /sys/class/leds/board0-2:green: /sys/class/leds/board0-2:red: /sys/class/leds/board0-3:blue: /sys/class/leds/board0-3:green: /sys/class/leds/board0-3:red: /sys/class/leds/board0-4:blue: /sys/class/leds/board0-4:green: /sys/class/leds/board0-4:red: /sys/class/leds/board0-5:blue: /sys/class/leds/board0-5:green: /sys/class/leds/board0-5:red: /sys/class/leds/board1-0:blue: /sys/class/leds/board1-0:green: /sys/class/leds/board1-0:red: /sys/class/leds/board1-1:blue: /sys/class/leds/board1-1:green: /sys/class/leds/board1-1:red: /sys/class/leds/board1-2:blue: /sys/class/leds/board1-2:green: /sys/class/leds/board1-2:red: /sys/class/leds/board1-3:blue: /sys/class/leds/board1-3:green: /sys/class/leds/board1-3:red: /sys/class/leds/board1-4:blue: /sys/class/leds/board1-4:green: /sys/class/leds/board1-4:red: /sys/class/leds/board1-5:blue: /sys/class/leds/board1-5:green: /sys/class/leds/board1-5:red: >>> How special hardware is this? Does it make sense to ask this question >>> for x86 users, for example? >> >> I think it's possible to connect it to x86 hardware, but I don't try it. >> Board need not only SPI connection, but good power supply. > > Aha, ok. > >>>> +/* CR0014114 SPI commands */ >>>> +#define CR0014114_SET_BRIGHTNESS 0x80 >>>> +#define CR0014114_INIT_REENUMERATE 0x81 >>>> +#define CR0014114_NEXT_REENUMERATE 0x82 >>> >>> can we s/cr0014114/cr00/g, or something? There are local to the >>> module, so they can be shorter... >> >> Sure, may be CMD_SET_BRIGHTNESS > > Please do the replace for all static symbols.> >> +fail: Of course (CMD_, DEF_, ..) >>>> + while (i--) >>>> + led_classdev_unregister(&priv->leds[i].ldev); >>> >>> Can devm_* be used to simplify this? >> >> I think no, because it will cause race condition. > > Please take a look at devm_led_classdev_register() and friends. It > should be possible to simplify code without races. I don't understand how I can call destroy_workqueue() after calling unregister leds. > Thanks, > Pavel > -- Best regards, Oleh Kravchenko
Attachment:
signature.asc
Description: OpenPGP digital signature
