Re: [PATCH v5 1/2] leds: core: Introduce generic pattern interface
On Mon 16 Jul 04:10 PDT 2018, 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
I'm happy to see that you found this patch useful and think the patch
looks good.
> ---
> Changes from v4:
> - Fix some typos.
> - Check if pattern_clear() is initialized.
>
> 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.
In the Qualcomm LPG it's possible to specify if the pattern should be
run through once of repeated continuously, so if you provide the means
to control this by a separately I don't see a problem with this.
Regards,
Bjorn
> ---
> 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..a040fd0 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 activate the pattern,
> + and empty string will disable the pattern.
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 3c7e348..5c6d2e9 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -74,6 +74,118 @@ 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,
Re: [PATCH v5 1/2] leds: core: Introduce generic pattern interface
On Mon 16 Jul 04:10 PDT 2018, 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
I'm happy to see that you found this patch useful and think the patch
looks good.
> ---
> Changes from v4:
> - Fix some typos.
> - Check if pattern_clear() is initialized.
>
> 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.
In the Qualcomm LPG it's possible to specify if the pattern should be
run through once of repeated continuously, so if you provide the means
to control this by a separately I don't see a problem with this.
Regards,
Bjorn
> ---
> 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..a040fd0 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 activate the pattern,
> + and empty string will disable the pattern.
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 3c7e348..5c6d2e9 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -74,6 +74,118 @@ 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,
Re: [PATCH v5 1/2] leds: core: Introduce generic pattern interface
On Mon 2018-07-16 11:51:01, David Lechner wrote: > On 07/16/2018 06:10 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. > > > > Hmm... I was hoping that this would be a generic pattern trigger that > could be used by any LED (e.g. gpio LEDs), not just LEDs with hardware > support. I guess I cannot really test this after all. > > I think it looks good for your use case though. Generic trigger doing patterns on any hardware would be welcome, too. I even have a copy of code somewhere, if you want to dust it off, clean up and merge... 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 v5 1/2] leds: core: Introduce generic pattern interface
On Mon 2018-07-16 11:51:01, David Lechner wrote: > On 07/16/2018 06:10 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. > > > > Hmm... I was hoping that this would be a generic pattern trigger that > could be used by any LED (e.g. gpio LEDs), not just LEDs with hardware > support. I guess I cannot really test this after all. > > I think it looks good for your use case though. Generic trigger doing patterns on any hardware would be welcome, too. I even have a copy of code somewhere, if you want to dust it off, clean up and merge... 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 v5 1/2] leds: core: Introduce generic pattern interface
On 07/16/2018 06:10 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. Hmm... I was hoping that this would be a generic pattern trigger that could be used by any LED (e.g. gpio LEDs), not just LEDs with hardware support. I guess I cannot really test this after all. I think it looks good for your use case though.
Re: [PATCH v5 1/2] leds: core: Introduce generic pattern interface
On 07/16/2018 06:10 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. Hmm... I was hoping that this would be a generic pattern trigger that could be used by any LED (e.g. gpio LEDs), not just LEDs with hardware support. I guess I cannot really test this after all. I think it looks good for your use case though.
