Re: [PATCH v4 1/2] leds: core: Introduce generic pattern interface

2018-07-15 Thread Baolin Wang
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

2018-07-15 Thread Baolin Wang
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

2018-07-14 Thread Pavel Machek

> @@ -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

2018-07-14 Thread Pavel Machek

> @@ -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

2018-07-13 Thread Baolin Wang
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

2018-07-13 Thread Baolin Wang
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

2018-07-13 Thread Jacek Anaszewski

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

2018-07-13 Thread Jacek Anaszewski

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;
+   }