lkml.org 
[lkml]   [2023]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 0/2] Change PWM-controlled LED pin active mode and algorithm
    +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 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
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2023-03-26 23:41    [W:3.795 / U:0.144 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site