| Message ID | 20241018034857.568-3-cwweng.linux@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for nuvoton ma35d1 pwm controller | expand |
On 18/10/2024 05:48, Chi-Wen Weng wrote: > This commit adds a generic PWM framework driver for Nuvoton MA35D1 > PWM controller. > > Signed-off-by: Chi-Wen Weng <cwweng.linux@gmail.com> ... > + > +static const struct of_device_id nuvoton_pwm_of_match[] = { > + { .compatible = "nuvoton,ma35d1-pwm" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match); > + > +static struct platform_driver nuvoton_pwm_driver = { > + .probe = nuvoton_pwm_probe, > + .driver = { > + .name = "nuvoton-pwm", > + .of_match_table = nuvoton_pwm_of_match, > + }, > +}; > +module_platform_driver(nuvoton_pwm_driver); > + > +MODULE_ALIAS("platform:nuvoton-pwm"); Drop, not needed and not correct either. If you need platform alias for non-OF binds, this is supposed to match OF ID table. > +MODULE_AUTHOR("Chi-Wen Weng <cwweng@nuvoton.com>"); > +MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver"); > +MODULE_LICENSE("GPL"); Best regards, Krzysztof
On Fri, Oct 18, 2024 at 03:48:57AM +0000, Chi-Wen Weng wrote: > This commit adds a generic PWM framework driver for Nuvoton MA35D1 > PWM controller. > > Signed-off-by: Chi-Wen Weng <cwweng.linux@gmail.com> > --- > drivers/pwm/Kconfig | 9 +++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-ma35d1.c | 169 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 179 insertions(+) > create mode 100644 drivers/pwm/pwm-ma35d1.c > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 0915c1e7df16..97b9e83af020 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -411,6 +411,15 @@ config PWM_LPSS_PLATFORM > To compile this driver as a module, choose M here: the module > will be called pwm-lpss-platform. > > +config PWM_MA35D1 > + tristate "Nuvoton MA35D1 PWM support" > + depends on ARCH_MA35 || COMPILE_TEST > + help > + Generic PWM framework driver for Nuvoton MA35D1. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-ma35d1. > + > config PWM_MESON > tristate "Amlogic Meson PWM driver" > depends on ARCH_MESON || COMPILE_TEST > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index 9081e0c0e9e0..c1d3a1d8add0 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -36,6 +36,7 @@ obj-$(CONFIG_PWM_LPC32XX) += pwm-lpc32xx.o > obj-$(CONFIG_PWM_LPSS) += pwm-lpss.o > obj-$(CONFIG_PWM_LPSS_PCI) += pwm-lpss-pci.o > obj-$(CONFIG_PWM_LPSS_PLATFORM) += pwm-lpss-platform.o > +obj-$(CONFIG_PWM_MA35D1) += pwm-ma35d1.o > obj-$(CONFIG_PWM_MESON) += pwm-meson.o > obj-$(CONFIG_PWM_MEDIATEK) += pwm-mediatek.o > obj-$(CONFIG_PWM_MICROCHIP_CORE) += pwm-microchip-core.o > diff --git a/drivers/pwm/pwm-ma35d1.c b/drivers/pwm/pwm-ma35d1.c > new file mode 100644 > index 000000000000..dc2f1f494a91 > --- /dev/null > +++ b/drivers/pwm/pwm-ma35d1.c > @@ -0,0 +1,169 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for the Nuvoton MA35D1 PWM controller > + * > + * Copyright (C) 2024 Nuvoton Corporation > + * Chi-Wen Weng <cwweng@nuvoton.com> > + */ > + > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/pwm.h> > +#include <linux/io.h> > +#include <linux/clk.h> > +#include <linux/math64.h> > + > +/* The following are registers for PWM controller */ > +#define REG_PWM_CTL0 (0x00) > +#define REG_PWM_CNTEN (0x20) > +#define REG_PWM_PERIOD0 (0x30) > +#define REG_PWM_CMPDAT0 (0x50) > +#define REG_PWM_WGCTL0 (0xB0) > +#define REG_PWM_POLCTL (0xD4) > +#define REG_PWM_POEN (0xD8) > + > +#define PWM_TOTAL_CHANNELS 6 > +#define PWM_CH_REG_SIZE 4 > + > +struct nuvoton_pwm { > + void __iomem *base; > + u64 clkrate; > +}; > + > +static inline struct nuvoton_pwm *to_nuvoton_pwm(struct pwm_chip *chip) > +{ > + return pwmchip_get_drvdata(chip); > +} > + > +static int nuvoton_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + struct nuvoton_pwm *nvtpwm; > + unsigned int ch = pwm->hwpwm; > + > + nvtpwm = to_nuvoton_pwm(chip); > + if (state->enabled) { > + u64 duty_cycles, period_cycles; > + > + /* Calculate the duty and period cycles */ > + duty_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, > + state->duty_cycle, NSEC_PER_SEC); > + if (duty_cycles > 0xFFFF) > + duty_cycles = 0xFFFF; > + > + period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, > + state->period, NSEC_PER_SEC); > + if (period_cycles > 0xFFFF) > + period_cycles = 0xFFFF; If a period is not supported, return -EINVAL - maybe even do a dev_err(). Same for the duty cycle above. It might make sense to calculate the period first, so that the error is more likely to be about the period than the duty cycle. Then again I don't know if all the drivers do this, but at least some of them do. > + > + /* Write the duty and period cycles to registers */ > + writel(duty_cycles, nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE)); > + writel(period_cycles, nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE)); > + /* Enable counter */ > + writel(readl(nvtpwm->base + REG_PWM_CNTEN) | BIT(ch), > + nvtpwm->base + REG_PWM_CNTEN); > + /* Enable output */ > + writel(readl(nvtpwm->base + REG_PWM_POEN) | BIT(ch), > + nvtpwm->base + REG_PWM_POEN); > + } else { > + /* Disable counter */ > + writel(readl(nvtpwm->base + REG_PWM_CNTEN) & ~BIT(ch), > + nvtpwm->base + REG_PWM_CNTEN); > + /* Disable output */ > + writel(readl(nvtpwm->base + REG_PWM_POEN) & ~BIT(ch), > + nvtpwm->base + REG_PWM_POEN); > + } > + > + /* Set polarity state to register */ > + if (state->polarity == PWM_POLARITY_NORMAL) > + writel(readl(nvtpwm->base + REG_PWM_POLCTL) & ~BIT(ch), > + nvtpwm->base + REG_PWM_POLCTL); > + else > + writel(readl(nvtpwm->base + REG_PWM_POLCTL) | BIT(ch), > + nvtpwm->base + REG_PWM_POLCTL); > + > + return 0; > +} > + > +static int nuvoton_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > + struct pwm_state *state) > +{ > + struct nuvoton_pwm *nvtpwm; > + unsigned int duty_cycles, period_cycles, cnten, outen, polarity; > + unsigned int ch = pwm->hwpwm; > + > + nvtpwm = to_nuvoton_pwm(chip); > + > + cnten = readl(nvtpwm->base + REG_PWM_CNTEN); > + outen = readl(nvtpwm->base + REG_PWM_POEN); > + duty_cycles = readl(nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE)); > + period_cycles = readl(nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE)); > + polarity = readl(nvtpwm->base + REG_PWM_POLCTL) & BIT(ch); > + > + state->enabled = (cnten & BIT(ch)) && (outen & BIT(ch)); > + state->polarity = polarity ? PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; > + state->duty_cycle = DIV64_U64_ROUND_UP((u64)duty_cycles * NSEC_PER_SEC, nvtpwm->clkrate); > + state->period = DIV64_U64_ROUND_UP((u64)period_cycles * NSEC_PER_SEC, nvtpwm->clkrate); > + > + return 0; > +} > + > +static const struct pwm_ops nuvoton_pwm_ops = { > + .apply = nuvoton_pwm_apply, > + .get_state = nuvoton_pwm_get_state, > +}; > + > +static int nuvoton_pwm_probe(struct platform_device *pdev) > +{ > + struct pwm_chip *chip; > + struct nuvoton_pwm *nvtpwm; > + struct clk *clk; > + int ret; > + > + chip = devm_pwmchip_alloc(&pdev->dev, PWM_TOTAL_CHANNELS, sizeof(*nvtpwm)); > + if (IS_ERR(chip)) > + return PTR_ERR(chip); > + > + nvtpwm = to_nuvoton_pwm(chip); > + > + nvtpwm->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(nvtpwm->base)) > + return PTR_ERR(nvtpwm->base); > + > + clk = devm_clk_get_enabled(&pdev->dev, NULL); > + if (IS_ERR(clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(clk), "unable to get the clock"); > + > + nvtpwm->clkrate = clk_get_rate(clk); > + if (nvtpwm->clkrate > NSEC_PER_SEC) > + return dev_err_probe(&pdev->dev, -EINVAL, "pwm clock out of range"); > + > + chip->ops = &nuvoton_pwm_ops; I think you can add chip->atomic = true; here Sean > + > + ret = devm_pwmchip_add(&pdev->dev, chip); > + if (ret < 0) > + return dev_err_probe(&pdev->dev, ret, "unable to add pwm chip"); > + > + return 0; > +} > + > +static const struct of_device_id nuvoton_pwm_of_match[] = { > + { .compatible = "nuvoton,ma35d1-pwm" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match); > + > +static struct platform_driver nuvoton_pwm_driver = { > + .probe = nuvoton_pwm_probe, > + .driver = { > + .name = "nuvoton-pwm", > + .of_match_table = nuvoton_pwm_of_match, > + }, > +}; > +module_platform_driver(nuvoton_pwm_driver); > + > +MODULE_ALIAS("platform:nuvoton-pwm"); > +MODULE_AUTHOR("Chi-Wen Weng <cwweng@nuvoton.com>"); > +MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver"); > +MODULE_LICENSE("GPL"); > -- > 2.25.1 >
Hello Sean, On Fri, Oct 18, 2024 at 08:46:28AM +0100, Sean Young wrote: > > +static int nuvoton_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > + const struct pwm_state *state) > > +{ > > + struct nuvoton_pwm *nvtpwm; > > + unsigned int ch = pwm->hwpwm; > > + > > + nvtpwm = to_nuvoton_pwm(chip); > > + if (state->enabled) { > > + u64 duty_cycles, period_cycles; > > + > > + /* Calculate the duty and period cycles */ > > + duty_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, > > + state->duty_cycle, NSEC_PER_SEC); > > + if (duty_cycles > 0xFFFF) > > + duty_cycles = 0xFFFF; > > + > > + period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, > > + state->period, NSEC_PER_SEC); > > + if (period_cycles > 0xFFFF) > > + period_cycles = 0xFFFF; > > If a period is not supported, return -EINVAL - maybe even do a dev_err(). > Same for the duty cycle above. It might make sense to calculate the period > first, so that the error is more likely to be about the period than the > duty cycle. That's a wrong advice. Drivers are supposed to implement the highest period possible that is not bigger than the requested one. So clamping the value to 0xFFFF looks right. However I wonder what happens in hardware if period_cycles == 0. If that disables the hardware that is something to catch and return an error for. > Then again I don't know if all the drivers do this, but at least some of > them do. Yeah, and I hesitate to align them because their behaviour might be relied on. But for new drivers the above rule applies. (And with the new waveform stuff, consumers can rely on the rounding rule and even query the resulting waveform before calling the equivalent of pwm_apply_might_sleep(). > > + chip->ops = &nuvoton_pwm_ops; > > I think you can add chip->atomic = true; here ack. Best regards Uwe
Hi Krzysztof, Thank you for your reply. On 2024/10/18 下午 02:01, Krzysztof Kozlowski wrote: > On 18/10/2024 05:48, Chi-Wen Weng wrote: >> This commit adds a generic PWM framework driver for Nuvoton MA35D1 >> PWM controller. >> >> Signed-off-by: Chi-Wen Weng <cwweng.linux@gmail.com> > > ... > > >> + >> +static const struct of_device_id nuvoton_pwm_of_match[] = { >> + { .compatible = "nuvoton,ma35d1-pwm" }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match); >> + >> +static struct platform_driver nuvoton_pwm_driver = { >> + .probe = nuvoton_pwm_probe, >> + .driver = { >> + .name = "nuvoton-pwm", >> + .of_match_table = nuvoton_pwm_of_match, >> + }, >> +}; >> +module_platform_driver(nuvoton_pwm_driver); >> + >> +MODULE_ALIAS("platform:nuvoton-pwm"); > Drop, not needed and not correct either. If you need platform alias for > non-OF binds, this is supposed to match OF ID table. Ok. I will drop it. >> +MODULE_AUTHOR("Chi-Wen Weng <cwweng@nuvoton.com>"); >> +MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver"); >> +MODULE_LICENSE("GPL"); > Best regards, > Krzysztof Thanks. Chi-Wen Weng
Hi Sean, Thank you for your reply. On 2024/10/18 下午 03:46, Sean Young wrote: > On Fri, Oct 18, 2024 at 03:48:57AM +0000, Chi-Wen Weng wrote: >> This commit adds a generic PWM framework driver for Nuvoton MA35D1 >> PWM controller. >> >> Signed-off-by: Chi-Wen Weng <cwweng.linux@gmail.com> >> --- >> drivers/pwm/Kconfig | 9 +++ >> drivers/pwm/Makefile | 1 + >> drivers/pwm/pwm-ma35d1.c | 169 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 179 insertions(+) >> create mode 100644 drivers/pwm/pwm-ma35d1.c >> >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig >> index 0915c1e7df16..97b9e83af020 100644 >> --- a/drivers/pwm/Kconfig >> +++ b/drivers/pwm/Kconfig >> @@ -411,6 +411,15 @@ config PWM_LPSS_PLATFORM >> To compile this driver as a module, choose M here: the module >> will be called pwm-lpss-platform. >> >> +config PWM_MA35D1 >> + tristate "Nuvoton MA35D1 PWM support" >> + depends on ARCH_MA35 || COMPILE_TEST >> + help >> + Generic PWM framework driver for Nuvoton MA35D1. >> + >> + To compile this driver as a module, choose M here: the module >> + will be called pwm-ma35d1. >> + >> config PWM_MESON >> tristate "Amlogic Meson PWM driver" >> depends on ARCH_MESON || COMPILE_TEST >> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile >> index 9081e0c0e9e0..c1d3a1d8add0 100644 >> --- a/drivers/pwm/Makefile >> +++ b/drivers/pwm/Makefile >> @@ -36,6 +36,7 @@ obj-$(CONFIG_PWM_LPC32XX) += pwm-lpc32xx.o >> obj-$(CONFIG_PWM_LPSS) += pwm-lpss.o >> obj-$(CONFIG_PWM_LPSS_PCI) += pwm-lpss-pci.o >> obj-$(CONFIG_PWM_LPSS_PLATFORM) += pwm-lpss-platform.o >> +obj-$(CONFIG_PWM_MA35D1) += pwm-ma35d1.o >> obj-$(CONFIG_PWM_MESON) += pwm-meson.o >> obj-$(CONFIG_PWM_MEDIATEK) += pwm-mediatek.o >> obj-$(CONFIG_PWM_MICROCHIP_CORE) += pwm-microchip-core.o >> diff --git a/drivers/pwm/pwm-ma35d1.c b/drivers/pwm/pwm-ma35d1.c >> new file mode 100644 >> index 000000000000..dc2f1f494a91 >> --- /dev/null >> +++ b/drivers/pwm/pwm-ma35d1.c >> @@ -0,0 +1,169 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Driver for the Nuvoton MA35D1 PWM controller >> + * >> + * Copyright (C) 2024 Nuvoton Corporation >> + * Chi-Wen Weng <cwweng@nuvoton.com> >> + */ >> + >> +#include <linux/mod_devicetable.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/pwm.h> >> +#include <linux/io.h> >> +#include <linux/clk.h> >> +#include <linux/math64.h> >> + >> +/* The following are registers for PWM controller */ >> +#define REG_PWM_CTL0 (0x00) >> +#define REG_PWM_CNTEN (0x20) >> +#define REG_PWM_PERIOD0 (0x30) >> +#define REG_PWM_CMPDAT0 (0x50) >> +#define REG_PWM_WGCTL0 (0xB0) >> +#define REG_PWM_POLCTL (0xD4) >> +#define REG_PWM_POEN (0xD8) >> + >> +#define PWM_TOTAL_CHANNELS 6 >> +#define PWM_CH_REG_SIZE 4 >> + >> +struct nuvoton_pwm { >> + void __iomem *base; >> + u64 clkrate; >> +}; >> + >> +static inline struct nuvoton_pwm *to_nuvoton_pwm(struct pwm_chip *chip) >> +{ >> + return pwmchip_get_drvdata(chip); >> +} >> + >> +static int nuvoton_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, >> + const struct pwm_state *state) >> +{ >> + struct nuvoton_pwm *nvtpwm; >> + unsigned int ch = pwm->hwpwm; >> + >> + nvtpwm = to_nuvoton_pwm(chip); >> + if (state->enabled) { >> + u64 duty_cycles, period_cycles; >> + >> + /* Calculate the duty and period cycles */ >> + duty_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, >> + state->duty_cycle, NSEC_PER_SEC); >> + if (duty_cycles > 0xFFFF) >> + duty_cycles = 0xFFFF; >> + >> + period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, >> + state->period, NSEC_PER_SEC); >> + if (period_cycles > 0xFFFF) >> + period_cycles = 0xFFFF; > If a period is not supported, return -EINVAL - maybe even do a dev_err(). > Same for the duty cycle above. It might make sense to calculate the period > first, so that the error is more likely to be about the period than the > duty cycle. > > Then again I don't know if all the drivers do this, but at least some of > them do. Uwe has explained it. Thanks Uwe. >> + >> + /* Write the duty and period cycles to registers */ >> + writel(duty_cycles, nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE)); >> + writel(period_cycles, nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE)); >> + /* Enable counter */ >> + writel(readl(nvtpwm->base + REG_PWM_CNTEN) | BIT(ch), >> + nvtpwm->base + REG_PWM_CNTEN); >> + /* Enable output */ >> + writel(readl(nvtpwm->base + REG_PWM_POEN) | BIT(ch), >> + nvtpwm->base + REG_PWM_POEN); >> + } else { >> + /* Disable counter */ >> + writel(readl(nvtpwm->base + REG_PWM_CNTEN) & ~BIT(ch), >> + nvtpwm->base + REG_PWM_CNTEN); >> + /* Disable output */ >> + writel(readl(nvtpwm->base + REG_PWM_POEN) & ~BIT(ch), >> + nvtpwm->base + REG_PWM_POEN); >> + } >> + >> + /* Set polarity state to register */ >> + if (state->polarity == PWM_POLARITY_NORMAL) >> + writel(readl(nvtpwm->base + REG_PWM_POLCTL) & ~BIT(ch), >> + nvtpwm->base + REG_PWM_POLCTL); >> + else >> + writel(readl(nvtpwm->base + REG_PWM_POLCTL) | BIT(ch), >> + nvtpwm->base + REG_PWM_POLCTL); >> + >> + return 0; >> +} >> + >> +static int nuvoton_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, >> + struct pwm_state *state) >> +{ >> + struct nuvoton_pwm *nvtpwm; >> + unsigned int duty_cycles, period_cycles, cnten, outen, polarity; >> + unsigned int ch = pwm->hwpwm; >> + >> + nvtpwm = to_nuvoton_pwm(chip); >> + >> + cnten = readl(nvtpwm->base + REG_PWM_CNTEN); >> + outen = readl(nvtpwm->base + REG_PWM_POEN); >> + duty_cycles = readl(nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE)); >> + period_cycles = readl(nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE)); >> + polarity = readl(nvtpwm->base + REG_PWM_POLCTL) & BIT(ch); >> + >> + state->enabled = (cnten & BIT(ch)) && (outen & BIT(ch)); >> + state->polarity = polarity ? PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; >> + state->duty_cycle = DIV64_U64_ROUND_UP((u64)duty_cycles * NSEC_PER_SEC, nvtpwm->clkrate); >> + state->period = DIV64_U64_ROUND_UP((u64)period_cycles * NSEC_PER_SEC, nvtpwm->clkrate); >> + >> + return 0; >> +} >> + >> +static const struct pwm_ops nuvoton_pwm_ops = { >> + .apply = nuvoton_pwm_apply, >> + .get_state = nuvoton_pwm_get_state, >> +}; >> + >> +static int nuvoton_pwm_probe(struct platform_device *pdev) >> +{ >> + struct pwm_chip *chip; >> + struct nuvoton_pwm *nvtpwm; >> + struct clk *clk; >> + int ret; >> + >> + chip = devm_pwmchip_alloc(&pdev->dev, PWM_TOTAL_CHANNELS, sizeof(*nvtpwm)); >> + if (IS_ERR(chip)) >> + return PTR_ERR(chip); >> + >> + nvtpwm = to_nuvoton_pwm(chip); >> + >> + nvtpwm->base = devm_platform_ioremap_resource(pdev, 0); >> + if (IS_ERR(nvtpwm->base)) >> + return PTR_ERR(nvtpwm->base); >> + >> + clk = devm_clk_get_enabled(&pdev->dev, NULL); >> + if (IS_ERR(clk)) >> + return dev_err_probe(&pdev->dev, PTR_ERR(clk), "unable to get the clock"); >> + >> + nvtpwm->clkrate = clk_get_rate(clk); >> + if (nvtpwm->clkrate > NSEC_PER_SEC) >> + return dev_err_probe(&pdev->dev, -EINVAL, "pwm clock out of range"); >> + >> + chip->ops = &nuvoton_pwm_ops; > I think you can add chip->atomic = true; here Ok. I will add it. Thanks. > > Sean > >> + >> + ret = devm_pwmchip_add(&pdev->dev, chip); >> + if (ret < 0) >> + return dev_err_probe(&pdev->dev, ret, "unable to add pwm chip"); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id nuvoton_pwm_of_match[] = { >> + { .compatible = "nuvoton,ma35d1-pwm" }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match); >> + >> +static struct platform_driver nuvoton_pwm_driver = { >> + .probe = nuvoton_pwm_probe, >> + .driver = { >> + .name = "nuvoton-pwm", >> + .of_match_table = nuvoton_pwm_of_match, >> + }, >> +}; >> +module_platform_driver(nuvoton_pwm_driver); >> + >> +MODULE_ALIAS("platform:nuvoton-pwm"); >> +MODULE_AUTHOR("Chi-Wen Weng <cwweng@nuvoton.com>"); >> +MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver"); >> +MODULE_LICENSE("GPL"); >> -- >> 2.25.1 Thanks. Chi-Wen Weng
Hi Uwe, Thank you for your reply. On 2024/10/18 下午 04:22, Uwe Kleine-König wrote: > Hello Sean, > > On Fri, Oct 18, 2024 at 08:46:28AM +0100, Sean Young wrote: >>> +static int nuvoton_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, >>> + const struct pwm_state *state) >>> +{ >>> + struct nuvoton_pwm *nvtpwm; >>> + unsigned int ch = pwm->hwpwm; >>> + >>> + nvtpwm = to_nuvoton_pwm(chip); >>> + if (state->enabled) { >>> + u64 duty_cycles, period_cycles; >>> + >>> + /* Calculate the duty and period cycles */ >>> + duty_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, >>> + state->duty_cycle, NSEC_PER_SEC); >>> + if (duty_cycles > 0xFFFF) >>> + duty_cycles = 0xFFFF; >>> + >>> + period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, >>> + state->period, NSEC_PER_SEC); >>> + if (period_cycles > 0xFFFF) >>> + period_cycles = 0xFFFF; >> If a period is not supported, return -EINVAL - maybe even do a dev_err(). >> Same for the duty cycle above. It might make sense to calculate the period >> first, so that the error is more likely to be about the period than the >> duty cycle. > That's a wrong advice. Drivers are supposed to implement the highest > period possible that is not bigger than the requested one. So clamping > the value to 0xFFFF looks right. Thanks for your explanation. > However I wonder what happens in hardware if period_cycles == 0. If that > disables the hardware that is something to catch and return an error > for. If period_cycles = 0, the waveform output will be always low. >> Then again I don't know if all the drivers do this, but at least some of >> them do. > Yeah, and I hesitate to align them because their behaviour might be > relied on. But for new drivers the above rule applies. > > (And with the new waveform stuff, consumers can rely on the rounding > rule and even query the resulting waveform before calling the equivalent > of pwm_apply_might_sleep(). > > >>> + chip->ops = &nuvoton_pwm_ops; >> I think you can add chip->atomic = true; here > ack. > > Best regards > Uwe Thanks. Chi-Wen Weng
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 0915c1e7df16..97b9e83af020 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -411,6 +411,15 @@ config PWM_LPSS_PLATFORM To compile this driver as a module, choose M here: the module will be called pwm-lpss-platform. +config PWM_MA35D1 + tristate "Nuvoton MA35D1 PWM support" + depends on ARCH_MA35 || COMPILE_TEST + help + Generic PWM framework driver for Nuvoton MA35D1. + + To compile this driver as a module, choose M here: the module + will be called pwm-ma35d1. + config PWM_MESON tristate "Amlogic Meson PWM driver" depends on ARCH_MESON || COMPILE_TEST diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 9081e0c0e9e0..c1d3a1d8add0 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -36,6 +36,7 @@ obj-$(CONFIG_PWM_LPC32XX) += pwm-lpc32xx.o obj-$(CONFIG_PWM_LPSS) += pwm-lpss.o obj-$(CONFIG_PWM_LPSS_PCI) += pwm-lpss-pci.o obj-$(CONFIG_PWM_LPSS_PLATFORM) += pwm-lpss-platform.o +obj-$(CONFIG_PWM_MA35D1) += pwm-ma35d1.o obj-$(CONFIG_PWM_MESON) += pwm-meson.o obj-$(CONFIG_PWM_MEDIATEK) += pwm-mediatek.o obj-$(CONFIG_PWM_MICROCHIP_CORE) += pwm-microchip-core.o diff --git a/drivers/pwm/pwm-ma35d1.c b/drivers/pwm/pwm-ma35d1.c new file mode 100644 index 000000000000..dc2f1f494a91 --- /dev/null +++ b/drivers/pwm/pwm-ma35d1.c @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for the Nuvoton MA35D1 PWM controller + * + * Copyright (C) 2024 Nuvoton Corporation + * Chi-Wen Weng <cwweng@nuvoton.com> + */ + +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/io.h> +#include <linux/clk.h> +#include <linux/math64.h> + +/* The following are registers for PWM controller */ +#define REG_PWM_CTL0 (0x00) +#define REG_PWM_CNTEN (0x20) +#define REG_PWM_PERIOD0 (0x30) +#define REG_PWM_CMPDAT0 (0x50) +#define REG_PWM_WGCTL0 (0xB0) +#define REG_PWM_POLCTL (0xD4) +#define REG_PWM_POEN (0xD8) + +#define PWM_TOTAL_CHANNELS 6 +#define PWM_CH_REG_SIZE 4 + +struct nuvoton_pwm { + void __iomem *base; + u64 clkrate; +}; + +static inline struct nuvoton_pwm *to_nuvoton_pwm(struct pwm_chip *chip) +{ + return pwmchip_get_drvdata(chip); +} + +static int nuvoton_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct nuvoton_pwm *nvtpwm; + unsigned int ch = pwm->hwpwm; + + nvtpwm = to_nuvoton_pwm(chip); + if (state->enabled) { + u64 duty_cycles, period_cycles; + + /* Calculate the duty and period cycles */ + duty_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, + state->duty_cycle, NSEC_PER_SEC); + if (duty_cycles > 0xFFFF) + duty_cycles = 0xFFFF; + + period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate, + state->period, NSEC_PER_SEC); + if (period_cycles > 0xFFFF) + period_cycles = 0xFFFF; + + /* Write the duty and period cycles to registers */ + writel(duty_cycles, nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE)); + writel(period_cycles, nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE)); + /* Enable counter */ + writel(readl(nvtpwm->base + REG_PWM_CNTEN) | BIT(ch), + nvtpwm->base + REG_PWM_CNTEN); + /* Enable output */ + writel(readl(nvtpwm->base + REG_PWM_POEN) | BIT(ch), + nvtpwm->base + REG_PWM_POEN); + } else { + /* Disable counter */ + writel(readl(nvtpwm->base + REG_PWM_CNTEN) & ~BIT(ch), + nvtpwm->base + REG_PWM_CNTEN); + /* Disable output */ + writel(readl(nvtpwm->base + REG_PWM_POEN) & ~BIT(ch), + nvtpwm->base + REG_PWM_POEN); + } + + /* Set polarity state to register */ + if (state->polarity == PWM_POLARITY_NORMAL) + writel(readl(nvtpwm->base + REG_PWM_POLCTL) & ~BIT(ch), + nvtpwm->base + REG_PWM_POLCTL); + else + writel(readl(nvtpwm->base + REG_PWM_POLCTL) | BIT(ch), + nvtpwm->base + REG_PWM_POLCTL); + + return 0; +} + +static int nuvoton_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct nuvoton_pwm *nvtpwm; + unsigned int duty_cycles, period_cycles, cnten, outen, polarity; + unsigned int ch = pwm->hwpwm; + + nvtpwm = to_nuvoton_pwm(chip); + + cnten = readl(nvtpwm->base + REG_PWM_CNTEN); + outen = readl(nvtpwm->base + REG_PWM_POEN); + duty_cycles = readl(nvtpwm->base + REG_PWM_CMPDAT0 + (ch * PWM_CH_REG_SIZE)); + period_cycles = readl(nvtpwm->base + REG_PWM_PERIOD0 + (ch * PWM_CH_REG_SIZE)); + polarity = readl(nvtpwm->base + REG_PWM_POLCTL) & BIT(ch); + + state->enabled = (cnten & BIT(ch)) && (outen & BIT(ch)); + state->polarity = polarity ? PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; + state->duty_cycle = DIV64_U64_ROUND_UP((u64)duty_cycles * NSEC_PER_SEC, nvtpwm->clkrate); + state->period = DIV64_U64_ROUND_UP((u64)period_cycles * NSEC_PER_SEC, nvtpwm->clkrate); + + return 0; +} + +static const struct pwm_ops nuvoton_pwm_ops = { + .apply = nuvoton_pwm_apply, + .get_state = nuvoton_pwm_get_state, +}; + +static int nuvoton_pwm_probe(struct platform_device *pdev) +{ + struct pwm_chip *chip; + struct nuvoton_pwm *nvtpwm; + struct clk *clk; + int ret; + + chip = devm_pwmchip_alloc(&pdev->dev, PWM_TOTAL_CHANNELS, sizeof(*nvtpwm)); + if (IS_ERR(chip)) + return PTR_ERR(chip); + + nvtpwm = to_nuvoton_pwm(chip); + + nvtpwm->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(nvtpwm->base)) + return PTR_ERR(nvtpwm->base); + + clk = devm_clk_get_enabled(&pdev->dev, NULL); + if (IS_ERR(clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(clk), "unable to get the clock"); + + nvtpwm->clkrate = clk_get_rate(clk); + if (nvtpwm->clkrate > NSEC_PER_SEC) + return dev_err_probe(&pdev->dev, -EINVAL, "pwm clock out of range"); + + chip->ops = &nuvoton_pwm_ops; + + ret = devm_pwmchip_add(&pdev->dev, chip); + if (ret < 0) + return dev_err_probe(&pdev->dev, ret, "unable to add pwm chip"); + + return 0; +} + +static const struct of_device_id nuvoton_pwm_of_match[] = { + { .compatible = "nuvoton,ma35d1-pwm" }, + {} +}; +MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match); + +static struct platform_driver nuvoton_pwm_driver = { + .probe = nuvoton_pwm_probe, + .driver = { + .name = "nuvoton-pwm", + .of_match_table = nuvoton_pwm_of_match, + }, +}; +module_platform_driver(nuvoton_pwm_driver); + +MODULE_ALIAS("platform:nuvoton-pwm"); +MODULE_AUTHOR("Chi-Wen Weng <cwweng@nuvoton.com>"); +MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver"); +MODULE_LICENSE("GPL");
This commit adds a generic PWM framework driver for Nuvoton MA35D1 PWM controller. Signed-off-by: Chi-Wen Weng <cwweng.linux@gmail.com> --- drivers/pwm/Kconfig | 9 +++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-ma35d1.c | 169 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+) create mode 100644 drivers/pwm/pwm-ma35d1.c