StarFive's Pulse Width Modulation driver support
Subsystems (2)
cover StarFive's Pulse Width Modulation driver support
Hi,
This patchset adds initial rudimentary support for the StarFive Pulse Width Modulation controller driver. And this driver will be used in StarFive's VisionFive 2 board.The first patch add Documentations for the device and Patch 2 adds device probe for the module.
The patch series is based on v6.2.
William Qiu (2):
dt-bindings: PWM: Add StarFive PWM module
pwm: starfive: Add PWM driver support
.../devicetree/bindings/pwm/pwm-starfive.yaml | 54 ++++ MAINTAINERS | 7 + drivers/pwm/Kconfig | 10 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-starfive-ptc.c | 256 ++++++++++++++++++ 5 files changed, 328 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/pwm-starfive.yaml create mode 100644 drivers/pwm/pwm-starfive-ptc.c
--
2.34.1
PATCH 1/2 dt-bindings: PWM: Add StarFive PWM module
Add documentation to describe StarFive Pulse Width Modulation controller driver.
Signed-off-by William Qiu <[email protected]>
--- .../devicetree/bindings/pwm/pwm-starfive.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/pwm-starfive.yaml
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +54 -0
@@ -0,0 +1,54 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/pwm-starfive.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive PWM controller + +maintainers: + - William Qiu <[email protected]> + +description: + StarFive SoCs contain PWM and when operating in PWM mode, the PTC core generates + binary signal with user-programmable low and high periods. Clock source for the + PWM can be either system clockor external clock. Each PWM timer block provides 8 + PWM channels. + +allOf: + - $ref: pwm.yaml# + +properties: + compatible: + items: + - const: starfive,jh7110-pwm + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + "#pwm-cells": + const: 3 + +required: + - compatible + - reg + - clocks + - resets + +additionalProperties: false + +examples: + - | + pwm@120d0000 { + compatible = "starfive,jh7110-pwm"; + reg = <0x120d0000 0x10000>; + clocks = <&syscrg 121>; + resets = <&syscrg 108>; + #pwm-cells=<3>; + };
Signature · 1 line
2.34.1
Re: [PATCH v1 1/2] dt-bindings: PWM: Add StarFive PWM module
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml
Quoted 14 lines · William Qiu
Add documentation to describe StarFive Pulse Width Modulation controller driver.
Signed-off-by William Qiu <[email protected]>--- .../devicetree/bindings/pwm/pwm-starfive.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/pwm-starfive.yamlDocumentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +0
-0
Filename based on compatible, so starfive,jh7110-pwm.yaml
Quoted 14 lines
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +13
-0@@ -0,0 +1,54 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/pwm-starfive.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive PWM controller + +maintainers: + - William Qiu <[email protected]> + +description: + StarFive SoCs contain PWM and when operating in PWM mode, the PTC core generates
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml line 23
Quoted 10 lines
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +10
-0+ binary signal with user-programmable low and high periods. Clock source for the + PWM can be either system clockor external clock. Each PWM timer block provides 8 + PWM channels. + +allOf: + - $ref: pwm.yaml# + +properties: + compatible: + items:
Drop items, not a list in your case.
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml line 25
Quoted 2 lines
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +2
-0+ - const: starfive,jh7110-pwm +
Best regards,
Krzysztof
Re: [PATCH v1 1/2] dt-bindings: PWM: Add StarFive PWM module
Quoted 36 lines · Krzysztof Kozlowski
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml
Quoted 14 lines · William Qiu
Add documentation to describe StarFive Pulse Width Modulation controller driver.
Signed-off-by William Qiu <[email protected]>--- .../devicetree/bindings/pwm/pwm-starfive.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/pwm-starfive.yamlDocumentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +0
-0Filename based on compatible, so starfive,jh7110-pwm.yaml
Quoted 18 lines
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +17
-0@@ -0,0 +1,54 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/pwm-starfive.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive PWM controller + +maintainers: + - William Qiu <[email protected]> + +description: + StarFive SoCs contain PWM and when operating in PWM mode, the PTC core generates + binary signal with user-programmable low and high periods. Clock source for the + PWM can be either system clockor external clock. Each PWM timer block provides 8 + PWM channels. +
Quoted 9 lines
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml line 23
Quoted 6 lines
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +6
-0+allOf: + - $ref: pwm.yaml# + +properties: + compatible: + items:Drop items, not a list in your case.
Will drop. Thanks for taking time reviewing this patch series.
Best regards
William
Quoted 7 lines
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml line 25
Quoted 2 lines
Documentation/devicetree/bindings/pwm/pwm-starfive.yaml new file +2
-0+ - const: starfive,jh7110-pwm +Best regards,
Krzysztof
PATCH 2/2 pwm: starfive: Add PWM driver support
In reply to William QiuAdd Pulse Width Modulation driver support for StarFive JH7110 soc.
Signed-off-by Hal Feng <[email protected]>
Signed-off-by William Qiu <[email protected]>
--- MAINTAINERS | 7 + drivers/pwm/Kconfig | 10 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-starfive-ptc.c | 256 +++++++++++++++++++++++++++++++++ 4 files changed, 274 insertions(+) create mode 100644 drivers/pwm/pwm-starfive-ptc.c
MAINTAINERS +7 -0
@@ -19929,6 +19929,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 <[email protected]> +M: Hal Feng <[email protected]> +S: Supported +F: Documentation/devicetree/bindings/pwm/pwm-starfive.yaml +F: drivers/pwm/pwm-starfive-ptc.c + STARFIVE JH71X0 RESET CONTROLLER DRIVERS M: Emil Renner Berthing <[email protected]> M: Hal Feng <[email protected]>
drivers/pwm/Kconfig +10 -0
@@ -536,6 +536,16 @@ config PWM_SPRD To compile this driver as a module, choose M here: the module will be called pwm-sprd. +config PWM_STARFIVE_PTC + tristate "StarFive PWM PTC support" + depends on OF + depends on COMMON_CLK + 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-ptc. + config PWM_STI tristate "STiH4xx PWM support" depends on ARCH_STI || COMPILE_TEST
drivers/pwm/Makefile +1 -0
@@ -49,6 +49,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_PTC) += pwm-starfive-ptc.o obj-$(CONFIG_PWM_STI) += pwm-sti.o obj-$(CONFIG_PWM_STM32) += pwm-stm32.o obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.o
drivers/pwm/pwm-starfive-ptc.c new file +256 -0
256 lines not on this page · raw message
Signature · 1 line
2.34.1
Re: [PATCH v1 2/2] pwm: starfive: Add PWM driver support
Quoted 93 lines · William Qiu
Add Pulse Width Modulation driver support for StarFive JH7110 soc.
Signed-off-by Hal Feng <[email protected]>Signed-off-by William Qiu <[email protected]>--- MAINTAINERS | 7 + drivers/pwm/Kconfig | 10 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-starfive-ptc.c | 256 +++++++++++++++++++++++++++++++++ 4 files changed, 274 insertions(+) create mode 100644 drivers/pwm/pwm-starfive-ptc.cMAINTAINERS +7
-0@@ -19929,6 +19929,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 <[email protected]> +M: Hal Feng <[email protected]> +S: Supported +F: Documentation/devicetree/bindings/pwm/pwm-starfive.yaml +F: drivers/pwm/pwm-starfive-ptc.c + STARFIVE JH71X0 RESET CONTROLLER DRIVERS M: Emil Renner Berthing <[email protected]> M: Hal Feng <[email protected]>drivers/pwm/Kconfig +10
-0@@ -536,6 +536,16 @@ config PWM_SPRD To compile this driver as a module, choose M here: the module will be called pwm-sprd. +config PWM_STARFIVE_PTC + tristate "StarFive PWM PTC support" + depends on OF + depends on COMMON_CLK + 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-ptc. + config PWM_STI tristate "STiH4xx PWM support" depends on ARCH_STI || COMPILE_TESTdrivers/pwm/Makefile +1
-0@@ -49,6 +49,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_PTC) += pwm-starfive-ptc.o obj-$(CONFIG_PWM_STI) += pwm-sti.o obj-$(CONFIG_PWM_STM32) += pwm-stm32.o obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.odrivers/pwm/pwm-starfive-ptc.c new file +23
-0@@ -0,0 +1,256 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* PWM driver for the StarFive JH7110 SoC +* +* Copyright (C) 2018 StarFive Technology Co., Ltd. +*/ + +#include <dt-bindings/pwm/pwm.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/slab.h> +#include <linux/clk.h> +#include <linux/reset.h> +#include <linux/io.h> + +/* how many parameters can be transferred to ptc */ +#define OF_PWM_N_CELLS 3 + +/* PTC Register offsets */ +#define REG_RPTC_CNTR 0x0 +#define REG_RPTC_HRC 0x4 +#define REG_RPTC_LRC 0x8
drivers/pwm/pwm-starfive-ptc.c line 33
Quoted 10 lines
drivers/pwm/pwm-starfive-ptc.c new file +10
-0+#define REG_RPTC_CTRL 0xC + +/* Bit for PWM clock */ +#define BIT_PWM_CLOCK_EN 31 + +/* Bit for clock gen soft reset */ +#define BIT_CLK_GEN_SOFT_RESET 13 + +#define NS_PER_SECOND 1000000000 +#define DEFAULT_FREQ_HZ 2000000
Drop unused defines.
Quoted 20 lines
drivers/pwm/pwm-starfive-ptc.c new file +20
-0+ +/* +* Access PTC register (cntr hrc lrc and ctrl), +* need to replace PWM_BASE_ADDR +*/ +#define REG_PTC_BASE_ADDR_SUB (base,N ) \ +((base) + (((N) > 3) ? (((N) % 4) * 0x10 + (1 << 15)) : ((N) * 0x10))) +#define REG_PTC_RPTC_CNTR (base,N ) (REG_PTC_BASE_ADDR_SUB(base, N)) +#define REG_PTC_RPTC_HRC (base,N ) (REG_PTC_BASE_ADDR_SUB(base, N) + 0x4) +#define REG_PTC_RPTC_LRC (base,N ) (REG_PTC_BASE_ADDR_SUB(base, N) + 0x8) +#define REG_PTC_RPTC_CTRL (base,N ) (REG_PTC_BASE_ADDR_SUB(base, N) + 0xC) + +/* PTC_RPTC_CTRL */ +#define PTC_EN BIT(0) +#define PTC_ECLK BIT(1) +#define PTC_NEC BIT(2) +#define PTC_OE BIT(3) +#define PTC_SIGNLE BIT(4) +#define PTC_INTE BIT(5) +#define PTC_INT BIT(6)
drivers/pwm/pwm-starfive-ptc.c line 63
Quoted 10 lines
drivers/pwm/pwm-starfive-ptc.c new file +10
-0+#define PTC_CNTRRST BIT(7) +#define PTC_CAPTE BIT(8) + +struct starfive_pwm_ptc_device { +struct pwm_chip chip; +struct clk *clk; +struct reset_control *rst; +void __iomem *regs; +int irq; +/*pwm apb clock frequency*/
Missing spaces. Use Linux coding style.
Quoted 9 lines
drivers/pwm/pwm-starfive-ptc.c new file +9
-0+unsigned int approx_freq; +}; + +static inline struct starfive_pwm_ptc_device * +chip_to_starfive_ptc (struct pwm_chip *c) +{ + return container_of(c, struct starfive_pwm_ptc_device, chip); +} +
(...)
Quoted 13 lines
drivers/pwm/pwm-starfive-ptc.c new file +13
-0+static int starfive_pwm_ptc_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct starfive_pwm_ptc_device *pwm; +struct pwm_chip *chip; +struct resource *res; +unsigned int clk_apb_freq; +int ret; + + pwm =devm_kzalloc (dev,sizeof (*pwm),GFP_KERNEL ); +if (!pwm) +return -ENOMEM ; +
drivers/pwm/pwm-starfive-ptc.c line 181
Quoted 10 lines
drivers/pwm/pwm-starfive-ptc.c new file +10
-0+ chip = &pwm->chip; + chip->dev = dev; + chip->ops = &starfive_pwm_ptc_ops; + chip->npwm =8 ; + + chip->of_pwm_n_cells =OF_PWM_N_CELLS ; + chip->base =-1 ; + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + pwm->regs =devm_ioremap_resource (dev, res);
Combine these two, there is a helper for it.
drivers/pwm/pwm-starfive-ptc.c line 183
Quoted 2 lines
drivers/pwm/pwm-starfive-ptc.c new file +2
-0+if (IS_ERR (pwm->regs)) { +dev_err (dev,"Unable to map IO resources\n" );
return dev_err_probe(), everywhere probably.
Quoted 15 lines
drivers/pwm/pwm-starfive-ptc.c new file +15
-0+return PTR_ERR (pwm->regs); + } + + pwm->clk =devm_clk_get (dev,NULL ); +if (IS_ERR (pwm->clk)) { +dev_err (dev,"Unable to get pwm clock\n" ); +return PTR_ERR (pwm->clk); + } + + pwm->rst =devm_reset_control_get_exclusive (dev,NULL ); +if (IS_ERR (pwm->rst)) { +dev_err (dev,"Unable to get pwm reset\n" ); +return PTR_ERR (pwm->rst); + } +
drivers/pwm/pwm-starfive-ptc.c line 208
Quoted 10 lines
drivers/pwm/pwm-starfive-ptc.c new file +10
-0+ ret =clk_prepare_enable (pwm->clk); +if (ret) { +dev_err (dev, +"Failed to enable pwm clock, %d\n" , ret); +return ret; + } + +reset_control_deassert (pwm->rst); + + clk_apb_freq = (unsigned int )clk_get_rate (pwm->clk);
Why do you need this local variable? And why the cast?
drivers/pwm/pwm-starfive-ptc.c line 211
Quoted 3 lines
drivers/pwm/pwm-starfive-ptc.c new file +3
-0+if (!clk_apb_freq) +dev_warn (dev, +"get pwm apb clock rate failed.\n" );
and pwm->approx_freq stays 0 which you later use for dividing. Did you actually test it? It should produce big splat...
drivers/pwm/pwm-starfive-ptc.c line 215
Quoted 4 lines
drivers/pwm/pwm-starfive-ptc.c new file +4
-0+else + pwm->approx_freq = clk_apb_freq; + + ret =pwmchip_add (chip);
devm
Quoted 23 lines
drivers/pwm/pwm-starfive-ptc.c new file +23
-0+if (ret <0 ) { +dev_err (dev,"cannot register PTC: %d\n" , ret); +clk_disable_unprepare (pwm->clk); +return ret; + } + +platform_set_drvdata (pdev, pwm); + +return 0 ; +} + +static int starfive_pwm_ptc_remove (struct platform_device *dev) +{ +struct starfive_pwm_ptc_device *pwm =platform_get_drvdata (dev); +struct pwm_chip *chip = &pwm->chip; + +pwmchip_remove (chip); + +return 0 ; +} + +static const struct of_device_id starfive_pwm_ptc_of_match[] = { + { .compatible ="starfive,jh7110-pwm" },
drivers/pwm/pwm-starfive-ptc.c line 248
Quoted 10 lines
drivers/pwm/pwm-starfive-ptc.c new file +10
-0+ {}, +}; +MODULE_DEVICE_TABLE (of, starfive_pwm_ptc_of_match); + +static struct platform_driver starfive_pwm_ptc_driver = { + .probe = starfive_pwm_ptc_probe, + .remove = starfive_pwm_ptc_remove, + .driver = { + .name ="pwm-starfive-ptc" , + .of_match_table =of_match_ptr (starfive_pwm_ptc_of_match),
of_match_ptr goes with maybe_unused, which you do not have. Anyway I am not sure what's the benefit of having it here, so just drop it.
Best regards,
Krzysztof
Re: [PATCH v1 2/2] pwm: starfive: Add PWM driver support
Quoted 98 lines · Krzysztof Kozlowski
Quoted 97 lines · William Qiu
Add Pulse Width Modulation driver support for StarFive JH7110 soc.
Signed-off-by Hal Feng <[email protected]>Signed-off-by William Qiu <[email protected]>--- MAINTAINERS | 7 + drivers/pwm/Kconfig | 10 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-starfive-ptc.c | 256 +++++++++++++++++++++++++++++++++ 4 files changed, 274 insertions(+) create mode 100644 drivers/pwm/pwm-starfive-ptc.cMAINTAINERS +7
-0@@ -19929,6 +19929,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 <[email protected]> +M: Hal Feng <[email protected]> +S: Supported +F: Documentation/devicetree/bindings/pwm/pwm-starfive.yaml +F: drivers/pwm/pwm-starfive-ptc.c + STARFIVE JH71X0 RESET CONTROLLER DRIVERS M: Emil Renner Berthing <[email protected]> M: Hal Feng <[email protected]>drivers/pwm/Kconfig +10
-0@@ -536,6 +536,16 @@ config PWM_SPRD To compile this driver as a module, choose M here: the module will be called pwm-sprd. +config PWM_STARFIVE_PTC + tristate "StarFive PWM PTC support" + depends on OF + depends on COMMON_CLK + 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-ptc. + config PWM_STI tristate "STiH4xx PWM support" depends on ARCH_STI || COMPILE_TESTdrivers/pwm/Makefile +1
-0@@ -49,6 +49,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_PTC) += pwm-starfive-ptc.o obj-$(CONFIG_PWM_STI) += pwm-sti.o obj-$(CONFIG_PWM_STM32) += pwm-stm32.o obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.odrivers/pwm/pwm-starfive-ptc.c new file +27
-0@@ -0,0 +1,256 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* PWM driver for the StarFive JH7110 SoC +* +* Copyright (C) 2018 StarFive Technology Co., Ltd. +*/ + +#include <dt-bindings/pwm/pwm.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/slab.h> +#include <linux/clk.h> +#include <linux/reset.h> +#include <linux/io.h> + +/* how many parameters can be transferred to ptc */ +#define OF_PWM_N_CELLS 3 + +/* PTC Register offsets */ +#define REG_RPTC_CNTR 0x0 +#define REG_RPTC_HRC 0x4 +#define REG_RPTC_LRC 0x8 +#define REG_RPTC_CTRL 0xC + +/* Bit for PWM clock */ +#define BIT_PWM_CLOCK_EN 31
Quoted 9 lines
drivers/pwm/pwm-starfive-ptc.c line 33
Quoted 6 lines
drivers/pwm/pwm-starfive-ptc.c new file +6
-0+ +/* Bit for clock gen soft reset */ +#define BIT_CLK_GEN_SOFT_RESET 13 + +#define NS_PER_SECOND 1000000000 +#define DEFAULT_FREQ_HZ 2000000Drop unused defines.
Will drop.
Quoted 24 lines
Quoted 24 lines
drivers/pwm/pwm-starfive-ptc.c new file +24
-0+ +/* +* Access PTC register (cntr hrc lrc and ctrl), +* need to replace PWM_BASE_ADDR +*/ +#define REG_PTC_BASE_ADDR_SUB (base,N ) \ +((base) + (((N) > 3) ? (((N) % 4) * 0x10 + (1 << 15)) : ((N) * 0x10))) +#define REG_PTC_RPTC_CNTR (base,N ) (REG_PTC_BASE_ADDR_SUB(base, N)) +#define REG_PTC_RPTC_HRC (base,N ) (REG_PTC_BASE_ADDR_SUB(base, N) + 0x4) +#define REG_PTC_RPTC_LRC (base,N ) (REG_PTC_BASE_ADDR_SUB(base, N) + 0x8) +#define REG_PTC_RPTC_CTRL (base,N ) (REG_PTC_BASE_ADDR_SUB(base, N) + 0xC) + +/* PTC_RPTC_CTRL */ +#define PTC_EN BIT(0) +#define PTC_ECLK BIT(1) +#define PTC_NEC BIT(2) +#define PTC_OE BIT(3) +#define PTC_SIGNLE BIT(4) +#define PTC_INTE BIT(5) +#define PTC_INT BIT(6) +#define PTC_CNTRRST BIT(7) +#define PTC_CAPTE BIT(8) + +struct starfive_pwm_ptc_device {
Quoted 9 lines
drivers/pwm/pwm-starfive-ptc.c line 63
Quoted 6 lines
drivers/pwm/pwm-starfive-ptc.c new file +6
-0+struct pwm_chip chip; +struct clk *clk; +struct reset_control *rst; +void __iomem *regs; +int irq; +/*pwm apb clock frequency*/ Missing spaces. Use Linux coding style.
Will fix.
Quoted 29 lines
Quoted 9 lines
drivers/pwm/pwm-starfive-ptc.c new file +9
-0+unsigned int approx_freq; +}; + +static inline struct starfive_pwm_ptc_device * +chip_to_starfive_ptc (struct pwm_chip *c) +{ + return container_of(c, struct starfive_pwm_ptc_device, chip); +} +(...)
Quoted 17 lines
drivers/pwm/pwm-starfive-ptc.c new file +17
-0+static int starfive_pwm_ptc_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct starfive_pwm_ptc_device *pwm; +struct pwm_chip *chip; +struct resource *res; +unsigned int clk_apb_freq; +int ret; + + pwm =devm_kzalloc (dev,sizeof (*pwm),GFP_KERNEL ); +if (!pwm) +return -ENOMEM ; + + chip = &pwm->chip; + chip->dev = dev; + chip->ops = &starfive_pwm_ptc_ops; + chip->npwm =8 ;
Quoted 9 lines
drivers/pwm/pwm-starfive-ptc.c line 181
Quoted 6 lines
drivers/pwm/pwm-starfive-ptc.c new file +6
-0+ + chip->of_pwm_n_cells =OF_PWM_N_CELLS ; + chip->base =-1 ; + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + pwm->regs =devm_ioremap_resource (dev, res);Combine these two, there is a helper for it.
Will update.
Quoted 5 lines
drivers/pwm/pwm-starfive-ptc.c line 183
Quoted 2 lines
drivers/pwm/pwm-starfive-ptc.c new file +2
-0+if (IS_ERR (pwm->regs)) { +dev_err (dev,"Unable to map IO resources\n" );return
dev_err_probe(), everywhere probably.
Will fix.
Quoted 19 lines
Quoted 19 lines
drivers/pwm/pwm-starfive-ptc.c new file +19
-0+return PTR_ERR (pwm->regs); + } + + pwm->clk =devm_clk_get (dev,NULL ); +if (IS_ERR (pwm->clk)) { +dev_err (dev,"Unable to get pwm clock\n" ); +return PTR_ERR (pwm->clk); + } + + pwm->rst =devm_reset_control_get_exclusive (dev,NULL ); +if (IS_ERR (pwm->rst)) { +dev_err (dev,"Unable to get pwm reset\n" ); +return PTR_ERR (pwm->rst); + } + + ret =clk_prepare_enable (pwm->clk); +if (ret) { +dev_err (dev, +"Failed to enable pwm clock, %d\n" , ret);
Quoted 9 lines
drivers/pwm/pwm-starfive-ptc.c line 208
Quoted 6 lines
drivers/pwm/pwm-starfive-ptc.c new file +6
-0+return ret; + } + +reset_control_deassert (pwm->rst); + + clk_apb_freq = (unsigned int )clk_get_rate (pwm->clk);Why do you need this local variable? And why the cast?
Will drop this local variable.
Quoted 7 lines
drivers/pwm/pwm-starfive-ptc.c line 211
Quoted 3 lines
drivers/pwm/pwm-starfive-ptc.c new file +3
-0+if (!clk_apb_freq) +dev_warn (dev, +"get pwm apb clock rate failed.\n" );and
pwm->approx_freqstays 0 which you later use for dividing. Did you actually test it? It should produce big splat...
Will update.
Quoted 7 lines
drivers/pwm/pwm-starfive-ptc.c line 215
Quoted 4 lines
drivers/pwm/pwm-starfive-ptc.c new file +4
-0+else + pwm->approx_freq = clk_apb_freq; + + ret =pwmchip_add (chip);devm
Will update.
Quoted 28 lines
Quoted 28 lines
drivers/pwm/pwm-starfive-ptc.c new file +28
-0+if (ret <0 ) { +dev_err (dev,"cannot register PTC: %d\n" , ret); +clk_disable_unprepare (pwm->clk); +return ret; + } + +platform_set_drvdata (pdev, pwm); + +return 0 ; +} + +static int starfive_pwm_ptc_remove (struct platform_device *dev) +{ +struct starfive_pwm_ptc_device *pwm =platform_get_drvdata (dev); +struct pwm_chip *chip = &pwm->chip; + +pwmchip_remove (chip); + +return 0 ; +} + +static const struct of_device_id starfive_pwm_ptc_of_match[] = { + { .compatible ="starfive,jh7110-pwm" }, + {}, +}; +MODULE_DEVICE_TABLE (of, starfive_pwm_ptc_of_match); + +static struct platform_driver starfive_pwm_ptc_driver = {
Quoted 9 lines
drivers/pwm/pwm-starfive-ptc.c line 248
Quoted 5 lines
drivers/pwm/pwm-starfive-ptc.c new file +5
-0+ .probe = starfive_pwm_ptc_probe, + .remove = starfive_pwm_ptc_remove, + .driver = { + .name = "pwm-starfive-ptc", + .of_match_table = of_match_ptr(starfive_pwm_ptc_of_match),
of_match_ptrgoes withmaybe_unused, which you do not have. Anyway I am not sure what's the benefit of having it here, so just drop it.
Will drop.
Thank you for spending time reviewing and providing helpful comments for this driver
Best regards
William
Quoted 3 lines
Best regards,
Krzysztof
Hey William,
Quoted 8 lines · William Qiu
Hi,
This patchset adds initial rudimentary support for the StarFive Pulse Width Modulation controller driver. And this driver will be used in StarFive's VisionFive 2 board.The first patch add Documentations for the device and Patch 2 adds device probe for the module.
Quoted 5 lines
The patch series is based on v6.2.
William Qiu (2):
dt-bindings: PWM: Add StarFive PWM module
pwm: starfive: Add PWM driver support
Is there a corresponding dts addition for this driver?
Cheers,
Conor.
Quoted 13 lines · Conor Dooley
Hey William,
Quoted 10 lines · William Qiu
Hi,
This patchset adds initial rudimentary support for the StarFive Pulse Width Modulation controller driver. And this driver will be used in StarFive's VisionFive 2 board.The first patch add Documentations for the device and Patch 2 adds device probe for the module.
The patch series is based on v6.2.
Quoted 8 lines
Quoted 3 lines
William Qiu (2):
dt-bindings: PWM: Add StarFive PWM module
pwm: starfive: Add PWM driver supportIs there a corresponding dts addition for this driver?
Cheers,
Conor
Hi Conor,
There is, but after communicating with Hal Feng, I decided to upload bindings and driver based on mainline first, and then upload dts after Hal Feng's minimal system is merged.
Best Regards
William