Messages in this thread | | | From | Nylon Chen <> | Date | Wed, 18 Jan 2023 10:30:52 +0800 | Subject | Re: [PATCH 0/2] Change PWM-controlled LED pin active mode and algorithm |
| |
Hi Jess,
as you said, I use LED directly connected to PWM logic to modify it.
As I stated in my previous article, the key is that the lower the PWM output is, the brighter the LED light is(active-low), the higher the PWM output is, the brighter the LED light is(active-high).
Therefore, I would point out the waveform diagram below, the output result remains the same for the circuit, but when you use active-low/active-high to look at it, you will get two completely different results of brightness.
e.g. duty=30s, period=100s, actvie-high = 30%, active-low = 70%
V ^ | | ----------| | | | | |______ |__________ > t
Jessica Clarke <jrtc27@jrtc27.com> 於 2023年1月14日 週六 上午3:24寫道: > > On 13 Jan 2023, at 18:32, Conor Dooley <conor@kernel.org> wrote: > > > > +CC Uwe, Thierry, linux-pwm > > > > Hey Nylon, > > > > Please run scripts/get_maintainer.pl before sending patches, you missed > > both me & the PWM maintainers unfortunately! > > AFAIK, the PWM maintainers use patchwork, so you will probably have to > > resend this patchset so that it is on their radar. > > I've marked the series as "Changes Requested" on the RISC-V one. > > > > On Fri, Jan 13, 2023 at 04:31:13PM +0800, Nylon Chen wrote: > > > >> According to the circuit diagram of User LEDs - RGB described in the > >> manual hifive-unmatched-schematics-v3.pdf[0]. > >> The behavior of PWM is acitve-high. > >> > >> According to the descriptionof PWM for pwmcmp in SiFive FU740-C000 > >> Manual[1]. > >> The pwm algorithm is (PW) pulse active time = (D) duty * (T) period[2]. > >> The `frac` variable is pulse "inactive" time so we need to invert it. > >> > >> So this patchset removes active-low in DTS and adds reverse logic to > >> the driver. > >> > >> [0]:https://sifive-china.oss-cn-zhangjiakou.aliyuncs.com/HiFIve%20Unmatched/hifive-unmatched-schematics-v3.pdf > >> [1]:https://sifive-china.oss-cn-zhangjiakou.aliyuncs.com/HiFIve%20Unmatched/fu740-c000-manual-v1p2.pdf > >> [2]:https://en.wikipedia.org/wiki/Duty_cycle > > > > Please delete link 2, convert the other two to standard Link: tags and > > put this information in the dts patch. Possibly into the PWM patch too, > > depending on what the PWM maintainers think. > > This info should be in the commit history IMO and the commit message for > > the dts patch says what's obvious from the diff without any explanation > > as to why. > > > > I did a bit of looking around on lore, to see if I could figure out > > why it was done like this in the first place, and I found: > > https://lore.kernel.org/linux-pwm/CAJ2_jOG2M03aLBgUOgGjWH9CUxq2aTG97eSX70=UaSbGCMMF_g@mail.gmail.com/ > > That DTS documentation makes no sense to me, why does what the LED is > wired to matter? Whether you have your transistor next to ground or > next to Vdd doesn’t matter, what matters is whether the transistor is > on or off. Maybe what they mean is whether the *PWM's output* / *the > transistor's input* is pulled to ground or Vdd? In which case the > property would indeed not apply here. > > Unless that’s written assuming the LED is wired directly to the PWM, in > which case it would make sense, but that’s a very narrow-minded view of > what the PWM output is (directly) driving. > > Jess > > > That doesn't explain the driver, but it does explain the dts being that > > way. Perhaps a Fixes tag is also in order? But only if both patches get > > one, otherwise backporting would lead to breakage. > > > > The min() construct appears to have been there since the RFC driver was > > first posted. > > > > Thanks, > > Conor. > > > >> > >> Nylon Chen (2): > >> riscv: dts: sifive unmatched: Remove PWM controlled LED's active-low > > > > nit: s/sifive unmatched:/sifive: unmatched:/ > > > >> properties > >> pwm: sifive: change the PWM controlled LED algorithm > >> > >> arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 4 ---- > >> drivers/pwm/pwm-sifive.c | 1 + > >> 2 files changed, 1 insertion(+), 4 deletions(-) > >> > >> -- > >> 2.36.1 > >> > >> > >> _______________________________________________ > >> linux-riscv mailing list > >> linux-riscv@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-riscv > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv >
| |