leds: oneshot - Allow default delay to be passed as an argument
PATCH 1/1 leds: oneshot - Allow default delay to be passed as an argument
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(-)
drivers/leds/trigger/ledtrig-oneshot.c +5 -2
@@ -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;
Signature · 6 lines
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
Hi Amitesh,
Quoted 24 lines · Amitesh Singh
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(-)drivers/leds/trigger/ledtrig-oneshot.c +3
-0@@ -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; };
drivers/leds/trigger/ledtrig-oneshot.c static void oneshot_trig_activate(struct led_classdev *led_cdev) · line 156
Quoted 10 lines
drivers/leds/trigger/ledtrig-oneshot.c +2
-2@@ -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.
Signature · 2 lines
Best regards, Jacek Anaszewski
Hello Jacek,
Quoted 38 lines · Jacek Anaszewski
Hi Amitesh,
Quoted 34 lines · Amitesh Singh
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(-)drivers/leds/trigger/ledtrig-oneshot.c +5
-2@@ -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;
Quoted 7 lines
drivers/leds/trigger/ledtrig-oneshot.c static void oneshot_trig_activate(struct led_classdev *led_cdev) · line 155
Quoted 3 lines
drivers/leds/trigger/ledtrig-oneshot.c +0
-0led_cdev->activated = true;Why do you need this module parameter? You can change
delay_onanddelay_offvalues 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.
Quoted 3 lines
--
Best regards,
Jacek Anaszewski
Regards
Amitesh Singh
A message that is not in the archive (sent off-list or not yet received): CABKcAmXSwgMbopurjrNU7fgPu58W-ZDbDi6VuUn2P2zOGATYLQ@mail.gmail.com
Quoted 52 lines · Amitesh Singh
Hello Jacek,
On Sat, Sep 3, 2016 at 7:14 PM, Jacek Anaszewski <[email protected] <mailto:[email protected]>> 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.
Quoted 5 lines
Yes, indeed. If someone wants to change
blink_delay, he has to change bothdelay_onanddelay_offin 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).
Signature · 2 lines
Best regards, Jacek Anaszewski
Quoted 65 lines · Jacek Anaszewski
Quoted 53 lines · Amitesh Singh
Hello Jacek,
On Sat, Sep 3, 2016 at 7:14 PM, Jacek Anaszewski <[email protected] <mailto:[email protected]>> 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.Quoted 5 lines
Yes, indeed. If someone wants to change
blink_delay, he has to change bothdelay_onanddelay_offin 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_delayparam value passed on module loading won't need to be altered later on.
Quoted 7 lines
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.
Quoted 3 lines
--
Best regards,
Jacek Anaszewski
Quoted 74 lines · Amitesh Singh
Quoted 65 lines · Jacek Anaszewski
Quoted 53 lines · Amitesh Singh
Hello Jacek,
On Sat, Sep 3, 2016 at 7:14 PM, Jacek Anaszewski <[email protected] <mailto:[email protected]>> 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.Quoted 5 lines
Yes, indeed. If someone wants to change
blink_delay, he has to change bothdelay_onanddelay_offin 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_delayparam value passed on module loading won't need to be altered later on.Quoted 7 lines
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).
Quoted 3 lines
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.
Signature · 2 lines
Best regards, Jacek Anaszewski