Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 13 Jan 2023 18:32:41 +0000
From:   Conor Dooley <conor@kernel.org>
To:     Nylon Chen <nylon.chen@sifive.com>
Cc:     paul.walmsley@sifive.com, palmer@dabbelt.com,
        linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
        nylon7717@gmail.com, zong.li@sifive.com, greentime.hu@sifive.com,
        vincent.chen@sifive.com, Thierry Reding <thierry.reding@gmail.com>,
        Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= 
        <u.kleine-koenig@pengutronix.de>, linux-pwm@vger.kernel.org
Subject: Re: [PATCH 0/2] Change PWM-controlled LED pin active mode and
 algorithm
Message-ID: <Y8GjySjm9OjoZvCF@spud>
References: <20230113083115.2590-1-nylon.chen@sifive.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="8H29E2czozb1Ysd/"
Content-Disposition: inline
In-Reply-To: <20230113083115.2590-1-nylon.chen@sifive.com>
Precedence: bulk


--8H29E2czozb1Ysd/
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

+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.
>=20
> According to the descriptionof PWM for pwmcmp in SiFive FU740-C000
> Manual[1].
> The pwm algorithm is (PW) pulse active time  =3D (D) duty * (T) period[2].
> The `frac` variable is pulse "inactive" time so we need to invert it.
>=20
> So this patchset removes active-low in DTS and adds reverse logic to
> the driver.
>=20
> [0]:https://sifive-china.oss-cn-zhangjiakou.aliyuncs.com/HiFIve%20Unmatch=
ed/hifive-unmatched-schematics-v3.pdf
> [1]:https://sifive-china.oss-cn-zhangjiakou.aliyuncs.com/HiFIve%20Unmatch=
ed/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=
=3DUaSbGCMMF_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.

>=20
> 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
>=20
>  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 4 ----
>  drivers/pwm/pwm-sifive.c                            | 1 +
>  2 files changed, 1 insertion(+), 4 deletions(-)
>=20
> --=20
> 2.36.1
>=20
>=20
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

--8H29E2czozb1Ysd/
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCY8GjyQAKCRB4tDGHoIJi
0te3AQCOUP2q+X8xQoPB6gQEJSA6kxsp3o3UBJtLo2y3rj1ghgEAv5fpkfmkc7fM
XrAwr8T1JhNctwLAS7OsG6CQibVFcQ0=
=wA6w
-----END PGP SIGNATURE-----

--8H29E2czozb1Ysd/--