* [PATCH v2 0/2] pwm: add driver for T-THEAD TH1520 SoC @ 2023-10-04 9:27 Jisheng Zhang 2023-10-04 9:27 ` [PATCH v2 1/2] dt-bindings: pwm: Add T-HEAD PWM controller Jisheng Zhang 2023-10-04 9:27 ` [PATCH v2 2/2] pwm: add T-HEAD PWM driver Jisheng Zhang 0 siblings, 2 replies; 7+ messages in thread From: Jisheng Zhang @ 2023-10-04 9:27 UTC (permalink / raw) To: Thierry Reding, Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-pwm, devicetree, linux-kernel, linux-riscv T-HEAD SoCs such as the TH1520 contain a PWM controller used to control the LCD backlight, fan and so on. Add the PWM driver support for it. Since the clk part isn't mainlined, so SoC dts(i) changes are not included in this series. However, it can be tested by using fixed-clock. since v1: - update commit msg and yaml filename to address Conor's comment - use devm_clk_get_enabled() and devm_pwmchip_add() - implement .get_state() - properly handle overflow - introduce thead_pwm_from_chip() inline function - document Limitations - address pm_runtime_get/put pingpong comment Jisheng Zhang (2): dt-bindings: pwm: Add T-HEAD PWM controller pwm: add T-HEAD PWM driver .../bindings/pwm/thead,th1520-pwm.yaml | 44 +++ MAINTAINERS | 1 + drivers/pwm/Kconfig | 11 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-thead.c | 274 ++++++++++++++++++ 5 files changed, 331 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml create mode 100644 drivers/pwm/pwm-thead.c -- 2.40.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] dt-bindings: pwm: Add T-HEAD PWM controller 2023-10-04 9:27 [PATCH v2 0/2] pwm: add driver for T-THEAD TH1520 SoC Jisheng Zhang @ 2023-10-04 9:27 ` Jisheng Zhang 2023-10-04 14:02 ` Uwe Kleine-König 2023-10-04 15:07 ` Rob Herring 2023-10-04 9:27 ` [PATCH v2 2/2] pwm: add T-HEAD PWM driver Jisheng Zhang 1 sibling, 2 replies; 7+ messages in thread From: Jisheng Zhang @ 2023-10-04 9:27 UTC (permalink / raw) To: Thierry Reding, Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-pwm, devicetree, linux-kernel, linux-riscv T-HEAD SoCs such as the TH1520 contain a PWM controller used to control the LCD backlight, fan and so on. Signed-off-by: Jisheng Zhang <jszhang@kernel.org> --- .../bindings/pwm/thead,th1520-pwm.yaml | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml diff --git a/Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml b/Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml new file mode 100644 index 000000000000..b9c88f758a39 --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/thead,th1520-pwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: T-HEAD TH1520 PWM + +maintainers: + - Jisheng Zhang <jszhang@kernel.org> + +allOf: + - $ref: pwm.yaml# + +properties: + compatible: + enum: + - thead,th1520-pwm + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + "#pwm-cells": + const: 2 + +required: + - compatible + - reg + - clocks + +additionalProperties: false + +examples: + - | + + pwm@ec01c000 { + compatible = "thead,th1520-pwm"; + reg = <0xec01c000 0x1000>; + clocks = <&clk 1>; + #pwm-cells = <2>; + }; -- 2.40.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: pwm: Add T-HEAD PWM controller 2023-10-04 9:27 ` [PATCH v2 1/2] dt-bindings: pwm: Add T-HEAD PWM controller Jisheng Zhang @ 2023-10-04 14:02 ` Uwe Kleine-König 2023-10-04 15:07 ` Rob Herring 1 sibling, 0 replies; 7+ messages in thread From: Uwe Kleine-König @ 2023-10-04 14:02 UTC (permalink / raw) To: Jisheng Zhang Cc: Thierry Reding, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-pwm, devicetree, linux-kernel, linux-riscv [-- Attachment #1.1: Type: text/plain, Size: 316 bytes --] Hello, On Wed, Oct 04, 2023 at 05:27:30PM +0800, Jisheng Zhang wrote: > + "#pwm-cells": > + const: 2 Please make this a 3. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: pwm: Add T-HEAD PWM controller 2023-10-04 9:27 ` [PATCH v2 1/2] dt-bindings: pwm: Add T-HEAD PWM controller Jisheng Zhang 2023-10-04 14:02 ` Uwe Kleine-König @ 2023-10-04 15:07 ` Rob Herring 1 sibling, 0 replies; 7+ messages in thread From: Rob Herring @ 2023-10-04 15:07 UTC (permalink / raw) To: Jisheng Zhang Cc: Krzysztof Kozlowski, Uwe Kleine-König, linux-kernel, linux-riscv, Thierry Reding, Conor Dooley, linux-pwm, Rob Herring, devicetree On Wed, 04 Oct 2023 17:27:30 +0800, Jisheng Zhang wrote: > T-HEAD SoCs such as the TH1520 contain a PWM controller used > to control the LCD backlight, fan and so on. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > .../bindings/pwm/thead,th1520-pwm.yaml | 44 +++++++++++++++++++ > 1 file changed, 44 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/thead,th1520-pwm.yaml > Reviewed-by: Rob Herring <robh@kernel.org> _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] pwm: add T-HEAD PWM driver 2023-10-04 9:27 [PATCH v2 0/2] pwm: add driver for T-THEAD TH1520 SoC Jisheng Zhang 2023-10-04 9:27 ` [PATCH v2 1/2] dt-bindings: pwm: Add T-HEAD PWM controller Jisheng Zhang @ 2023-10-04 9:27 ` Jisheng Zhang 2023-10-04 14:01 ` Uwe Kleine-König 1 sibling, 1 reply; 7+ messages in thread From: Jisheng Zhang @ 2023-10-04 9:27 UTC (permalink / raw) To: Thierry Reding, Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-pwm, devicetree, linux-kernel, linux-riscv T-HEAD SoCs such as the TH1520 contain a PWM controller used to control the LCD backlight, fan and so on. Add driver for it. Signed-off-by: Jisheng Zhang <jszhang@kernel.org> --- MAINTAINERS | 1 + drivers/pwm/Kconfig | 11 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-thead.c | 274 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 287 insertions(+) create mode 100644 drivers/pwm/pwm-thead.c diff --git a/MAINTAINERS b/MAINTAINERS index d55e40060c46..86cf0926dbfc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -18482,6 +18482,7 @@ L: linux-riscv@lists.infradead.org S: Maintained F: arch/riscv/boot/dts/thead/ F: drivers/usb/dwc3/dwc3-thead.c +F: drivers/pwm/pwm-thead.c RNBD BLOCK DRIVERS M: Md. Haris Iqbal <haris.iqbal@ionos.com> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 8ebcddf91f7b..428fa365a19a 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -637,6 +637,17 @@ config PWM_TEGRA To compile this driver as a module, choose M here: the module will be called pwm-tegra. +config PWM_THEAD + tristate "T-HEAD PWM support" + depends on ARCH_THEAD || COMPILE_TEST + depends on HAS_IOMEM + help + Generic PWM framework driver for the PWFM controller found on THEAD + SoCs. + + To compile this driver as a module, choose M here: the module + will be called pwm-thead. + config PWM_TIECAP tristate "ECAP PWM support" depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index c822389c2a24..4c317e6316e8 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -59,6 +59,7 @@ obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o obj-$(CONFIG_PWM_SUNPLUS) += pwm-sunplus.o obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o +obj-$(CONFIG_PWM_THEAD) += pwm-thead.o obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o obj-$(CONFIG_PWM_TWL) += pwm-twl.o diff --git a/drivers/pwm/pwm-thead.c b/drivers/pwm/pwm-thead.c new file mode 100644 index 000000000000..ba1e3a4f1027 --- /dev/null +++ b/drivers/pwm/pwm-thead.c @@ -0,0 +1,274 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * T-HEAD PWM driver + * + * Copyright (C) 2021 Alibaba Group Holding Limited. + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> + * + * Limitations: + * - The THEAD_PWM_START bit is only effective when 0 -> 1, which is used to + * start the channel, 1 -> 0 doesn't change anything. so 0 % duty cycle is + * used to "disable" the channel. + * - The PWM_CFG_UPDATE atomically updates and only updates period and duty. + * - To update period and duty, PWM_CFG_UPDATE needs to go through 0 -> 1 step, + * I.E if PWM_CFG_UPDATE is already 1, it's necessary to clear it to 0 + * beforehand. + * - Polarity can only be changed if never started before. + */ + +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/pwm.h> +#include <linux/slab.h> + +#define THEAD_PWM_MAX_NUM 6 +#define THEAD_PWM_MAX_PERIOD GENMASK(31, 0) +#define THEAD_PWM_MAX_DUTY GENMASK(31, 0) + +#define THEAD_PWM_CHN_BASE(n) ((n) * 0x20) +#define THEAD_PWM_CTRL(n) (THEAD_PWM_CHN_BASE(n) + 0x00) +#define THEAD_PWM_RPT(n) (THEAD_PWM_CHN_BASE(n) + 0x04) +#define THEAD_PWM_PER(n) (THEAD_PWM_CHN_BASE(n) + 0x08) +#define THEAD_PWM_FP(n) (THEAD_PWM_CHN_BASE(n) + 0x0c) +#define THEAD_PWM_STATUS(n) (THEAD_PWM_CHN_BASE(n) + 0x10) + +/* bit definition PWM_CTRL */ +#define THEAD_PWM_START BIT(0) +#define THEAD_PWM_SOFT_RST BIT(1) +#define THEAD_PWM_CFG_UPDATE BIT(2) +#define THEAD_PWM_INT_EN BIT(3) +#define THEAD_PWM_MODE_MASK GENMASK(5, 4) +#define THEAD_PWM_ONE_SHOT_MODE FIELD_PREP(THEAD_PWM_MODE_MASK, 1) +#define THEAD_PWM_CONTINUOUS_MODE FIELD_PREP(THEAD_PWM_MODE_MASK, 2) +#define THEAD_PWM_EVTRIG_MASK GENMASK(7, 6) +#define THEAD_PWM_FPOUT BIT(8) +#define THEAD_PWM_INFACTOUT BIT(9) + +struct thead_pwm_chip { + struct pwm_chip chip; + void __iomem *mmio_base; + struct clk *clk; + bool ever_started; +}; + +static inline struct thead_pwm_chip *thead_pwm_from_chip(struct pwm_chip *chip) +{ + return container_of(chip, struct thead_pwm_chip, chip); +} + +static int thead_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct thead_pwm_chip *priv = thead_pwm_from_chip(chip); + u64 period_cycle, duty_cycle, rate; + u32 val; + + /* if ever started, can't change the polarity */ + if (priv->ever_started && state->polarity != pwm->state.polarity) + return -EINVAL; + + if (!state->enabled) { + if (pwm->state.enabled) { + val = readl(priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); + val &= ~THEAD_PWM_CFG_UPDATE; + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); + + writel(0, priv->mmio_base + THEAD_PWM_FP(pwm->hwpwm)); + + val |= THEAD_PWM_CFG_UPDATE; + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); + } + return 0; + } + + if (!pwm->state.enabled) + pm_runtime_get_sync(chip->dev); + + val = readl(priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); + val &= ~THEAD_PWM_CFG_UPDATE; + + if (state->polarity == PWM_POLARITY_INVERSED) + val &= ~THEAD_PWM_FPOUT; + else + val |= THEAD_PWM_FPOUT; + + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); + + rate = clk_get_rate(priv->clk); + /* + * The following calculations might overflow if clk is bigger + * than 1 GHz. In practise it's 24MHz, so this limitation + * is only theoretic. + */ + if (rate > (u64)NSEC_PER_SEC) + return -EINVAL; + + period_cycle = mul_u64_u64_div_u64(rate, state->period, NSEC_PER_SEC); + if (period_cycle > THEAD_PWM_MAX_PERIOD) + period_cycle = THEAD_PWM_MAX_PERIOD; + /* + * With limitation above we have period_cycle <= THEAD_PWM_MAX_PERIOD, + * so this cannot overflow. + */ + writel((u32)period_cycle, priv->mmio_base + THEAD_PWM_PER(pwm->hwpwm)); + + duty_cycle = mul_u64_u64_div_u64(rate, state->duty_cycle, NSEC_PER_SEC); + if (duty_cycle > THEAD_PWM_MAX_DUTY) + duty_cycle = THEAD_PWM_MAX_DUTY; + /* + * With limitation above we have duty_cycle <= THEAD_PWM_MAX_PERIOD, + * so this cannot overflow. + */ + writel((u32)duty_cycle, priv->mmio_base + THEAD_PWM_FP(pwm->hwpwm)); + + val |= THEAD_PWM_CFG_UPDATE; + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); + + if (!pwm->state.enabled) { + val |= THEAD_PWM_START; + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); + priv->ever_started = true; + } + + return 0; +} + +static int thead_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct thead_pwm_chip *priv = thead_pwm_from_chip(chip); + u64 rate = clk_get_rate(priv->clk); + u32 val; + + pm_runtime_get_sync(chip->dev); + + val = readl(priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); + state->enabled = !!(val & THEAD_PWM_START); + if (val & THEAD_PWM_FPOUT) + state->polarity = PWM_POLARITY_NORMAL; + else + state->polarity = PWM_POLARITY_INVERSED; + + val = readl(priv->mmio_base + THEAD_PWM_PER(pwm->hwpwm)); + /* + * val 32 bits, multiply NSEC_PER_SEC, won't overflow. + */ + state->period = DIV64_U64_ROUND_UP((u64)val * NSEC_PER_SEC, rate); + + val = readl(priv->mmio_base + THEAD_PWM_FP(pwm->hwpwm)); + /* + * val 32 bits, multiply NSEC_PER_SEC, won't overflow. + */ + state->duty_cycle = DIV64_U64_ROUND_UP((u64)val * NSEC_PER_SEC, rate); + + pm_runtime_put_sync(chip->dev); + + return 0; +} + +static const struct pwm_ops thead_pwm_ops = { + .apply = thead_pwm_apply, + .get_state = thead_pwm_get_state, + .owner = THIS_MODULE, +}; + +static int __maybe_unused thead_pwm_runtime_suspend(struct device *dev) +{ + struct thead_pwm_chip *priv = dev_get_drvdata(dev); + + clk_disable_unprepare(priv->clk); + + return 0; +} + +static int __maybe_unused thead_pwm_runtime_resume(struct device *dev) +{ + struct thead_pwm_chip *priv = dev_get_drvdata(dev); + int ret; + + ret = clk_prepare_enable(priv->clk); + if (ret) { + dev_err(dev, "failed to enable pwm clock(%d)\n", ret); + return ret; + } + + return 0; +} + +static int thead_pwm_probe(struct platform_device *pdev) +{ + u32 val = THEAD_PWM_INFACTOUT | THEAD_PWM_FPOUT | THEAD_PWM_CONTINUOUS_MODE; + struct thead_pwm_chip *priv; + int ret, i; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + platform_set_drvdata(pdev, priv); + + priv->mmio_base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(priv->mmio_base)) + return PTR_ERR(priv->mmio_base); + + priv->clk = devm_clk_get_enabled(&pdev->dev, NULL); + if (IS_ERR(priv->clk)) + return PTR_ERR(priv->clk); + + priv->chip.ops = &thead_pwm_ops; + priv->chip.dev = &pdev->dev; + priv->chip.npwm = THEAD_PWM_MAX_NUM; + + /* set normal polarity and other proper bits for all channels */ + for (i = 0; i < priv->chip.npwm; i++) + writel(val, priv->mmio_base + THEAD_PWM_CTRL(i)); + + ret = devm_pwmchip_add(&pdev->dev, &priv->chip); + if (ret) + return ret; + + pm_runtime_enable(&pdev->dev); + + return 0; +} + +static void thead_pwm_remove(struct platform_device *pdev) +{ + pm_runtime_disable(&pdev->dev); +} + +static const struct of_device_id thead_pwm_dt_ids[] = { + {.compatible = "thead,th1520-pwm",}, + {/* sentinel */} +}; +MODULE_DEVICE_TABLE(of, thead_pwm_dt_ids); + +static const struct dev_pm_ops thead_pwm_pm_ops = { + SET_RUNTIME_PM_OPS(thead_pwm_runtime_suspend, thead_pwm_runtime_resume, NULL) +}; + +static struct platform_driver thead_pwm_driver = { + .driver = { + .name = "thead-pwm", + .of_match_table = thead_pwm_dt_ids, + .pm = &thead_pwm_pm_ops, + }, + .probe = thead_pwm_probe, + .remove_new = thead_pwm_remove, +}; +module_platform_driver(thead_pwm_driver); + +MODULE_AUTHOR("Wei Liu <lw312886@linux.alibaba.com>"); +MODULE_AUTHOR("Jisheng Zhang <jszhang@kernel.org>"); +MODULE_DESCRIPTION("T-HEAD pwm driver"); +MODULE_LICENSE("GPL v2"); -- 2.40.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] pwm: add T-HEAD PWM driver 2023-10-04 9:27 ` [PATCH v2 2/2] pwm: add T-HEAD PWM driver Jisheng Zhang @ 2023-10-04 14:01 ` Uwe Kleine-König 2023-10-05 14:06 ` Jisheng Zhang 0 siblings, 1 reply; 7+ messages in thread From: Uwe Kleine-König @ 2023-10-04 14:01 UTC (permalink / raw) To: Jisheng Zhang Cc: Thierry Reding, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-pwm, devicetree, linux-kernel, linux-riscv [-- Attachment #1.1: Type: text/plain, Size: 12463 bytes --] On Wed, Oct 04, 2023 at 05:27:31PM +0800, Jisheng Zhang wrote: > T-HEAD SoCs such as the TH1520 contain a PWM controller used > to control the LCD backlight, fan and so on. Add driver for it. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > MAINTAINERS | 1 + > drivers/pwm/Kconfig | 11 ++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-thead.c | 274 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 287 insertions(+) > create mode 100644 drivers/pwm/pwm-thead.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d55e40060c46..86cf0926dbfc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -18482,6 +18482,7 @@ L: linux-riscv@lists.infradead.org > S: Maintained > F: arch/riscv/boot/dts/thead/ > F: drivers/usb/dwc3/dwc3-thead.c > +F: drivers/pwm/pwm-thead.c > > RNBD BLOCK DRIVERS > M: Md. Haris Iqbal <haris.iqbal@ionos.com> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 8ebcddf91f7b..428fa365a19a 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -637,6 +637,17 @@ config PWM_TEGRA > To compile this driver as a module, choose M here: the module > will be called pwm-tegra. > > +config PWM_THEAD > + tristate "T-HEAD PWM support" > + depends on ARCH_THEAD || COMPILE_TEST > + depends on HAS_IOMEM > + help > + Generic PWM framework driver for the PWFM controller found on THEAD > + SoCs. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-thead. > + > config PWM_TIECAP > tristate "ECAP PWM support" > depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index c822389c2a24..4c317e6316e8 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -59,6 +59,7 @@ obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o > obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o > obj-$(CONFIG_PWM_SUNPLUS) += pwm-sunplus.o > obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o > +obj-$(CONFIG_PWM_THEAD) += pwm-thead.o > obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o > obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o > obj-$(CONFIG_PWM_TWL) += pwm-twl.o > diff --git a/drivers/pwm/pwm-thead.c b/drivers/pwm/pwm-thead.c > new file mode 100644 > index 000000000000..ba1e3a4f1027 > --- /dev/null > +++ b/drivers/pwm/pwm-thead.c > @@ -0,0 +1,274 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * T-HEAD PWM driver > + * > + * Copyright (C) 2021 Alibaba Group Holding Limited. > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + * > + * Limitations: > + * - The THEAD_PWM_START bit is only effective when 0 -> 1, which is used to > + * start the channel, 1 -> 0 doesn't change anything. so 0 % duty cycle is > + * used to "disable" the channel. > + * - The PWM_CFG_UPDATE atomically updates and only updates period and duty. > + * - To update period and duty, PWM_CFG_UPDATE needs to go through 0 -> 1 step, > + * I.E if PWM_CFG_UPDATE is already 1, it's necessary to clear it to 0 > + * beforehand. > + * - Polarity can only be changed if never started before. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/bitops.h> > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> Looking at 0a41b0c5d97a3758ad102cec469aaa79c7d406b7 I think you want linux/mod_devicetable.h here instead of of_device.h. > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/pwm.h> > +#include <linux/slab.h> > + > +#define THEAD_PWM_MAX_NUM 6 > +#define THEAD_PWM_MAX_PERIOD GENMASK(31, 0) > +#define THEAD_PWM_MAX_DUTY GENMASK(31, 0) > + > +#define THEAD_PWM_CHN_BASE(n) ((n) * 0x20) > +#define THEAD_PWM_CTRL(n) (THEAD_PWM_CHN_BASE(n) + 0x00) > +#define THEAD_PWM_RPT(n) (THEAD_PWM_CHN_BASE(n) + 0x04) > +#define THEAD_PWM_PER(n) (THEAD_PWM_CHN_BASE(n) + 0x08) > +#define THEAD_PWM_FP(n) (THEAD_PWM_CHN_BASE(n) + 0x0c) > +#define THEAD_PWM_STATUS(n) (THEAD_PWM_CHN_BASE(n) + 0x10) > + > +/* bit definition PWM_CTRL */ > +#define THEAD_PWM_START BIT(0) This is a bit in THEAD_PWM_CTRL(n), so I'd propose THEAD_PWM_CTRL_START as a name. > +#define THEAD_PWM_SOFT_RST BIT(1) > +#define THEAD_PWM_CFG_UPDATE BIT(2) > +#define THEAD_PWM_INT_EN BIT(3) > +#define THEAD_PWM_MODE_MASK GENMASK(5, 4) I'd drop "_MASK" here (and add _CTRL). > +#define THEAD_PWM_ONE_SHOT_MODE FIELD_PREP(THEAD_PWM_MODE_MASK, 1) This is unused > +#define THEAD_PWM_CONTINUOUS_MODE FIELD_PREP(THEAD_PWM_MODE_MASK, 2) THEAD_PWM_CTRL_MODE_CONTINUOUS ? > +#define THEAD_PWM_EVTRIG_MASK GENMASK(7, 6) > +#define THEAD_PWM_FPOUT BIT(8) > +#define THEAD_PWM_INFACTOUT BIT(9) > + > +struct thead_pwm_chip { > + struct pwm_chip chip; > + void __iomem *mmio_base; > + struct clk *clk; > + bool ever_started; > +}; > + > +static inline struct thead_pwm_chip *thead_pwm_from_chip(struct pwm_chip *chip) > +{ > + return container_of(chip, struct thead_pwm_chip, chip); > +} > + > +static int thead_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + struct thead_pwm_chip *priv = thead_pwm_from_chip(chip); > + u64 period_cycle, duty_cycle, rate; > + u32 val; > + > + /* if ever started, can't change the polarity */ > + if (priv->ever_started && state->polarity != pwm->state.polarity) > + return -EINVAL; > + > + if (!state->enabled) { > + if (pwm->state.enabled) { > + val = readl(priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > + val &= ~THEAD_PWM_CFG_UPDATE; > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > + > + writel(0, priv->mmio_base + THEAD_PWM_FP(pwm->hwpwm)); > + > + val |= THEAD_PWM_CFG_UPDATE; > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > + } > + return 0; > + } > + > + if (!pwm->state.enabled) > + pm_runtime_get_sync(chip->dev); pm_runtime_get_sync() returns an int that you shouldn't ignore. > + val = readl(priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > + val &= ~THEAD_PWM_CFG_UPDATE; > + > + if (state->polarity == PWM_POLARITY_INVERSED) > + val &= ~THEAD_PWM_FPOUT; > + else > + val |= THEAD_PWM_FPOUT; What happens here if the bootloader already touched that flag? Or the driver is reloaded/rebound? > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > + > + rate = clk_get_rate(priv->clk); > + /* > + * The following calculations might overflow if clk is bigger > + * than 1 GHz. In practise it's 24MHz, so this limitation > + * is only theoretic. > + */ > + if (rate > (u64)NSEC_PER_SEC) this cast isn't needed. > + return -EINVAL; > + > + period_cycle = mul_u64_u64_div_u64(rate, state->period, NSEC_PER_SEC); > + if (period_cycle > THEAD_PWM_MAX_PERIOD) > + period_cycle = THEAD_PWM_MAX_PERIOD; > + /* > + * With limitation above we have period_cycle <= THEAD_PWM_MAX_PERIOD, > + * so this cannot overflow. > + */ > + writel((u32)period_cycle, priv->mmio_base + THEAD_PWM_PER(pwm->hwpwm)); This cast can also be dropped. > + > + duty_cycle = mul_u64_u64_div_u64(rate, state->duty_cycle, NSEC_PER_SEC); > + if (duty_cycle > THEAD_PWM_MAX_DUTY) > + duty_cycle = THEAD_PWM_MAX_DUTY; > + /* > + * With limitation above we have duty_cycle <= THEAD_PWM_MAX_PERIOD, > + * so this cannot overflow. > + */ > + writel((u32)duty_cycle, priv->mmio_base + THEAD_PWM_FP(pwm->hwpwm)); ... > + > + val |= THEAD_PWM_CFG_UPDATE; > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > + > + if (!pwm->state.enabled) { > + val |= THEAD_PWM_START; > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > + priv->ever_started = true; > + } Further above you conditionally call pm_runtime_get_sync(), there should be a matching pm_runtime_put(). > + > + return 0; > +} > + > +static int thead_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > + struct pwm_state *state) > +{ > + struct thead_pwm_chip *priv = thead_pwm_from_chip(chip); > + u64 rate = clk_get_rate(priv->clk); > + u32 val; > + > + pm_runtime_get_sync(chip->dev); > + > + val = readl(priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > + state->enabled = !!(val & THEAD_PWM_START); > + if (val & THEAD_PWM_FPOUT) > + state->polarity = PWM_POLARITY_NORMAL; > + else > + state->polarity = PWM_POLARITY_INVERSED; > + > + val = readl(priv->mmio_base + THEAD_PWM_PER(pwm->hwpwm)); > + /* > + * val 32 bits, multiply NSEC_PER_SEC, won't overflow. > + */ > + state->period = DIV64_U64_ROUND_UP((u64)val * NSEC_PER_SEC, rate); > + > + val = readl(priv->mmio_base + THEAD_PWM_FP(pwm->hwpwm)); > + /* > + * val 32 bits, multiply NSEC_PER_SEC, won't overflow. > + */ > + state->duty_cycle = DIV64_U64_ROUND_UP((u64)val * NSEC_PER_SEC, rate); > + > + pm_runtime_put_sync(chip->dev); > + > + return 0; > +} > + > +static const struct pwm_ops thead_pwm_ops = { > + .apply = thead_pwm_apply, > + .get_state = thead_pwm_get_state, > + .owner = THIS_MODULE, > +}; > + > +static int __maybe_unused thead_pwm_runtime_suspend(struct device *dev) > +{ > + struct thead_pwm_chip *priv = dev_get_drvdata(dev); > + > + clk_disable_unprepare(priv->clk); > + > + return 0; > +} > + > +static int __maybe_unused thead_pwm_runtime_resume(struct device *dev) > +{ > + struct thead_pwm_chip *priv = dev_get_drvdata(dev); > + int ret; > + > + ret = clk_prepare_enable(priv->clk); > + if (ret) { > + dev_err(dev, "failed to enable pwm clock(%d)\n", ret); > + return ret; > + } > + > + return 0; This can be simplified to: ret = clk_prepare_enable(priv->clk); if (ret) dev_err(dev, "failed to enable pwm clock(%pe)\n", ERR_PTR(ret)); return ret; (Note that I used %pe instead of %d which is nicer for error codes) Having said that, I'm unsure if emitting an error message is useful. Maybe the core emits a message anyhow? > +} > + > +static int thead_pwm_probe(struct platform_device *pdev) > +{ > + u32 val = THEAD_PWM_INFACTOUT | THEAD_PWM_FPOUT | THEAD_PWM_CONTINUOUS_MODE; > + struct thead_pwm_chip *priv; > + int ret, i; > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, priv); > + > + priv->mmio_base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(priv->mmio_base)) > + return PTR_ERR(priv->mmio_base); > + > + priv->clk = devm_clk_get_enabled(&pdev->dev, NULL); > + if (IS_ERR(priv->clk)) > + return PTR_ERR(priv->clk); > + > + priv->chip.ops = &thead_pwm_ops; > + priv->chip.dev = &pdev->dev; > + priv->chip.npwm = THEAD_PWM_MAX_NUM; > + > + /* set normal polarity and other proper bits for all channels */ Please don't. You're supposed to keep the state of the hardware in probe. Consider a bootloader that enabled the backlight of an LCD that shows a splash screen. > + for (i = 0; i < priv->chip.npwm; i++) > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(i)); > + > + ret = devm_pwmchip_add(&pdev->dev, &priv->chip); > + if (ret) > + return ret; > + > + pm_runtime_enable(&pdev->dev); devm_pm_runtime_enable() and then drop .remove() > + > + return 0; > +} > + > +static void thead_pwm_remove(struct platform_device *pdev) > +{ > + pm_runtime_disable(&pdev->dev); > +} > + > +static const struct of_device_id thead_pwm_dt_ids[] = { > + {.compatible = "thead,th1520-pwm",}, > + {/* sentinel */} > +}; > +MODULE_DEVICE_TABLE(of, thead_pwm_dt_ids); > + > +static const struct dev_pm_ops thead_pwm_pm_ops = { > + SET_RUNTIME_PM_OPS(thead_pwm_runtime_suspend, thead_pwm_runtime_resume, NULL) > +}; > + > +static struct platform_driver thead_pwm_driver = { > + .driver = { > + .name = "thead-pwm", > + .of_match_table = thead_pwm_dt_ids, > + .pm = &thead_pwm_pm_ops, > + }, > + .probe = thead_pwm_probe, > + .remove_new = thead_pwm_remove, > +}; > +module_platform_driver(thead_pwm_driver); Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] pwm: add T-HEAD PWM driver 2023-10-04 14:01 ` Uwe Kleine-König @ 2023-10-05 14:06 ` Jisheng Zhang 0 siblings, 0 replies; 7+ messages in thread From: Jisheng Zhang @ 2023-10-05 14:06 UTC (permalink / raw) To: Uwe Kleine-König Cc: Thierry Reding, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-pwm, devicetree, linux-kernel, linux-riscv On Wed, Oct 04, 2023 at 04:01:30PM +0200, Uwe Kleine-König wrote: > On Wed, Oct 04, 2023 at 05:27:31PM +0800, Jisheng Zhang wrote: > > T-HEAD SoCs such as the TH1520 contain a PWM controller used > > to control the LCD backlight, fan and so on. Add driver for it. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- ... Hi Uwe, Thanks a lot for your review and nice suggestions. v3 has been sent out. And I want to add more comments to your questions here. > > + > > +static int thead_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > + const struct pwm_state *state) > > +{ > > + struct thead_pwm_chip *priv = thead_pwm_from_chip(chip); > > + u64 period_cycle, duty_cycle, rate; > > + u32 val; > > + > > + /* if ever started, can't change the polarity */ > > + if (priv->ever_started && state->polarity != pwm->state.polarity) > > + return -EINVAL; This is the polority check[1] for ever started channel. > > + > > + if (!state->enabled) { > > + if (pwm->state.enabled) { > > + val = readl(priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > > + val &= ~THEAD_PWM_CFG_UPDATE; > > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > > + > > + writel(0, priv->mmio_base + THEAD_PWM_FP(pwm->hwpwm)); > > + > > + val |= THEAD_PWM_CFG_UPDATE; > > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > > + } > > + return 0; > > + } > > + > > + if (!pwm->state.enabled) > > + pm_runtime_get_sync(chip->dev); > > pm_runtime_get_sync() returns an int that you shouldn't ignore. In v3 I switch to pm_runtime_resume_and_get() because it can simplify the error handling code. > > > + val = readl(priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > > + val &= ~THEAD_PWM_CFG_UPDATE; > > + > > + if (state->polarity == PWM_POLARITY_INVERSED) > > + val &= ~THEAD_PWM_FPOUT; > > + else > > + val |= THEAD_PWM_FPOUT; > > What happens here if the bootloader already touched that flag? Or the > driver is reloaded/rebound? Only polarity can't be changed once started, so if bootloader already configured polarity and started the pwm channel, and we want to change to a different polarity, the check[1] in the beginning of this function will fail so return -EINVAL. > > > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > > + > > + rate = clk_get_rate(priv->clk); > > + /* > > + * The following calculations might overflow if clk is bigger > > + * than 1 GHz. In practise it's 24MHz, so this limitation > > + * is only theoretic. > > + */ > > + if (rate > (u64)NSEC_PER_SEC) > > this cast isn't needed. > > > + return -EINVAL; > > + > > + period_cycle = mul_u64_u64_div_u64(rate, state->period, NSEC_PER_SEC); > > + if (period_cycle > THEAD_PWM_MAX_PERIOD) > > + period_cycle = THEAD_PWM_MAX_PERIOD; > > + /* > > + * With limitation above we have period_cycle <= THEAD_PWM_MAX_PERIOD, > > + * so this cannot overflow. > > + */ > > + writel((u32)period_cycle, priv->mmio_base + THEAD_PWM_PER(pwm->hwpwm)); > > This cast can also be dropped. > > > + > > + duty_cycle = mul_u64_u64_div_u64(rate, state->duty_cycle, NSEC_PER_SEC); > > + if (duty_cycle > THEAD_PWM_MAX_DUTY) > > + duty_cycle = THEAD_PWM_MAX_DUTY; > > + /* > > + * With limitation above we have duty_cycle <= THEAD_PWM_MAX_PERIOD, > > + * so this cannot overflow. > > + */ > > + writel((u32)duty_cycle, priv->mmio_base + THEAD_PWM_FP(pwm->hwpwm)); > > ... > > > + > > + val |= THEAD_PWM_CFG_UPDATE; > > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > > + > > + if (!pwm->state.enabled) { > > + val |= THEAD_PWM_START; > > + writel(val, priv->mmio_base + THEAD_PWM_CTRL(pwm->hwpwm)); > > + priv->ever_started = true; > > + } > > Further above you conditionally call pm_runtime_get_sync(), there should > be a matching pm_runtime_put(). In v3, I call pm_runtime_put_sync() when pwm channel is disabled. Thanks _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-10-05 14:19 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-10-04 9:27 [PATCH v2 0/2] pwm: add driver for T-THEAD TH1520 SoC Jisheng Zhang 2023-10-04 9:27 ` [PATCH v2 1/2] dt-bindings: pwm: Add T-HEAD PWM controller Jisheng Zhang 2023-10-04 14:02 ` Uwe Kleine-König 2023-10-04 15:07 ` Rob Herring 2023-10-04 9:27 ` [PATCH v2 2/2] pwm: add T-HEAD PWM driver Jisheng Zhang 2023-10-04 14:01 ` Uwe Kleine-König 2023-10-05 14:06 ` Jisheng Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).