Re: [PATCH] leds: add LED driver for CR0014114 board

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

On Sat 2017-08-12 14:34:02, Oleh Kravchenko wrote:
> 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:

Hmm. Well, this works if you can't provide better names. Still "board"
is somehow generic, andsomeone else might want to use it. "board" ->
"crms"?

> >>>> +	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.

Do you actually need the workqueues? It should be possible to avoid
them, using workqueue support in the core.
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]