linux-kernel - Re: [PATCH v5 2/4] pwm: starfive: Add PWM driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZR_NIya611D9LyeB@orome.fritz.box>
Date:   Fri, 6 Oct 2023 11:02:27 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     William Qiu <william.qiu@...rfivetech.com>
Cc:     Emil Renner Berthing <emil.renner.berthing@...onical.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-riscv@...ts.infradead.org, linux-pwm@...r.kernel.org,
        Emil Renner Berthing <kernel@...il.dk>,
        Rob Herring <robh+dt@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Hal Feng <hal.feng@...rfivetech.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>
Subject: Re: [PATCH v5 2/4] pwm: starfive: Add PWM driver support

On Mon, Sep 25, 2023 at 06:27:16PM +0800, William Qiu wrote:
> 
> 
> On 2023/9/23 20:08, Emil Renner Berthing wrote:
> > William Qiu wrote:
> >> Add Pulse Width Modulation driver support for StarFive
> >> JH7100 and JH7110 SoC.
> >>
> >> Co-developed-by: Hal Feng <hal.feng@...rfivetech.com>
> >> Signed-off-by: Hal Feng <hal.feng@...rfivetech.com>
> >> Signed-off-by: William Qiu <william.qiu@...rfivetech.com>
> >> ---
> >>  MAINTAINERS                |   7 ++
> >>  drivers/pwm/Kconfig        |   9 ++
> >>  drivers/pwm/Makefile       |   1 +
> >>  drivers/pwm/pwm-starfive.c | 190 +++++++++++++++++++++++++++++++++++++
> >>  4 files changed, 207 insertions(+)
> >>  create mode 100644 drivers/pwm/pwm-starfive.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index bf0f54c24f81..bc2155bd2712 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -20495,6 +20495,13 @@ F:	drivers/pinctrl/starfive/pinctrl-starfive-jh71*
> >>  F:	include/dt-bindings/pinctrl/pinctrl-starfive-jh7100.h
> >>  F:	include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h
> >>
> >> +STARFIVE JH71X0 PWM DRIVERS
> >> +M:	William Qiu <william.qiu@...rfivetech.com>
> >> +M:	Hal Feng <hal.feng@...rfivetech.com>
> >> +S:	Supported
> >> +F:	Documentation/devicetree/bindings/pwm/starfive,jh7100-pwm.yaml
> >> +F:	drivers/pwm/pwm-starfive-ptc.c
> >> +
> >>  STARFIVE JH71X0 RESET CONTROLLER DRIVERS
> >>  M:	Emil Renner Berthing <kernel@...il.dk>
> >>  M:	Hal Feng <hal.feng@...rfivetech.com>
> >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> >> index 8ebcddf91f7b..e2ee0169f6e4 100644
> >> --- a/drivers/pwm/Kconfig
> >> +++ b/drivers/pwm/Kconfig
> >> @@ -569,6 +569,15 @@ config PWM_SPRD
> >>  	  To compile this driver as a module, choose M here: the module
> >>  	  will be called pwm-sprd.
> >>
> >> +config PWM_STARFIVE
> >> +	tristate "StarFive PWM support"
> >> +	depends on ARCH_STARFIVE || COMPILE_TEST
> >> +	help
> >> +	  Generic PWM framework driver for StarFive SoCs.
> >> +
> >> +	  To compile this driver as a module, choose M here: the module
> >> +	  will be called pwm-starfive.
> >> +
> >>  config PWM_STI
> >>  	tristate "STiH4xx PWM support"
> >>  	depends on ARCH_STI || COMPILE_TEST
> >> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> >> index c822389c2a24..93b954376873 100644
> >> --- a/drivers/pwm/Makefile
> >> +++ b/drivers/pwm/Makefile
> >> @@ -52,6 +52,7 @@ obj-$(CONFIG_PWM_SIFIVE)	+= pwm-sifive.o
> >>  obj-$(CONFIG_PWM_SL28CPLD)	+= pwm-sl28cpld.o
> >>  obj-$(CONFIG_PWM_SPEAR)		+= pwm-spear.o
> >>  obj-$(CONFIG_PWM_SPRD)		+= pwm-sprd.o
> >> +obj-$(CONFIG_PWM_STARFIVE)	+= pwm-starfive.o
> >>  obj-$(CONFIG_PWM_STI)		+= pwm-sti.o
> >>  obj-$(CONFIG_PWM_STM32)		+= pwm-stm32.o
> >>  obj-$(CONFIG_PWM_STM32_LP)	+= pwm-stm32-lp.o
> >> diff --git a/drivers/pwm/pwm-starfive.c b/drivers/pwm/pwm-starfive.c
> > 
> > Hi William,
> > 
> > You never answered my questions about what PTC is short for and if there are
> > other PWMs on the JH7110. You just removed -ptc from the name of this file..
> > 
> Hi Emil,
> 
> The PTC, short for PWM/TIMER/CONUTER, comes from OpenCore's ip, but only PWM
> mode is used in the JH7110. So the register still has the word "PTC".
> s the best way to change all the prefix to STARFIVE?

This is the first time I see mentioned that this is based on an Open-
Cores IP. It's definitely something you want to note somewhere so that
others can reuse this driver if they've incorporated the same IP into
their device.

Given the above it might be better to name this something different
entirely. The original OpenCores PTC IP seems to be single-instance,
but that's about the only difference here (i.e. the OpenCores IP lists
one clock and one reset, which this driver supports).

So it'd be easy to turn this into a generic OpenCores driver and then
use the starfive compatible string(s) to parameterize (number of
instances, register stride, etc.).

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)