Add support for Allwinner PWM on D1/T113s/R329 SoCs
Subsystems (4)
v10 cover Add support for Allwinner PWM on D1/T113s/R329 SoCs
v2:
- fix dt-bindings
- fix a remark in the driver
v3:
- fix dt-bindings
- fix sunxi-d1s-t113.dtsi
v4:
- fix a remark in the driver
v5:
- dropped unused varibale in the driver
- fix dt-bindings
v6:
- add apb0 clock
v7:
- fix a remark in the driver
- add maintainer
v8:
- fix compile driver for 6.8-rc
v9:
- fix a remark in the driver
- fix dt-bindings
- rename apb0 -> apb
v10:
- fix a remark in the driver
- fix compile driver for 6.12-rc2
Aleksandr Shubin (3):
dt-bindings: pwm: Add binding for Allwinner D1/T113-S3/R329 PWM
controller
pwm: Add Allwinner's D1/T113-S3/R329 SoCs PWM support
riscv: dts: allwinner: d1: Add pwm node
.../bindings/pwm/allwinner,sun20i-pwm.yaml | 84 ++++ .../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 12 + drivers/pwm/Kconfig | 10 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-sun20i.c | 379 ++++++++++++++++++ 5 files changed, 486 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml create mode 100644 drivers/pwm/pwm-sun20i.c
Signature · 1 line
2.25.1
PATCH v10 1/3 dt-bindings: pwm: Add binding for Allwinner D1/T113-S3/R329 PWM controller
Allwinner's D1, T113-S3 and R329 SoCs have a new pwm controller witch is different from the previous pwm-sun4i.
The D1 and T113 are identical in terms of peripherals, they differ only in the architecture of the CPU core, and even share the majority of their DT. Because of that, using the same compatible makes sense.
The R329 is a different SoC though, and should have a different compatible string added, especially as there is a difference in the number of channels.
D1 and T113s SoCs have one PWM controller with 8 channels. R329 SoC has two PWM controllers in both power domains, one of them has 9 channels (CPUX one) and the other has 6 (CPUS one).
Add a device tree binding for them.
--- .../bindings/pwm/allwinner,sun20i-pwm.yaml | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml
Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +84 -0
@@ -0,0 +1,84 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/allwinner,sun20i-pwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Allwinner D1, T113-S3 and R329 PWM + +maintainers: + - Aleksandr Shubin <[email protected]> + - Brandon Cheo Fusi <[email protected]> + +properties: + compatible: + oneOf: + - const: allwinner,sun20i-d1-pwm + - items: + - const: allwinner,sun50i-r329-pwm + - const: allwinner,sun20i-d1-pwm + + reg: + maxItems: 1 + + "#pwm-cells": + const: 3 + + clocks: + items: + - description: Bus clock + - description: 24 MHz oscillator + - description: APB clock + + clock-names: + items: + - const: bus + - const: hosc + - const: apb + + resets: + maxItems: 1 + + allwinner,pwm-channels: + $ref: /schemas/types.yaml#/definitions/uint32 + description: The number of PWM channels configured for this instance + enum: [6, 9] + +allOf: + - $ref: pwm.yaml# + + - if: + properties: + compatible: + contains: + const: allwinner,sun50i-r329-pwm + + then: + required: + - allwinner,pwm-channels + +unevaluatedProperties: false + +required: + - compatible + - reg + - "#pwm-cells" + - clocks + - clock-names + - resets + +examples: + - | + #include <dt-bindings/clock/sun20i-d1-ccu.h> + #include <dt-bindings/reset/sun20i-d1-ccu.h> + + pwm: pwm@2000c00 { + compatible = "allwinner,sun20i-d1-pwm"; + reg = <0x02000c00 0x400>; + clocks = <&ccu CLK_BUS_PWM>, <&dcxo>, <&ccu CLK_APB0>; + clock-names = "bus", "hosc", "apb"; + resets = <&ccu RST_BUS_PWM>; + #pwm-cells = <0x3>; + }; + +...
Signature · 1 line
2.25.1
Re: [PATCH v10 1/3] dt-bindings: pwm: Add binding for Allwinner D1/T113-S3/R329 PWM controller
Hello,
Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml line 45
Quoted 4 lines · Aleksandr Shubin
Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +4
-0+ allwinner,pwm-channels: + $ref: /schemas/types.yaml#/definitions/uint32 + description: The number of PWM channels configured for this instance + enum: [6, 9]
I wonder if the number of channels is a property common enough that we can use "num-pwm-channels" here instead of a vendor specific property. Or would you suggest a different name? gpio-controller nodes have "ngpios", so maybe "npwms"?
A quick grep suggests we already have:
fsl,pwm-number in mxs-pwm.yaml st,pwm-num-chan in pwm-st.txt snps,pwm-number in snps,dw-apb-timers-pwm2.yaml
As a follow up this could then be used by pwmchip_alloc() to determine the number of channels if the passed npwm value is 0.
Best regards
Uwe
Re: [PATCH v10 1/3] dt-bindings: pwm: Add binding for Allwinner D1/T113-S3/R329 PWM controller
Hello Uwe,
I appreciate your suggestion to use a more standardized property name like "pwm-number" instead of vendor-specific names.
Since the name "pwm-number" is present in two drivers, we could consider using this name here as an option. Or perhaps we should choose a new common name "npwms" as you suggested?
Please let me know what you think about this.
Best regards,
Aleksandr.
вт, 29 окт. 2024 г. в 11:56, Uwe Kleine-König <[email protected]>:
Quoted 25 lines
Hello,
Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml line 45
Quoted 4 lines · Aleksandr Shubin
Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml new file +4
-0+ allwinner,pwm-channels: + $ref: /schemas/types.yaml#/definitions/uint32 + description: The number of PWM channels configured for this instance + enum: [6, 9]I wonder if the number of channels is a property common enough that we can use "num-pwm-channels" here instead of a vendor specific property. Or would you suggest a different name? gpio-controller nodes have "ngpios", so maybe "npwms"?
A quick grep suggests we already have:
fsl,pwm-number in
mxs-pwm.yamlst,pwm-num-chan in pwm-st.txt snps,pwm-number in snps,dw-apb-timers-pwm2.yamlAs a follow up this could then be used by
pwmchip_alloc()to determine the number of channels if the passed npwm value is 0.Best regards
Uwe
PATCH v10 2/3 pwm: Add Allwinner's D1/T113-S3/R329 SoCs PWM support
In reply to Aleksandr ShubinAllwinner's D1, T113-S3 and R329 SoCs have a quite different PWM controllers with ones supported by pwm-sun4i driver.
This patch adds a PWM controller driver for Allwinner's D1, T113-S3 and R329 SoCs. The main difference between these SoCs is the number of channels defined by the DT property.
--- drivers/pwm/Kconfig | 10 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-sun20i.c | 379 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 390 insertions(+) create mode 100644 drivers/pwm/pwm-sun20i.c
drivers/pwm/Kconfig +10 -0
@@ -652,6 +652,16 @@ config PWM_SUN4I To compile this driver as a module, choose M here: the module will be called pwm-sun4i. +config PWM_SUN20I + tristate "Allwinner D1/T113s/R329 PWM support" + depends on ARCH_SUNXI || COMPILE_TEST + depends on COMMON_CLK + help + Generic PWM framework driver for Allwinner D1/T113s/R329 SoCs. + + To compile this driver as a module, choose M here: the module + will be called pwm-sun20i. + config PWM_SUNPLUS tristate "Sunplus PWM support" depends on ARCH_SUNPLUS || COMPILE_TEST
drivers/pwm/Makefile +1 -0
@@ -60,6 +60,7 @@ obj-$(CONFIG_PWM_STM32) += pwm-stm32.o obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.o obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o +obj-$(CONFIG_PWM_SUN20I) += pwm-sun20i.o obj-$(CONFIG_PWM_SUNPLUS) += pwm-sunplus.o obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
drivers/pwm/pwm-sun20i.c new file +379 -0
379 lines not on this page · raw message
Signature · 1 line
2.25.1
Re: [PATCH v10 2/3] pwm: Add Allwinner's D1/T113-S3/R329 SoCs PWM support
Hello,
Quoted 41 lines · Aleksandr Shubin
[...]
drivers/pwm/pwm-sun20i.c new file +34
-0@@ -0,0 +1,379 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* PWM Controller Driver for sunxi platforms (D1, T113-S3 and R329) +* +* Limitations: +* - When the parameters change, current running period will not be completed +* and run new settings immediately. +* - It output HIGH-Z state when PWM channel disabled. +* +* Copyright (c) 2023 Aleksandr Shubin <[email protected]> +*/ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/reset.h> + +#define SUN20I_PWM_CLK_CFG (chan) (0x20 + ((chan) * 0x4)) +#define SUN20I_PWM_CLK_CFG_SRC GENMASK(8, 7) +#define SUN20I_PWM_CLK_CFG_DIV_M GENMASK(3, 0) +#define SUN20I_PWM_CLK_DIV_M_MAX 8 + +#define SUN20I_PWM_CLK_GATE 0x40 +#define SUN20I_PWM_CLK_GATE_BYPASS (chan) BIT((chan) + 16) +#define SUN20I_PWM_CLK_GATE_GATING (chan) BIT(chan) + +#define SUN20I_PWM_ENABLE 0x80 +#define SUN20I_PWM_ENABLE_EN (chan) BIT(chan) +
drivers/pwm/pwm-sun20i.c line 44
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+#define SUN20I_PWM_CTL (chan) (0x100 + (chan) * 0x20) +#define SUN20I_PWM_CTL_ACT_STA BIT(8) +#define SUN20I_PWM_CTL_PRESCAL_K GENMASK(7, 0) +#define SUN20I_PWM_CTL_PRESCAL_K_MAX field_max(SUN20I_PWM_CTL_PRESCAL_K) + +#define SUN20I_PWM_PERIOD (chan) (0x104 + (chan) * 0x20) +#define SUN20I_PWM_PERIOD_ENTIRE_CYCLE GENMASK(31, 16) +#define SUN20I_PWM_PERIOD_ACT_CYCLE GENMASK(15, 0) + +#define SUN20I_PWM_PCNTR_SIZE BIT(16)
It's a bit unfortunate that SUN20I_PWM_CLK_CFG is passed hwpwm/2 while SUN20I_PWM_CTL gets a plain hwpwm. I suggest to at least name the parameters differently.
Quoted 9 lines
drivers/pwm/pwm-sun20i.c new file +9
-0+/* +* SUN20I_PWM_MAGIC is used to quickly compute the values of the clock dividers +* div_m (SUN20I_PWM_CLK_CFG_DIV_M) & prescale_k (SUN20I_PWM_CTL_PRESCAL_K) +* without using a loop. These dividers limit the # of cycles in a period +* to SUN20I_PWM_PCNTR_SIZE by applying a scaling factor of +* 1/(div_m * (prescale_k + 1)) to the clock source. +* +* SUN20I_PWM_MAGIC is derived by solving for div_m and prescale_k +* such that for a given requested period,
drivers/pwm/pwm-sun20i.c line 64
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+* +* i) div_m is minimized for any prescale_k ≤ SUN20I_PWM_CTL_PRESCAL_K_MAX, +* ii) prescale_k is minimized. +* +* The derivation proceeds as follows, with val = # of cycles for requested +* period: +* +* for a given value of div_m we want the smallest prescale_k such that +* +* (val >> div_m) // (prescale_k + 1) ≤ 65536 (SUN20I_PWM_PCNTR_SIZE)
^
I'd add a = here ---------------------------------' to make it clear that SUN20I_PWM_PCNTR_SIZE is a description for 65536 and this isn't a multiplication.
Quoted 22 lines
drivers/pwm/pwm-sun20i.c new file +22
-0+* +* This is equivalent to: +* +* (val >> div_m) ≤ 65536 * (prescale_k + 1) + prescale_k +* ⟺ (val >> div_m) ≤ 65537 * prescale_k + 65536 +* ⟺ (val >> div_m) - 65536 ≤ 65537 * prescale_k +* ⟺ ((val >> div_m) - 65536) / 65537 ≤ prescale_k +* +* As prescale_k is integer, this becomes +* +* ((val >> div_m) - 65536) // 65537 ≤ prescale_k +* +* And is minimized at +* +* ((val >> div_m) - 65536) // 65537 +* +* Now we pick the smallest div_m that satifies prescale_k ≤ 255 +* (i.e SUN20I_PWM_CTL_PRESCAL_K_MAX), +* +* ((val >> div_m) - 65536) // 65537 ≤ 255 +* ⟺ (val >> div_m) - 65536 ≤ 255 * 65537 + 65536 +* ⟺ val >> div_m ≤ 255 * 65537 + 2 * 65536
drivers/pwm/pwm-sun20i.c line 96
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+* ⟺ val >> div_m < (255 * 65537 + 2 * 65536 + 1) +* ⟺ div_m = fls((val) / (255 * 65537 + 2 * 65536 + 1)) +* +* Suggested by Uwe Kleine-König +*/ +#define SUN20I_PWM_MAGIC (255 * 65537 + 2 * 65536 + 1) + +struct sun20i_pwm_chip { +struct clk *clk_bus, *clk_hosc, *clk_apb; +struct reset_control *rst;
clk_bus and rst are only used in probe and so can be a local variable there.
drivers/pwm/pwm-sun20i.c line 98
Quoted 2 lines
drivers/pwm/pwm-sun20i.c new file +2
-0+void __iomem *base; +struct mutex mutex;/* Protect PWM apply state */
Since commit 1cc2e1faafb3 ("pwm: Add more locking") that currently waits in next for the merge window the callbacks for a single chip are serialized, so (unless I miss something) this mutex can be dropped.
Quoted 27 lines
drivers/pwm/pwm-sun20i.c new file +27
-0+}; + +static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip (struct pwm_chip *chip) +{ +return pwmchip_get_drvdata (chip); +} + +static inline u32 sun20i_pwm_readl (struct sun20i_pwm_chip *chip, +unsigned long offset) +{ +return readl (chip->base + offset); +} + +static inline void sun20i_pwm_writel (struct sun20i_pwm_chip *chip, +u32 val,unsigned long offset) +{ +writel (val, chip->base + offset); +} + +static int sun20i_pwm_get_state (struct pwm_chip *chip, +struct pwm_device *pwm, +struct pwm_state *state) +{ +struct sun20i_pwm_chip *sun20i_chip =to_sun20i_pwm_chip (chip); +u16 ent_cycle, act_cycle, prescale_k; +u64 clk_rate, tmp; +u8 div_m;
drivers/pwm/pwm-sun20i.c line 135
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+u32 val; + +mutex_lock (&sun20i_chip->mutex); + + val =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CLK_CFG (pwm->hwpwm /2 )); + div_m =FIELD_GET (SUN20I_PWM_CLK_CFG_DIV_M , val); +if (div_m >SUN20I_PWM_CLK_DIV_M_MAX ) + div_m =SUN20I_PWM_CLK_DIV_M_MAX ; + +if (FIELD_GET (SUN20I_PWM_CLK_CFG_SRC , val) ==0 )
SUN20I_PWM_CLK_CFG_SRC is two bits wide. Do all values != 0 mean APB?
Quoted 77 lines
drivers/pwm/pwm-sun20i.c new file +77
-0+ clk_rate =clk_get_rate (sun20i_chip->clk_hosc); + else + clk_rate =clk_get_rate (sun20i_chip->clk_apb); + + val =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CTL (pwm->hwpwm)); + state->polarity = (SUN20I_PWM_CTL_ACT_STA & val) ? +PWM_POLARITY_NORMAL :PWM_POLARITY_INVERSED ; + + prescale_k =FIELD_GET (SUN20I_PWM_CTL_PRESCAL_K , val) +1 ; + + val =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_ENABLE ); + state->enabled = (SUN20I_PWM_ENABLE_EN (pwm->hwpwm) & val) ? true : false; + + val =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_PERIOD (pwm->hwpwm)); + +mutex_unlock (&sun20i_chip->mutex); + + act_cycle =FIELD_GET (SUN20I_PWM_PERIOD_ACT_CYCLE , val); + ent_cycle =FIELD_GET (SUN20I_PWM_PERIOD_ENTIRE_CYCLE , val); + +/* +* The duration of the active phase should not be longer +* than the duration of the period +*/ +if (act_cycle > ent_cycle) + act_cycle = ent_cycle; + +/* +* We have act_cycle <= ent_cycle <= 0xffff, prescale_k <= 0x100, +* div_m <= 8. So the multiplication fits into an u64 without +* overflow. +*/ + tmp = ((u64)(act_cycle) * prescale_k << div_m) *NSEC_PER_SEC ; + state->duty_cycle =DIV_ROUND_UP_ULL (tmp, clk_rate); + tmp = ((u64)(ent_cycle) * prescale_k << div_m) *NSEC_PER_SEC ; + state->period =DIV_ROUND_UP_ULL (tmp, clk_rate); + +return 0 ; +} + +static int sun20i_pwm_apply (struct pwm_chip *chip,struct pwm_device *pwm, +const struct pwm_state *state) +{ +struct sun20i_pwm_chip *sun20i_chip =to_sun20i_pwm_chip (chip); +u64 bus_rate, hosc_rate, val, ent_cycle, act_cycle; +u32 clk_gate, clk_cfg, pwm_en, ctl, reg_period; +u32 prescale_k, div_m; +bool use_bus_clk; + +guard (mutex)(&sun20i_chip->mutex); + + pwm_en =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_ENABLE ); + +if (state->enabled != pwm->state.enabled) { + clk_gate =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CLK_GATE ); + +if (!state->enabled) { + clk_gate &= ~SUN20I_PWM_CLK_GATE_GATING (pwm->hwpwm); + pwm_en &= ~SUN20I_PWM_ENABLE_EN (pwm->hwpwm); +sun20i_pwm_writel (sun20i_chip, pwm_en,SUN20I_PWM_ENABLE ); +sun20i_pwm_writel (sun20i_chip, clk_gate,SUN20I_PWM_CLK_GATE ); + +return 0 ; + } + } + + ctl =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CTL (pwm->hwpwm)); + clk_cfg =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CLK_CFG (pwm->hwpwm /2 )); + hosc_rate =clk_get_rate (sun20i_chip->clk_hosc); + bus_rate =clk_get_rate (sun20i_chip->clk_apb); +if (pwm_en &SUN20I_PWM_ENABLE_EN (pwm->hwpwm ^1 )) { +/* if the neighbor channel is enabled, check period only */ + use_bus_clk =FIELD_GET (SUN20I_PWM_CLK_CFG_SRC , clk_cfg) !=0 ; + val =mul_u64_u64_div_u64 (state->period, + (use_bus_clk ? bus_rate : hosc_rate), +NSEC_PER_SEC ); +
drivers/pwm/pwm-sun20i.c line 222
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+ div_m =FIELD_GET (SUN20I_PWM_CLK_CFG_DIV_M , clk_cfg); + }else { +/* +* Select the clock source based on the period, +* since bus_rate > hosc_rate, which means bus_rate +* can provide a higher frequency than hosc_rate. +*/ + use_bus_clk = false; + val =mul_u64_u64_div_u64 (state->period, hosc_rate,NSEC_PER_SEC ); +if (val <=1 ) {
Why is val == 1 already problematic?
Quoted 5 lines
drivers/pwm/pwm-sun20i.c new file +5
-0+ use_bus_clk = true; + val =mul_u64_u64_div_u64 (state->period, bus_rate,NSEC_PER_SEC ); +if (val <=1 ) +return -EINVAL ; + }
drivers/pwm/pwm-sun20i.c line 237
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+ div_m =fls (DIV_ROUND_DOWN_ULL (val,SUN20I_PWM_MAGIC )); +if (div_m >SUN20I_PWM_CLK_DIV_M_MAX ) +return -EINVAL ; + +/* set up the CLK_DIV_M and clock CLK_SRC */ + clk_cfg =FIELD_PREP (SUN20I_PWM_CLK_CFG_DIV_M , div_m); + clk_cfg |=FIELD_PREP (SUN20I_PWM_CLK_CFG_SRC , use_bus_clk); + +sun20i_pwm_writel (sun20i_chip, clk_cfg,SUN20I_PWM_CLK_CFG (pwm->hwpwm /2 )); + }
Does writing SUN20I_PWM_CLK_CFG already influence the output? If so this needs mentioning in the Limitations paragraph at the driver's top as the output might glitch more than suggested there currently.
drivers/pwm/pwm-sun20i.c line 241
Quoted 3 lines
drivers/pwm/pwm-sun20i.c new file +3
-0+/* calculate prescale_k, PWM entire cycle */ + ent_cycle = val >> div_m; + prescale_k =DIV_ROUND_DOWN_ULL (ent_cycle,65537 );
A #define for 65537?
drivers/pwm/pwm-sun20i.c line 248
Quoted 7 lines
drivers/pwm/pwm-sun20i.c new file +7
-0+if (prescale_k >SUN20I_PWM_CTL_PRESCAL_K_MAX ) + prescale_k =SUN20I_PWM_CTL_PRESCAL_K_MAX ; + +do_div (ent_cycle, prescale_k +1 ); + +/* for N cycles, PPRx.PWM_ENTIRE_CYCLE = (N-1) */ + reg_period =FIELD_PREP (SUN20I_PWM_PERIOD_ENTIRE_CYCLE , ent_cycle -1 );
Is ent_cycle known to be ≥ 1?
Quoted 19 lines
drivers/pwm/pwm-sun20i.c new file +19
-0+/* set duty cycle */ + val =mul_u64_u64_div_u64 (state->duty_cycle, + (use_bus_clk ? bus_rate : hosc_rate), +NSEC_PER_SEC ); + act_cycle = val >> div_m; +do_div (act_cycle, prescale_k +1 ); + +/* +* The formula of the output period and the duty-cycle for PWM are as follows. +* T period = PWM0_PRESCALE_K / PWM01_CLK * (PPR0.PWM_ENTIRE_CYCLE + 1) +* T high-level = PWM0_PRESCALE_K / PWM01_CLK * PPR0.PWM_ACT_CYCLE +* Duty-cycle = T high-level / T period +*/ + reg_period |=FIELD_PREP (SUN20I_PWM_PERIOD_ACT_CYCLE , act_cycle); +sun20i_pwm_writel (sun20i_chip, reg_period,SUN20I_PWM_PERIOD (pwm->hwpwm)); + + ctl =FIELD_PREP (SUN20I_PWM_CTL_PRESCAL_K , prescale_k); +if (state->polarity ==PWM_POLARITY_NORMAL ) + ctl |=SUN20I_PWM_CTL_ACT_STA ;
drivers/pwm/pwm-sun20i.c line 278
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+ +sun20i_pwm_writel (sun20i_chip, ctl,SUN20I_PWM_CTL (pwm->hwpwm)); + +if (state->enabled != pwm->state.enabled && state->enabled) { + clk_gate &= ~SUN20I_PWM_CLK_GATE_BYPASS (pwm->hwpwm); + clk_gate |=SUN20I_PWM_CLK_GATE_GATING (pwm->hwpwm); + pwm_en |=SUN20I_PWM_ENABLE_EN (pwm->hwpwm); +sun20i_pwm_writel (sun20i_chip, pwm_en,SUN20I_PWM_ENABLE ); +sun20i_pwm_writel (sun20i_chip, clk_gate,SUN20I_PWM_CLK_GATE ); + }
There is an early return at the start of the function for state->enabled != pwm->state.enabled && !state->enabled . So just checking for
if (state->enabled != pwm->state.enabled)
is enough.
drivers/pwm/pwm-sun20i.c line 290
Quoted 12 lines
drivers/pwm/pwm-sun20i.c new file +12
-0+ +return 0 ; +} + +static const struct pwm_ops sun20i_pwm_ops = { + .apply = sun20i_pwm_apply, + .get_state = sun20i_pwm_get_state, +}; + +static const struct of_device_id sun20i_pwm_dt_ids[] = { + { .compatible ="allwinner,sun20i-d1-pwm" }, + { },
No comma after the sentinel entry please.
Quoted 19 lines
drivers/pwm/pwm-sun20i.c new file +19
-0+}; +MODULE_DEVICE_TABLE (of, sun20i_pwm_dt_ids); + +static void sun20i_pwm_reset_ctrl_release (void *data) +{ +struct reset_control *rst = data; + +reset_control_assert (rst); +} + +static int sun20i_pwm_probe (struct platform_device *pdev) +{ +struct pwm_chip *chip; +struct sun20i_pwm_chip *sun20i_chip; +int ret; + + chip =devm_pwmchip_alloc (&pdev->dev,8 ,sizeof (*sun20i_chip)); +if (IS_ERR (chip)) +return PTR_ERR (chip);
drivers/pwm/pwm-sun20i.c line 319
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+ sun20i_chip =to_sun20i_pwm_chip (chip); + + sun20i_chip->base =devm_platform_ioremap_resource (pdev,0 ); +if (IS_ERR (sun20i_chip->base)) +return PTR_ERR (sun20i_chip->base); + + sun20i_chip->clk_bus =devm_clk_get_enabled (&pdev->dev,"bus" ); +if (IS_ERR (sun20i_chip->clk_bus)) +return dev_err_probe (&pdev->dev,PTR_ERR (sun20i_chip->clk_bus), +"failed to get bus clock\n" );
I like error messages starting with a capital letter. Would you mind converting accordingly?
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+ sun20i_chip->clk_hosc =devm_clk_get_enabled (&pdev->dev,"hosc" ); +if (IS_ERR (sun20i_chip->clk_hosc)) +return dev_err_probe (&pdev->dev,PTR_ERR (sun20i_chip->clk_hosc), +"failed to get hosc clock\n" ); + + sun20i_chip->clk_apb =devm_clk_get_enabled (&pdev->dev,"apb" ); +if (IS_ERR (sun20i_chip->clk_apb)) +return dev_err_probe (&pdev->dev,PTR_ERR (sun20i_chip->clk_apb), +"failed to get apb clock\n" ); +
drivers/pwm/pwm-sun20i.c line 340
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+if (clk_get_rate (sun20i_chip->clk_apb) >clk_get_rate (sun20i_chip->clk_hosc)) +dev_info (&pdev->dev,"apb clock must be greater than hosc clock" ); + + sun20i_chip->rst =devm_reset_control_get_exclusive (&pdev->dev,NULL ); +if (IS_ERR (sun20i_chip->rst)) +return dev_err_probe (&pdev->dev,PTR_ERR (sun20i_chip->rst), +"failed to get bus reset\n" ); + + ret =of_property_read_u32 (pdev->dev.of_node,"allwinner,pwm-channels" , + &chip->npwm);
error checking for ret?
drivers/pwm/pwm-sun20i.c line 345
Quoted 4 lines
drivers/pwm/pwm-sun20i.c new file +4
-0+if (chip->npwm >16 ) { +dev_info (&pdev->dev,"limiting number of PWM lines from %u to 16" , + chip->npwm); + chip->npwm =16 ;
Layer violation; drivers are not supposed to assign npwm. Also above you only allocated 8. Better check allwinner,pwm-channels before calling devm_pwmchip_alloc().
drivers/pwm/pwm-sun20i.c line 355
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+ } + +/* Deassert reset */ + ret =reset_control_deassert (sun20i_chip->rst); +if (ret) +return dev_err_probe (&pdev->dev, ret,"failed to deassert reset\n" ); + + ret =devm_add_action_or_reset (&pdev->dev, sun20i_pwm_reset_ctrl_release, sun20i_chip->rst); +if (ret) +return ret;
There is devm_reset_control_get_exclusive_asserted() scheduled to go into v6.13-rc1. Please group the operations concerning rst together such that it can be converted trivially to that function.
drivers/pwm/pwm-sun20i.c line 366
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+ chip->ops = &sun20i_pwm_ops; + +mutex_init (&sun20i_chip->mutex); + + ret =devm_pwmchip_add (&pdev->dev, chip); +if (ret <0 ) +return dev_err_probe (&pdev->dev, ret,"failed to add PWM chip\n" ); + +return 0 ; +}
Best regards
Uwe
Re: [PATCH v10 2/3] pwm: Add Allwinner's D1/T113-S3/R329 SoCs PWM support
In reply to Aleksandr ShubinQuoted 6 lines · Aleksandr Shubin
Allwinner's D1, T113-S3 and R329 SoCs have a quite different PWM controllers with ones supported by pwm-sun4i driver.
This patch adds a PWM controller driver for Allwinner's D1, T113-S3 and R329 SoCs. The main difference between these SoCs is the number of channels defined by the DT property.
Thanks for your efforts. A133 uses the same IP and tried using the pwm for LVDS backlight with below diff,
arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi +10 -0
@@ -204,6 +204,16 @@ watchdog@30090a0 { clocks = <&dcxo24M>; }; + pwm: pwm@300a000 { + compatible = "allwinner,sun20i-d1-pwm"; + reg = <0x300a000 0x400>; + clocks = <&ccu CLK_BUS_PWM>, <&dcxo24M>, <&ccu CLK_APB1>; + clock-names = "bus", "hosc", "apb"; + resets = <&ccu RST_BUS_PWM>; + #pwm-cells = <0x3>; + status = "disabled"; + }; +
arch/arm64/boot/dts/allwinner/sun50i-a133-helper-board.dts +8 -0
@@ -19,6 +19,14 @@ aliases { serial0 = &uart0; }; + backlight: backlight { + compatible = "pwm-backlight"; + pwms = <&pwm 0 50000 0>; + brightness-levels = <0 5 10 15 20 30 40 55 70 85 100>; + default-brightness-level = <2>; + enable-gpios = <&pio 3 23 GPIO_ACTIVE_HIGH>; /* PD23 */ + }; +
Although the backlight tunrned on and pwm probed fine, I couldn't really control the backlight brightness levels.
root@helper-board-a133:~# dmesg | grep -i pwm [ 0.969739] sun20i-pwm 300a000.pwm: apb clock must be greater than hosc clock [ 1.486000] sun20i-pwm 300a000.pwm: .apply is not idempotent (ena=1 pol=0 5000/49959) -> (ena=1 pol=0 5000/49917) [ 410.760633] sun20i-pwm 300a000.pwm: .apply is not idempotent (ena=1 pol=0 49959/49959) -> (ena=1 pol=0 49917/49917) [ 425.284650] sun20i-pwm 300a000.pwm: .apply is not idempotent (ena=1 pol=0 5000/49959) -> (ena=1 pol=0 5000/49917) [ 428.180657] sun20i-pwm 300a000.pwm: .apply is not idempotent (ena=1 pol=0 2500/49959) -> (ena=1 pol=0 2500/49917) [ 431.952648] sun20i-pwm 300a000.pwm: .apply is not idempotent (ena=1 pol=0 49959/49959) -> (ena=1 pol=0 49917/49917)
Did I miss something?
Quoted 361 lines
Co-developed-by Brandon Cheo Fusi <[email protected]>Signed-off-by Brandon Cheo Fusi <[email protected]>Signed-off-by Aleksandr Shubin <[email protected]>--- drivers/pwm/Kconfig | 10 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-sun20i.c | 379 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 390 insertions(+) create mode 100644 drivers/pwm/pwm-sun20i.cdrivers/pwm/Kconfig +10
-0@@ -652,6 +652,16 @@ config PWM_SUN4I To compile this driver as a module, choose M here: the module will be called pwm-sun4i. +config PWM_SUN20I + tristate "Allwinner D1/T113s/R329 PWM support" + depends on ARCH_SUNXI || COMPILE_TEST + depends on COMMON_CLK + help + Generic PWM framework driver for Allwinner D1/T113s/R329 SoCs. + + To compile this driver as a module, choose M here: the module + will be called pwm-sun20i. + config PWM_SUNPLUS tristate "Sunplus PWM support" depends on ARCH_SUNPLUS || COMPILE_TESTdrivers/pwm/Makefile +1
-0@@ -60,6 +60,7 @@ obj-$(CONFIG_PWM_STM32) += pwm-stm32.o obj-$(CONFIG_PWM_STM32_LP) += pwm-stm32-lp.o obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o +obj-$(CONFIG_PWM_SUN20I) += pwm-sun20i.o obj-$(CONFIG_PWM_SUNPLUS) += pwm-sunplus.o obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.odrivers/pwm/pwm-sun20i.c new file +311
-0@@ -0,0 +1,379 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* PWM Controller Driver for sunxi platforms (D1, T113-S3 and R329) +* +* Limitations: +* - When the parameters change, current running period will not be completed +* and run new settings immediately. +* - It output HIGH-Z state when PWM channel disabled. +* +* Copyright (c) 2023 Aleksandr Shubin <[email protected]> +*/ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/reset.h> + +#define SUN20I_PWM_CLK_CFG (chan) (0x20 + ((chan) * 0x4)) +#define SUN20I_PWM_CLK_CFG_SRC GENMASK(8, 7) +#define SUN20I_PWM_CLK_CFG_DIV_M GENMASK(3, 0) +#define SUN20I_PWM_CLK_DIV_M_MAX 8 + +#define SUN20I_PWM_CLK_GATE 0x40 +#define SUN20I_PWM_CLK_GATE_BYPASS (chan) BIT((chan) + 16) +#define SUN20I_PWM_CLK_GATE_GATING (chan) BIT(chan) + +#define SUN20I_PWM_ENABLE 0x80 +#define SUN20I_PWM_ENABLE_EN (chan) BIT(chan) + +#define SUN20I_PWM_CTL (chan) (0x100 + (chan) * 0x20) +#define SUN20I_PWM_CTL_ACT_STA BIT(8) +#define SUN20I_PWM_CTL_PRESCAL_K GENMASK(7, 0) +#define SUN20I_PWM_CTL_PRESCAL_K_MAX field_max(SUN20I_PWM_CTL_PRESCAL_K) + +#define SUN20I_PWM_PERIOD (chan) (0x104 + (chan) * 0x20) +#define SUN20I_PWM_PERIOD_ENTIRE_CYCLE GENMASK(31, 16) +#define SUN20I_PWM_PERIOD_ACT_CYCLE GENMASK(15, 0) + +#define SUN20I_PWM_PCNTR_SIZE BIT(16) + +/* +* SUN20I_PWM_MAGIC is used to quickly compute the values of the clock dividers +* div_m (SUN20I_PWM_CLK_CFG_DIV_M) & prescale_k (SUN20I_PWM_CTL_PRESCAL_K) +* without using a loop. These dividers limit the # of cycles in a period +* to SUN20I_PWM_PCNTR_SIZE by applying a scaling factor of +* 1/(div_m * (prescale_k + 1)) to the clock source. +* +* SUN20I_PWM_MAGIC is derived by solving for div_m and prescale_k +* such that for a given requested period, +* +* i) div_m is minimized for any prescale_k ≤ SUN20I_PWM_CTL_PRESCAL_K_MAX, +* ii) prescale_k is minimized. +* +* The derivation proceeds as follows, with val = # of cycles for requested +* period: +* +* for a given value of div_m we want the smallest prescale_k such that +* +* (val >> div_m) // (prescale_k + 1) ≤ 65536 (SUN20I_PWM_PCNTR_SIZE) +* +* This is equivalent to: +* +* (val >> div_m) ≤ 65536 * (prescale_k + 1) + prescale_k +* ⟺ (val >> div_m) ≤ 65537 * prescale_k + 65536 +* ⟺ (val >> div_m) - 65536 ≤ 65537 * prescale_k +* ⟺ ((val >> div_m) - 65536) / 65537 ≤ prescale_k +* +* As prescale_k is integer, this becomes +* +* ((val >> div_m) - 65536) // 65537 ≤ prescale_k +* +* And is minimized at +* +* ((val >> div_m) - 65536) // 65537 +* +* Now we pick the smallest div_m that satifies prescale_k ≤ 255 +* (i.e SUN20I_PWM_CTL_PRESCAL_K_MAX), +* +* ((val >> div_m) - 65536) // 65537 ≤ 255 +* ⟺ (val >> div_m) - 65536 ≤ 255 * 65537 + 65536 +* ⟺ val >> div_m ≤ 255 * 65537 + 2 * 65536 +* ⟺ val >> div_m < (255 * 65537 + 2 * 65536 + 1) +* ⟺ div_m = fls((val) / (255 * 65537 + 2 * 65536 + 1)) +* +* Suggested by Uwe Kleine-König +*/ +#define SUN20I_PWM_MAGIC (255 * 65537 + 2 * 65536 + 1) + +struct sun20i_pwm_chip { +struct clk *clk_bus, *clk_hosc, *clk_apb; +struct reset_control *rst; +void __iomem *base; +struct mutex mutex;/* Protect PWM apply state */ +}; + +static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip (struct pwm_chip *chip) +{ +return pwmchip_get_drvdata (chip); +} + +static inline u32 sun20i_pwm_readl (struct sun20i_pwm_chip *chip, +unsigned long offset) +{ +return readl (chip->base + offset); +} + +static inline void sun20i_pwm_writel (struct sun20i_pwm_chip *chip, +u32 val,unsigned long offset) +{ +writel (val, chip->base + offset); +} + +static int sun20i_pwm_get_state (struct pwm_chip *chip, +struct pwm_device *pwm, +struct pwm_state *state) +{ +struct sun20i_pwm_chip *sun20i_chip =to_sun20i_pwm_chip (chip); +u16 ent_cycle, act_cycle, prescale_k; +u64 clk_rate, tmp; +u8 div_m; +u32 val; + +mutex_lock (&sun20i_chip->mutex); + + val =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CLK_CFG (pwm->hwpwm /2 )); + div_m =FIELD_GET (SUN20I_PWM_CLK_CFG_DIV_M , val); +if (div_m >SUN20I_PWM_CLK_DIV_M_MAX ) + div_m =SUN20I_PWM_CLK_DIV_M_MAX ; + +if (FIELD_GET (SUN20I_PWM_CLK_CFG_SRC , val) ==0 ) + clk_rate =clk_get_rate (sun20i_chip->clk_hosc); +else + clk_rate =clk_get_rate (sun20i_chip->clk_apb); + + val =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CTL (pwm->hwpwm)); + state->polarity = (SUN20I_PWM_CTL_ACT_STA & val) ? +PWM_POLARITY_NORMAL :PWM_POLARITY_INVERSED ; + + prescale_k =FIELD_GET (SUN20I_PWM_CTL_PRESCAL_K , val) +1 ; + + val =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_ENABLE ); + state->enabled = (SUN20I_PWM_ENABLE_EN (pwm->hwpwm) & val) ? true : false; + + val =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_PERIOD (pwm->hwpwm)); + +mutex_unlock (&sun20i_chip->mutex); + + act_cycle =FIELD_GET (SUN20I_PWM_PERIOD_ACT_CYCLE , val); + ent_cycle =FIELD_GET (SUN20I_PWM_PERIOD_ENTIRE_CYCLE , val); + +/* +* The duration of the active phase should not be longer +* than the duration of the period +*/ +if (act_cycle > ent_cycle) + act_cycle = ent_cycle; + +/* +* We have act_cycle <= ent_cycle <= 0xffff, prescale_k <= 0x100, +* div_m <= 8. So the multiplication fits into an u64 without +* overflow. +*/ + tmp = ((u64)(act_cycle) * prescale_k << div_m) *NSEC_PER_SEC ; + state->duty_cycle =DIV_ROUND_UP_ULL (tmp, clk_rate); + tmp = ((u64)(ent_cycle) * prescale_k << div_m) *NSEC_PER_SEC ; + state->period =DIV_ROUND_UP_ULL (tmp, clk_rate); + +return 0 ; +} + +static int sun20i_pwm_apply (struct pwm_chip *chip,struct pwm_device *pwm, +const struct pwm_state *state) +{ +struct sun20i_pwm_chip *sun20i_chip =to_sun20i_pwm_chip (chip); +u64 bus_rate, hosc_rate, val, ent_cycle, act_cycle; +u32 clk_gate, clk_cfg, pwm_en, ctl, reg_period; +u32 prescale_k, div_m; +bool use_bus_clk; + +guard (mutex)(&sun20i_chip->mutex); + + pwm_en =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_ENABLE ); + +if (state->enabled != pwm->state.enabled) { + clk_gate =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CLK_GATE ); + +if (!state->enabled) { + clk_gate &= ~SUN20I_PWM_CLK_GATE_GATING (pwm->hwpwm); + pwm_en &= ~SUN20I_PWM_ENABLE_EN (pwm->hwpwm); +sun20i_pwm_writel (sun20i_chip, pwm_en,SUN20I_PWM_ENABLE ); +sun20i_pwm_writel (sun20i_chip, clk_gate,SUN20I_PWM_CLK_GATE ); + +return 0 ; + } + } + + ctl =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CTL (pwm->hwpwm)); + clk_cfg =sun20i_pwm_readl (sun20i_chip,SUN20I_PWM_CLK_CFG (pwm->hwpwm /2 )); + hosc_rate =clk_get_rate (sun20i_chip->clk_hosc); + bus_rate =clk_get_rate (sun20i_chip->clk_apb); +if (pwm_en &SUN20I_PWM_ENABLE_EN (pwm->hwpwm ^1 )) { +/* if the neighbor channel is enabled, check period only */ + use_bus_clk =FIELD_GET (SUN20I_PWM_CLK_CFG_SRC , clk_cfg) !=0 ; + val =mul_u64_u64_div_u64 (state->period, + (use_bus_clk ? bus_rate : hosc_rate), +NSEC_PER_SEC ); + + div_m =FIELD_GET (SUN20I_PWM_CLK_CFG_DIV_M , clk_cfg); + }else { +/* +* Select the clock source based on the period, +* since bus_rate > hosc_rate, which means bus_rate +* can provide a higher frequency than hosc_rate. +*/ + use_bus_clk = false; + val =mul_u64_u64_div_u64 (state->period, hosc_rate,NSEC_PER_SEC ); +if (val <=1 ) { + use_bus_clk = true; + val =mul_u64_u64_div_u64 (state->period, bus_rate,NSEC_PER_SEC ); +if (val <=1 ) +return -EINVAL ; + } + div_m =fls (DIV_ROUND_DOWN_ULL (val,SUN20I_PWM_MAGIC )); +if (div_m >SUN20I_PWM_CLK_DIV_M_MAX ) +return -EINVAL ; + +/* set up the CLK_DIV_M and clock CLK_SRC */ + clk_cfg =FIELD_PREP (SUN20I_PWM_CLK_CFG_DIV_M , div_m); + clk_cfg |=FIELD_PREP (SUN20I_PWM_CLK_CFG_SRC , use_bus_clk); + +sun20i_pwm_writel (sun20i_chip, clk_cfg,SUN20I_PWM_CLK_CFG (pwm->hwpwm /2 )); + } + +/* calculate prescale_k, PWM entire cycle */ + ent_cycle = val >> div_m; + prescale_k =DIV_ROUND_DOWN_ULL (ent_cycle,65537 ); +if (prescale_k >SUN20I_PWM_CTL_PRESCAL_K_MAX ) + prescale_k =SUN20I_PWM_CTL_PRESCAL_K_MAX ; + +do_div (ent_cycle, prescale_k +1 ); + +/* for N cycles, PPRx.PWM_ENTIRE_CYCLE = (N-1) */ + reg_period =FIELD_PREP (SUN20I_PWM_PERIOD_ENTIRE_CYCLE , ent_cycle -1 ); + +/* set duty cycle */ + val =mul_u64_u64_div_u64 (state->duty_cycle, + (use_bus_clk ? bus_rate : hosc_rate), +NSEC_PER_SEC ); + act_cycle = val >> div_m; +do_div (act_cycle, prescale_k +1 ); + +/* +* The formula of the output period and the duty-cycle for PWM are as follows. +* T period = PWM0_PRESCALE_K / PWM01_CLK * (PPR0.PWM_ENTIRE_CYCLE + 1) +* T high-level = PWM0_PRESCALE_K / PWM01_CLK * PPR0.PWM_ACT_CYCLE +* Duty-cycle = T high-level / T period +*/ + reg_period |=FIELD_PREP (SUN20I_PWM_PERIOD_ACT_CYCLE , act_cycle); +sun20i_pwm_writel (sun20i_chip, reg_period,SUN20I_PWM_PERIOD (pwm->hwpwm)); + + ctl =FIELD_PREP (SUN20I_PWM_CTL_PRESCAL_K , prescale_k); +if (state->polarity ==PWM_POLARITY_NORMAL ) + ctl |=SUN20I_PWM_CTL_ACT_STA ; + +sun20i_pwm_writel (sun20i_chip, ctl,SUN20I_PWM_CTL (pwm->hwpwm)); + +if (state->enabled != pwm->state.enabled && state->enabled) { + clk_gate &= ~SUN20I_PWM_CLK_GATE_BYPASS (pwm->hwpwm); + clk_gate |=SUN20I_PWM_CLK_GATE_GATING (pwm->hwpwm); + pwm_en |=SUN20I_PWM_ENABLE_EN (pwm->hwpwm); +sun20i_pwm_writel (sun20i_chip, pwm_en,SUN20I_PWM_ENABLE ); +sun20i_pwm_writel (sun20i_chip, clk_gate,SUN20I_PWM_CLK_GATE ); + } + +return 0 ; +} + +static const struct pwm_ops sun20i_pwm_ops = { + .apply = sun20i_pwm_apply, + .get_state = sun20i_pwm_get_state, +}; + +static const struct of_device_id sun20i_pwm_dt_ids[] = { + { .compatible ="allwinner,sun20i-d1-pwm" }, + { }, +}; +MODULE_DEVICE_TABLE (of, sun20i_pwm_dt_ids); + +static void sun20i_pwm_reset_ctrl_release (void *data) +{ +struct reset_control *rst = data; + +reset_control_assert (rst); +} + +static int sun20i_pwm_probe (struct platform_device *pdev) +{ +struct pwm_chip *chip; +struct sun20i_pwm_chip *sun20i_chip; +int ret; + + chip =devm_pwmchip_alloc (&pdev->dev,8 ,sizeof (*sun20i_chip)); +if (IS_ERR (chip)) +return PTR_ERR (chip); + sun20i_chip =to_sun20i_pwm_chip (chip); +
drivers/pwm/pwm-sun20i.c line 321
Quoted 10 lines
drivers/pwm/pwm-sun20i.c new file +10
-0+ sun20i_chip->base =devm_platform_ioremap_resource (pdev,0 ); +if (IS_ERR (sun20i_chip->base)) +return PTR_ERR (sun20i_chip->base); + + sun20i_chip->clk_bus =devm_clk_get_enabled (&pdev->dev,"bus" ); +if (IS_ERR (sun20i_chip->clk_bus)) +return dev_err_probe (&pdev->dev,PTR_ERR (sun20i_chip->clk_bus), +"failed to get bus clock\n" ); + + sun20i_chip->clk_hosc =devm_clk_get_enabled (&pdev->dev,"hosc" );
A133 also have r_pwm,
+ r_pwm: pwm@7020c00 {
+ compatible = "allwinner,sun20i-d1-pwm"; + reg = <0x7020c00 0x400>; + clocks = <&ccu CLK_R_APB1_PWM>, <&ccu CLK_R_APB1_BUS_PWM>, <&ccu CLK_APB1>; + clock-names = "bus", "mod", "apb"; + resets = <&r_ccu RST_R_APB1_BUS_PWM>; + #pwm-cells = <0x3>; + status = "disabled";
+ };
+
with "bus" clock and "mod" clock for gating it. Calling it as "hosc" doesn't makes sense IMO. pwm-sun4i.c handles it already. Do we also need to handled "mod"?
drivers/pwm/pwm-sun20i.c line 332
Quoted 11 lines
drivers/pwm/pwm-sun20i.c new file +11
-0+if (IS_ERR (sun20i_chip->clk_hosc)) +return dev_err_probe (&pdev->dev,PTR_ERR (sun20i_chip->clk_hosc), +"failed to get hosc clock\n" ); + + sun20i_chip->clk_apb =devm_clk_get_enabled (&pdev->dev,"apb" ); +if (IS_ERR (sun20i_chip->clk_apb)) +return dev_err_probe (&pdev->dev,PTR_ERR (sun20i_chip->clk_apb), +"failed to get apb clock\n" ); + +if (clk_get_rate (sun20i_chip->clk_apb) >clk_get_rate (sun20i_chip->clk_hosc)) +dev_info (&pdev->dev,"apb clock must be greater than hosc clock" );
This is always the case if am not wrong. This condition check needed?
Thanks,
Parthiban
Quoted 47 lines
drivers/pwm/pwm-sun20i.c new file +47
-0+ + sun20i_chip->rst =devm_reset_control_get_exclusive (&pdev->dev,NULL ); +if (IS_ERR (sun20i_chip->rst)) +return dev_err_probe (&pdev->dev,PTR_ERR (sun20i_chip->rst), +"failed to get bus reset\n" ); + + ret =of_property_read_u32 (pdev->dev.of_node,"allwinner,pwm-channels" , + &chip->npwm); + +if (chip->npwm >16 ) { +dev_info (&pdev->dev,"limiting number of PWM lines from %u to 16" , + chip->npwm); + chip->npwm =16 ; + } + +/* Deassert reset */ + ret =reset_control_deassert (sun20i_chip->rst); +if (ret) +return dev_err_probe (&pdev->dev, ret,"failed to deassert reset\n" ); + + ret =devm_add_action_or_reset (&pdev->dev, sun20i_pwm_reset_ctrl_release, sun20i_chip->rst); +if (ret) +return ret; + + chip->ops = &sun20i_pwm_ops; + +mutex_init (&sun20i_chip->mutex); + + ret =devm_pwmchip_add (&pdev->dev, chip); +if (ret <0 ) +return dev_err_probe (&pdev->dev, ret,"failed to add PWM chip\n" ); + +return 0 ; +} + +static struct platform_driver sun20i_pwm_driver = { + .driver = { + .name ="sun20i-pwm" , + .of_match_table = sun20i_pwm_dt_ids, + }, + .probe = sun20i_pwm_probe, +}; +module_platform_driver (sun20i_pwm_driver); + +MODULE_AUTHOR ("Aleksandr Shubin <[email protected]>" ); +MODULE_DESCRIPTION ("Allwinner sun20i PWM driver" ); +MODULE_LICENSE ("GPL" );
PATCH v10 3/3 riscv: dts: allwinner: d1: Add pwm node
In reply to Aleksandr ShubinD1 and T113s contain a pwm controller with 8 channels. This controller is supported by the sun20i-pwm driver.
Add a device tree node for it.
--- arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi | 12 ++++++++++++ 1 file changed, 12 insertions(+)
arch/riscv/boot/dts/allwinner/sunxi-d1s-t113.dtsi +12 -0
@@ -145,6 +145,18 @@ uart3_pb_pins: uart3-pb-pins { }; }; + pwm: pwm@2000c00 { + compatible = "allwinner,sun20i-d1-pwm"; + reg = <0x02000c00 0x400>; + clocks = <&ccu CLK_BUS_PWM>, + <&dcxo>, + <&ccu CLK_APB0>; + clock-names = "bus", "hosc", "apb"; + resets = <&ccu RST_BUS_PWM>; + status = "disabled"; + #pwm-cells = <0x3>; + }; + ccu: clock-controller@2001000 { compatible = "allwinner,sun20i-d1-ccu"; reg = <0x2001000 0x1000>;
Signature · 1 line
2.25.1
Quoted 40 lines · Aleksandr Shubin
v2:
- fix dt-bindings
- fix a remark in the driverv3:
- fix dt-bindings
- fixsunxi-d1s-t113.dtsiv4:
- fix a remark in the driverv5:
- dropped unused varibale in the driver
- fix dt-bindingsv6:
- add apb0 clockv7:
- fix a remark in the driver
- add maintainerv8:
- fix compile driver for 6.8-rcv9:
- fix a remark in the driver
- fix dt-bindings
- rename apb0 -> apbv10:
- fix a remark in the driver
- fix compile driver for 6.12-rc2Aleksandr Shubin (3):
dt-bindings: pwm: Add binding for Allwinner D1/T113-S3/R329 PWM
controller
pwm: Add Allwinner's D1/T113-S3/R329 SoCs PWM support
riscv: dts: allwinner: d1: Add pwm node
Quoted 12 lines
.../bindings/pwm/allwinner,sun20i-pwm.yaml | 84 ++++ .../boot/dts/allwinner/sunxi-d1s-t113.dtsi | 12 + drivers/pwm/Kconfig | 10 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-sun20i.c | 379 ++++++++++++++++++ 5 files changed, 486 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/allwinner,sun20i-pwm.yaml create mode 100644 drivers/pwm/pwm-sun20i.cSignature · 1 line
2.25.1
This patch has been idle for a few months, would you mind if I took over shepherding it to mainline? While I don't have *this* hardware to test it with I have an H616 which has a similar pwm controller (and for which I was going to add support for once this made it to mainline). Otherwise, I can just wait patiently and then tack on my additions once this has been accepted.
Thank you,
Chris