Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface
On 14 July 2018 at 14:29, Pavel Machek wrote: > >> @@ -446,4 +455,14 @@ static inline void >> led_classdev_notify_brightness_hw_changed( >> struct led_classdev *led_cdev, enum led_brightness brightness) { } >> #endif >> >> +/** >> + * struct led_pattern - brigheness value in a pattern > > "brightness". Will fix the typo. Thanks. -- Baolin Wang Best Regards
Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface
On 14 July 2018 at 14:29, Pavel Machek wrote: > >> @@ -446,4 +455,14 @@ static inline void >> led_classdev_notify_brightness_hw_changed( >> struct led_classdev *led_cdev, enum led_brightness brightness) { } >> #endif >> >> +/** >> + * struct led_pattern - brigheness value in a pattern > > "brightness". Will fix the typo. Thanks. -- Baolin Wang Best Regards
Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface
> @@ -446,4 +455,14 @@ static inline void > led_classdev_notify_brightness_hw_changed( > struct led_classdev *led_cdev, enum led_brightness brightness) { } > #endif > > +/** > + * struct led_pattern - brigheness value in a pattern "brightness". Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface
> @@ -446,4 +455,14 @@ static inline void > led_classdev_notify_brightness_hw_changed( > struct led_classdev *led_cdev, enum led_brightness brightness) { } > #endif > > +/** > + * struct led_pattern - brigheness value in a pattern "brightness". Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface
Hi Jacek, On 14 July 2018 at 04:07, Jacek Anaszewski wrote: > Hi Baolin, > > Thank you for the update. > > > On 07/13/2018 08:21 AM, Baolin Wang wrote: >> >> From: Bjorn Andersson >> >> Some LED controllers have support for autonomously controlling >> brightness over time, according to some preprogrammed pattern or >> function. >> >> This adds a new optional operator that LED class drivers can implement >> if they support such functionality as well as a new device attribute to >> configure the pattern for a given LED. >> >> [Baolin Wang did some improvements.] >> >> Signed-off-by: Bjorn Andersson >> Signed-off-by: Baolin Wang >> --- >> Changes from v3: >> - Move the check in pattern_show() to of_led_classdev_register(). >> - Add more documentation to explain how to set/clear one pattern. >> >> Changes from v2: >> - Change kernel version to 4.19. >> - Force user to return error pointer if failed to issue pattern_get(). >> - Use strstrip() to trim trailing newline. >> - Other optimization. >> >> Changes from v1: >> - Add some comments suggested by Pavel. >> - Change 'delta_t' can be 0. >> >> Note: I removed the pattern repeat check and will get the repeat number by >> adding >> one extra file named 'pattern_repeat' according to previous discussion. >> --- >> Documentation/ABI/testing/sysfs-class-led | 20 + >> drivers/leds/led-class.c | 118 >> + >> include/linux/leds.h | 19 + >> 3 files changed, 157 insertions(+) >> >> diff --git a/Documentation/ABI/testing/sysfs-class-led >> b/Documentation/ABI/testing/sysfs-class-led >> index 5f67f7a..f4b73ad 100644 >> --- a/Documentation/ABI/testing/sysfs-class-led >> +++ b/Documentation/ABI/testing/sysfs-class-led >> @@ -61,3 +61,23 @@ Description: >> gpio and backlight triggers. In case of the backlight >> trigger, >> it is useful when driving a LED which is intended to >> indicate >> a device in a standby like state. >> + >> +What: /sys/class/leds//pattern >> +Date: July 2018 >> +KernelVersion: 4.19 >> +Description: >> + Specify a pattern for the LED, for LED hardware that >> support >> + altering the brightness as a function of time. >> + >> + The pattern is given by a series of tuples, of brightness >> and >> + duration (ms). The LED is expected to traverse the series >> and >> + each brightness value for the specified duration. Duration >> of >> + 0 means brightness should immediately change to new value. >> + >> + As LED hardware might have different capabilities and >> precision >> + the requested pattern might be slighly adjusted by the >> driver >> + and the resulting pattern of such operation should be >> returned >> + when this file is read. >> + >> + Writing non-empty string to this file will active the >> pattern, > > > s/active/activate/ Will fix the typo. > > >> + and empty string will disable the pattern. >> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c >> index 3c7e348..0992a0e 100644 >> --- a/drivers/leds/led-class.c >> +++ b/drivers/leds/led-class.c >> @@ -74,6 +74,119 @@ static ssize_t max_brightness_show(struct device *dev, >> } >> static DEVICE_ATTR_RO(max_brightness); >> +static ssize_t pattern_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct led_pattern *pattern; >> + size_t offset = 0; >> + int count, n, i; >> + >> + pattern = led_cdev->pattern_get(led_cdev, ); >> + if (IS_ERR(pattern)) >> + return PTR_ERR(pattern); >> + >> + for (i = 0; i < count; i++) { >> + n = snprintf(buf + offset, PAGE_SIZE - offset, "%d %d ", >> +pattern[i].brightness, pattern[i].delta_t); >> + >> + if (offset + n >= PAGE_SIZE) >> + goto err_nospc; >> + >> + offset += n; >> + } >> + >> + buf[offset - 1] = '\n'; >> + >> + kfree(pattern); >> + return offset; >> + >> +err_nospc: >> + kfree(pattern); >> + return -ENOSPC; >> +} >> + >> +static ssize_t pattern_store(struct device *dev, >> +struct device_attribute *attr, >> +const char *buf, size_t size) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct led_pattern *pattern = NULL; >> + char *sbegin, *elem, *s; >> + unsigned long val; >> + int ret = 0, len = 0; >> + bool odd = true; >> + >> + sbegin = kstrndup(buf, size, GFP_KERNEL); >> + if (!sbegin) >> + return -ENOMEM; >> + >> + /* >> +* Trim trailing newline, if
Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface
Hi Jacek, On 14 July 2018 at 04:07, Jacek Anaszewski wrote: > Hi Baolin, > > Thank you for the update. > > > On 07/13/2018 08:21 AM, Baolin Wang wrote: >> >> From: Bjorn Andersson >> >> Some LED controllers have support for autonomously controlling >> brightness over time, according to some preprogrammed pattern or >> function. >> >> This adds a new optional operator that LED class drivers can implement >> if they support such functionality as well as a new device attribute to >> configure the pattern for a given LED. >> >> [Baolin Wang did some improvements.] >> >> Signed-off-by: Bjorn Andersson >> Signed-off-by: Baolin Wang >> --- >> Changes from v3: >> - Move the check in pattern_show() to of_led_classdev_register(). >> - Add more documentation to explain how to set/clear one pattern. >> >> Changes from v2: >> - Change kernel version to 4.19. >> - Force user to return error pointer if failed to issue pattern_get(). >> - Use strstrip() to trim trailing newline. >> - Other optimization. >> >> Changes from v1: >> - Add some comments suggested by Pavel. >> - Change 'delta_t' can be 0. >> >> Note: I removed the pattern repeat check and will get the repeat number by >> adding >> one extra file named 'pattern_repeat' according to previous discussion. >> --- >> Documentation/ABI/testing/sysfs-class-led | 20 + >> drivers/leds/led-class.c | 118 >> + >> include/linux/leds.h | 19 + >> 3 files changed, 157 insertions(+) >> >> diff --git a/Documentation/ABI/testing/sysfs-class-led >> b/Documentation/ABI/testing/sysfs-class-led >> index 5f67f7a..f4b73ad 100644 >> --- a/Documentation/ABI/testing/sysfs-class-led >> +++ b/Documentation/ABI/testing/sysfs-class-led >> @@ -61,3 +61,23 @@ Description: >> gpio and backlight triggers. In case of the backlight >> trigger, >> it is useful when driving a LED which is intended to >> indicate >> a device in a standby like state. >> + >> +What: /sys/class/leds//pattern >> +Date: July 2018 >> +KernelVersion: 4.19 >> +Description: >> + Specify a pattern for the LED, for LED hardware that >> support >> + altering the brightness as a function of time. >> + >> + The pattern is given by a series of tuples, of brightness >> and >> + duration (ms). The LED is expected to traverse the series >> and >> + each brightness value for the specified duration. Duration >> of >> + 0 means brightness should immediately change to new value. >> + >> + As LED hardware might have different capabilities and >> precision >> + the requested pattern might be slighly adjusted by the >> driver >> + and the resulting pattern of such operation should be >> returned >> + when this file is read. >> + >> + Writing non-empty string to this file will active the >> pattern, > > > s/active/activate/ Will fix the typo. > > >> + and empty string will disable the pattern. >> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c >> index 3c7e348..0992a0e 100644 >> --- a/drivers/leds/led-class.c >> +++ b/drivers/leds/led-class.c >> @@ -74,6 +74,119 @@ static ssize_t max_brightness_show(struct device *dev, >> } >> static DEVICE_ATTR_RO(max_brightness); >> +static ssize_t pattern_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct led_pattern *pattern; >> + size_t offset = 0; >> + int count, n, i; >> + >> + pattern = led_cdev->pattern_get(led_cdev, ); >> + if (IS_ERR(pattern)) >> + return PTR_ERR(pattern); >> + >> + for (i = 0; i < count; i++) { >> + n = snprintf(buf + offset, PAGE_SIZE - offset, "%d %d ", >> +pattern[i].brightness, pattern[i].delta_t); >> + >> + if (offset + n >= PAGE_SIZE) >> + goto err_nospc; >> + >> + offset += n; >> + } >> + >> + buf[offset - 1] = '\n'; >> + >> + kfree(pattern); >> + return offset; >> + >> +err_nospc: >> + kfree(pattern); >> + return -ENOSPC; >> +} >> + >> +static ssize_t pattern_store(struct device *dev, >> +struct device_attribute *attr, >> +const char *buf, size_t size) >> +{ >> + struct led_classdev *led_cdev = dev_get_drvdata(dev); >> + struct led_pattern *pattern = NULL; >> + char *sbegin, *elem, *s; >> + unsigned long val; >> + int ret = 0, len = 0; >> + bool odd = true; >> + >> + sbegin = kstrndup(buf, size, GFP_KERNEL); >> + if (!sbegin) >> + return -ENOMEM; >> + >> + /* >> +* Trim trailing newline, if
Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface
Hi Baolin, Thank you for the update. On 07/13/2018 08:21 AM, Baolin Wang wrote: From: Bjorn Andersson Some LED controllers have support for autonomously controlling brightness over time, according to some preprogrammed pattern or function. This adds a new optional operator that LED class drivers can implement if they support such functionality as well as a new device attribute to configure the pattern for a given LED. [Baolin Wang did some improvements.] Signed-off-by: Bjorn Andersson Signed-off-by: Baolin Wang --- Changes from v3: - Move the check in pattern_show() to of_led_classdev_register(). - Add more documentation to explain how to set/clear one pattern. Changes from v2: - Change kernel version to 4.19. - Force user to return error pointer if failed to issue pattern_get(). - Use strstrip() to trim trailing newline. - Other optimization. Changes from v1: - Add some comments suggested by Pavel. - Change 'delta_t' can be 0. Note: I removed the pattern repeat check and will get the repeat number by adding one extra file named 'pattern_repeat' according to previous discussion. --- Documentation/ABI/testing/sysfs-class-led | 20 + drivers/leds/led-class.c | 118 + include/linux/leds.h | 19 + 3 files changed, 157 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led index 5f67f7a..f4b73ad 100644 --- a/Documentation/ABI/testing/sysfs-class-led +++ b/Documentation/ABI/testing/sysfs-class-led @@ -61,3 +61,23 @@ Description: gpio and backlight triggers. In case of the backlight trigger, it is useful when driving a LED which is intended to indicate a device in a standby like state. + +What: /sys/class/leds//pattern +Date: July 2018 +KernelVersion: 4.19 +Description: + Specify a pattern for the LED, for LED hardware that support + altering the brightness as a function of time. + + The pattern is given by a series of tuples, of brightness and + duration (ms). The LED is expected to traverse the series and + each brightness value for the specified duration. Duration of + 0 means brightness should immediately change to new value. + + As LED hardware might have different capabilities and precision + the requested pattern might be slighly adjusted by the driver + and the resulting pattern of such operation should be returned + when this file is read. + + Writing non-empty string to this file will active the pattern, s/active/activate/ + and empty string will disable the pattern. diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index 3c7e348..0992a0e 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -74,6 +74,119 @@ static ssize_t max_brightness_show(struct device *dev, } static DEVICE_ATTR_RO(max_brightness); +static ssize_t pattern_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_pattern *pattern; + size_t offset = 0; + int count, n, i; + + pattern = led_cdev->pattern_get(led_cdev, ); + if (IS_ERR(pattern)) + return PTR_ERR(pattern); + + for (i = 0; i < count; i++) { + n = snprintf(buf + offset, PAGE_SIZE - offset, "%d %d ", +pattern[i].brightness, pattern[i].delta_t); + + if (offset + n >= PAGE_SIZE) + goto err_nospc; + + offset += n; + } + + buf[offset - 1] = '\n'; + + kfree(pattern); + return offset; + +err_nospc: + kfree(pattern); + return -ENOSPC; +} + +static ssize_t pattern_store(struct device *dev, +struct device_attribute *attr, +const char *buf, size_t size) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_pattern *pattern = NULL; + char *sbegin, *elem, *s; + unsigned long val; + int ret = 0, len = 0; + bool odd = true; + + sbegin = kstrndup(buf, size, GFP_KERNEL); + if (!sbegin) + return -ENOMEM; + + /* +* Trim trailing newline, if the remaining string is empty, +* clear the pattern. +*/ + s = strstrip(sbegin); + if (!*s) { + if (led_cdev->pattern_clear) + ret = led_cdev->pattern_clear(led_cdev); Why you don't require pattern_clear op to be initialized? Without it there seems to be no other way to disable the pattern. Please fail in led_classdev_register() if it is NULL, similarly as when pattern_get is NULL. + goto out; + }
Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface
Hi Baolin, Thank you for the update. On 07/13/2018 08:21 AM, Baolin Wang wrote: From: Bjorn Andersson Some LED controllers have support for autonomously controlling brightness over time, according to some preprogrammed pattern or function. This adds a new optional operator that LED class drivers can implement if they support such functionality as well as a new device attribute to configure the pattern for a given LED. [Baolin Wang did some improvements.] Signed-off-by: Bjorn Andersson Signed-off-by: Baolin Wang --- Changes from v3: - Move the check in pattern_show() to of_led_classdev_register(). - Add more documentation to explain how to set/clear one pattern. Changes from v2: - Change kernel version to 4.19. - Force user to return error pointer if failed to issue pattern_get(). - Use strstrip() to trim trailing newline. - Other optimization. Changes from v1: - Add some comments suggested by Pavel. - Change 'delta_t' can be 0. Note: I removed the pattern repeat check and will get the repeat number by adding one extra file named 'pattern_repeat' according to previous discussion. --- Documentation/ABI/testing/sysfs-class-led | 20 + drivers/leds/led-class.c | 118 + include/linux/leds.h | 19 + 3 files changed, 157 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led index 5f67f7a..f4b73ad 100644 --- a/Documentation/ABI/testing/sysfs-class-led +++ b/Documentation/ABI/testing/sysfs-class-led @@ -61,3 +61,23 @@ Description: gpio and backlight triggers. In case of the backlight trigger, it is useful when driving a LED which is intended to indicate a device in a standby like state. + +What: /sys/class/leds//pattern +Date: July 2018 +KernelVersion: 4.19 +Description: + Specify a pattern for the LED, for LED hardware that support + altering the brightness as a function of time. + + The pattern is given by a series of tuples, of brightness and + duration (ms). The LED is expected to traverse the series and + each brightness value for the specified duration. Duration of + 0 means brightness should immediately change to new value. + + As LED hardware might have different capabilities and precision + the requested pattern might be slighly adjusted by the driver + and the resulting pattern of such operation should be returned + when this file is read. + + Writing non-empty string to this file will active the pattern, s/active/activate/ + and empty string will disable the pattern. diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index 3c7e348..0992a0e 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -74,6 +74,119 @@ static ssize_t max_brightness_show(struct device *dev, } static DEVICE_ATTR_RO(max_brightness); +static ssize_t pattern_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_pattern *pattern; + size_t offset = 0; + int count, n, i; + + pattern = led_cdev->pattern_get(led_cdev, ); + if (IS_ERR(pattern)) + return PTR_ERR(pattern); + + for (i = 0; i < count; i++) { + n = snprintf(buf + offset, PAGE_SIZE - offset, "%d %d ", +pattern[i].brightness, pattern[i].delta_t); + + if (offset + n >= PAGE_SIZE) + goto err_nospc; + + offset += n; + } + + buf[offset - 1] = '\n'; + + kfree(pattern); + return offset; + +err_nospc: + kfree(pattern); + return -ENOSPC; +} + +static ssize_t pattern_store(struct device *dev, +struct device_attribute *attr, +const char *buf, size_t size) +{ + struct led_classdev *led_cdev = dev_get_drvdata(dev); + struct led_pattern *pattern = NULL; + char *sbegin, *elem, *s; + unsigned long val; + int ret = 0, len = 0; + bool odd = true; + + sbegin = kstrndup(buf, size, GFP_KERNEL); + if (!sbegin) + return -ENOMEM; + + /* +* Trim trailing newline, if the remaining string is empty, +* clear the pattern. +*/ + s = strstrip(sbegin); + if (!*s) { + if (led_cdev->pattern_clear) + ret = led_cdev->pattern_clear(led_cdev); Why you don't require pattern_clear op to be initialized? Without it there seems to be no other way to disable the pattern. Please fail in led_classdev_register() if it is NULL, similarly as when pattern_get is NULL. + goto out; + }