[PATCH 2/2] pwm: add T-HEAD PWM driver
Emil Renner Berthing
emil.renner.berthing at canonical.com
Fri Sep 29 04:26:35 PDT 2023
Jisheng Zhang wrote:
> T-HEAD SoCs such as the TH1520 contain a PWM controller used
> among other things to control the LCD backlight, fan and so on.
> Add driver for it.
>
> Signed-off-by: Jisheng Zhang <jszhang at kernel.org>
> ---
> MAINTAINERS | 1 +
> drivers/pwm/Kconfig | 11 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-thead.c | 289 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 302 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 at 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 at 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..8339f5617b6f
> --- /dev/null
> +++ b/drivers/pwm/pwm-thead.c
> @@ -0,0 +1,289 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * T-HEAD PWM driver
> + *
> + * Copyright (C) 2021 Alibaba Group Holding Limited.
> + */
> +
> +#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 MAX_PWM_NUM 6
> +
> +#define LIGHT_PWM_CHN_BASE(n) ((n) * 0x20)
> +#define LIGHT_PWM_CTRL(n) (LIGHT_PWM_CHN_BASE(n) + 0x00)
> +#define LIGHT_PWM_RPT(n) (LIGHT_PWM_CHN_BASE(n) + 0x04)
> +#define LIGHT_PWM_PER(n) (LIGHT_PWM_CHN_BASE(n) + 0x08)
> +#define LIGHT_PWM_FP(n) (LIGHT_PWM_CHN_BASE(n) + 0x0c)
> +#define LIGHT_PWM_STATUS(n) (LIGHT_PWM_CHN_BASE(n) + 0x10)
> +
> +/* bit definition PWM_CTRL */
> +#define PWM_START BIT(0)
> +#define PWM_SOFT_RST BIT(1)
> +#define PWM_CFG_UPDATE BIT(2)
> +#define PWM_INT_EN BIT(3)
> +#define PWM_ONE_SHOT_MODE BIT(4)
> +#define PWM_CONTINUOUS_MODE BIT(5)
> +#define PWM_EVT_RISING_TRIG_UNDER_ONE_SHOT BIT(6)
> +#define PWM_EVT_FALLING_TRIG_UNDER_ONE_SHOT BIT(7)
> +#define PWM_FPOUT BIT(8)
> +#define PWM_INFACTOUT BIT(9)
Hi Jisheng,
I'd be worried that these defines one day clash with the PWM framework. Maybe
just keep the LIGHT_PWM_ prefix?
> +
> +struct thead_pwm_chip {
> + struct clk *clk;
> + void __iomem *mmio_base;
> + struct pwm_chip chip;
> +};
> +
> +#define to_thead_pwm_chip(chip) container_of(chip, struct thead_pwm_chip, chip)
> +
> +static int thead_pwm_clk_prepare_enable(struct thead_pwm_chip *pc)
> +{
> + return clk_prepare_enable(pc->clk);
> +}
> +
> +static void thead_pwm_clk_disable_unprepare(struct thead_pwm_chip *pc)
> +{
> + clk_disable_unprepare(pc->clk);
> +}
These two wrappers don't seem to add a lot of value compared to just writing
clk_*(pc->clk) directly and they're not used as callbacks.
> +
> +static int thead_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct thead_pwm_chip *pc = to_thead_pwm_chip(chip);
> + u32 value;
> + int ret;
> +
> + ret = pm_runtime_get_sync(chip->dev);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to clock on the pwm device(%d)\n", ret);
> + return ret;
> + }
> +
> + value = readl(pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> + value |= PWM_START;
> + writel(value, pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> +
> + return 0;
> +}
> +
> +static void thead_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct thead_pwm_chip *pc = to_thead_pwm_chip(chip);
> + u32 value;
> +
> + value = readl(pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> + value &= ~PWM_START;
> + writel(value, pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> +
> + pm_runtime_put_sync(chip->dev);
> +}
> +
> +static int thead_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> + int duty_ns, int period_ns)
> +{
> + struct thead_pwm_chip *pc = to_thead_pwm_chip(chip);
> + unsigned long rate = clk_get_rate(pc->clk);
> + unsigned long duty_cycle, period_cycle;
> + u32 pwm_cfg = PWM_INFACTOUT | PWM_FPOUT | PWM_CONTINUOUS_MODE | PWM_INT_EN;
> + int ret;
> +
> + if (duty_ns > period_ns) {
> + dev_err(chip->dev, "invalid pwm configure\n");
> + return -EINVAL;
> + }
> +
> + ret = pm_runtime_get_sync(chip->dev);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to clock on the pwm device(%d)\n", ret);
> + return ret;
> + }
> +
> + writel(pwm_cfg, pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> +
> + period_cycle = period_ns * rate;
> + do_div(period_cycle, NSEC_PER_SEC);
I thought do_div() was only needed to do 64bit by 32bit divisions on
32bit hardware, but here
period_cycle is an unsigned long, so only 32bit on 32bit hardware.
> + writel(period_cycle, pc->mmio_base + LIGHT_PWM_PER(pwm->hwpwm));
> +
> + duty_cycle = duty_ns * rate;
> + do_div(duty_cycle, NSEC_PER_SEC);
Same as above.
> + writel(duty_cycle, pc->mmio_base + LIGHT_PWM_FP(pwm->hwpwm));
> +
> + pwm_cfg = readl(pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> + pwm_cfg |= PWM_CFG_UPDATE;
> + writel(pwm_cfg, pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> +
> + pm_runtime_put_sync(chip->dev);
> +
> + return 0;
> +}
> +
> +static int thead_pwm_set_polarity(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + enum pwm_polarity polarity)
> +{
> + struct thead_pwm_chip *pc = to_thead_pwm_chip(chip);
> + u32 value = readl(pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> + int ret;
> +
> + ret = pm_runtime_get_sync(chip->dev);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to clock on the pwm device(%d)\n", ret);
> + return ret;
> + }
> +
> + if (polarity == PWM_POLARITY_NORMAL)
> + value |= PWM_FPOUT;
> + else
> + value &= ~PWM_FPOUT;
> +
> + writel(value, pc->mmio_base + LIGHT_PWM_CTRL(pwm->hwpwm));
> +
> + pm_runtime_put_sync(chip->dev);
> +
> + return 0;
> +}
> +
> +static int thead_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> + const struct pwm_state *state)
> +{
> + int err;
> + bool enabled = pwm->state.enabled;
> +
> + if (state->polarity != pwm->state.polarity)
> + thead_pwm_set_polarity(chip, pwm, state->polarity);
> +
> + if (!state->enabled) {
> + if (enabled)
> + thead_pwm_disable(chip, pwm);
> + return 0;
> + }
> +
> + err = thead_pwm_config(chip, pwm, state->duty_cycle, state->period);
> + if (err)
> + return err;
> +
> + if (!enabled)
> + return thead_pwm_enable(chip, pwm);
> +
> + return 0;
> +}
> +
> +static const struct pwm_ops thead_pwm_ops = {
> + .apply = thead_pwm_apply,
> + .owner = THIS_MODULE,
> +};
> +
> +static int __maybe_unused thead_pwm_runtime_suspend(struct device *dev)
> +{
> + struct thead_pwm_chip *pc = dev_get_drvdata(dev);
> +
> + thead_pwm_clk_disable_unprepare(pc);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused thead_pwm_runtime_resume(struct device *dev)
> +{
> + struct thead_pwm_chip *pc = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = thead_pwm_clk_prepare_enable(pc);
> + 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)
> +{
> + struct thead_pwm_chip *pc;
> + int ret;
> +
> + pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
> + if (!pc)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, pc);
> +
> + pc->mmio_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(pc->mmio_base))
> + return PTR_ERR(pc->mmio_base);
> +
> + pc->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(pc->clk))
> + return PTR_ERR(pc->clk);
A lot of other drivers have something like
return dev_err_probe(&pdev->dev, PTR_ERR(pc->clk), "failed to get clock\n");
when failing to get clocks.
> +
> + pm_runtime_enable(&pdev->dev);
> + pm_runtime_get_noresume(&pdev->dev);
> + ret = thead_pwm_clk_prepare_enable(pc);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to enable pwm clock(%d)\n", ret);
> + goto err_pm_disable;
> + }
> +
> + pc->chip.ops = &thead_pwm_ops;
> + pc->chip.dev = &pdev->dev;
> + pc->chip.npwm = MAX_PWM_NUM;
> +
> + ret = pwmchip_add(&pc->chip);
> + if (ret)
> + goto err_clk_disable;
> +
> + pm_runtime_put(&pdev->dev);
> +
> + return 0;
> +
> +err_clk_disable:
> + thead_pwm_clk_disable_unprepare(pc);
> +err_pm_disable:
> + pm_runtime_disable(&pdev->dev);
Here you disable the clock and then pm_runtime_disable()..
> + return ret;
> +}
> +
> +static void thead_pwm_remove(struct platform_device *pdev)
> +{
> + struct thead_pwm_chip *pc = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(&pdev->dev);
> + thead_pwm_clk_disable_unprepare(pc);
..but here it's the other way around. It may not make a difference, but let's
be consistent anyway.
> + pwmchip_remove(&pc->chip);
> +}
> +
> +static const struct of_device_id thead_pwm_dt_ids[] = {
> + {.compatible = "thead,th1520-pwm",},
> + {/* sentinel */}
> +};
> +
> +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 at linux.alibaba.com>");
wei.liu is a very uncommon name, do you mean Wei Liu?
> +MODULE_AUTHOR("Jisheng Zhang <jszhang at kernel.org>");
> +MODULE_DESCRIPTION("T-HEAD pwm driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.40.1
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
More information about the linux-riscv
mailing list