* [PATCH] leds: oneshot - Allow default delay to be passed as an argument
@ 2016-09-03 9:03 Amitesh Singh
2016-09-03 13:44 ` Jacek Anaszewski
0 siblings, 1 reply; 7+ messages in thread
From: Amitesh Singh @ 2016-09-03 9:03 UTC (permalink / raw)
To: Richard Purdie, Anaszewski, linux-leds
This patch facilates the blink delay to be passed as
an argument at the time of module loading.
e.g.
insmod ledtrigg-oneshot.ko default_delay=100
---
drivers/leds/trigger/ledtrig-oneshot.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
index b8ea9f0..95933a1 100644
--- a/drivers/leds/trigger/ledtrig-oneshot.c
+++ b/drivers/leds/trigger/ledtrig-oneshot.c
@@ -22,6 +22,9 @@
#define DEFAULT_DELAY 100
+static unsigned long default_delay = DEFAULT_DELAY;
+module_param(default_delay, ulong, S_IRUGO|S_IWUSR);
+
struct oneshot_trig_data {
unsigned int invert;
};
@@ -146,8 +149,8 @@ static void oneshot_trig_activate(struct led_classdev *led_cdev)
if (rc)
goto err_out_invert;
- led_cdev->blink_delay_on = DEFAULT_DELAY;
- led_cdev->blink_delay_off = DEFAULT_DELAY;
+ led_cdev->blink_delay_on = default_delay;
+ led_cdev->blink_delay_off = default_delay;
led_cdev->activated = true;
--
2.7.4
This patch facilates the blink delay to be passed as
an argument at the time of module loading.
e.g.
insmod ledtrigg-oneshot.ko default_delay=100
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] leds: oneshot - Allow default delay to be passed as an argument
2016-09-03 9:03 [PATCH] leds: oneshot - Allow default delay to be passed as an argument Amitesh Singh
@ 2016-09-03 13:44 ` Jacek Anaszewski
2016-09-03 15:39 ` Amitesh Singh
[not found] ` <CABKcAmXSwgMbopurjrNU7fgPu58W-ZDbDi6VuUn2P2zOGATYLQ@mail.gmail.com>
0 siblings, 2 replies; 7+ messages in thread
From: Jacek Anaszewski @ 2016-09-03 13:44 UTC (permalink / raw)
To: Amitesh Singh, Richard Purdie, Anaszewski, linux-leds
Hi Amitesh,
On 09/03/2016 11:03 AM, Amitesh Singh wrote:
> This patch facilates the blink delay to be passed as
> an argument at the time of module loading.
> e.g.
> insmod ledtrigg-oneshot.ko default_delay=100
> ---
> drivers/leds/trigger/ledtrig-oneshot.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
> index b8ea9f0..95933a1 100644
> --- a/drivers/leds/trigger/ledtrig-oneshot.c
> +++ b/drivers/leds/trigger/ledtrig-oneshot.c
> @@ -22,6 +22,9 @@
>
> #define DEFAULT_DELAY 100
>
> +static unsigned long default_delay = DEFAULT_DELAY;
> +module_param(default_delay, ulong, S_IRUGO|S_IWUSR);
> +
> struct oneshot_trig_data {
> unsigned int invert;
> };
> @@ -146,8 +149,8 @@ static void oneshot_trig_activate(struct led_classdev *led_cdev)
> if (rc)
> goto err_out_invert;
>
> - led_cdev->blink_delay_on = DEFAULT_DELAY;
> - led_cdev->blink_delay_off = DEFAULT_DELAY;
> + led_cdev->blink_delay_on = default_delay;
> + led_cdev->blink_delay_off = default_delay;
>
> led_cdev->activated = true;
>
>
Why do you need this module parameter? You can change
delay_on and delay_off values from sysfs.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] leds: oneshot - Allow default delay to be passed as an argument
2016-09-03 13:44 ` Jacek Anaszewski
@ 2016-09-03 15:39 ` Amitesh Singh
[not found] ` <CABKcAmXSwgMbopurjrNU7fgPu58W-ZDbDi6VuUn2P2zOGATYLQ@mail.gmail.com>
1 sibling, 0 replies; 7+ messages in thread
From: Amitesh Singh @ 2016-09-03 15:39 UTC (permalink / raw)
To: Jacek Anaszewski; +Cc: Richard Purdie, Anaszewski, linux-leds
Hello Jacek,
On Sat, Sep 3, 2016 at 7:14 PM, Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:
> Hi Amitesh,
>
>
> On 09/03/2016 11:03 AM, Amitesh Singh wrote:
>>
>> This patch facilates the blink delay to be passed as
>> an argument at the time of module loading.
>> e.g.
>> insmod ledtrigg-oneshot.ko default_delay=100
>> ---
>> drivers/leds/trigger/ledtrig-oneshot.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/leds/trigger/ledtrig-oneshot.c
>> b/drivers/leds/trigger/ledtrig-oneshot.c
>> index b8ea9f0..95933a1 100644
>> --- a/drivers/leds/trigger/ledtrig-oneshot.c
>> +++ b/drivers/leds/trigger/ledtrig-oneshot.c
>> @@ -22,6 +22,9 @@
>>
>> #define DEFAULT_DELAY 100
>>
>> +static unsigned long default_delay = DEFAULT_DELAY;
>> +module_param(default_delay, ulong, S_IRUGO|S_IWUSR);
>> +
>> struct oneshot_trig_data {
>> unsigned int invert;
>> };
>> @@ -146,8 +149,8 @@ static void oneshot_trig_activate(struct led_classdev
>> *led_cdev)
>> if (rc)
>> goto err_out_invert;
>>
>> - led_cdev->blink_delay_on = DEFAULT_DELAY;
>> - led_cdev->blink_delay_off = DEFAULT_DELAY;
>> + led_cdev->blink_delay_on = default_delay;
>> + led_cdev->blink_delay_off = default_delay;
>>
>> led_cdev->activated = true;
>>
>>
>
> Why do you need this module parameter? You can change
> delay_on and delay_off values from sysfs.
>
Yes, indeed. If someone wants to change blink_delay, he has to change
both delay_on and delay_off in case you want to blink with constant
time ON and OFF (1 / (T + T) = 1/2T)
This patch facilitates you to modify delay in one step in case of
delay on and off are same which I think, is most used case.
> --
> Best regards,
> Jacek Anaszewski
Regards
Amitesh Singh
^ permalink raw reply [flat|nested] 7+ messages in thread[parent not found: <CABKcAmXSwgMbopurjrNU7fgPu58W-ZDbDi6VuUn2P2zOGATYLQ@mail.gmail.com>]
* Re: [PATCH] leds: oneshot - Allow default delay to be passed as an argument
[not found] ` <CABKcAmXSwgMbopurjrNU7fgPu58W-ZDbDi6VuUn2P2zOGATYLQ@mail.gmail.com>
@ 2016-09-03 18:12 ` Jacek Anaszewski
2016-09-06 15:05 ` Amitesh Singh
0 siblings, 1 reply; 7+ messages in thread
From: Jacek Anaszewski @ 2016-09-03 18:12 UTC (permalink / raw)
To: Amitesh Singh; +Cc: Richard Purdie, Anaszewski, linux-leds
On 09/03/2016 05:35 PM, Amitesh Singh wrote:
> Hello Jacek,
>
>
> On Sat, Sep 3, 2016 at 7:14 PM, Jacek Anaszewski
> <jacek.anaszewski@gmail.com <mailto:jacek.anaszewski@gmail.com>> wrote:
>
> Hi Amitesh,
>
>
> On 09/03/2016 11:03 AM, Amitesh Singh wrote:
>
> This patch facilates the blink delay to be passed as
> an argument at the time of module loading.
> e.g.
> insmod ledtrigg-oneshot.ko default_delay=100
> ---
> drivers/leds/trigger/ledtrig-oneshot.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-oneshot.c
> b/drivers/leds/trigger/ledtrig-oneshot.c
> index b8ea9f0..95933a1 100644
> --- a/drivers/leds/trigger/ledtrig-oneshot.c
> +++ b/drivers/leds/trigger/ledtrig-oneshot.c
> @@ -22,6 +22,9 @@
>
> #define DEFAULT_DELAY 100
>
> +static unsigned long default_delay = DEFAULT_DELAY;
> +module_param(default_delay, ulong, S_IRUGO|S_IWUSR);
> +
> struct oneshot_trig_data {
> unsigned int invert;
> };
> @@ -146,8 +149,8 @@ static void oneshot_trig_activate(struct
> led_classdev *led_cdev)
> if (rc)
> goto err_out_invert;
>
> - led_cdev->blink_delay_on = DEFAULT_DELAY;
> - led_cdev->blink_delay_off = DEFAULT_DELAY;
> + led_cdev->blink_delay_on = default_delay;
> + led_cdev->blink_delay_off = default_delay;
>
> led_cdev->activated = true;
>
>
>
>
> Why do you need this module parameter? You can change
> delay_on and delay_off values from sysfs.
>
> Yes, indeed. If someone wants to change blink_delay, he has to change
> both delay_on and delay_off in case you want to blink with constant time
> ON and OFF (1 / (T + T) = 1/2T)
> This patch facilitates you to modify delay in one step in case of delay
> on and off are same which I think, is most used case.
I don't think this is real improvement. We spare two sysfs file
write operations only in case the default_delay param value passed
on module loading won't need to be altered later on.
Moreover, three sysfs file writes needed for configuring oneshot
trigger (e.g. echo "oneshot" > trigger; echo 100 > delay_on;
echo 50 > delay_off) are not too expensive taking into account
usual frequency of setting LED trigger (surely less often than
once per second).
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] leds: oneshot - Allow default delay to be passed as an argument
2016-09-03 18:12 ` Jacek Anaszewski
@ 2016-09-06 15:05 ` Amitesh Singh
2016-09-06 18:52 ` Jacek Anaszewski
0 siblings, 1 reply; 7+ messages in thread
From: Amitesh Singh @ 2016-09-06 15:05 UTC (permalink / raw)
To: Jacek Anaszewski; +Cc: Richard Purdie, Anaszewski, linux-leds
On Sat, Sep 3, 2016 at 11:42 PM, Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:
> On 09/03/2016 05:35 PM, Amitesh Singh wrote:
>>
>> Hello Jacek,
>>
>>
>> On Sat, Sep 3, 2016 at 7:14 PM, Jacek Anaszewski
>> <jacek.anaszewski@gmail.com <mailto:jacek.anaszewski@gmail.com>> wrote:
>>
>> Hi Amitesh,
>>
>>
>> On 09/03/2016 11:03 AM, Amitesh Singh wrote:
>>
>> This patch facilates the blink delay to be passed as
>> an argument at the time of module loading.
>> e.g.
>> insmod ledtrigg-oneshot.ko default_delay=100
>> ---
>> drivers/leds/trigger/ledtrig-oneshot.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/leds/trigger/ledtrig-oneshot.c
>> b/drivers/leds/trigger/ledtrig-oneshot.c
>> index b8ea9f0..95933a1 100644
>> --- a/drivers/leds/trigger/ledtrig-oneshot.c
>> +++ b/drivers/leds/trigger/ledtrig-oneshot.c
>> @@ -22,6 +22,9 @@
>>
>> #define DEFAULT_DELAY 100
>>
>> +static unsigned long default_delay = DEFAULT_DELAY;
>> +module_param(default_delay, ulong, S_IRUGO|S_IWUSR);
>> +
>> struct oneshot_trig_data {
>> unsigned int invert;
>> };
>> @@ -146,8 +149,8 @@ static void oneshot_trig_activate(struct
>> led_classdev *led_cdev)
>> if (rc)
>> goto err_out_invert;
>>
>> - led_cdev->blink_delay_on = DEFAULT_DELAY;
>> - led_cdev->blink_delay_off = DEFAULT_DELAY;
>> + led_cdev->blink_delay_on = default_delay;
>> + led_cdev->blink_delay_off = default_delay;
>>
>> led_cdev->activated = true;
>>
>>
>>
>>
>> Why do you need this module parameter? You can change
>> delay_on and delay_off values from sysfs.
>>
>> Yes, indeed. If someone wants to change blink_delay, he has to change
>> both delay_on and delay_off in case you want to blink with constant time
>> ON and OFF (1 / (T + T) = 1/2T)
>> This patch facilitates you to modify delay in one step in case of delay
>> on and off are same which I think, is most used case.
>
>
> I don't think this is real improvement. We spare two sysfs file
> write operations only in case the default_delay param value passed
> on module loading won't need to be altered later on.
>
> Moreover, three sysfs file writes needed for configuring oneshot
> trigger (e.g. echo "oneshot" > trigger; echo 100 > delay_on;
> echo 50 > delay_off) are not too expensive taking into account
> usual frequency of setting LED trigger (surely less often than
> once per second).
>
>
This is for configuring the led trigger delay on/off parameter when I
instantiate the driver from kernel space.
I am instantiating oneshot trigger at the time of boot up.
> --
> Best regards,
> Jacek Anaszewski
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] leds: oneshot - Allow default delay to be passed as an argument
2016-09-06 15:05 ` Amitesh Singh
@ 2016-09-06 18:52 ` Jacek Anaszewski
0 siblings, 0 replies; 7+ messages in thread
From: Jacek Anaszewski @ 2016-09-06 18:52 UTC (permalink / raw)
To: Amitesh Singh; +Cc: Richard Purdie, Anaszewski, linux-leds
On 09/06/2016 05:05 PM, Amitesh Singh wrote:
> On Sat, Sep 3, 2016 at 11:42 PM, Jacek Anaszewski
> <jacek.anaszewski@gmail.com> wrote:
>> On 09/03/2016 05:35 PM, Amitesh Singh wrote:
>>>
>>> Hello Jacek,
>>>
>>>
>>> On Sat, Sep 3, 2016 at 7:14 PM, Jacek Anaszewski
>>> <jacek.anaszewski@gmail.com <mailto:jacek.anaszewski@gmail.com>> wrote:
>>>
>>> Hi Amitesh,
>>>
>>>
>>> On 09/03/2016 11:03 AM, Amitesh Singh wrote:
>>>
>>> This patch facilates the blink delay to be passed as
>>> an argument at the time of module loading.
>>> e.g.
>>> insmod ledtrigg-oneshot.ko default_delay=100
>>> ---
>>> drivers/leds/trigger/ledtrig-oneshot.c | 7 +++++--
>>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/leds/trigger/ledtrig-oneshot.c
>>> b/drivers/leds/trigger/ledtrig-oneshot.c
>>> index b8ea9f0..95933a1 100644
>>> --- a/drivers/leds/trigger/ledtrig-oneshot.c
>>> +++ b/drivers/leds/trigger/ledtrig-oneshot.c
>>> @@ -22,6 +22,9 @@
>>>
>>> #define DEFAULT_DELAY 100
>>>
>>> +static unsigned long default_delay = DEFAULT_DELAY;
>>> +module_param(default_delay, ulong, S_IRUGO|S_IWUSR);
>>> +
>>> struct oneshot_trig_data {
>>> unsigned int invert;
>>> };
>>> @@ -146,8 +149,8 @@ static void oneshot_trig_activate(struct
>>> led_classdev *led_cdev)
>>> if (rc)
>>> goto err_out_invert;
>>>
>>> - led_cdev->blink_delay_on = DEFAULT_DELAY;
>>> - led_cdev->blink_delay_off = DEFAULT_DELAY;
>>> + led_cdev->blink_delay_on = default_delay;
>>> + led_cdev->blink_delay_off = default_delay;
>>>
>>> led_cdev->activated = true;
>>>
>>>
>>>
>>>
>>> Why do you need this module parameter? You can change
>>> delay_on and delay_off values from sysfs.
>>>
>>> Yes, indeed. If someone wants to change blink_delay, he has to change
>>> both delay_on and delay_off in case you want to blink with constant time
>>> ON and OFF (1 / (T + T) = 1/2T)
>>> This patch facilitates you to modify delay in one step in case of delay
>>> on and off are same which I think, is most used case.
>>
>>
>> I don't think this is real improvement. We spare two sysfs file
>> write operations only in case the default_delay param value passed
>> on module loading won't need to be altered later on.
>>
>> Moreover, three sysfs file writes needed for configuring oneshot
>> trigger (e.g. echo "oneshot" > trigger; echo 100 > delay_on;
>> echo 50 > delay_off) are not too expensive taking into account
>> usual frequency of setting LED trigger (surely less often than
>> once per second).
>>
>>
> This is for configuring the led trigger delay on/off parameter when I
> instantiate the driver from kernel space.
> I am instantiating oneshot trigger at the time of boot up.
In other use cases, when the delay intervals need to be altered
later, it doesn't introduce any gain. Even in your use case
the gain is almost unnoticeable.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] leds: oneshot - Allow default delay to be passed as an argument
@ 2016-09-03 8:39 Amitesh Singh
0 siblings, 0 replies; 7+ messages in thread
From: Amitesh Singh @ 2016-09-03 8:39 UTC (permalink / raw)
To: Richard Purdie, acek Anaszewski, linux-leds
This patch facilates the blink delay to be passed as
an argument at the time of module loading.
e.g.
insmod ledtrigg-oneshot.ko default_delay=100
---
drivers/leds/trigger/ledtrig-oneshot.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
index b8ea9f0..accb241 100644
--- a/drivers/leds/trigger/ledtrig-oneshot.c
+++ b/drivers/leds/trigger/ledtrig-oneshot.c
@@ -22,6 +22,9 @@
#define DEFAULT_DELAY 100
+static unsigned long default_delay = DEFAULT_DELAY;
+module_param(default_delay, ulong, S_IRUGO|S_IWUSR);
+
struct oneshot_trig_data {
unsigned int invert;
};
--
2.7.4
This patch facilates the blink delay to be passed as
an argument at the time of module loading.
e.g.
insmod ledtrigg-oneshot.ko default_delay=100
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-09-06 19:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-03 9:03 [PATCH] leds: oneshot - Allow default delay to be passed as an argument Amitesh Singh
2016-09-03 13:44 ` Jacek Anaszewski
2016-09-03 15:39 ` Amitesh Singh
[not found] ` <CABKcAmXSwgMbopurjrNU7fgPu58W-ZDbDi6VuUn2P2zOGATYLQ@mail.gmail.com>
2016-09-03 18:12 ` Jacek Anaszewski
2016-09-06 15:05 ` Amitesh Singh
2016-09-06 18:52 ` Jacek Anaszewski
-- strict thread matches above, loose matches on Subject: below --
2016-09-03 8:39 Amitesh Singh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).