* [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting @ 2024-08-16 10:26 FUKAUMI Naoki 2024-08-21 16:11 ` Lee Jones 0 siblings, 1 reply; 11+ messages in thread From: FUKAUMI Naoki @ 2024-08-16 10:26 UTC (permalink / raw) To: linux-leds; +Cc: pavel, lee, FUKAUMI Naoki from drivers/leds/leds-pwm.c:led_pwm_set(), /* * Disabling a PWM doesn't guarantee that it emits the inactive level. * So keep it on. Only for suspending the PWM should be disabled because * otherwise it refuses to suspend. The possible downside is that the * LED might stay (or even go) on. */ do the same in led_pwm_mc_set(). this fixes LEDs light up without any setting (i.e. brightness is 0) on Radxa E25. Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> --- drivers/leds/rgb/leds-pwm-multicolor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c index e1a81e0109e8..7155339c075e 100644 --- a/drivers/leds/rgb/leds-pwm-multicolor.c +++ b/drivers/leds/rgb/leds-pwm-multicolor.c @@ -50,7 +50,7 @@ static int led_pwm_mc_set(struct led_classdev *cdev, duty = priv->leds[i].state.period - duty; priv->leds[i].state.duty_cycle = duty; - priv->leds[i].state.enabled = duty > 0; + priv->leds[i].state.enabled = !(cdev->flags & LED_SUSPENDED); ret = pwm_apply_might_sleep(priv->leds[i].pwm, &priv->leds[i].state); if (ret) -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-08-16 10:26 [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting FUKAUMI Naoki @ 2024-08-21 16:11 ` Lee Jones 2024-08-21 21:26 ` FUKAUMI Naoki 0 siblings, 1 reply; 11+ messages in thread From: Lee Jones @ 2024-08-21 16:11 UTC (permalink / raw) To: FUKAUMI Naoki; +Cc: linux-leds, pavel On Fri, 16 Aug 2024, FUKAUMI Naoki wrote: > from drivers/leds/leds-pwm.c:led_pwm_set(), > > /* > * Disabling a PWM doesn't guarantee that it emits the inactive level. > * So keep it on. Only for suspending the PWM should be disabled because > * otherwise it refuses to suspend. The possible downside is that the > * LED might stay (or even go) on. > */ > > do the same in led_pwm_mc_set(). > > this fixes LEDs light up without any setting (i.e. brightness is 0) on > Radxa E25. Please redraft the commit message into proper sentences, complete with correct grammar. All sentences should start with an uppercase char. Copying comment-blocks into commit messages to save authors from writing one isn't really the done thing. > Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> > --- > drivers/leds/rgb/leds-pwm-multicolor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c > index e1a81e0109e8..7155339c075e 100644 > --- a/drivers/leds/rgb/leds-pwm-multicolor.c > +++ b/drivers/leds/rgb/leds-pwm-multicolor.c > @@ -50,7 +50,7 @@ static int led_pwm_mc_set(struct led_classdev *cdev, > duty = priv->leds[i].state.period - duty; > > priv->leds[i].state.duty_cycle = duty; > - priv->leds[i].state.enabled = duty > 0; > + priv->leds[i].state.enabled = !(cdev->flags & LED_SUSPENDED); > ret = pwm_apply_might_sleep(priv->leds[i].pwm, > &priv->leds[i].state); > if (ret) > -- > 2.43.0 > -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-08-21 16:11 ` Lee Jones @ 2024-08-21 21:26 ` FUKAUMI Naoki 2024-08-22 7:22 ` FUKAUMI Naoki 0 siblings, 1 reply; 11+ messages in thread From: FUKAUMI Naoki @ 2024-08-21 21:26 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds, pavel Hi, On 8/22/24 01:11, Lee Jones wrote: > On Fri, 16 Aug 2024, FUKAUMI Naoki wrote: > >> from drivers/leds/leds-pwm.c:led_pwm_set(), >> >> /* >> * Disabling a PWM doesn't guarantee that it emits the inactive level. >> * So keep it on. Only for suspending the PWM should be disabled because >> * otherwise it refuses to suspend. The possible downside is that the >> * LED might stay (or even go) on. >> */ >> >> do the same in led_pwm_mc_set(). >> >> this fixes LEDs light up without any setting (i.e. brightness is 0) on >> Radxa E25. > > Please redraft the commit message into proper sentences, complete with > correct grammar. All sentences should start with an uppercase char. Thank you for your review! I'll do it in next ver. > Copying comment-blocks into commit messages to save authors from writing > one isn't really the done thing. Sorry, I could not understand this part. What should I do for this? Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. >> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >> --- >> drivers/leds/rgb/leds-pwm-multicolor.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/rgb/leds-pwm-multicolor.c >> index e1a81e0109e8..7155339c075e 100644 >> --- a/drivers/leds/rgb/leds-pwm-multicolor.c >> +++ b/drivers/leds/rgb/leds-pwm-multicolor.c >> @@ -50,7 +50,7 @@ static int led_pwm_mc_set(struct led_classdev *cdev, >> duty = priv->leds[i].state.period - duty; >> >> priv->leds[i].state.duty_cycle = duty; >> - priv->leds[i].state.enabled = duty > 0; >> + priv->leds[i].state.enabled = !(cdev->flags & LED_SUSPENDED); >> ret = pwm_apply_might_sleep(priv->leds[i].pwm, >> &priv->leds[i].state); >> if (ret) >> -- >> 2.43.0 >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-08-21 21:26 ` FUKAUMI Naoki @ 2024-08-22 7:22 ` FUKAUMI Naoki 2024-08-22 8:24 ` Lee Jones 2024-12-10 7:36 ` FUKAUMI Naoki 0 siblings, 2 replies; 11+ messages in thread From: FUKAUMI Naoki @ 2024-08-22 7:22 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds, pavel Hi, On 8/22/24 06:26, FUKAUMI Naoki wrote: > Hi, > > On 8/22/24 01:11, Lee Jones wrote: >> On Fri, 16 Aug 2024, FUKAUMI Naoki wrote: >> >>> from drivers/leds/leds-pwm.c:led_pwm_set(), >>> >>> /* >>> * Disabling a PWM doesn't guarantee that it emits the inactive level. >>> * So keep it on. Only for suspending the PWM should be disabled >>> because >>> * otherwise it refuses to suspend. The possible downside is that the >>> * LED might stay (or even go) on. >>> */ >>> >>> do the same in led_pwm_mc_set(). >>> >>> this fixes LEDs light up without any setting (i.e. brightness is 0) on >>> Radxa E25. >> >> Please redraft the commit message into proper sentences, complete with >> correct grammar. All sentences should start with an uppercase char. > > Thank you for your review! > > I'll do it in next ver. > >> Copying comment-blocks into commit messages to save authors from writing >> one isn't really the done thing. > > Sorry, I could not understand this part. What should I do for this? I (probably) understand what you said. I need to explain detail in my own words and use correct English. unfortunately my knowledge about electrical circuits and English is not sufficient for now. I need some time to learn some things for writing "why my fix is correct". then, I will not be able to send v2 so soon. if someone who understand well about this problem and the fix, feel free to reuse my patch (or make better patch). Best regards -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. > Best regards, > > -- > FUKAUMI Naoki > Radxa Computer (Shenzhen) Co., Ltd. > >>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >>> --- >>> drivers/leds/rgb/leds-pwm-multicolor.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c >>> b/drivers/leds/rgb/leds-pwm-multicolor.c >>> index e1a81e0109e8..7155339c075e 100644 >>> --- a/drivers/leds/rgb/leds-pwm-multicolor.c >>> +++ b/drivers/leds/rgb/leds-pwm-multicolor.c >>> @@ -50,7 +50,7 @@ static int led_pwm_mc_set(struct led_classdev *cdev, >>> duty = priv->leds[i].state.period - duty; >>> priv->leds[i].state.duty_cycle = duty; >>> - priv->leds[i].state.enabled = duty > 0; >>> + priv->leds[i].state.enabled = !(cdev->flags & LED_SUSPENDED); >>> ret = pwm_apply_might_sleep(priv->leds[i].pwm, >>> &priv->leds[i].state); >>> if (ret) >>> -- >>> 2.43.0 >>> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-08-22 7:22 ` FUKAUMI Naoki @ 2024-08-22 8:24 ` Lee Jones 2024-12-10 7:36 ` FUKAUMI Naoki 1 sibling, 0 replies; 11+ messages in thread From: Lee Jones @ 2024-08-22 8:24 UTC (permalink / raw) To: FUKAUMI Naoki; +Cc: linux-leds, pavel On Thu, 22 Aug 2024, FUKAUMI Naoki wrote: > Hi, > > On 8/22/24 06:26, FUKAUMI Naoki wrote: > > Hi, > > > > On 8/22/24 01:11, Lee Jones wrote: > > > On Fri, 16 Aug 2024, FUKAUMI Naoki wrote: > > > > > > > from drivers/leds/leds-pwm.c:led_pwm_set(), > > > > > > > > /* > > > > * Disabling a PWM doesn't guarantee that it emits the inactive level. > > > > * So keep it on. Only for suspending the PWM should be > > > > disabled because > > > > * otherwise it refuses to suspend. The possible downside is that the > > > > * LED might stay (or even go) on. > > > > */ > > > > > > > > do the same in led_pwm_mc_set(). > > > > > > > > this fixes LEDs light up without any setting (i.e. brightness is 0) on > > > > Radxa E25. > > > > > > Please redraft the commit message into proper sentences, complete with > > > correct grammar. All sentences should start with an uppercase char. > > > > Thank you for your review! > > > > I'll do it in next ver. > > > > > Copying comment-blocks into commit messages to save authors from writing > > > one isn't really the done thing. > > > > Sorry, I could not understand this part. What should I do for this? > > I (probably) understand what you said. I need to explain detail in my own > words and use correct English. > > unfortunately my knowledge about electrical circuits and English is not > sufficient for now. > I need some time to learn some things for writing "why my fix is correct". > > then, I will not be able to send v2 so soon. if someone who understand well > about this problem and the fix, feel free to reuse my patch (or make better > patch). You understood enough to read the comment and realise that it was related. Simply reword the comment block that you copied and pasted to explain why this is also relevant to your device. This is the sort of thing that AI might be able to help you with. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-08-22 7:22 ` FUKAUMI Naoki 2024-08-22 8:24 ` Lee Jones @ 2024-12-10 7:36 ` FUKAUMI Naoki 2024-12-12 19:04 ` Lee Jones 1 sibling, 1 reply; 11+ messages in thread From: FUKAUMI Naoki @ 2024-12-10 7:36 UTC (permalink / raw) To: linux-leds Hi, Could you help me(write good commit message), anyone? Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. On 8/22/24 16:22, FUKAUMI Naoki wrote: > Hi, > > On 8/22/24 06:26, FUKAUMI Naoki wrote: >> Hi, >> >> On 8/22/24 01:11, Lee Jones wrote: >>> On Fri, 16 Aug 2024, FUKAUMI Naoki wrote: >>> >>>> from drivers/leds/leds-pwm.c:led_pwm_set(), >>>> >>>> /* >>>> * Disabling a PWM doesn't guarantee that it emits the inactive level. >>>> * So keep it on. Only for suspending the PWM should be disabled >>>> because >>>> * otherwise it refuses to suspend. The possible downside is that the >>>> * LED might stay (or even go) on. >>>> */ >>>> >>>> do the same in led_pwm_mc_set(). >>>> >>>> this fixes LEDs light up without any setting (i.e. brightness is 0) on >>>> Radxa E25. >>> >>> Please redraft the commit message into proper sentences, complete with >>> correct grammar. All sentences should start with an uppercase char. >> >> Thank you for your review! >> >> I'll do it in next ver. >> >>> Copying comment-blocks into commit messages to save authors from writing >>> one isn't really the done thing. >> >> Sorry, I could not understand this part. What should I do for this? > > I (probably) understand what you said. I need to explain detail in my > own words and use correct English. > > unfortunately my knowledge about electrical circuits and English is not > sufficient for now. > I need some time to learn some things for writing "why my fix is correct". > > then, I will not be able to send v2 so soon. if someone who understand > well about this problem and the fix, feel free to reuse my patch (or > make better patch). > > Best regards > > -- > FUKAUMI Naoki > Radxa Computer (Shenzhen) Co., Ltd. > >> Best regards, >> >> -- >> FUKAUMI Naoki >> Radxa Computer (Shenzhen) Co., Ltd. >> >>>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com> >>>> --- >>>> drivers/leds/rgb/leds-pwm-multicolor.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/leds/rgb/leds-pwm-multicolor.c b/drivers/leds/ >>>> rgb/leds-pwm-multicolor.c >>>> index e1a81e0109e8..7155339c075e 100644 >>>> --- a/drivers/leds/rgb/leds-pwm-multicolor.c >>>> +++ b/drivers/leds/rgb/leds-pwm-multicolor.c >>>> @@ -50,7 +50,7 @@ static int led_pwm_mc_set(struct led_classdev *cdev, >>>> duty = priv->leds[i].state.period - duty; >>>> priv->leds[i].state.duty_cycle = duty; >>>> - priv->leds[i].state.enabled = duty > 0; >>>> + priv->leds[i].state.enabled = !(cdev->flags & LED_SUSPENDED); >>>> ret = pwm_apply_might_sleep(priv->leds[i].pwm, >>>> &priv->leds[i].state); >>>> if (ret) >>>> -- >>>> 2.43.0 >>>> >>> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-12-10 7:36 ` FUKAUMI Naoki @ 2024-12-12 19:04 ` Lee Jones 2024-12-12 21:19 ` FUKAUMI Naoki 0 siblings, 1 reply; 11+ messages in thread From: Lee Jones @ 2024-12-12 19:04 UTC (permalink / raw) To: FUKAUMI Naoki; +Cc: linux-leds On Tue, 10 Dec 2024, FUKAUMI Naoki wrote: > Hi, > > Could you help me(write good commit message), anyone? Surely you know what the changes you authored do and why you authored them? Write out a nice description in your first language then use a free online translation tool to convert it into English. It doesn't have to be a novel. Just describe what you're doing and why it's needed. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-12-12 19:04 ` Lee Jones @ 2024-12-12 21:19 ` FUKAUMI Naoki 2024-12-13 16:35 ` Lee Jones 0 siblings, 1 reply; 11+ messages in thread From: FUKAUMI Naoki @ 2024-12-12 21:19 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds Hi, On 12/13/24 04:04, Lee Jones wrote: > On Tue, 10 Dec 2024, FUKAUMI Naoki wrote: > >> Hi, >> >> Could you help me(write good commit message), anyone? > > Surely you know what the changes you authored do and why you authored > them? Write out a nice description in your first language then use > a free online translation tool to convert it into English. It doesn't > have to be a novel. Just describe what you're doing and why it's > needed. I have no knowledge about electrical circuits... Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-12-12 21:19 ` FUKAUMI Naoki @ 2024-12-13 16:35 ` Lee Jones 2024-12-13 21:11 ` FUKAUMI Naoki 0 siblings, 1 reply; 11+ messages in thread From: Lee Jones @ 2024-12-13 16:35 UTC (permalink / raw) To: FUKAUMI Naoki; +Cc: linux-leds On Fri, 13 Dec 2024, FUKAUMI Naoki wrote: > Hi, > > On 12/13/24 04:04, Lee Jones wrote: > > On Tue, 10 Dec 2024, FUKAUMI Naoki wrote: > > > > > Hi, > > > > > > Could you help me(write good commit message), anyone? > > > > Surely you know what the changes you authored do and why you authored > > them? Write out a nice description in your first language then use > > a free online translation tool to convert it into English. It doesn't > > have to be a novel. Just describe what you're doing and why it's > > needed. > > I have no knowledge about electrical circuits... You must know why you made this change? -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-12-13 16:35 ` Lee Jones @ 2024-12-13 21:11 ` FUKAUMI Naoki 2024-12-16 9:51 ` Lee Jones 0 siblings, 1 reply; 11+ messages in thread From: FUKAUMI Naoki @ 2024-12-13 21:11 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds Hi Lee, On 12/14/24 01:35, Lee Jones wrote: > On Fri, 13 Dec 2024, FUKAUMI Naoki wrote: > >> Hi, >> >> On 12/13/24 04:04, Lee Jones wrote: >>> On Tue, 10 Dec 2024, FUKAUMI Naoki wrote: >>> >>>> Hi, >>>> >>>> Could you help me(write good commit message), anyone? >>> >>> Surely you know what the changes you authored do and why you authored >>> them? Write out a nice description in your first language then use >>> a free online translation tool to convert it into English. It doesn't >>> have to be a novel. Just describe what you're doing and why it's >>> needed. >> >> I have no knowledge about electrical circuits... > > You must know why you made this change? I noticed strange behavior on Radxa E25 which have pwm-multicolor. I compared leds-pwm.c:led_pwm_set() and leds-pwm-multicolor.c:led_pwm_mc_set(), and found difference. Do the same fixed the problem, so I just thought this is correct fix. No knowledge was required. Btw, this is enough for me. https://lore.kernel.org/linux-leds/d7d930bc-4c82-4272-b2c6-88f7cac5a3e1@chaosfield.at/T/#t Please ignore my patch. Thank you very much for your help, and sorry for wasting your time. Best regards, -- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting 2024-12-13 21:11 ` FUKAUMI Naoki @ 2024-12-16 9:51 ` Lee Jones 0 siblings, 0 replies; 11+ messages in thread From: Lee Jones @ 2024-12-16 9:51 UTC (permalink / raw) To: FUKAUMI Naoki; +Cc: linux-leds On Sat, 14 Dec 2024, FUKAUMI Naoki wrote: > Hi Lee, > > On 12/14/24 01:35, Lee Jones wrote: > > On Fri, 13 Dec 2024, FUKAUMI Naoki wrote: > > > > > Hi, > > > > > > On 12/13/24 04:04, Lee Jones wrote: > > > > On Tue, 10 Dec 2024, FUKAUMI Naoki wrote: > > > > > > > > > Hi, > > > > > > > > > > Could you help me(write good commit message), anyone? > > > > > > > > Surely you know what the changes you authored do and why you authored > > > > them? Write out a nice description in your first language then use > > > > a free online translation tool to convert it into English. It doesn't > > > > have to be a novel. Just describe what you're doing and why it's > > > > needed. > > > > > > I have no knowledge about electrical circuits... > > > > You must know why you made this change? > > I noticed strange behavior on Radxa E25 which have pwm-multicolor. > > I compared leds-pwm.c:led_pwm_set() and > leds-pwm-multicolor.c:led_pwm_mc_set(), and found difference. > > Do the same fixed the problem, so I just thought this is correct fix. > No knowledge was required. > > Btw, this is enough for me. > > https://lore.kernel.org/linux-leds/d7d930bc-4c82-4272-b2c6-88f7cac5a3e1@chaosfield.at/T/#t > > Please ignore my patch. Thank you very much for your help, and sorry for > wasting your time. You didn't waste my time. If you see any other issues, please feel free to submit them. -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-12-16 9:51 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-16 10:26 [PATCH] leds: pwm-multicolor: fix multicolor PWM LED lights up without any setting FUKAUMI Naoki 2024-08-21 16:11 ` Lee Jones 2024-08-21 21:26 ` FUKAUMI Naoki 2024-08-22 7:22 ` FUKAUMI Naoki 2024-08-22 8:24 ` Lee Jones 2024-12-10 7:36 ` FUKAUMI Naoki 2024-12-12 19:04 ` Lee Jones 2024-12-12 21:19 ` FUKAUMI Naoki 2024-12-13 16:35 ` Lee Jones 2024-12-13 21:11 ` FUKAUMI Naoki 2024-12-16 9:51 ` Lee Jones
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).