Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Xiaotong Lu
Cc: Baolin Wang
---
Documentation/devicetree/bindings/leds/leds-sc27xx-bltc.txt | 10 ++
1 file changed, 6 insertions(+)
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Oleh Kravchenko
---
.../devicetree/bindings/leds/leds-cr0014114.txt| 26 --
1 file changed, 19 insertions(+), 7 deletions(
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
Cc: Dan Murphy
---
drivers/leds/leds-lm3601x.c | 45 -
1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/drivers/leds/leds
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
Cc: Dan Murphy
---
drivers/leds/leds-lm3692x.c | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/leds/leds-lm3692x.c b
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Also, fix malformed syntax of address-cells and size-cells
in the example.
Signed-off-by: Jacek Anaszewski
Cc: Sakari Ailus
---
.../devicetree/bindings/leds/ams,as3645a.txt | 22
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
---
drivers/leds/leds-aat1290.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
index 43bd8a4
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
Cc: Simon Shields
---
drivers/leds/leds-an30259a.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/leds-an30259a.c b/drivers
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Linus Walleij
---
.../devicetree/bindings/leds/leds-gpio.txt | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
Cc: Linus Walleij
---
drivers/leds/leds-gpio.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Simon Shields
---
.../devicetree/bindings/leds/leds-an30259a.txt | 22 +-
1 file changed, 17 insertions(+), 5 deletions(
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
Cc: Sakari Ailus
---
drivers/leds/leds-as3645a.c | 65 -
1 file changed, 29 insertions(+), 36 deletions(-)
diff --git a/drivers/leds/leds
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
---
Documentation/devicetree/bindings/leds/leds-aat1290.txt | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/Documentation
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
Cc: Oleh Kravchenko
---
drivers/leds/leds-cr0014114.c | 29 ++---
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/leds/leds-cr0014114.c b
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Dan Murphy
---
Documentation/devicetree/bindings/leds/leds-lm3692x.txt | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Daniel Mack
---
drivers/leds/leds-lt3593.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Daniel Mack
---
Documentation/devicetree/bindings/leds/leds-lt3593.txt | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --
Qemu,
by stubbing the driver internals where hardware interaction was needed
for proper probing.
Thanks,
Jacek Anaszewski
[0] https://lore.kernel.org/patchwork/patch/858993/
Jacek Anaszewski (24):
leds: class: Improve LED and LED flash class registration API
leds: core: Add support for composi
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Dan Murphy
---
Documentation/devicetree/bindings/leds/leds-lp8860.txt | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --
Add common LED function definitions for use in Device Tree.
The function names were extracted from existing dts files
after eliminating oddities.
Signed-off-by: Jacek Anaszewski
Cc: Baolin Wang
Cc: Daniel Mack
Cc: Dan Murphy
Cc: Linus Walleij
Cc: Oleh Kravchenko
Cc: Sakari Ailus
Cc: Simon
Refer to new "function" and "color" properties and mark "label"
as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Dan Murphy
---
Documentation/devicetree/bindings/leds/leds-lm3601x.txt | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --
Introduce dedicated properties for conveying information about
LED function and color. Mark old "label" property as deprecated.
Signed-off-by: Jacek Anaszewski
Cc: Baolin Wang
Cc: Daniel Mack
Cc: Dan Murphy
Cc: Linus Walleij
Cc: Oleh Kravchenko
Cc: Sakari Ailus
Cc: Simon S
Switch to using generic LED support for composing LED class
device name.
Signed-off-by: Jacek Anaszewski
Signed-off-by: Dan Murphy
---
drivers/leds/leds-lp8860.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/leds/leds
s adopted for LED class device name.
Signed-off-by: Jacek Anaszewski
Cc: Baolin Wang
Cc: Daniel Mack
Cc: Dan Murphy
Cc: Linus Walleij
Cc: Oleh Kravchenko
Cc: Sakari Ailus
Cc: Simon Shields
Cc: Xiaotong Lu
---
Documentation/leds/leds-class.txt | 2 +-
drivers/
err = -EINVAL;
We don't fail on attempt of setting brightness > max_brightness, but
only truncate it. In this case we could do the same for consistency.
> + goto out;
> + }
> +
> if (ccount != 2) {
> data->npatterns = 0;
> err = -EINVAL;
>
>
>
>
--
Best regards,
Jacek Anaszewski
dat->gpiod pointer for OF defined GPIO leds (2018-10-26
20:51:36 +0200)
Thanks,
Jacek Anaszewski
LED fix for 4-20-rc1
Liviu Dudau (1):
leds: gpio: set led_dat->g
Hi Dan,
Thank you for the patch. Nonetheless, I've just applied similar Liviu's
patch [0], since it arrived one week ago already.
I'll send it upstream with LED fixes for 4-20-rc2.
[0] https://lkml.org/lkml/2018/10/18/154
Best regards,
Jacek Anaszewski
On 10/25/2018 05:06
already gets the needed pointer,
>> we just need to assign it to the relevant gpio_led_data structure.
>>
>> Fixes: 45d4c6de4e49 ("leds: gpio: Try to lookup gpiod from device")
>> Cc: Linus Walleij
>> Cc: Jacek Anaszewski
>> Signed-off-by: Liviu Dud
> Signed-off-by: Pavel Machek
> Suggested-by: Jacek Anaszewski
>
>
> diff --git a/drivers/leds/trigger/ledtrig-pattern.c
> b/drivers/leds/trigger/ledtrig-pattern.c
> index ce7acd1..bc5f495 100644
> --- a/drivers/leds/trigger/ledtrig-pattern.c
> +++ b/drivers/le
runtime ramping or startup/shutdown ramping could also be done
> via sysfs.
> I am not dedicated to having it in the DT file I was following prior art.
>
> Jacek
>
> Do you have an opinion on this?
This is this problem with the Device Tree's scope of responsibility.
It is defined as a means for "describing the hardware", but often
this rule is abused by the properties that fall into "configuration"
category. E.g. default-state, retain-state-suspended from leds-gpio.txt
or linux-default-trigger from common LED bindings.
In some cases this is justified. The question is whether it is something
that necessarily needs to be configured on driver probing? If not, then
I'd go for sysfs interface.
--
Best regards,
Jacek Anaszewski
On 10/25/2018 11:22 AM, Andy Shevchenko wrote:
> On Wed, Oct 24, 2018 at 10:07:30PM +0200, Jacek Anaszewski wrote:
>
>> Yet, there are many out of LED subsystem files to update:
>>
>> find -name "*.c" -o -name "*.h" | xargs grep "enum led_brightne
On 10/23/2018 09:30 PM, Pavel Machek wrote:
> On Tue 2018-10-23 21:09:54, Jacek Anaszewski wrote:
>> On 10/23/2018 08:54 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>>> + led->field = devm_regmap_field_alloc(dev, regmap, conf);
>>>>> + if (I
d-controller:flash on nokia n900 (so everything in
> software).
>
>> OK, let's abide by constant update interval for now.
>>
>> Thank you for your work on this patch set throughout
>> all these months. We will have -rc8, so one week of testing
>> before sending upstream should be enough.
>>
>> Patch set applied to the for-next branch of linux-leds.git.
>
> Thanks!
> Pavel
>
--
Best regards,
Jacek Anaszewski
hange? Traditionally,
>> you can see that I've pulled from just seeing the end result when it
>> actually hits the public tree (which is yet another step removed from
>> the steps above - I do build tests between every pull, but I generally
>> tend to push out the end result in batches, usually a couple of times
>> a day).
>>
>> Comments?
>
> Welcome back!
>
> I have no strong opinions, in regards to the acks.
>
> Your current approach, with no ack at all, just means that I have to
> do "git remote update" a few times, which I probably would have done
> anyways. So, to me, feel free to pick whatever option that makes the
> life easiest for you.
Same for me, I do the update anyway to see if and how my pull request
has been merged.
--
Best regards,
Jacek Anaszewski
On 10/23/2018 09:23 PM, Joe Perches wrote:
> On Tue, 2018-10-23 at 20:50 +0200, Jacek Anaszewski wrote:
>>> diff --git a/drivers/leds/leds-upboard.c b/drivers/leds/leds-upboard.c
>>> new file mode 100644
>>> index 000..34a6973
>>> --- /dev/null
>>&g
not handle the led brightness or simply don't
care about it. Conceptually said devices want to just switch on
or off the led. It is useless in this case to have a 255 range
of brightness, while just having an LED_ON and LED_OFF improves
the boolean meaning of the led status.
Si
regmap_field_alloc(dev, regmap, conf);
> + if (IS_ERR(led->field))
> + return PTR_ERR(led->field);
> +
> + led->cdev.max_brightness = 1;
s/1/LED_ON/
> + led->cdev.brightness_get = upboard_led_brightness_get;
> + led->cdev.brightness_set = upboard_led_brightness_set;
> + led->cdev.name = upboard_led_names[led_index];
> +
> + return devm_led_classdev_register(dev, &led->cdev);
> +}
> +
> +static struct platform_driver upboard_led_driver = {
> + .driver = {
> + .name = "upboard-led",
> + },
> +};
> +
> +module_platform_driver_probe(upboard_led_driver, upboard_led_probe);
> +
> +MODULE_ALIAS("platform:upboard-led");
> +MODULE_AUTHOR("Javier Arteaga ");
> +MODULE_DESCRIPTION("UP Board LED driver");
> +MODULE_LICENSE("GPL v2");
>
--
Best regards,
Jacek Anaszewski
ts to existing LED class drivers:
- leds-pwm: don't print error message on -EPROBE_DEFER
- leds-gpio: try to lookup gpiod from device
- leds-as3645a: convert to using %pOFn instead of device_node.name
Thanks,
Jacek Anaszewski
The following changes si
On 10/15/2018 02:56 AM, Rob Herring wrote:
> On Sat, Oct 13, 2018 at 1:46 PM Jacek Anaszewski
> wrote:
>>
>> On 10/12/2018 08:03 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>>>>>> Signed-off-by: Dan Murphy
>>>>>>>
>>
n basing
on these bindings?
This patch fixes the issues of bindings in a way that would change
the ABI, if only it existed. But it apparently doesn't exist in
mainline. Unless a DT documentation itself constitutes an ABI.
I'd like to have it clarified at this occasion, and that's
ed_cdev->activated = true;
> +
> + return 0;
> +}
> +
> +static void pattern_trig_deactivate(struct led_classdev *led_cdev)
> +{
> + struct pattern_trig_data *data = led_cdev->trigger_data;
> +
> + if (!led_cdev->activated)
> + return;
> +
> + if (led_cdev->pattern_clear)
> + led_cdev->pattern_clear(led_cdev);
> +
> + del_timer_sync(&data->timer);
> +
> + led_set_brightness(led_cdev, LED_OFF);
> + kfree(data);
> + led_cdev->activated = false;
> +}
> +
> +static struct led_trigger pattern_led_trigger = {
> + .name = "pattern",
> + .activate = pattern_trig_activate,
> + .deactivate = pattern_trig_deactivate,
> + .groups = pattern_trig_groups,
> +};
> +
> +static int __init pattern_trig_init(void)
> +{
> + return led_trigger_register(&pattern_led_trigger);
> +}
> +
> +static void __exit pattern_trig_exit(void)
> +{
> + led_trigger_unregister(&pattern_led_trigger);
> +}
> +
> +module_init(pattern_trig_init);
> +module_exit(pattern_trig_exit);
> +
> +MODULE_AUTHOR("Raphael Teysseyre +MODULE_AUTHOR("Baolin Wang +MODULE_DESCRIPTION("LED Pattern trigger");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 834683d..7393a31 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -22,6 +22,7 @@
> #include
>
> struct device;
> +struct led_pattern;
> /*
> * LED Core
> */
> @@ -88,6 +89,10 @@ struct led_classdev {
>unsigned long *delay_on,
>unsigned long *delay_off);
>
> + int (*pattern_set)(struct led_classdev *led_cdev,
> +struct led_pattern *pattern, u32 len, int repeat);
> + int (*pattern_clear)(struct led_classdev *led_cdev);
> +
> struct device *dev;
> const struct attribute_group**groups;
>
> @@ -472,4 +477,14 @@ static inline void
> led_classdev_notify_brightness_hw_changed(
> struct led_classdev *led_cdev, enum led_brightness brightness) { }
> #endif
>
> +/**
> + * struct led_pattern - pattern interval settings
> + * @delta_t: pattern interval delay, in milliseconds
> + * @brightness: pattern interval brightness
> + */
> +struct led_pattern {
> + u32 delta_t;
> + int brightness;
> +};
> +
> #endif /* __LINUX_LEDS_H_INCLUDED */
>
OK, let's abide by constant update interval for now.
Thank you for your work on this patch set throughout
all these months. We will have -rc8, so one week of testing
before sending upstream should be enough.
Patch set applied to the for-next branch of linux-leds.git.
--
Best regards,
Jacek Anaszewski
dalone device and not a cell of any
MFD device.
Waiting for DT maintainer's ack.
Best regards,
Jacek Anaszewski
>>> index c885cf89b8ce..920f910be4e9 100644
>>> --- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt
ed_cdev->pattern_clear = NULL;
> + }
> +
> + data->is_indefinite = true;
> + data->last_repeat = -1;
> + mutex_init(&data->lock);
> + data->led_cdev = led_cdev;
> + led_set_trigger_data(led_cdev, data);
> + timer_setup(&data->timer, pattern_trig_timer_function, 0);
> + led_cdev->activated = true;
> +
> + return 0;
> +}
> +
> +static void pattern_trig_deactivate(struct led_classdev *led_cdev)
> +{
> + struct pattern_trig_data *data = led_cdev->trigger_data;
> +
> + if (!led_cdev->activated)
> + return;
> +
> + if (led_cdev->pattern_clear)
> + led_cdev->pattern_clear(led_cdev);
> +
> + del_timer_sync(&data->timer);
> +
> + led_set_brightness(led_cdev, LED_OFF);
> + kfree(data);
> + led_cdev->activated = false;
> +}
> +
> +static struct led_trigger pattern_led_trigger = {
> + .name = "pattern",
> + .activate = pattern_trig_activate,
> + .deactivate = pattern_trig_deactivate,
> + .groups = pattern_trig_groups,
> +};
> +
> +static int __init pattern_trig_init(void)
> +{
> + return led_trigger_register(&pattern_led_trigger);
> +}
> +
> +static void __exit pattern_trig_exit(void)
> +{
> + led_trigger_unregister(&pattern_led_trigger);
> +}
> +
> +module_init(pattern_trig_init);
> +module_exit(pattern_trig_exit);
> +
> +MODULE_AUTHOR("Raphael Teysseyre +MODULE_AUTHOR("Baolin Wang +MODULE_DESCRIPTION("LED Pattern trigger");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 834683d..7393a31 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -22,6 +22,7 @@
> #include
>
> struct device;
> +struct led_pattern;
> /*
> * LED Core
> */
> @@ -88,6 +89,10 @@ struct led_classdev {
>unsigned long *delay_on,
>unsigned long *delay_off);
>
> + int (*pattern_set)(struct led_classdev *led_cdev,
> +struct led_pattern *pattern, u32 len, int repeat);
> + int (*pattern_clear)(struct led_classdev *led_cdev);
> +
> struct device *dev;
> const struct attribute_group**groups;
>
> @@ -472,4 +477,14 @@ static inline void
> led_classdev_notify_brightness_hw_changed(
> struct led_classdev *led_cdev, enum led_brightness brightness) { }
> #endif
>
> +/**
> + * struct led_pattern - pattern interval settings
> + * @delta_t: pattern interval delay, in milliseconds
> + * @brightness: pattern interval brightness
> + */
> +struct led_pattern {
> + u32 delta_t;
> + int brightness;
> +};
> +
> #endif /* __LINUX_LEDS_H_INCLUDED */
>
--
Best regards,
Jacek Anaszewski
Hi Baolin,
On 10/09/2018 02:01 PM, Baolin Wang wrote:
> Hi Jacek and Pavel,
>
> On 5 October 2018 at 04:00, Jacek Anaszewski
> wrote:
>> Hi Baolin,
>>
>> On 10/03/2018 03:21 AM, Baolin Wang wrote:
>>> Hi Jacek,
>>>
>>> On 3 October 20
Hi Baolin,
On 10/03/2018 03:21 AM, Baolin Wang wrote:
> Hi Jacek,
>
> On 3 October 2018 at 04:25, Jacek Anaszewski
> wrote:
>> Hi Baolin,
>>
>> Thank you for the v14. We'll probably need v15, though :-)
>>
>> I added the comments in the code b
re is a problem with the binding,
> submit patches to fix the problem.
[0]
https://lore.kernel.org/lkml/1448521025-2796-1-git-send-email-milo@ti.com/
[1] https://lore.kernel.org/lkml/56656468.8020...@samsung.com/
[2]
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1341860.html
--
Best regards,
Jacek Anaszewski
data->led_cdev = led_cdev;
> + led_set_trigger_data(led_cdev, data);
> + timer_setup(&data->timer, pattern_trig_timer_function, 0);
> + led_cdev->activated = true;
> +
> + return 0;
> +}
> +
> +static void pattern_trig_deactivate(struct led_classdev *led_cdev)
> +{
> + struct pattern_trig_data *data = led_cdev->trigger_data;
> +
> + if (!led_cdev->activated)
> + return;
> +
> + if (led_cdev->pattern_clear)
> + led_cdev->pattern_clear(led_cdev);
> +
> + del_timer_sync(&data->timer);
> +
> + led_set_brightness(led_cdev, LED_OFF);
> + kfree(data);
> + led_cdev->activated = false;
> +}
> +
> +static struct led_trigger pattern_led_trigger = {
> + .name = "pattern",
> + .activate = pattern_trig_activate,
> + .deactivate = pattern_trig_deactivate,
> + .groups = pattern_trig_groups,
> +};
> +
> +static int __init pattern_trig_init(void)
> +{
> + return led_trigger_register(&pattern_led_trigger);
> +}
> +
> +static void __exit pattern_trig_exit(void)
> +{
> + led_trigger_unregister(&pattern_led_trigger);
> +}
> +
> +module_init(pattern_trig_init);
> +module_exit(pattern_trig_exit);
> +
> +MODULE_AUTHOR("Raphael Teysseyre +MODULE_AUTHOR("Baolin Wang +MODULE_DESCRIPTION("LED Pattern trigger");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 834683d..7393a31 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -22,6 +22,7 @@
> #include
>
> struct device;
> +struct led_pattern;
> /*
> * LED Core
> */
> @@ -88,6 +89,10 @@ struct led_classdev {
>unsigned long *delay_on,
>unsigned long *delay_off);
>
> + int (*pattern_set)(struct led_classdev *led_cdev,
> +struct led_pattern *pattern, u32 len, int repeat);
> + int (*pattern_clear)(struct led_classdev *led_cdev);
> +
> struct device *dev;
> const struct attribute_group**groups;
>
> @@ -472,4 +477,14 @@ static inline void
> led_classdev_notify_brightness_hw_changed(
> struct led_classdev *led_cdev, enum led_brightness brightness) { }
> #endif
>
> +/**
> + * struct led_pattern - pattern interval settings
> + * @delta_t: pattern interval delay, in milliseconds
> + * @brightness: pattern interval brightness
> + */
> +struct led_pattern {
> + u32 delta_t;
> + int brightness;
> +};
> +
> #endif /* __LINUX_LEDS_H_INCLUDED */
>
--
Best regards,
Jacek Anaszewski
pport
for non-MFD devices from MFD subsystem to the LED subsystem. And yes -
along with the bindings. This is semantically correct, and yet we don't
have mainline users.
Pavel - you will have to engage more people for your crusade to prevail.
For now, to speed up the things, I am forced to ignore your NAK.
So NAK to your NAK. Sorry.
--
Best regards,
Jacek Anaszewski
Hi Baolin,
On 10/01/2018 03:18 PM, Baolin Wang wrote:
> Hi Jacek,
>
> On 30 September 2018 at 23:33, Jacek Anaszewski
> wrote:
>> Hi Baolin,
>>
>> Thank you for adding the dimming support.
>>
>> I've tested it and detected severe problem when
&g
v;
> + led_set_trigger_data(led_cdev, data);
> + timer_setup(&data->timer, pattern_trig_timer_function, 0);
> + led_cdev->activated = true;
> +
> + return 0;
> +}
> +
> +static void pattern_trig_deactivate(struct led_classdev *led_cdev)
> +{
> + struct pattern_trig_data *data = led_cdev->trigger_data;
> +
> + if (!led_cdev->activated)
> + return;
> +
> + if (led_cdev->pattern_clear)
> + led_cdev->pattern_clear(led_cdev);
> +
> + del_timer_sync(&data->timer);
> +
> + led_set_brightness(led_cdev, LED_OFF);
> + kfree(data);
> + led_cdev->activated = false;
> +}
> +
> +static struct led_trigger pattern_led_trigger = {
> + .name = "pattern",
> + .activate = pattern_trig_activate,
> + .deactivate = pattern_trig_deactivate,
> + .groups = pattern_trig_groups,
> +};
> +
> +static int __init pattern_trig_init(void)
> +{
> + return led_trigger_register(&pattern_led_trigger);
> +}
> +
> +static void __exit pattern_trig_exit(void)
> +{
> + led_trigger_unregister(&pattern_led_trigger);
> +}
> +
> +module_init(pattern_trig_init);
> +module_exit(pattern_trig_exit);
> +
> +MODULE_AUTHOR("Raphael Teysseyre +MODULE_AUTHOR("Baolin Wang +MODULE_DESCRIPTION("LED Pattern trigger");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 834683d..7393a31 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -22,6 +22,7 @@
> #include
>
> struct device;
> +struct led_pattern;
> /*
> * LED Core
> */
> @@ -88,6 +89,10 @@ struct led_classdev {
>unsigned long *delay_on,
>unsigned long *delay_off);
>
> + int (*pattern_set)(struct led_classdev *led_cdev,
> +struct led_pattern *pattern, u32 len, int repeat);
> + int (*pattern_clear)(struct led_classdev *led_cdev);
> +
> struct device *dev;
> const struct attribute_group**groups;
>
> @@ -472,4 +477,14 @@ static inline void
> led_classdev_notify_brightness_hw_changed(
> struct led_classdev *led_cdev, enum led_brightness brightness) { }
> #endif
>
> +/**
> + * struct led_pattern - pattern interval settings
> + * @delta_t: pattern interval delay, in milliseconds
> + * @brightness: pattern interval brightness
> + */
> +struct led_pattern {
> + u32 delta_t;
> + int brightness;
> +};
> +
> #endif /* __LINUX_LEDS_H_INCLUDED */
>
--
Best regards,
Jacek Anaszewski
Hi Baolin,
On 09/25/2018 01:15 PM, Baolin Wang wrote:
> On 23 September 2018 at 20:25, Jacek Anaszewski
> wrote:
>> On 09/22/2018 09:44 PM, Pavel Machek wrote:
>>> On Sat 2018-09-22 00:18:13, Pavel Machek wrote:
>>>> On Sat 2018-09-22 00:11:29, Jacek Anaszewski
On 09/22/2018 09:44 PM, Pavel Machek wrote:
> On Sat 2018-09-22 00:18:13, Pavel Machek wrote:
>> On Sat 2018-09-22 00:11:29, Jacek Anaszewski wrote:
>>> On 09/21/2018 11:17 PM, Pavel Machek wrote:
>>>> On Fri 2018-09-21 22:59:40, Jacek Anaszewski wrote:
>>>
On 09/21/2018 11:17 PM, Pavel Machek wrote:
> On Fri 2018-09-21 22:59:40, Jacek Anaszewski wrote:
>> Hi Baolin,
>>
>> On 09/21/2018 05:31 AM, Baolin Wang wrote:
>>> Hi Jacek and Pavel,
>>>
>>> On 11 September 2018 at 10:47, Baolin Wang wrote:
&
flag to determine if software pattern
>> or hardware pattern.
>> - Re-implement pattern_trig_store_pattern() function.
>> - Remove pattern_get() interface.
>> - Improve comments.
>> - Other small optimization.
>> ---
>
> Do you have any comments for the v12 patch set? Thanks.
We will probably have to remove hw_pattern from ledtrig-pattern
since we are unable to come up with generic interface for it.
Unless thread [0] will end up with some brilliant ideas. So far
we're waiting for Pavel's reply.
[0] https://lkml.org/lkml/2018/9/13/1216
--
Best regards,
Jacek Anaszewski
Dan,
On 09/17/2018 05:24 PM, Dan Murphy wrote:
> Jacek
>
> On 09/15/2018 03:00 PM, Jacek Anaszewski wrote:
>> Hi Pavel.
>>
>> On 09/14/2018 11:42 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>>> You may want to learn more about device tree
nightmare" to modify, and care about "easy to
>>> maintain" more than "binary size".
>>
>> Easy to maintain will be a dedicated LED class driver.
>
> You mean, 3 dedicated LED class drivers and 3 MFD drivers with LED
> parts? We'll need complex driver anyway, and I'd really like to have
> just one.
In the LED subsystem we can wrap common functionalities
into a library object. MFD driver will be able to reuse it then.
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git/commit/?h=droid4-pending-v4.19&id=d774c7e447ac911e73a1b3c775e6d89f0422218c
--
Best regards,
Jacek Anaszewski
that is easier to
> maintain is important, and having many drivers are harder to maintain
> than one driver.
>
> Milo's code looks better than yours in that regard. I disagree about
> Milo's code being "nightmare" to modify, and care about "easy to
> maintain" more than "binary size".
Easy to maintain will be a dedicated LED class driver.
--
Best regards,
Jacek Anaszewski
return -EINVAL;
> if (pattern[3].brightness != 0)
> return -EINVAL;
> if (pattern[1].brightness != pattern[2].brightness)
> return -EINVAL;
>
> ..so if user writes something unexpected, he gets the error back.
>
> What am I missing?
I suppose that breathing pattern means smooth rise and fall of
brightness. I'd even say that it should be a non-linear function.
I'm not sure if you noticed my quotation of your statement from the
previous message, because the modification you proposed doesn't
introduce any novelty to the related matter.
The difference between pattern and discussed hw_pattern
semantics is that:
- hw_pattern semantics for leds-sc27xx-bltc.c requires only four
tuples to setup the breathing pattern.
- the same four tuples written to the pattern file would result
in four brightness changes
--
Best regards,
Jacek Anaszewski
t was proposed
in [0] for drivers/leds/leds-sc27xx-bltc.c.
> anything single LED can do in finite time. You are right, that
> [brightness delta_t] sequence may get rather long, and it may be hard
> to turn that sequence into parameters. Are there any _interesting_
> sequences hardware can do but [brightness delta_t] can not store
> reasonably?
Please propose the implementation of pattern_set for
drivers/leds/leds-sc27xx-bltc.c breathing pattern, that will
setup breathing mode basing on the values from tuples.
Use Baolin's patch [0] for a reference of what hardware expects.
[0] https://lore.kernel.org/patchwork/patch/984246/
--
Best regards,
Jacek Anaszewski
f our drivers.
>
> Unless there's something fatally wrong with the binding... but in such
> case we'd like to know what is wrong.
Dangling references ?
> [And yes, I recognize current situation is ... not ideal and I'm
> willing to help. But I'm not sure this is step in right direction.]
>
> Thanks,
--
Best regards,
Jacek Anaszewski
users and to me it would
>> be more logical if this denotes the number of times the pattern should
>> be repeated, with e.g. negative numbers denoting infinite.
>>
>> In particular I expect to have to explain why my driver expects that you
>> write 0 in the file named "repeat" to make it repeat and 1 to make it
>> not repeat.
>
> Ok, -1 works for me :-).
>
> Thanks,
> Pavel
>
--
Best regards,
Jacek Anaszewski
Dan,
On 09/10/2018 09:51 PM, Dan Murphy wrote:
> Jacek
>
> On 09/10/2018 02:07 PM, Jacek Anaszewski wrote:
>> Dan, Pavel,
>>
>> On 09/10/2018 04:37 PM, Dan Murphy wrote:
>>> Jacek
>>>
>>> On 09/08/2018 02:53 PM, Jacek Anaszewski wrote:
>&g
+
> drivers/leds/leds-an30259a.c | 368 ++
> 4 files changed, 422 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/leds-an30259a.txt
> create mode 100644 drivers/leds/leds-an30259a.c
>
Applied to the for-next branch of linux-leds.git.
--
Best regards,
Jacek Anaszewski
Dan, Pavel,
On 09/10/2018 04:37 PM, Dan Murphy wrote:
> Jacek
>
> On 09/08/2018 02:53 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> On 09/07/2018 03:52 PM, Dan Murphy wrote:
>> [...]
>>>>
>>>>> And I think Jacek pointed out that the bindin
%s: %d\n",
> - led->name, ret);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "unable to request PWM for %s: %d\n",
> + led->name, ret);
> return ret;
> }
>
>
Applied.
--
Best regards,
Jacek Anaszewski
with e.g. negative numbers denoting infinite.
Sounds reasonable. Let's change this semantics as you propose.
> In particular I expect to have to explain why my driver expects that you
> write 0 in the file named "repeat" to make it repeat and 1 to make it
> not repeat.
[0] https://lkml.org/lkml/2018/9/3/1192
--
Best regards,
Jacek Anaszewski
d some maintainer weigh in here.
>>
>> Hehe. I'm maintnainer. Fun.
>
> I know. I want to see if there was any other opinion. Especially for the
> LED driver.
>
[...]
I have a question - is this lm3697 LED controller a cell of some MFD
device? Or is it a self-contained chip?
--
Best regards,
Jacek Anaszewski
= t + SC27XX_LEDS_STEP / 2;
v = clamp(v, SC27XX_DELTA_T_MIN, SC27XX_DELTA_T_MAX);
offset = v - SC27XX_DELTA_T_MIN;
offset = SC27XX_LEDS_STEP * (offset / SC27XX_LEDS_STEP);
*delta_t = SC27XX_DELTA_T_MIN + offset;
}
Then call the function for each delta_t be
priv;
> led->ldev.name = led->name;
> led->ldev.brightness_set_blocking = sc27xx_led_set;
> + led->ldev.pattern_set = sc27xx_led_pattern_set;
> + led->ldev.pattern_clear = sc27xx_led_pattern_clear;
> + led->ldev.default_trigger = "pattern";
>
> err = devm_led_classdev_register(dev, &led->ldev);
> if (err)
> @@ -241,4 +333,5 @@ static int sc27xx_led_remove(struct platform_device *pdev)
>
> MODULE_DESCRIPTION("Spreadtrum SC27xx breathing light controller driver");
> MODULE_AUTHOR("Xiaotong Lu ");
> +MODULE_AUTHOR("Baolin Wang ");
> MODULE_LICENSE("GPL v2");
>
--
Best regards,
Jacek Anaszewski
and if we want to be consistent - we'd need to require defining target
brightness for each stage properly, i.e.
echo "100 500 100 500 0 500 0 500" > pattern
Which would mean:
1. Rise from brightness 0 to 100 for 500 ms.
2. Keep brightness 100 for 500 ms.
3. Fall from brightness 100 to 0 for 500 ms.
4. Keep brightness 0 for 500 ms.
--
Best regards,
Jacek Anaszewski
s still more work to do, but that will be stuff like RGB
> LED synchronization.)
>
> (And yes, one of the LED chip has pattern engine that can compute
> prime numbers on its own. I don't expect to support
> _that_. Fortunately, nobody but me is likely to want that pattern, so
> we are still okay :-)
>
> https://gitlab.com/tui/tui/blob/master/ofone/tests.notcc/primes.nc
>
> )
> Pavel
>
--
Best regards,
Jacek Anaszewski
t;npatterns)
> + return 0;
> +
> + if (data->is_hw_pattern) {
> + return led_cdev->pattern_set(led_cdev, data->patterns,
> + data->npatterns, data->repeat);
> + }
I have doubts here if it is a good idea to enforce array of tuples
as a generic interface for all hw_patterns. It may not fit well for
every hw pattern engine. It seems that the only feasible solution will
be allowing drivers to come up with their own interfaces, i.e. the
approach you proposed at first for your driver. It seems that the
ledtrig-pattern with software pattern mechanism will be just
a nice side effect of this series :-)
Unless someone will propose a better solution.
We need a broader consensus here. I'd like to hear Pavel's opinion,
since he's been always in favor of common pattern interface, and
inspired this work.
Pavel?
Best regards,
Jacek Anaszewski
> + del_timer_sync(&data->timer);
> +
> + led_set_brightness(led_cdev, LED_OFF);
> + kfree(data);
> + led_cdev->activated = false;
> +}
> +
> +static struct led_trigger pattern_led_trigger = {
> + .name = "pattern",
> + .activate = pattern_trig_activate,
> + .deactivate = pattern_trig_deactivate,
> + .groups = pattern_trig_groups,
> +};
> +
> +static int __init pattern_trig_init(void)
> +{
> + return led_trigger_register(&pattern_led_trigger);
> +}
> +
> +static void __exit pattern_trig_exit(void)
> +{
> + led_trigger_unregister(&pattern_led_trigger);
> +}
> +
> +module_init(pattern_trig_init);
> +module_exit(pattern_trig_exit);
> +
> +MODULE_AUTHOR("Raphael Teysseyre +MODULE_AUTHOR("Baolin Wang +MODULE_DESCRIPTION("LED Pattern trigger");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 834683d..74fc2c6 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -22,6 +22,7 @@
> #include
>
> struct device;
> +struct led_pattern;
> /*
> * LED Core
> */
> @@ -88,6 +89,11 @@ struct led_classdev {
>unsigned long *delay_on,
>unsigned long *delay_off);
>
> + int (*pattern_set)(struct led_classdev *led_cdev,
> +struct led_pattern *pattern, int len,
> +unsigned int repeat);
> + int (*pattern_clear)(struct led_classdev *led_cdev);
> +
> struct device *dev;
> const struct attribute_group**groups;
>
> @@ -472,4 +478,14 @@ static inline void
> led_classdev_notify_brightness_hw_changed(
> struct led_classdev *led_cdev, enum led_brightness brightness) { }
> #endif
>
> +/**
> + * struct led_pattern - pattern interval settings
> + * @delta_t: pattern interval delay, in milliseconds
> + * @brightness: pattern interval brightness
> + */
> +struct led_pattern {
> + int delta_t;
> + int brightness;
> +};
> +
> #endif /* __LINUX_LEDS_H_INCLUDED */
>
--
Best regards,
Jacek Anaszewski
Hi Pavel,
Thank you for the patch.
Looking for DT bindings I can find the following
Documentation/devicetree/bindings/mfd/ti-lmu.txt,
which for LED bindings redirects to:
"[2] ../leds/leds-lm3633.txt",
but it is not present in the mainline.
Best regards,
Jacek Anaszewski
On 08/
Hi Baolin,
On 08/29/2018 11:48 AM, Baolin Wang wrote:
> Hi Jacek,
>
> On 29 August 2018 at 04:25, Jacek Anaszewski
> wrote:
>> On 08/25/2018 09:51 AM, Baolin Wang wrote:
>>> On 25 August 2018 at 04:44, Jacek Anaszewski
>>> wrote:
>>>> On 08/24
On 08/28/2018 11:13 PM, Bjorn Andersson wrote:
> On Tue 28 Aug 13:25 PDT 2018, Jacek Anaszewski wrote:
>
>> On 08/25/2018 09:51 AM, Baolin Wang wrote:
>>> On 25 August 2018 at 04:44, Jacek Anaszewski
>>> wrote:
>>>> On 08/24/2018 10:12 PM, Pavel Mach
Hi Rob,
Thank you for the patch.
On 08/28/2018 03:52 AM, Rob Herring wrote:
> In preparation to remove the node name pointer from struct device_node,
> convert printf users to use the %pOFn format specifier.
>
> Cc: Sakari Ailus
> Cc: Jacek Anaszewski
> Cc: Pavel Mac
On 08/25/2018 09:51 AM, Baolin Wang wrote:
> On 25 August 2018 at 04:44, Jacek Anaszewski
> wrote:
>> On 08/24/2018 10:12 PM, Pavel Machek wrote:
>>> On Fri 2018-08-24 21:49:50, Jacek Anaszewski wrote:
>>>> Hi Pavel,
>>>>
>>>> On 08/24/20
On 08/24/2018 10:12 PM, Pavel Machek wrote:
> On Fri 2018-08-24 21:49:50, Jacek Anaszewski wrote:
>> Hi Pavel,
>>
>> On 08/24/2018 12:11 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>>> I think that it would be more flexible if software pattern fallb
ns,
+ data->npatterns, data->repeat);
+ }
> If human can not tell the difference, it probably is.
>
> We may want to do something more formal later.
--
Best regards,
Jacek Anaszewski
t;>> + if (led->hvled_strings[i] == LM3697_HVLED_ASSIGNMENT)
>>> + control_bank_config |= 1 << i;
>>> + }
>>
>> Extra {}s.
>>
>>> + priv->regulator = devm_regulator_get(&priv->client->dev, "vled");
>>> + if (IS_ERR(priv->regulator))
>>> + priv->regulator = NULL;
>>
>> If vled regulator is specified in dt and _get fails, is it worth a
>> warning?
>>
>> Pavel
>>
>
> Do you want v6 or a new patch on top?
Please submit v6.
--
Best regards,
Jacek Anaszewski
+vled-supply = <&vbatt>;
>> +
>> +led@0 {
>> +reg = <0>;
>> + led-sources = <1 1 1>;
>> +label = "white:backlight_cluster";
>> +linux,default-trigger = "backlight";
>> +};
>> +}
>> +
>> +For more product information please see the link below:
>> +http://www.ti.com/lit/ds/symlink/lm3697.pdf
>
--
Best regards,
Jacek Anaszewski
Dan,
On 08/16/2018 10:44 PM, Dan Murphy wrote:
> Jacek
>
> On 08/16/2018 02:58 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> Thank you for the patch.
>>
>> I didn't review DT parsing details in v3, but now I've produced
>> diff between v3 and
p;client->dev, "Failed to allocate register map: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = lm3697_probe_dt(led);
> + if (ret)
> + return ret;
> +
> + ret = lm3697_init(led);
Let's simplify it:
return lm3697_init(led);
> +
> + return ret;
> +}
> +
> +static int lm3697_remove(struct i2c_client *client)
> +{
> + struct lm3697 *led = i2c_get_clientdata(client);
> + int ret;
> +
> + ret = regmap_update_bits(led->regmap, LM3697_CTRL_ENABLE,
> + LM3697_CTRL_A_B_EN, 0);
> + if (ret) {
> + dev_err(&led->client->dev, "Failed to disable the device\n");
> + return ret;
> + }
> +
> + if (led->enable_gpio)
> + gpiod_direction_output(led->enable_gpio, 0);
> +
> + if (led->regulator) {
> + ret = regulator_disable(led->regulator);
> + if (ret)
> + dev_err(&led->client->dev,
> + "Failed to disable regulator\n");
> + }
> +
> + mutex_destroy(&led->lock);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id lm3697_id[] = {
> + { "lm3697", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, lm3697_id);
> +
> +static const struct of_device_id of_lm3697_leds_match[] = {
> + { .compatible = "ti,lm3697", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_lm3697_leds_match);
> +
> +static struct i2c_driver lm3697_driver = {
> + .driver = {
> + .name = "lm3697",
> + .of_match_table = of_lm3697_leds_match,
> + },
> + .probe = lm3697_probe,
> + .remove = lm3697_remove,
> + .id_table = lm3697_id,
> +};
> +module_i2c_driver(lm3697_driver);
> +
> +MODULE_DESCRIPTION("Texas Instruments LM3697 LED driver");
> +MODULE_AUTHOR("Dan Murphy ");
> +MODULE_LICENSE("GPL v2");
>
--
Best regards,
Jacek Anaszewski
ot;;
> + reg = <0x36>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> + vled-supply = <&vbatt>;
> +
> + led@0 {
> + reg = <0>;
> + led-sources = <1 1 1>;
> + label = "white:backlight_cluster";
> + linux,default-trigger = "backlight";
> + };
> +}
> +
> +For more product information please see the link below:
> +http://www.ti.com/lit/ds/symlink/lm3697.pdf
>
--
Best regards,
Jacek Anaszewski
Dan,
On 08/15/2018 07:30 PM, Dan Murphy wrote:
> Jacek
>
> On 08/15/2018 11:56 AM, Jacek Anaszewski wrote:
>> Hi Dan,
>>
>> Thank you for the update.
>>
>> On 08/15/2018 06:17 PM, Dan Murphy wrote:
>>> Add the device tree bindings for the lm
+ reg = <0x36>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + enable-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> + vled-supply = <&vbatt>;
> +
> + led@0 {
> + reg = <0>;
> + led-sources = <0x1 0x2 0x3>;
led-sources = <1 1 1>;
> + label = "white:backlight_cluster";
> + linux,default-trigger = "backlight";
> + };
> +}
> +
> +For more product information please see the link below:
> +http://www.ti.com/lit/ds/symlink/lm3697.pdf
>
--
Best regards,
Jacek Anaszewski
meone who can actually test and confirm that the
netdev
trigger works for can devices.
Thanks,
Jacek Anaszewski
The following changes since commit ce397d215ccd07b8ae3f71db689aedb85d56ab40:
Linux 4.18-rc1 (2018-06-17 08:04:49 +0900)
are available in the git repository at:
Hi Baolin,
On 08/10/2018 05:26 PM, Baolin Wang wrote:
> Hi Jacek,
>
> On 9 August 2018 at 21:21, Jacek Anaszewski
> wrote:
>> Hi Baolin,
>>
>> On 08/09/2018 07:48 AM, Baolin Wang wrote:
>> [...]
>>>>>>> +static int
Dan,
On 08/09/2018 03:30 PM, Dan Murphy wrote:
> Jacek and Pavel
>
> On 08/09/2018 07:09 AM, Jacek Anaszewski wrote:
>> Dan,
>>
>> On 08/08/2018 11:45 PM, Dan Murphy wrote:
>>> Jacek
>>>
>>> On 08/08/2018 04:09 PM, Jacek Anaszewski wrote:
>
evel rise per
1ms. So, the pattern for rising edge could look like (assuming we
stop at 254):
0 1 2 1 4 1 6 1 8 1 10 1 ... 254 1
Now, I'm starting to wonder if we shouldn't have specialized trigger
for breathing patterns, that would accept brightness level change per
time period. Pattern trigger needs more flexibility and inferring if the
hardware can handle given series of pattern intervals would entail
unnecessary code complexity.
Such breathing trigger would require triplets comprised of
start brightness, end brightness and a duration of the brightness
transition.
--
Best regards,
Jacek Anaszewski
Dan,
On 08/08/2018 11:45 PM, Dan Murphy wrote:
> Jacek
>
> On 08/08/2018 04:09 PM, Jacek Anaszewski wrote:
>> Hi Dan,
>>
>> On 08/08/2018 11:04 PM, Dan Murphy wrote:
>>> On 08/08/2018 04:02 PM, Pavel Machek wrote:
>>>> Hi!
>>>>
>>
Hi Baolin,
On 08/08/2018 08:01 AM, Baolin Wang wrote:
> Hi Jacek,
>
> On 8 August 2018 at 05:54, Jacek Anaszewski
> wrote:
>> Hi Baolin,
>>
>> Thank you for addressing the review remarks.
>> Since the patch set is targeted for 4.19, then we have three week
k A can control 2
>>> LED strings and control
>>> bank b can control 1 LED string.
>>>
>>
>> Can we forget about the LED strings, and just expose the sinks as
>> Linux LED devices?
>
> 2 sinks 3 LED strings. How do you know which LED string is which and what
> bank it belongs
> to when setting the brightness. Each Bank has a separate register for
> brightness control.
Just a blind shot, without going into details - could you please check
if led-sources property documented in the common LED bindings couldn't
help here?
--
Best regards,
Jacek Anaszewski
Hi Michal.
Thanks for the heads-up.
"dt-" prefix is indeed more preferred than "dt: ".
Dan - I will fix it by myself, no need to resend.
Best regards,
Jacek Anaszewski
On 08/08/2018 09:56 AM, Michal Vokáč wrote:
> On 7.8.2018 18:04, Dan Murphy wrote:
>> Add the
are engine will be used.
Possible options coming to mind:
- an interface will be provided to determine max difference between
the settings supported by the hardware and the settings requested by
the user, that will result in aligning user's setting to the hardware
capabilities
- the above alignment rate will be predefined instead
- hardware engine will be used only if user requests supported settings
on the whole span of the requested pattern
- in each of the above cases it would be worth to think of the
interface to show the scope of the settings supported by hardware
The same issue applies to the already available timer trigger.
So far the policy implemented by the drivers implementing blink_set
op varies.
--
Best regards,
Jacek Anaszewski
data *data = led_cdev->trigger_data;
>>> + unsigned long res;
>>> + int err;
>>> +
>>> + err = kstrtoul(buf, 10, &res);
>>> + if (err)
>>> + return err;
>>> +
>>> + if (!led_cdev->pattern_set)
>>> + del_timer_sync(&data->timer);
>>
>> Is there a reason for not having this check under mutex?
>
> We will hold the mutex in pattern_trig_timer_function(), so if we do
> del_timer_sync() under the mutex protection, we may meet dead-lock
> issue. Moreover, the del_timer_sync() will make sure deactivating one
> timer is safe.
Ack.
--
Best regards,
Jacek Anaszewski
;
> +}
> +
> +module_init(pattern_trig_init);
> +module_exit(pattern_trig_exit);
> +
> +MODULE_AUTHOR("Raphael Teysseyre +MODULE_AUTHOR("Baolin Wang +MODULE_DESCRIPTION("LED Pattern trigger");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 834683d..c54712c 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -22,6 +22,7 @@
> #include
>
> struct device;
> +struct led_pattern;
> /*
> * LED Core
> */
> @@ -88,6 +89,11 @@ struct led_classdev {
>unsigned long *delay_on,
>unsigned long *delay_off);
>
> + int (*pattern_set)(struct led_classdev *led_cdev,
> +struct led_pattern *pattern, int len,
> +unsigned repeat);
> + int (*pattern_clear)(struct led_classdev *led_cdev);
> +
> struct device *dev;
> const struct attribute_group**groups;
>
> @@ -472,4 +478,14 @@ static inline void
> led_classdev_notify_brightness_hw_changed(
> struct led_classdev *led_cdev, enum led_brightness brightness) { }
> #endif
>
> +/**
> + * struct led_pattern - brightness value in a pattern
Since this structure describes single pattern interval and not
the whole pattern, please change its description to:
pattern interval settings
> + * @delta_t: delay until next entry, in milliseconds
@delta_t: pattern interval delay, in milliseconds
> + * @brightness: brightness at time = 0
@brightness: pattern interval brightness
> + */
> +struct led_pattern {
> + int delta_t;
> + int brightness;
> +};
> +
> #endif /* __LINUX_LEDS_H_INCLUDED */
>
--
Best regards,
Jacek Anaszewski
",
> + .activate = pattern_trig_activate,
> + .deactivate = pattern_trig_deactivate,
> + .groups = pattern_trig_groups,
> +};
> +
> +static int __init pattern_trig_init(void)
> +{
> + return led_trigger_register(&pattern_led_trigger);
> +}
> +
> +static void __exit pattern_trig_exit(void)
> +{
> + led_trigger_unregister(&pattern_led_trigger);
> +}
> +
> +module_init(pattern_trig_init);
> +module_exit(pattern_trig_exit);
> +
> +MODULE_AUTHOR("Raphael Teysseyre +MODULE_AUTHOR("Baolin Wang +MODULE_DESCRIPTION("LED Pattern trigger");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 834683d..c54712c 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -22,6 +22,7 @@
> #include
>
> struct device;
> +struct led_pattern;
> /*
> * LED Core
> */
> @@ -88,6 +89,11 @@ struct led_classdev {
>unsigned long *delay_on,
>unsigned long *delay_off);
>
> + int (*pattern_set)(struct led_classdev *led_cdev,
> +struct led_pattern *pattern, int len,
> +unsigned repeat);
> + int (*pattern_clear)(struct led_classdev *led_cdev);
> +
> struct device *dev;
> const struct attribute_group**groups;
>
> @@ -472,4 +478,14 @@ static inline void
> led_classdev_notify_brightness_hw_changed(
> struct led_classdev *led_cdev, enum led_brightness brightness) { }
> #endif
>
> +/**
> + * struct led_pattern - brightness value in a pattern
> + * @delta_t: delay until next entry, in milliseconds
> + * @brightness: brightness at time = 0
> + */
> +struct led_pattern {
> + int delta_t;
> + int brightness;
> +};
> +
> #endif /* __LINUX_LEDS_H_INCLUDED */
>
--
Best regards,
Jacek Anaszewski
Hi Dan,
Thank you for the patch.
Typo in the patch title:
s/di: /dt-/
And maybe let's change the following text to:
"Add bindings for lm3697 LED driver"
Best regards,
Jacek Anaszewski
On 08/03/2018 05:02 PM, Dan Murphy wrote:
> Introduce the device tree bindings for the l
Hi Dan,
Thank you for the patch.
Typo in the patch title:
s/di: /dt-/
And maybe let's change the following text to:
"Add bindings for lm3697 LED driver"
Best regards,
Jacek Anaszewski
On 08/03/2018 05:02 PM, Dan Murphy wrote:
> Introduce the device tree bindings for the l
601 - 700 of 1409 matches
Mail list logo
