| Message ID | 20230627082334.1253020-3-privatesub2@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Allwinner PWM on D1/T113s/R329 SoCs | expand |
On Di, 2023-06-27 at 11:23 +0300, Aleksandr Shubin wrote: [...] > +static int sun20i_pwm_probe(struct platform_device *pdev) > +{ [...] > + sun20i_chip->rst = devm_reset_control_get(&pdev->dev, NULL); Please use devm_reset_control_get_exclusive() directly. regards Philipp
Hello, On Tue, Jun 27, 2023 at 11:23:25AM +0300, Aleksandr Shubin wrote: > 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. > > Signed-off-by: Aleksandr Shubin <privatesub2@gmail.com> > --- > drivers/pwm/Kconfig | 10 ++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-sun20i.c | 322 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 333 insertions(+) > create mode 100644 drivers/pwm/pwm-sun20i.c > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 8df861b1f4a3..05c48a36969e 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -594,6 +594,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 > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index 19899b912e00..cea872e22c78 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -55,6 +55,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 > diff --git a/drivers/pwm/pwm-sun20i.c b/drivers/pwm/pwm-sun20i.c > new file mode 100644 > index 000000000000..63e9c64e0e18 > --- /dev/null > +++ b/drivers/pwm/pwm-sun20i.c > @@ -0,0 +1,322 @@ > +// 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 <privatesub2@gmail.com> > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/pwm.h> > +#include <linux/reset.h> > + > +#define PWM_CLK_CFG_REG(chan) (0x20 + (((chan) >> 1) * 0x4)) > +#define PWM_CLK_SRC GENMASK(8, 7) > +#define PWM_CLK_DIV_M GENMASK(3, 0) > + > +#define PWM_CLK_GATE_REG 0x40 > +#define PWM_CLK_BYPASS(chan) BIT((chan) - 16) > +#define PWM_CLK_GATING(chan) BIT(chan) > + > +#define PWM_ENABLE_REG 0x80 > +#define PWM_EN(chan) BIT(chan) > + > +#define PWM_CTL_REG(chan) (0x100 + (chan) * 0x20) > +#define PWM_ACT_STA BIT(8) > +#define PWM_PRESCAL_K GENMASK(7, 0) > + > +#define PWM_PERIOD_REG(chan) (0x104 + (chan) * 0x20) > +#define PWM_ENTIRE_CYCLE GENMASK(31, 16) > +#define PWM_ACT_CYCLE GENMASK(15, 0) Can you please adapt the register field names to include the register name? I'd use: #define PWM_CTL(chan) (0x100 + (chan) * 0x20) #define PWM_CTL_ACT_STA BIT(8) #define PWM_CTL_PRESCAL_K GENMASK(7, 0) then you get a chance to spot when PWM_CLK_BYPASS(x) is written to PWM_CLK_CFG. > +struct sun20i_pwm_chip { > + struct pwm_chip chip; > + struct clk *clk_bus, *clk_hosc; > + struct reset_control *rst; > + void __iomem *base; > + /* Mutex to protect pwm apply state */ > + struct mutex mutex; > +}; > + > +static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip(struct pwm_chip *chip) > +{ > + return container_of(chip, struct sun20i_pwm_chip, 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); > + u64 clk_rate, tmp; > + u32 val; > + u16 ent_cycle, act_cycle; > + u8 prescal, div_id; > + > + mutex_lock(&sun20i_chip->mutex); > + > + val = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG_REG(pwm->hwpwm)); > + div_id = FIELD_GET(PWM_CLK_DIV_M, val); > + if (FIELD_GET(PWM_CLK_SRC, val) == 0) > + clk_rate = clk_get_rate(sun20i_chip->clk_hosc); > + else > + clk_rate = clk_get_rate(sun20i_chip->clk_bus); > + > + val = sun20i_pwm_readl(sun20i_chip, PWM_CTL_REG(pwm->hwpwm)); > + state->polarity = (PWM_ACT_STA & val) ? PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED; > + > + prescal = FIELD_GET(PWM_PRESCAL_K, val) + 1; If PWM_PRESCAL_K is 0xff, prescal ends up being 0. This isn't right, is it? > + val = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE_REG); > + state->enabled = (PWM_EN(pwm->hwpwm) & val) ? true : false; > + > + val = sun20i_pwm_readl(sun20i_chip, PWM_PERIOD_REG(pwm->hwpwm)); > + act_cycle = FIELD_GET(PWM_ACT_CYCLE, val); > + ent_cycle = FIELD_GET(PWM_ENTIRE_CYCLE, val); > + if (act_cycle > ent_cycle) > + act_cycle = ent_cycle; > + A comment that with the width of the used factors this cannot overflow would be nice here. > + tmp = (u64)(act_cycle) * prescal * (1U << div_id) * NSEC_PER_SEC; Can be simplified to: tmp = (u64)act_cycle * prescal << div_id * NSEC_PER_SEC; > + state->duty_cycle = DIV_ROUND_UP_ULL(tmp, clk_rate); > + tmp = (u64)(ent_cycle) * prescal * (1U << div_id) * NSEC_PER_SEC; > + state->period = DIV_ROUND_UP_ULL(tmp, clk_rate); > + mutex_unlock(&sun20i_chip->mutex); > + > + return 0; > +} > + > +static int sun20i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + int ret = 0; > + u32 clk_gate, clk_cfg, pwm_en, ctl, period; > + u64 bus_rate, hosc_rate, clk_div, val; > + u16 prescaler, div_m; > + bool use_bus_clk, calc_div_m; > + struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip); > + > + mutex_lock(&sun20i_chip->mutex); > + > + pwm_en = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE_REG); > + > + if (state->enabled != pwm->state.enabled) > + clk_gate = sun20i_pwm_readl(sun20i_chip, PWM_CLK_GATE_REG); > + > + if (state->enabled != pwm->state.enabled && !state->enabled) { > + clk_gate &= ~PWM_CLK_GATING(pwm->hwpwm); > + pwm_en &= ~PWM_EN(pwm->hwpwm); > + sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE_REG); > + sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE_REG); > + } > + > + if (state->polarity != pwm->state.polarity || > + state->duty_cycle != pwm->state.duty_cycle || > + state->period != pwm->state.period) { > + ctl = sun20i_pwm_readl(sun20i_chip, PWM_CTL_REG(pwm->hwpwm)); > + clk_cfg = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG_REG(pwm->hwpwm)); > + hosc_rate = clk_get_rate(sun20i_chip->clk_hosc); > + bus_rate = clk_get_rate(sun20i_chip->clk_bus); > + if (pwm_en & PWM_EN(pwm->hwpwm ^ 1)) { > + /* if the neighbor channel is enable, check period only */ > + use_bus_clk = FIELD_GET(PWM_CLK_SRC, clk_cfg) != 0; > + if (use_bus_clk) > + val = state->period * bus_rate; > + else > + val = state->period * hosc_rate; > + do_div(val, NSEC_PER_SEC); > + > + div_m = FIELD_GET(PWM_CLK_DIV_M, clk_cfg); > + calc_div_m = false; > + } else { > + /* check period and select clock source */ > + use_bus_clk = false; > + val = state->period * hosc_rate; > + do_div(val, NSEC_PER_SEC); > + if (val <= 1) { > + use_bus_clk = true; > + val = state->period * bus_rate; > + do_div(val, NSEC_PER_SEC); > + if (val <= 1) { > + ret = -EINVAL; > + goto unlock_mutex; > + } > + } > + div_m = 0; > + calc_div_m = true; > + > + /* set up the CLK_DIV_M and clock CLK_SRC */ > + clk_cfg = FIELD_PREP(PWM_CLK_DIV_M, div_m); > + clk_cfg |= FIELD_PREP(PWM_CLK_SRC, use_bus_clk ? 1 : 0); > + > + sun20i_pwm_writel(sun20i_chip, clk_cfg, PWM_CLK_CFG_REG(pwm->hwpwm)); > + } > + > + /* calculate prescaler, M factor, PWM entire cycle */ > + clk_div = val; This assignment is useless as it is overwritten in the loop below, isn't it? > + for (prescaler = 0;; prescaler++) { > + if (prescaler >= 256) { > + if (calc_div_m) { > + prescaler = 0; > + div_m++; > + if (div_m >= 9) { > + ret = -EINVAL; > + goto unlock_mutex; > + } > + } else { > + ret = -EINVAL; > + goto unlock_mutex; > + } > + } > + > + clk_div = val >> div_m; > + do_div(clk_div, prescaler + 1); > + if (clk_div <= 65534) > + break; This can be calculated without a loop. > + } > + > + period = FIELD_PREP(PWM_ENTIRE_CYCLE, clk_div); > + > + /* set duty cycle */ > + if (use_bus_clk) > + val = state->duty_cycle * bus_rate; > + else > + val = state->duty_cycle * hosc_rate; > + do_div(val, NSEC_PER_SEC); > + clk_div = val >> div_m; > + do_div(clk_div, prescaler + 1); > + > + if (state->duty_cycle == state->period) > + clk_div++; I don't understand that one. Can you explain that in a comment please? > + period |= FIELD_PREP(PWM_ACT_CYCLE, clk_div); > + sun20i_pwm_writel(sun20i_chip, period, PWM_PERIOD_REG(pwm->hwpwm)); > + > + ctl = FIELD_PREP(PWM_PRESCAL_K, prescaler); > + if (state->polarity == PWM_POLARITY_NORMAL) > + ctl |= PWM_ACT_STA; > + > + sun20i_pwm_writel(sun20i_chip, ctl, PWM_CTL_REG(pwm->hwpwm)); Is this racy? I.e. does the write to PWM_PERIOD_REG(pwm->hwpwm) above already has an effect before PWM_CTL_REG(pwm->hwpwm) is written? > + } > + > + if (state->enabled != pwm->state.enabled && state->enabled) { > + clk_gate &= ~PWM_CLK_BYPASS(pwm->hwpwm); > + clk_gate |= PWM_CLK_GATING(pwm->hwpwm); > + pwm_en |= PWM_EN(pwm->hwpwm); > + sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE_REG); > + sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE_REG); This is (I guess) racy. If your PWM is running with .period = 10000 .duty_cyle = 0 .enabled = true and you configure it to .period = 10000 .duty_cyle = 10000 .enabled = false you get a short spike. For a enabled=true -> enabled=false transition you should disable first before configuring duty+period (or skip the latter completely). > + } > + > +unlock_mutex: > + mutex_unlock(&sun20i_chip->mutex); > + > + return ret; > +} Best regards Uwe
Hi Uwe, thanks for having a look! Sorry for replying so late. пн, 10 июл. 2023 г. в 12:14, Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > > Hello, > > On Tue, Jun 27, 2023 at 11:23:25AM +0300, Aleksandr Shubin wrote: > > 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. > > > > Signed-off-by: Aleksandr Shubin <privatesub2@gmail.com> > > --- > > drivers/pwm/Kconfig | 10 ++ > > drivers/pwm/Makefile | 1 + > > drivers/pwm/pwm-sun20i.c | 322 +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 333 insertions(+) > > create mode 100644 drivers/pwm/pwm-sun20i.c > > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > > index 8df861b1f4a3..05c48a36969e 100644 > > --- a/drivers/pwm/Kconfig > > +++ b/drivers/pwm/Kconfig > > @@ -594,6 +594,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 > > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > > index 19899b912e00..cea872e22c78 100644 > > --- a/drivers/pwm/Makefile > > +++ b/drivers/pwm/Makefile > > @@ -55,6 +55,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 > > diff --git a/drivers/pwm/pwm-sun20i.c b/drivers/pwm/pwm-sun20i.c > > new file mode 100644 > > index 000000000000..63e9c64e0e18 > > --- /dev/null > > +++ b/drivers/pwm/pwm-sun20i.c > > @@ -0,0 +1,322 @@ > > +// 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 <privatesub2@gmail.com> > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/clk.h> > > +#include <linux/err.h> > > +#include <linux/io.h> > > +#include <linux/module.h> > > +#include <linux/of_device.h> > > +#include <linux/pwm.h> > > +#include <linux/reset.h> > > + > > +#define PWM_CLK_CFG_REG(chan) (0x20 + (((chan) >> 1) * 0x4)) > > +#define PWM_CLK_SRC GENMASK(8, 7) > > +#define PWM_CLK_DIV_M GENMASK(3, 0) > > + > > +#define PWM_CLK_GATE_REG 0x40 > > +#define PWM_CLK_BYPASS(chan) BIT((chan) - 16) > > +#define PWM_CLK_GATING(chan) BIT(chan) > > + > > +#define PWM_ENABLE_REG 0x80 > > +#define PWM_EN(chan) BIT(chan) > > + > > +#define PWM_CTL_REG(chan) (0x100 + (chan) * 0x20) > > +#define PWM_ACT_STA BIT(8) > > +#define PWM_PRESCAL_K GENMASK(7, 0) > > + > > +#define PWM_PERIOD_REG(chan) (0x104 + (chan) * 0x20) > > +#define PWM_ENTIRE_CYCLE GENMASK(31, 16) > > +#define PWM_ACT_CYCLE GENMASK(15, 0) > > Can you please adapt the register field names to include the register > name? I'd use: > > #define PWM_CTL(chan) (0x100 + (chan) * 0x20) > #define PWM_CTL_ACT_STA BIT(8) > #define PWM_CTL_PRESCAL_K GENMASK(7, 0) > > then you get a chance to spot when PWM_CLK_BYPASS(x) is written to > PWM_CLK_CFG. > > > > +struct sun20i_pwm_chip { > > + struct pwm_chip chip; > > + struct clk *clk_bus, *clk_hosc; > > + struct reset_control *rst; > > + void __iomem *base; > > + /* Mutex to protect pwm apply state */ > > + struct mutex mutex; > > +}; > > + > > +static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip(struct pwm_chip *chip) > > +{ > > + return container_of(chip, struct sun20i_pwm_chip, 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); > > + u64 clk_rate, tmp; > > + u32 val; > > + u16 ent_cycle, act_cycle; > > + u8 prescal, div_id; > > + > > + mutex_lock(&sun20i_chip->mutex); > > + > > + val = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG_REG(pwm->hwpwm)); > > + div_id = FIELD_GET(PWM_CLK_DIV_M, val); > > + if (FIELD_GET(PWM_CLK_SRC, val) == 0) > > + clk_rate = clk_get_rate(sun20i_chip->clk_hosc); > > + else > > + clk_rate = clk_get_rate(sun20i_chip->clk_bus); > > + > > + val = sun20i_pwm_readl(sun20i_chip, PWM_CTL_REG(pwm->hwpwm)); > > + state->polarity = (PWM_ACT_STA & val) ? PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED; > > + > > + prescal = FIELD_GET(PWM_PRESCAL_K, val) + 1; > > If PWM_PRESCAL_K is 0xff, prescal ends up being 0. This isn't right, is > it? > > > + val = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE_REG); > > + state->enabled = (PWM_EN(pwm->hwpwm) & val) ? true : false; > > + > > + val = sun20i_pwm_readl(sun20i_chip, PWM_PERIOD_REG(pwm->hwpwm)); > > + act_cycle = FIELD_GET(PWM_ACT_CYCLE, val); > > + ent_cycle = FIELD_GET(PWM_ENTIRE_CYCLE, val); > > + if (act_cycle > ent_cycle) > > + act_cycle = ent_cycle; > > + > > A comment that with the width of the used factors this cannot overflow > would be nice here. > > > + tmp = (u64)(act_cycle) * prescal * (1U << div_id) * NSEC_PER_SEC; > > Can be simplified to: > > tmp = (u64)act_cycle * prescal << div_id * NSEC_PER_SEC; > > > + state->duty_cycle = DIV_ROUND_UP_ULL(tmp, clk_rate); > > + tmp = (u64)(ent_cycle) * prescal * (1U << div_id) * NSEC_PER_SEC; > > + state->period = DIV_ROUND_UP_ULL(tmp, clk_rate); > > + mutex_unlock(&sun20i_chip->mutex); > > + > > + return 0; > > +} > > + > > +static int sun20i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > + const struct pwm_state *state) > > +{ > > + int ret = 0; > > + u32 clk_gate, clk_cfg, pwm_en, ctl, period; > > + u64 bus_rate, hosc_rate, clk_div, val; > > + u16 prescaler, div_m; > > + bool use_bus_clk, calc_div_m; > > + struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip); > > + > > + mutex_lock(&sun20i_chip->mutex); > > + > > + pwm_en = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE_REG); > > + > > + if (state->enabled != pwm->state.enabled) > > + clk_gate = sun20i_pwm_readl(sun20i_chip, PWM_CLK_GATE_REG); > > + > > + if (state->enabled != pwm->state.enabled && !state->enabled) { > > + clk_gate &= ~PWM_CLK_GATING(pwm->hwpwm); > > + pwm_en &= ~PWM_EN(pwm->hwpwm); > > + sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE_REG); > > + sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE_REG); > > + } > > + > > + if (state->polarity != pwm->state.polarity || > > + state->duty_cycle != pwm->state.duty_cycle || > > + state->period != pwm->state.period) { > > + ctl = sun20i_pwm_readl(sun20i_chip, PWM_CTL_REG(pwm->hwpwm)); > > + clk_cfg = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG_REG(pwm->hwpwm)); > > + hosc_rate = clk_get_rate(sun20i_chip->clk_hosc); > > + bus_rate = clk_get_rate(sun20i_chip->clk_bus); > > + if (pwm_en & PWM_EN(pwm->hwpwm ^ 1)) { > > + /* if the neighbor channel is enable, check period only */ > > + use_bus_clk = FIELD_GET(PWM_CLK_SRC, clk_cfg) != 0; > > + if (use_bus_clk) > > + val = state->period * bus_rate; > > + else > > + val = state->period * hosc_rate; > > + do_div(val, NSEC_PER_SEC); > > + > > + div_m = FIELD_GET(PWM_CLK_DIV_M, clk_cfg); > > + calc_div_m = false; > > + } else { > > + /* check period and select clock source */ > > + use_bus_clk = false; > > + val = state->period * hosc_rate; > > + do_div(val, NSEC_PER_SEC); > > + if (val <= 1) { > > + use_bus_clk = true; > > + val = state->period * bus_rate; > > + do_div(val, NSEC_PER_SEC); > > + if (val <= 1) { > > + ret = -EINVAL; > > + goto unlock_mutex; > > + } > > + } > > + div_m = 0; > > + calc_div_m = true; > > + > > + /* set up the CLK_DIV_M and clock CLK_SRC */ > > + clk_cfg = FIELD_PREP(PWM_CLK_DIV_M, div_m); > > + clk_cfg |= FIELD_PREP(PWM_CLK_SRC, use_bus_clk ? 1 : 0); > > + > > + sun20i_pwm_writel(sun20i_chip, clk_cfg, PWM_CLK_CFG_REG(pwm->hwpwm)); > > + } > > + > > + /* calculate prescaler, M factor, PWM entire cycle */ > > + clk_div = val; > > This assignment is useless as it is overwritten in the loop below, isn't > it? > > > + for (prescaler = 0;; prescaler++) { > > + if (prescaler >= 256) { > > + if (calc_div_m) { > > + prescaler = 0; > > + div_m++; > > + if (div_m >= 9) { > > + ret = -EINVAL; > > + goto unlock_mutex; > > + } > > + } else { > > + ret = -EINVAL; > > + goto unlock_mutex; > > + } > > + } > > + > > + clk_div = val >> div_m; > > + do_div(clk_div, prescaler + 1); > > + if (clk_div <= 65534) > > + break; > > This can be calculated without a loop. Point me please where can I see the calculation of two divisors without a loop? > > > + } > > + > > + period = FIELD_PREP(PWM_ENTIRE_CYCLE, clk_div); > > + > > + /* set duty cycle */ > > + if (use_bus_clk) > > + val = state->duty_cycle * bus_rate; > > + else > > + val = state->duty_cycle * hosc_rate; > > + do_div(val, NSEC_PER_SEC); > > + clk_div = val >> div_m; > > + do_div(clk_div, prescaler + 1); > > + > > + if (state->duty_cycle == state->period) > > + clk_div++; > > I don't understand that one. Can you explain that in a comment please? The formula of the output period and the duty-cycle for PWM are as follows. T period = (PWM01_CLK / PWM0_PRESCALE_K)^-1 * (PPR0.PWM_ENTIRE_CYCLE + 1) T high-level = (PWM01_CLK / PWM0_PRESCALE_K)^-1 * PPR0.PWM_ACT_CYCLE Duty-cycle = T high-level / T period In accordance with this formula, in order to set the duty-cycle to 100%, it is necessary that PWM_ACT_CYCLE >= PWM_ENTIRE_CYCLE + 1 > > > + period |= FIELD_PREP(PWM_ACT_CYCLE, clk_div); > > + sun20i_pwm_writel(sun20i_chip, period, PWM_PERIOD_REG(pwm->hwpwm)); > > + > > + ctl = FIELD_PREP(PWM_PRESCAL_K, prescaler); > > + if (state->polarity == PWM_POLARITY_NORMAL) > > + ctl |= PWM_ACT_STA; > > + > > + sun20i_pwm_writel(sun20i_chip, ctl, PWM_CTL_REG(pwm->hwpwm)); > > Is this racy? I.e. does the write to PWM_PERIOD_REG(pwm->hwpwm) above > already has an effect before PWM_CTL_REG(pwm->hwpwm) is written? > > > + } > > + > > + if (state->enabled != pwm->state.enabled && state->enabled) { > > + clk_gate &= ~PWM_CLK_BYPASS(pwm->hwpwm); > > + clk_gate |= PWM_CLK_GATING(pwm->hwpwm); > > + pwm_en |= PWM_EN(pwm->hwpwm); > > + sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE_REG); > > + sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE_REG); > > This is (I guess) racy. If your PWM is running with > > .period = 10000 > .duty_cyle = 0 > .enabled = true > > and you configure it to > > .period = 10000 > .duty_cyle = 10000 > .enabled = false > > you get a short spike. For a enabled=true -> enabled=false transition > you should disable first before configuring duty+period (or skip the > latter completely). When switching enabled=true -> enabled=false then before setting the period, the enable register will be written false and the pvm will become inactive. this is the place: if (state->enabled != pwm->state.enabled && !state->enabled) { > > > + } > > + > > +unlock_mutex: > > + mutex_unlock(&sun20i_chip->mutex); > > + > > + return ret; > > +} > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ | Cheers, Aleksandr
Hello, On Fri, Aug 04, 2023 at 10:19:26AM +0300, Александр Шубин wrote: > пн, 10 июл. 2023 г. в 12:14, Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > > On Tue, Jun 27, 2023 at 11:23:25AM +0300, Aleksandr Shubin wrote: > > > 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. > > > > > > Signed-off-by: Aleksandr Shubin <privatesub2@gmail.com> > > > --- > > > drivers/pwm/Kconfig | 10 ++ > > > drivers/pwm/Makefile | 1 + > > > drivers/pwm/pwm-sun20i.c | 322 +++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 333 insertions(+) > > > create mode 100644 drivers/pwm/pwm-sun20i.c > > > > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > > > index 8df861b1f4a3..05c48a36969e 100644 > > > --- a/drivers/pwm/Kconfig > > > +++ b/drivers/pwm/Kconfig > > > @@ -594,6 +594,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 > > > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > > > index 19899b912e00..cea872e22c78 100644 > > > --- a/drivers/pwm/Makefile > > > +++ b/drivers/pwm/Makefile > > > @@ -55,6 +55,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 > > > diff --git a/drivers/pwm/pwm-sun20i.c b/drivers/pwm/pwm-sun20i.c > > > new file mode 100644 > > > index 000000000000..63e9c64e0e18 > > > --- /dev/null > > > +++ b/drivers/pwm/pwm-sun20i.c > > > @@ -0,0 +1,322 @@ > > > +// 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 <privatesub2@gmail.com> > > > + */ > > > + > > > +#include <linux/bitfield.h> > > > +#include <linux/clk.h> > > > +#include <linux/err.h> > > > +#include <linux/io.h> > > > +#include <linux/module.h> > > > +#include <linux/of_device.h> > > > +#include <linux/pwm.h> > > > +#include <linux/reset.h> > > > + > > > +#define PWM_CLK_CFG_REG(chan) (0x20 + (((chan) >> 1) * 0x4)) > > > +#define PWM_CLK_SRC GENMASK(8, 7) > > > +#define PWM_CLK_DIV_M GENMASK(3, 0) > > > + > > > +#define PWM_CLK_GATE_REG 0x40 > > > +#define PWM_CLK_BYPASS(chan) BIT((chan) - 16) > > > +#define PWM_CLK_GATING(chan) BIT(chan) > > > + > > > +#define PWM_ENABLE_REG 0x80 > > > +#define PWM_EN(chan) BIT(chan) > > > + > > > +#define PWM_CTL_REG(chan) (0x100 + (chan) * 0x20) > > > +#define PWM_ACT_STA BIT(8) > > > +#define PWM_PRESCAL_K GENMASK(7, 0) > > > + > > > +#define PWM_PERIOD_REG(chan) (0x104 + (chan) * 0x20) > > > +#define PWM_ENTIRE_CYCLE GENMASK(31, 16) > > > +#define PWM_ACT_CYCLE GENMASK(15, 0) > > > > Can you please adapt the register field names to include the register > > name? I'd use: > > > > #define PWM_CTL(chan) (0x100 + (chan) * 0x20) > > #define PWM_CTL_ACT_STA BIT(8) > > #define PWM_CTL_PRESCAL_K GENMASK(7, 0) > > > > then you get a chance to spot when PWM_CLK_BYPASS(x) is written to > > PWM_CLK_CFG. > > > > > > > +struct sun20i_pwm_chip { > > > + struct pwm_chip chip; > > > + struct clk *clk_bus, *clk_hosc; > > > + struct reset_control *rst; > > > + void __iomem *base; > > > + /* Mutex to protect pwm apply state */ > > > + struct mutex mutex; > > > +}; > > > + > > > +static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip(struct pwm_chip *chip) > > > +{ > > > + return container_of(chip, struct sun20i_pwm_chip, 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); > > > + u64 clk_rate, tmp; > > > + u32 val; > > > + u16 ent_cycle, act_cycle; > > > + u8 prescal, div_id; > > > + > > > + mutex_lock(&sun20i_chip->mutex); > > > + > > > + val = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG_REG(pwm->hwpwm)); > > > + div_id = FIELD_GET(PWM_CLK_DIV_M, val); > > > + if (FIELD_GET(PWM_CLK_SRC, val) == 0) > > > + clk_rate = clk_get_rate(sun20i_chip->clk_hosc); > > > + else > > > + clk_rate = clk_get_rate(sun20i_chip->clk_bus); > > > + > > > + val = sun20i_pwm_readl(sun20i_chip, PWM_CTL_REG(pwm->hwpwm)); > > > + state->polarity = (PWM_ACT_STA & val) ? PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED; > > > + > > > + prescal = FIELD_GET(PWM_PRESCAL_K, val) + 1; > > > > If PWM_PRESCAL_K is 0xff, prescal ends up being 0. This isn't right, is > > it? > > > > > + val = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE_REG); > > > + state->enabled = (PWM_EN(pwm->hwpwm) & val) ? true : false; > > > + > > > + val = sun20i_pwm_readl(sun20i_chip, PWM_PERIOD_REG(pwm->hwpwm)); > > > + act_cycle = FIELD_GET(PWM_ACT_CYCLE, val); > > > + ent_cycle = FIELD_GET(PWM_ENTIRE_CYCLE, val); > > > + if (act_cycle > ent_cycle) > > > + act_cycle = ent_cycle; > > > + > > > > A comment that with the width of the used factors this cannot overflow > > would be nice here. > > > > > + tmp = (u64)(act_cycle) * prescal * (1U << div_id) * NSEC_PER_SEC; > > > > Can be simplified to: > > > > tmp = (u64)act_cycle * prescal << div_id * NSEC_PER_SEC; > > > > > + state->duty_cycle = DIV_ROUND_UP_ULL(tmp, clk_rate); > > > + tmp = (u64)(ent_cycle) * prescal * (1U << div_id) * NSEC_PER_SEC; > > > + state->period = DIV_ROUND_UP_ULL(tmp, clk_rate); > > > + mutex_unlock(&sun20i_chip->mutex); > > > + > > > + return 0; > > > +} > > > + > > > +static int sun20i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > > + const struct pwm_state *state) > > > +{ > > > + int ret = 0; > > > + u32 clk_gate, clk_cfg, pwm_en, ctl, period; > > > + u64 bus_rate, hosc_rate, clk_div, val; > > > + u16 prescaler, div_m; > > > + bool use_bus_clk, calc_div_m; > > > + struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip); > > > + > > > + mutex_lock(&sun20i_chip->mutex); > > > + > > > + pwm_en = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE_REG); > > > + > > > + if (state->enabled != pwm->state.enabled) > > > + clk_gate = sun20i_pwm_readl(sun20i_chip, PWM_CLK_GATE_REG); > > > + > > > + if (state->enabled != pwm->state.enabled && !state->enabled) { > > > + clk_gate &= ~PWM_CLK_GATING(pwm->hwpwm); > > > + pwm_en &= ~PWM_EN(pwm->hwpwm); > > > + sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE_REG); > > > + sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE_REG); > > > + } > > > + > > > + if (state->polarity != pwm->state.polarity || > > > + state->duty_cycle != pwm->state.duty_cycle || > > > + state->period != pwm->state.period) { > > > + ctl = sun20i_pwm_readl(sun20i_chip, PWM_CTL_REG(pwm->hwpwm)); > > > + clk_cfg = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG_REG(pwm->hwpwm)); > > > + hosc_rate = clk_get_rate(sun20i_chip->clk_hosc); > > > + bus_rate = clk_get_rate(sun20i_chip->clk_bus); > > > + if (pwm_en & PWM_EN(pwm->hwpwm ^ 1)) { > > > + /* if the neighbor channel is enable, check period only */ > > > + use_bus_clk = FIELD_GET(PWM_CLK_SRC, clk_cfg) != 0; > > > + if (use_bus_clk) > > > + val = state->period * bus_rate; > > > + else > > > + val = state->period * hosc_rate; > > > + do_div(val, NSEC_PER_SEC); > > > + > > > + div_m = FIELD_GET(PWM_CLK_DIV_M, clk_cfg); > > > + calc_div_m = false; > > > + } else { > > > + /* check period and select clock source */ > > > + use_bus_clk = false; > > > + val = state->period * hosc_rate; > > > + do_div(val, NSEC_PER_SEC); > > > + if (val <= 1) { > > > + use_bus_clk = true; > > > + val = state->period * bus_rate; > > > + do_div(val, NSEC_PER_SEC); > > > + if (val <= 1) { > > > + ret = -EINVAL; > > > + goto unlock_mutex; > > > + } > > > + } > > > + div_m = 0; > > > + calc_div_m = true; > > > + > > > + /* set up the CLK_DIV_M and clock CLK_SRC */ > > > + clk_cfg = FIELD_PREP(PWM_CLK_DIV_M, div_m); > > > + clk_cfg |= FIELD_PREP(PWM_CLK_SRC, use_bus_clk ? 1 : 0); > > > + > > > + sun20i_pwm_writel(sun20i_chip, clk_cfg, PWM_CLK_CFG_REG(pwm->hwpwm)); > > > + } > > > + > > > + /* calculate prescaler, M factor, PWM entire cycle */ > > > + clk_div = val; > > > > This assignment is useless as it is overwritten in the loop below, isn't > > it? > > > > > + for (prescaler = 0;; prescaler++) { > > > + if (prescaler >= 256) { > > > + if (calc_div_m) { > > > + prescaler = 0; > > > + div_m++; > > > + if (div_m >= 9) { > > > + ret = -EINVAL; > > > + goto unlock_mutex; > > > + } > > > + } else { > > > + ret = -EINVAL; > > > + goto unlock_mutex; > > > + } > > > + } > > > + > > > + clk_div = val >> div_m; > > > + do_div(clk_div, prescaler + 1); > > > + if (clk_div <= 65534) > > > + break; > > > > This can be calculated without a loop. > > Point me please where can I see the calculation of two divisors without a loop? for a given value of div_m you search the smallest prescaler such that (val >> div_m) // (prescaler + 1) ≤ 65534 (Using Python syntax where // denotes the usual round-down-to-next-integer division and / normal exact division.) This is equivalent to: (val >> div_m) // (prescaler + 1) ≤ 65534 ⟺ (val >> div_m) ≤ 65534 * (prescaler + 1) + prescaler ⟺ (val >> div_m) ≤ 65535 * prescaler + 65534 ⟺ (val >> div_m) - 65534 ≤ 65535 * prescaler ⟺ ((val >> div_m) - 65534) / 65535 ≤ prescaler And as prescaler is integer, this is ... ⟺ ((val >> div_m) - 65534) // 65535 ≤ prescaler So the prescaler value you're looking for is: ((val >> div_m) - 65534) // 65535 And then you have to pick the smallest div_m such that prescaler ≤ 255: ((val >> div_m) - 65534) // 65535 ≤ 255 ⟺ (val >> div_m) - 65534 ≤ 255 * 65535 + 65534 ⟺ val >> div_m ≤ 255 * 65535 + 2 * 65534 ⟺ val >> div_m ≤ 16842493 ⟺ val >> div_m < 16842494 so div_m is fls((val) / 16842494). You might want to double check this and explain the algorithm in a comment similar to the above calculation. > > > + } > > > + > > > + period = FIELD_PREP(PWM_ENTIRE_CYCLE, clk_div); > > > + > > > + /* set duty cycle */ > > > + if (use_bus_clk) > > > + val = state->duty_cycle * bus_rate; > > > + else > > > + val = state->duty_cycle * hosc_rate; maybe better use: if (use_bus_clk) rate = bus_rate; else rate = hosc_rate; val = state->duty_cycle * rate > > > + do_div(val, NSEC_PER_SEC); > > > + clk_div = val >> div_m; > > > + do_div(clk_div, prescaler + 1); > > > + > > > + if (state->duty_cycle == state->period) > > > + clk_div++; > > > > I don't understand that one. Can you explain that in a comment please? > > The formula of the output period and the duty-cycle for PWM are as follows. > T period = (PWM01_CLK / PWM0_PRESCALE_K)^-1 * (PPR0.PWM_ENTIRE_CYCLE + 1) That can be simpler written as: Tperiod = PWM0_PRESCALE_K / PWM01_CLK * (PPR0.PWM_ENTIRE_CYCLE + 1) right? > T high-level = (PWM01_CLK / PWM0_PRESCALE_K)^-1 * PPR0.PWM_ACT_CYCLE simlar: Thigh = PWM0_PRESCALE_K / PWM01_CLK * PPR0.PWM_ACT_CYCLE > Duty-cycle = T high-level / T period > In accordance with this formula, in order to set the duty-cycle to 100%, > it is necessary that PWM_ACT_CYCLE >= PWM_ENTIRE_CYCLE + 1 The +1 is also relevant for duty_cycles other than state->period, right? > > > + period |= FIELD_PREP(PWM_ACT_CYCLE, clk_div); > > > + sun20i_pwm_writel(sun20i_chip, period, PWM_PERIOD_REG(pwm->hwpwm)); > > > + > > > + ctl = FIELD_PREP(PWM_PRESCAL_K, prescaler); > > > + if (state->polarity == PWM_POLARITY_NORMAL) > > > + ctl |= PWM_ACT_STA; > > > + > > > + sun20i_pwm_writel(sun20i_chip, ctl, PWM_CTL_REG(pwm->hwpwm)); > > > > Is this racy? I.e. does the write to PWM_PERIOD_REG(pwm->hwpwm) above > > already has an effect before PWM_CTL_REG(pwm->hwpwm) is written? > > > > > + } > > > + > > > + if (state->enabled != pwm->state.enabled && state->enabled) { > > > + clk_gate &= ~PWM_CLK_BYPASS(pwm->hwpwm); > > > + clk_gate |= PWM_CLK_GATING(pwm->hwpwm); > > > + pwm_en |= PWM_EN(pwm->hwpwm); > > > + sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE_REG); > > > + sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE_REG); > > > > This is (I guess) racy. If your PWM is running with > > > > .period = 10000 > > .duty_cyle = 0 > > .enabled = true > > > > and you configure it to > > > > .period = 10000 > > .duty_cyle = 10000 > > .enabled = false > > > > you get a short spike. For a enabled=true -> enabled=false transition > > you should disable first before configuring duty+period (or skip the > > latter completely). > > When switching enabled=true -> enabled=false then before setting the period, > the enable register will be written false and the pvm will become inactive. > this is the place: > if (state->enabled != pwm->state.enabled && !state->enabled) { Ah, it seems I missed that (or I just don't understand any more what meant back then :-) Best regards Uwe
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 8df861b1f4a3..05c48a36969e 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -594,6 +594,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 diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 19899b912e00..cea872e22c78 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -55,6 +55,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 diff --git a/drivers/pwm/pwm-sun20i.c b/drivers/pwm/pwm-sun20i.c new file mode 100644 index 000000000000..63e9c64e0e18 --- /dev/null +++ b/drivers/pwm/pwm-sun20i.c @@ -0,0 +1,322 @@ +// 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 <privatesub2@gmail.com> + */ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/pwm.h> +#include <linux/reset.h> + +#define PWM_CLK_CFG_REG(chan) (0x20 + (((chan) >> 1) * 0x4)) +#define PWM_CLK_SRC GENMASK(8, 7) +#define PWM_CLK_DIV_M GENMASK(3, 0) + +#define PWM_CLK_GATE_REG 0x40 +#define PWM_CLK_BYPASS(chan) BIT((chan) - 16) +#define PWM_CLK_GATING(chan) BIT(chan) + +#define PWM_ENABLE_REG 0x80 +#define PWM_EN(chan) BIT(chan) + +#define PWM_CTL_REG(chan) (0x100 + (chan) * 0x20) +#define PWM_ACT_STA BIT(8) +#define PWM_PRESCAL_K GENMASK(7, 0) + +#define PWM_PERIOD_REG(chan) (0x104 + (chan) * 0x20) +#define PWM_ENTIRE_CYCLE GENMASK(31, 16) +#define PWM_ACT_CYCLE GENMASK(15, 0) + +struct sun20i_pwm_chip { + struct pwm_chip chip; + struct clk *clk_bus, *clk_hosc; + struct reset_control *rst; + void __iomem *base; + /* Mutex to protect pwm apply state */ + struct mutex mutex; +}; + +static inline struct sun20i_pwm_chip *to_sun20i_pwm_chip(struct pwm_chip *chip) +{ + return container_of(chip, struct sun20i_pwm_chip, 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); + u64 clk_rate, tmp; + u32 val; + u16 ent_cycle, act_cycle; + u8 prescal, div_id; + + mutex_lock(&sun20i_chip->mutex); + + val = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG_REG(pwm->hwpwm)); + div_id = FIELD_GET(PWM_CLK_DIV_M, val); + if (FIELD_GET(PWM_CLK_SRC, val) == 0) + clk_rate = clk_get_rate(sun20i_chip->clk_hosc); + else + clk_rate = clk_get_rate(sun20i_chip->clk_bus); + + val = sun20i_pwm_readl(sun20i_chip, PWM_CTL_REG(pwm->hwpwm)); + state->polarity = (PWM_ACT_STA & val) ? PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED; + + prescal = FIELD_GET(PWM_PRESCAL_K, val) + 1; + + val = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE_REG); + state->enabled = (PWM_EN(pwm->hwpwm) & val) ? true : false; + + val = sun20i_pwm_readl(sun20i_chip, PWM_PERIOD_REG(pwm->hwpwm)); + act_cycle = FIELD_GET(PWM_ACT_CYCLE, val); + ent_cycle = FIELD_GET(PWM_ENTIRE_CYCLE, val); + if (act_cycle > ent_cycle) + act_cycle = ent_cycle; + + tmp = (u64)(act_cycle) * prescal * (1U << div_id) * NSEC_PER_SEC; + state->duty_cycle = DIV_ROUND_UP_ULL(tmp, clk_rate); + tmp = (u64)(ent_cycle) * prescal * (1U << div_id) * NSEC_PER_SEC; + state->period = DIV_ROUND_UP_ULL(tmp, clk_rate); + mutex_unlock(&sun20i_chip->mutex); + + return 0; +} + +static int sun20i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + int ret = 0; + u32 clk_gate, clk_cfg, pwm_en, ctl, period; + u64 bus_rate, hosc_rate, clk_div, val; + u16 prescaler, div_m; + bool use_bus_clk, calc_div_m; + struct sun20i_pwm_chip *sun20i_chip = to_sun20i_pwm_chip(chip); + + mutex_lock(&sun20i_chip->mutex); + + pwm_en = sun20i_pwm_readl(sun20i_chip, PWM_ENABLE_REG); + + if (state->enabled != pwm->state.enabled) + clk_gate = sun20i_pwm_readl(sun20i_chip, PWM_CLK_GATE_REG); + + if (state->enabled != pwm->state.enabled && !state->enabled) { + clk_gate &= ~PWM_CLK_GATING(pwm->hwpwm); + pwm_en &= ~PWM_EN(pwm->hwpwm); + sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE_REG); + sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE_REG); + } + + if (state->polarity != pwm->state.polarity || + state->duty_cycle != pwm->state.duty_cycle || + state->period != pwm->state.period) { + ctl = sun20i_pwm_readl(sun20i_chip, PWM_CTL_REG(pwm->hwpwm)); + clk_cfg = sun20i_pwm_readl(sun20i_chip, PWM_CLK_CFG_REG(pwm->hwpwm)); + hosc_rate = clk_get_rate(sun20i_chip->clk_hosc); + bus_rate = clk_get_rate(sun20i_chip->clk_bus); + if (pwm_en & PWM_EN(pwm->hwpwm ^ 1)) { + /* if the neighbor channel is enable, check period only */ + use_bus_clk = FIELD_GET(PWM_CLK_SRC, clk_cfg) != 0; + if (use_bus_clk) + val = state->period * bus_rate; + else + val = state->period * hosc_rate; + do_div(val, NSEC_PER_SEC); + + div_m = FIELD_GET(PWM_CLK_DIV_M, clk_cfg); + calc_div_m = false; + } else { + /* check period and select clock source */ + use_bus_clk = false; + val = state->period * hosc_rate; + do_div(val, NSEC_PER_SEC); + if (val <= 1) { + use_bus_clk = true; + val = state->period * bus_rate; + do_div(val, NSEC_PER_SEC); + if (val <= 1) { + ret = -EINVAL; + goto unlock_mutex; + } + } + div_m = 0; + calc_div_m = true; + + /* set up the CLK_DIV_M and clock CLK_SRC */ + clk_cfg = FIELD_PREP(PWM_CLK_DIV_M, div_m); + clk_cfg |= FIELD_PREP(PWM_CLK_SRC, use_bus_clk ? 1 : 0); + + sun20i_pwm_writel(sun20i_chip, clk_cfg, PWM_CLK_CFG_REG(pwm->hwpwm)); + } + + /* calculate prescaler, M factor, PWM entire cycle */ + clk_div = val; + for (prescaler = 0;; prescaler++) { + if (prescaler >= 256) { + if (calc_div_m) { + prescaler = 0; + div_m++; + if (div_m >= 9) { + ret = -EINVAL; + goto unlock_mutex; + } + } else { + ret = -EINVAL; + goto unlock_mutex; + } + } + + clk_div = val >> div_m; + do_div(clk_div, prescaler + 1); + if (clk_div <= 65534) + break; + } + + period = FIELD_PREP(PWM_ENTIRE_CYCLE, clk_div); + + /* set duty cycle */ + if (use_bus_clk) + val = state->duty_cycle * bus_rate; + else + val = state->duty_cycle * hosc_rate; + do_div(val, NSEC_PER_SEC); + clk_div = val >> div_m; + do_div(clk_div, prescaler + 1); + + if (state->duty_cycle == state->period) + clk_div++; + period |= FIELD_PREP(PWM_ACT_CYCLE, clk_div); + sun20i_pwm_writel(sun20i_chip, period, PWM_PERIOD_REG(pwm->hwpwm)); + + ctl = FIELD_PREP(PWM_PRESCAL_K, prescaler); + if (state->polarity == PWM_POLARITY_NORMAL) + ctl |= PWM_ACT_STA; + + sun20i_pwm_writel(sun20i_chip, ctl, PWM_CTL_REG(pwm->hwpwm)); + } + + if (state->enabled != pwm->state.enabled && state->enabled) { + clk_gate &= ~PWM_CLK_BYPASS(pwm->hwpwm); + clk_gate |= PWM_CLK_GATING(pwm->hwpwm); + pwm_en |= PWM_EN(pwm->hwpwm); + sun20i_pwm_writel(sun20i_chip, pwm_en, PWM_ENABLE_REG); + sun20i_pwm_writel(sun20i_chip, clk_gate, PWM_CLK_GATE_REG); + } + +unlock_mutex: + mutex_unlock(&sun20i_chip->mutex); + + return ret; +} + +static const struct pwm_ops sun20i_pwm_ops = { + .get_state = sun20i_pwm_get_state, + .apply = sun20i_pwm_apply, + .owner = THIS_MODULE, +}; + +static const struct of_device_id sun20i_pwm_dt_ids[] = { + { .compatible = "allwinner,sun20i-d1-pwm" }, + { }, +}; +MODULE_DEVICE_TABLE(of, sun20i_pwm_dt_ids); + +static int sun20i_pwm_probe(struct platform_device *pdev) +{ + struct sun20i_pwm_chip *sun20i_chip; + int ret; + + sun20i_chip = devm_kzalloc(&pdev->dev, sizeof(*sun20i_chip), GFP_KERNEL); + if (!sun20i_chip) + return -ENOMEM; + + 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"); + 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->rst = devm_reset_control_get(&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", + &sun20i_chip->chip.npwm); + if (ret) + sun20i_chip->chip.npwm = 8; + + /* Deassert reset */ + ret = reset_control_deassert(sun20i_chip->rst); + if (ret) + return dev_err_probe(&pdev->dev, ret, "failed to deassert reset\n"); + + sun20i_chip->chip.dev = &pdev->dev; + sun20i_chip->chip.ops = &sun20i_pwm_ops; + + mutex_init(&sun20i_chip->mutex); + + ret = pwmchip_add(&sun20i_chip->chip); + if (ret < 0) { + reset_control_assert(sun20i_chip->rst); + return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n"); + } + + platform_set_drvdata(pdev, sun20i_chip); + + return 0; +} + +static void sun20i_pwm_remove(struct platform_device *pdev) +{ + struct sun20i_pwm_chip *sun20i_chip = platform_get_drvdata(pdev); + + pwmchip_remove(&sun20i_chip->chip); + + reset_control_assert(sun20i_chip->rst); +} + +static struct platform_driver sun20i_pwm_driver = { + .driver = { + .name = "sun20i-pwm", + .of_match_table = sun20i_pwm_dt_ids, + }, + .probe = sun20i_pwm_probe, + .remove_new = sun20i_pwm_remove, +}; +module_platform_driver(sun20i_pwm_driver); + +MODULE_AUTHOR("Aleksandr Shubin <privatesub2@gmail.com>"); +MODULE_DESCRIPTION("Allwinner sun20i PWM driver"); +MODULE_LICENSE("GPL");
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. Signed-off-by: Aleksandr Shubin <privatesub2@gmail.com> --- drivers/pwm/Kconfig | 10 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-sun20i.c | 322 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 333 insertions(+) create mode 100644 drivers/pwm/pwm-sun20i.c