Messages in this thread | | | Date | Fri, 12 Sep 2014 08:45:03 +0200 | From | Jacek Anaszewski <> | Subject | Re: [PATCH/RFC v5 2/4] leds: implement sysfs interface locking mechanism |
| |
Hi Bryan,
Thanks for a review.
On 09/12/2014 03:11 AM, Bryan Wu wrote: > On Wed, Aug 20, 2014 at 6:41 AM, Jacek Anaszewski > <j.anaszewski@samsung.com> wrote: >> Add a mechanism for locking LED subsystem sysfs interface. >> This patch prepares ground for addition of LED Flash Class >> extension, whose API will be integrated with V4L2 Flash API. >> Such a fusion enforces introducing a locking scheme, which >> will secure consistent access to the LED Flash Class device. >> >> The mechanism being introduced allows for disabling LED >> subsystem sysfs interface by calling led_sysfs_lock function >> and enabling it by calling led_sysfs_unlock. The functions >> alter the LED_SYSFS_LOCK flag state and must be called >> under mutex lock. The state of the lock is checked with use >> of led_sysfs_is_locked function. Such a design allows for >> providing immediate feedback to the user space on whether >> the LED Flash Class device is available or is under V4L2 Flash >> sub-device control. >> >> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com> >> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >> Cc: Bryan Wu <cooloney@gmail.com> >> Cc: Richard Purdie <rpurdie@rpsys.net> >> --- >> drivers/leds/led-class.c | 23 ++++++++++++++++++++--- >> drivers/leds/led-core.c | 18 ++++++++++++++++++ >> drivers/leds/led-triggers.c | 15 ++++++++++++--- >> include/linux/leds.h | 32 ++++++++++++++++++++++++++++++++ >> 4 files changed, 82 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c >> index 6f82a76..0bc0ba9 100644 >> --- a/drivers/leds/led-class.c >> +++ b/drivers/leds/led-class.c >> @@ -39,17 +39,31 @@ static ssize_t brightness_store(struct device *dev, >> { >> struct led_classdev *led_cdev = dev_get_drvdata(dev); >> unsigned long state; >> - ssize_t ret = -EINVAL; >> + ssize_t ret; >> + >> +#ifdef CONFIG_V4L2_FLASH_LED_CLASS > > Can we remove this #ifdef? Following code looks good to the common LED class.
Sure.
>> + mutex_lock(&led_cdev->led_lock); > > Can we choose more meaningful name instead of led_lock here? > Then use led_sysfs_enable() instead of led_sysfs_unlock() > led_sysfs_disable instead of led_sysfs_lock() > led_sysfs_is_disabled instead of led_sysfs_is_locked() > > And the flag LED_SYSFS_LOCK -> LED_SYSFS_DISABLE > > I was just confused by the name lock and unlock and mutex lock. > > The idea looks good to me.
Indeed, this naming will be more intuitive.
Regards, Jacek
| |