* [PATCH v2 0/2] pwm: Add the Ingenic X1600 PWM controller @ 2026-10-25 19:16 Olivier Louvignes 2026-10-25 19:16 ` [PATCH v2 1/2] dt-bindings: " Olivier Louvignes 2026-10-25 19:16 ` [PATCH v2 2/2] pwm: Add a driver for " Olivier Louvignes 0 siblings, 2 replies; 5+ messages in thread From: Olivier Louvignes @ 2026-10-25 19:16 UTC (permalink / raw) To: Uwe Kleine-König Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, linux-pwm, devicetree, linux-mips, linux-kernel, Olivier Louvignes The Ingenic X1600 has a PWM controller of its own, separate from the TCU timers that pwm-jz4740 drives on the earlier Ingenic SoCs. This adds its binding and a driver built on the waveform callbacks. Changes in v2 (both found by the Sashiko review of v1): - A channel that is off with both counts still loaded, as one stopped behind the driver's back leaves it, now reads back as disabled. v1 read it as running, so asking for the same waveform again didn't switch it on. A channel off with one count zero is how 0 and 100% are written; the idle level gives which. - A clock rate of 0 is refused with -EINVAL when rounding a waveform and when writing one, instead of switching the channel off silently or dividing by zero in the update timeout. Tested on a HiBy R1 (X1600), whose backlight is channel 0, through pwm-backlight: brightness 1, 25, 50, 60 and 100% give the expected high and low counts, 0 and 100% switch the channel off at the right idle level, and an inversed 250 us of 1 ms on channel 1 reads back as INL 0, LOW 9375 then HIGH 28125 with the clock divided by 8. PWM_DEBUG stayed silent. For v2 also: channel 1 running 250 us of 1 ms, switched off through ENC directly, then requested again through sysfs, reads back as disabled, and the same waveform asked again switches it back on; v1's driver read it as enabled and left it off. The X1600 has no device tree upstream yet; its support is being posted one subsystem at a time. v1: https://lore.kernel.org/r/20261025084643.93167-1-olivier@mgcrea.io Olivier Louvignes (2): dt-bindings: pwm: Add the Ingenic X1600 PWM controller pwm: Add a driver for the Ingenic X1600 PWM controller .../bindings/pwm/ingenic,x1600-pwm.yaml | 47 +++ MAINTAINERS | 6 + drivers/pwm/Kconfig | 11 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-ingenic-x1600.c | 280 ++++++++++++++++++ 5 files changed, 345 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/ingenic,x1600-pwm.yaml create mode 100644 drivers/pwm/pwm-ingenic-x1600.c base-commit: 00e917df050d79ac26ec700717680ad6421d2808 -- 2.39.5 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] dt-bindings: pwm: Add the Ingenic X1600 PWM controller 2026-10-25 19:16 [PATCH v2 0/2] pwm: Add the Ingenic X1600 PWM controller Olivier Louvignes @ 2026-10-25 19:16 ` Olivier Louvignes 2026-10-27 18:44 ` Krzysztof Kozlowski 2026-10-25 19:16 ` [PATCH v2 2/2] pwm: Add a driver for " Olivier Louvignes 1 sibling, 1 reply; 5+ messages in thread From: Olivier Louvignes @ 2026-10-25 19:16 UTC (permalink / raw) To: Uwe Kleine-König Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, linux-pwm, devicetree, linux-mips, linux-kernel, Olivier Louvignes The X1600 has an eight-channel PWM controller of its own, separate from the TCU timers that drive the PWMs of earlier Ingenic SoCs (ingenic,tcu.yaml), so it gets a binding of its own. Signed-off-by: Olivier Louvignes <olivier@mgcrea.io> --- .../bindings/pwm/ingenic,x1600-pwm.yaml | 47 +++++++++++++++++++ MAINTAINERS | 5 ++ 2 files changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/ingenic,x1600-pwm.yaml diff --git a/Documentation/devicetree/bindings/pwm/ingenic,x1600-pwm.yaml b/Documentation/devicetree/bindings/pwm/ingenic,x1600-pwm.yaml new file mode 100644 index 000000000000..3ecaa92a5ca2 --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/ingenic,x1600-pwm.yaml @@ -0,0 +1,47 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pwm/ingenic,x1600-pwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ingenic X1600 PWM controller + +maintainers: + - Olivier Louvignes <olivier@mgcrea.io> + +description: + The eight-channel PWM controller of the Ingenic X1600 SoC, separate from + the TCU timers that drive the PWMs of earlier Ingenic SoCs. + +allOf: + - $ref: pwm.yaml# + +properties: + compatible: + const: ingenic,x1600-pwm + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + "#pwm-cells": + const: 3 + +required: + - compatible + - reg + - clocks + - "#pwm-cells" + +unevaluatedProperties: false + +examples: + - | + pwm@134c0000 { + compatible = "ingenic,x1600-pwm"; + reg = <0x134c0000 0x200>; + clocks = <&cgu 21>; + #pwm-cells = <3>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 3a19da74d00c..c905aa13bcbc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12878,6 +12878,11 @@ F: include/linux/mfd/ingenic-tcu.h F: sound/soc/codecs/jz47* F: sound/soc/jz4740/ +INGENIC X1600 PWM DRIVER +M: Olivier Louvignes <olivier@mgcrea.io> +S: Maintained +F: Documentation/devicetree/bindings/pwm/ingenic,x1600-pwm.yaml + INJOINIC IP5xxx POWER BANK IC DRIVER M: Samuel Holland <samuel@sholland.org> S: Maintained -- 2.39.5 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: pwm: Add the Ingenic X1600 PWM controller 2026-10-25 19:16 ` [PATCH v2 1/2] dt-bindings: " Olivier Louvignes @ 2026-10-27 18:44 ` Krzysztof Kozlowski 0 siblings, 0 replies; 5+ messages in thread From: Krzysztof Kozlowski @ 2026-10-27 18:44 UTC (permalink / raw) To: Olivier Louvignes Cc: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, linux-pwm, devicetree, linux-mips, linux-kernel On Sun, Oct 25, 2026 at 08:16:23PM +0100, Olivier Louvignes wrote: > The X1600 has an eight-channel PWM controller of its own, separate from > the TCU timers that drive the PWMs of earlier Ingenic SoCs > (ingenic,tcu.yaml), so it gets a binding of its own. > > Signed-off-by: Olivier Louvignes <olivier@mgcrea.io> > --- > .../bindings/pwm/ingenic,x1600-pwm.yaml | 47 +++++++++++++++++++ > MAINTAINERS | 5 ++ > 2 files changed, 52 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/ingenic,x1600-pwm.yaml Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] pwm: Add a driver for the Ingenic X1600 PWM controller 2026-10-25 19:16 [PATCH v2 0/2] pwm: Add the Ingenic X1600 PWM controller Olivier Louvignes 2026-10-25 19:16 ` [PATCH v2 1/2] dt-bindings: " Olivier Louvignes @ 2026-10-25 19:16 ` Olivier Louvignes 2026-10-26 8:04 ` Uwe Kleine-König 1 sibling, 1 reply; 5+ messages in thread From: Olivier Louvignes @ 2026-10-25 19:16 UTC (permalink / raw) To: Uwe Kleine-König Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, linux-pwm, devicetree, linux-mips, linux-kernel, Olivier Louvignes The X1600 has a PWM controller of its own, separate from the TCU timers that pwm-jz4740 drives on earlier Ingenic SoCs (X1600 programming manual, chapter 13): eight channels, each with a power-of-two clock divider and a waveform word holding the counts at the high and at the low level, the channel's initial level picking which comes first. So the duty offset is 0 or period - duty, and since neither count may be zero, a duty cycle of 0 or 100% switches the channel off with its idle level set accordingly. The driver implements the waveform callbacks. It follows the manual's order: the divider is only changed with the channel off, and a running channel takes a new word through the update register. Tested on a HiBy R1, whose backlight is channel 0. Signed-off-by: Olivier Louvignes <olivier@mgcrea.io> --- v2: - Read a channel that is off with both counts loaded as disabled. - Refuse a clock rate of 0 in round_waveform_tohw() and write_waveform(). MAINTAINERS | 1 + drivers/pwm/Kconfig | 11 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-ingenic-x1600.c | 280 ++++++++++++++++++++++++++++++++ 4 files changed, 293 insertions(+) create mode 100644 drivers/pwm/pwm-ingenic-x1600.c diff --git a/MAINTAINERS b/MAINTAINERS index c905aa13bcbc..c090512468a4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12882,6 +12882,7 @@ INGENIC X1600 PWM DRIVER M: Olivier Louvignes <olivier@mgcrea.io> S: Maintained F: Documentation/devicetree/bindings/pwm/ingenic,x1600-pwm.yaml +F: drivers/pwm/pwm-ingenic-x1600.c INJOINIC IP5xxx POWER BANK IC DRIVER M: Samuel Holland <samuel@sholland.org> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 729776086879..244c8f4f140e 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -381,6 +381,17 @@ config PWM_JZ4740 To compile this driver as a module, choose M here: the module will be called pwm-jz4740. +config PWM_INGENIC_X1600 + tristate "Ingenic X1600 PWM support" + depends on MACH_INGENIC || COMPILE_TEST + depends on COMMON_CLK && OF && HAS_IOMEM + help + Generic PWM framework driver for the PWM controller of the + Ingenic X1600. + + To compile this driver as a module, choose M here: the module + will be called pwm-ingenic-x1600. + config PWM_KEEMBAY tristate "Intel Keem Bay PWM driver" depends on ARCH_KEEMBAY || COMPILE_TEST diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 5630a521a7cf..5c59b54e0029 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -31,6 +31,7 @@ obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o obj-$(CONFIG_PWM_INTEL_LGM) += pwm-intel-lgm.o obj-$(CONFIG_PWM_IPQ) += pwm-ipq.o obj-$(CONFIG_PWM_IQS620A) += pwm-iqs620a.o +obj-$(CONFIG_PWM_INGENIC_X1600) += pwm-ingenic-x1600.o obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o obj-$(CONFIG_PWM_KEEMBAY) += pwm-keembay.o obj-$(CONFIG_PWM_LOONGSON) += pwm-loongson.o diff --git a/drivers/pwm/pwm-ingenic-x1600.c b/drivers/pwm/pwm-ingenic-x1600.c new file mode 100644 index 000000000000..5865ff0cf298 --- /dev/null +++ b/drivers/pwm/pwm-ingenic-x1600.c @@ -0,0 +1,280 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * PWM controller of the Ingenic X1600 + * + * Copyright (c) 2026 Olivier Louvignes <olivier@mgcrea.io> + * + * Eight channels, each with a power-of-two clock divider and a waveform word + * holding the counts at the high and at the low level; the channel's initial + * level picks which comes first (X1600 programming manual, chapter 13). Unlike + * the JZ47xx and X1000, whose PWMs are TCU timer channels (pwm-jz4740), the + * X1600 has this separate controller. + * + * Limitations: + * - The duty offset is 0 or period - duty (the high level last). + * - Neither count may be zero, so a duty cycle of 0 or 100% switches the + * channel off and holds its output at the idle level instead. + * - A disabled channel drives its idle level, low. + * - Changing the divider or the initial level stops the channel first, which + * cuts the current period short. + */ + +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/math64.h> +#include <linux/minmax.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pwm.h> +#include <linux/time.h> +#include <linux/types.h> + +#define X1600_PWM_ENS 0x00 /* write 1: switch a channel on */ +#define X1600_PWM_ENC 0x04 /* write 1: switch a channel off */ +#define X1600_PWM_EN 0x08 /* the channels switched on */ +#define X1600_PWM_UPT 0x10 /* write 1: load the waveform word */ +#define X1600_PWM_BUSY 0x14 /* a load is pending */ +#define X1600_PWM_MS 0x20 /* 0: waveform word, 1: DMA */ +#define X1600_PWM_INL 0x24 /* the level a period starts with */ +#define X1600_PWM_IDL 0x28 /* the level while off */ +#define X1600_PWM_CCFG(n) (0x40 + 4 * (n)) +#define X1600_PWM_WCFG(n) (0x80 + 4 * (n)) + +#define X1600_PWM_CCFG_PRESCALE GENMASK(3, 0) /* divide by 2^PRESCALE */ +#define X1600_PWM_WCFG_HIGH GENMASK(31, 16) +#define X1600_PWM_WCFG_LOW GENMASK(15, 0) + +#define X1600_PWM_NUM 8 +#define X1600_PWM_MAX_COUNT 0xffff +#define X1600_PWM_MAX_PRESCALE 7 + +struct x1600_pwm { + void __iomem *base; + struct clk *clk; +}; + +struct x1600_pwm_wfhw { + u32 wcfg; + u8 prescale; + bool enabled; + bool inl; + bool idl; +}; + +static u64 x1600_pwm_counts(u64 ns, unsigned long rate, unsigned int prescale) +{ + return mul_u64_u64_div_u64(ns, rate, (u64)NSEC_PER_SEC << prescale); +} + +static u64 x1600_pwm_ns(u64 counts, unsigned long rate, unsigned int prescale) +{ + return DIV64_U64_ROUND_UP((counts * NSEC_PER_SEC) << prescale, rate); +} + +static int x1600_pwm_round_waveform_tohw(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_waveform *wf, void *_wfhw) +{ + struct x1600_pwm *x = pwmchip_get_drvdata(chip); + unsigned long rate = clk_get_rate(x->clk); + struct x1600_pwm_wfhw *wfhw = _wfhw; + u64 period, high, offset; + unsigned int prescale; + int ret = 0; + + *wfhw = (struct x1600_pwm_wfhw){ }; + if (!wf->period_length_ns) + return 0; + + if (!rate) + return -EINVAL; + + for (prescale = 0; prescale < X1600_PWM_MAX_PRESCALE; prescale++) + if (x1600_pwm_counts(wf->period_length_ns, rate, prescale) <= + X1600_PWM_MAX_COUNT) + break; + + period = min(x1600_pwm_counts(wf->period_length_ns, rate, prescale), + X1600_PWM_MAX_COUNT); + if (period < 2) { + period = 2; + ret = 1; + } + high = min(x1600_pwm_counts(wf->duty_length_ns, rate, prescale), period); + offset = x1600_pwm_counts(wf->duty_offset_ns, rate, prescale); + + wfhw->prescale = prescale; + wfhw->wcfg = FIELD_PREP(X1600_PWM_WCFG_HIGH, high) | + FIELD_PREP(X1600_PWM_WCFG_LOW, period - high); + wfhw->enabled = high && high < period; + /* Low first only if the requested offset reaches period - duty. */ + wfhw->inl = !wfhw->enabled || offset < period - high; + wfhw->idl = high == period; + + return ret; +} + +static int x1600_pwm_round_waveform_fromhw(struct pwm_chip *chip, struct pwm_device *pwm, + const void *_wfhw, struct pwm_waveform *wf) +{ + struct x1600_pwm *x = pwmchip_get_drvdata(chip); + unsigned long rate = clk_get_rate(x->clk); + const struct x1600_pwm_wfhw *wfhw = _wfhw; + u32 high = FIELD_GET(X1600_PWM_WCFG_HIGH, wfhw->wcfg); + u32 low = FIELD_GET(X1600_PWM_WCFG_LOW, wfhw->wcfg); + + *wf = (struct pwm_waveform){ }; + + /* + * A channel that is off drives its idle level. Both counts set means + * it was stopped with a waveform still loaded: it is off. One count + * zero is how 0 and 100% are written, and the idle level says which. + */ + if (!wfhw->enabled && high && low) + return 0; + + if (!rate) + return -EINVAL; + + wf->period_length_ns = x1600_pwm_ns(high + low, rate, wfhw->prescale); + if (!wfhw->enabled) { + wf->duty_length_ns = wfhw->idl ? wf->period_length_ns : 0; + return 0; + } + + wf->duty_length_ns = x1600_pwm_ns(high, rate, wfhw->prescale); + wf->duty_offset_ns = wfhw->inl ? 0 : x1600_pwm_ns(low, rate, wfhw->prescale); + + return 0; +} + +static int x1600_pwm_read_waveform(struct pwm_chip *chip, struct pwm_device *pwm, + void *_wfhw) +{ + struct x1600_pwm *x = pwmchip_get_drvdata(chip); + struct x1600_pwm_wfhw *wfhw = _wfhw; + unsigned int ch = pwm->hwpwm; + + wfhw->enabled = readl(x->base + X1600_PWM_EN) & BIT(ch); + wfhw->inl = readl(x->base + X1600_PWM_INL) & BIT(ch); + wfhw->idl = readl(x->base + X1600_PWM_IDL) & BIT(ch); + wfhw->prescale = FIELD_GET(X1600_PWM_CCFG_PRESCALE, + readl(x->base + X1600_PWM_CCFG(ch))); + wfhw->wcfg = readl(x->base + X1600_PWM_WCFG(ch)); + + return 0; +} + +static void x1600_pwm_set_bit(struct x1600_pwm *x, unsigned int reg, + unsigned int ch, bool set) +{ + u32 val = readl(x->base + reg); + + if (set) + val |= BIT(ch); + else + val &= ~BIT(ch); + writel(val, x->base + reg); +} + +static int x1600_pwm_write_waveform(struct pwm_chip *chip, struct pwm_device *pwm, + const void *_wfhw) +{ + struct x1600_pwm *x = pwmchip_get_drvdata(chip); + const struct x1600_pwm_wfhw *wfhw = _wfhw; + unsigned long rate = clk_get_rate(x->clk); + struct x1600_pwm_wfhw cur; + unsigned int ch = pwm->hwpwm; + u32 busy; + + if (!rate) + return -EINVAL; + + x1600_pwm_read_waveform(chip, pwm, &cur); + + /* + * Running with the same divider and initial level: load the new word, + * which the channel takes at the end of its period. + */ + if (cur.enabled && wfhw->enabled && cur.prescale == wfhw->prescale && + cur.inl == wfhw->inl) { + writel(wfhw->wcfg, x->base + X1600_PWM_WCFG(ch)); + writel(BIT(ch), x->base + X1600_PWM_UPT); + return readl_poll_timeout(x->base + X1600_PWM_BUSY, busy, + !(busy & BIT(ch)), 10, + DIV_ROUND_UP_ULL(2ULL * USEC_PER_SEC * + (X1600_PWM_MAX_COUNT << cur.prescale), + rate)); + } + + /* The idle level first, so that stopping holds the right level. */ + x1600_pwm_set_bit(x, X1600_PWM_IDL, ch, wfhw->idl); + if (cur.enabled) + writel(BIT(ch), x->base + X1600_PWM_ENC); + + /* The prescaler only changes while the channel is off. */ + writel(FIELD_PREP(X1600_PWM_CCFG_PRESCALE, wfhw->prescale), + x->base + X1600_PWM_CCFG(ch)); + x1600_pwm_set_bit(x, X1600_PWM_INL, ch, wfhw->inl); + writel(wfhw->wcfg, x->base + X1600_PWM_WCFG(ch)); + x1600_pwm_set_bit(x, X1600_PWM_MS, ch, false); + + if (wfhw->enabled) + writel(BIT(ch), x->base + X1600_PWM_ENS); + + return 0; +} + +static const struct pwm_ops x1600_pwm_ops = { + .sizeof_wfhw = sizeof(struct x1600_pwm_wfhw), + .round_waveform_tohw = x1600_pwm_round_waveform_tohw, + .round_waveform_fromhw = x1600_pwm_round_waveform_fromhw, + .read_waveform = x1600_pwm_read_waveform, + .write_waveform = x1600_pwm_write_waveform, +}; + +static int x1600_pwm_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct pwm_chip *chip; + struct x1600_pwm *x; + + chip = devm_pwmchip_alloc(dev, X1600_PWM_NUM, sizeof(*x)); + if (IS_ERR(chip)) + return PTR_ERR(chip); + x = pwmchip_get_drvdata(chip); + + x->base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(x->base)) + return PTR_ERR(x->base); + + x->clk = devm_clk_get_enabled(dev, NULL); + if (IS_ERR(x->clk)) + return dev_err_probe(dev, PTR_ERR(x->clk), "Unable to get clock\n"); + + chip->ops = &x1600_pwm_ops; + return devm_pwmchip_add(dev, chip); +} + +static const struct of_device_id x1600_pwm_of_match[] = { + { .compatible = "ingenic,x1600-pwm" }, + { } +}; +MODULE_DEVICE_TABLE(of, x1600_pwm_of_match); + +static struct platform_driver x1600_pwm_driver = { + .driver = { + .name = "ingenic-x1600-pwm", + .of_match_table = x1600_pwm_of_match, + }, + .probe = x1600_pwm_probe, +}; +module_platform_driver(x1600_pwm_driver); + +MODULE_AUTHOR("Olivier Louvignes <olivier@mgcrea.io>"); +MODULE_DESCRIPTION("Ingenic X1600 PWM driver"); +MODULE_LICENSE("GPL"); -- 2.39.5 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] pwm: Add a driver for the Ingenic X1600 PWM controller 2026-10-25 19:16 ` [PATCH v2 2/2] pwm: Add a driver for " Olivier Louvignes @ 2026-10-26 8:04 ` Uwe Kleine-König 0 siblings, 0 replies; 5+ messages in thread From: Uwe Kleine-König @ 2026-10-26 8:04 UTC (permalink / raw) To: Olivier Louvignes Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Cercueil, H . Nikolaus Schaller, linux-pwm, devicetree, linux-mips, linux-kernel [-- Attachment #1: Type: text/plain, Size: 12029 bytes --] Hello, On Sun, Oct 25, 2026 at 08:16:24PM +0100, Olivier Louvignes wrote: > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 729776086879..244c8f4f140e 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -381,6 +381,17 @@ config PWM_JZ4740 > To compile this driver as a module, choose M here: the module > will be called pwm-jz4740. > > +config PWM_INGENIC_X1600 Alphabetic ordering please. > + tristate "Ingenic X1600 PWM support" > + depends on MACH_INGENIC || COMPILE_TEST > + depends on COMMON_CLK && OF && HAS_IOMEM > + help > + Generic PWM framework driver for the PWM controller of the > + Ingenic X1600. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-ingenic-x1600. > + > config PWM_KEEMBAY > tristate "Intel Keem Bay PWM driver" > depends on ARCH_KEEMBAY || COMPILE_TEST > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index 5630a521a7cf..5c59b54e0029 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -31,6 +31,7 @@ obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o > obj-$(CONFIG_PWM_INTEL_LGM) += pwm-intel-lgm.o > obj-$(CONFIG_PWM_IPQ) += pwm-ipq.o > obj-$(CONFIG_PWM_IQS620A) += pwm-iqs620a.o > +obj-$(CONFIG_PWM_INGENIC_X1600) += pwm-ingenic-x1600.o Alphabetic ordering please. > obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o > obj-$(CONFIG_PWM_KEEMBAY) += pwm-keembay.o > obj-$(CONFIG_PWM_LOONGSON) += pwm-loongson.o > diff --git a/drivers/pwm/pwm-ingenic-x1600.c b/drivers/pwm/pwm-ingenic-x1600.c > new file mode 100644 > index 000000000000..5865ff0cf298 > --- /dev/null > +++ b/drivers/pwm/pwm-ingenic-x1600.c > @@ -0,0 +1,280 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * PWM controller of the Ingenic X1600 > + * > + * Copyright (c) 2026 Olivier Louvignes <olivier@mgcrea.io> > + * > + * Eight channels, each with a power-of-two clock divider and a waveform word > + * holding the counts at the high and at the low level; the channel's initial > + * level picks which comes first (X1600 programming manual, chapter 13). Unlike > + * the JZ47xx and X1000, whose PWMs are TCU timer channels (pwm-jz4740), the > + * X1600 has this separate controller. A link to a reference manual would be awesome (if available). > + * Limitations: > + * - The duty offset is 0 or period - duty (the high level last). > + * - Neither count may be zero, so a duty cycle of 0 or 100% switches the > + * channel off and holds its output at the idle level instead. That reads like 100% wasn't possible, but looking at the code it is?! > + * - A disabled channel drives its idle level, low. > + * - Changing the divider or the initial level stops the channel first, which > + * cuts the current period short. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/bits.h> > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/iopoll.h> > +#include <linux/math64.h> > +#include <linux/minmax.h> > +#include <linux/mod_devicetable.h> That is wrong, of_device_id is already provided by <linux/platform_device.h> and <linux/mod_devicetable.h> pulls in many unneeded dependencies. So just drop the latter. > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/pwm.h> > +#include <linux/time.h> > +#include <linux/types.h> > + > +#define X1600_PWM_ENS 0x00 /* write 1: switch a channel on */ > +#define X1600_PWM_ENC 0x04 /* write 1: switch a channel off */ > +#define X1600_PWM_EN 0x08 /* the channels switched on */ > +#define X1600_PWM_UPT 0x10 /* write 1: load the waveform word */ > +#define X1600_PWM_BUSY 0x14 /* a load is pending */ > +#define X1600_PWM_MS 0x20 /* 0: waveform word, 1: DMA */ > +#define X1600_PWM_INL 0x24 /* the level a period starts with */ > +#define X1600_PWM_IDL 0x28 /* the level while off */ > +#define X1600_PWM_CCFG(n) (0x40 + 4 * (n)) > +#define X1600_PWM_WCFG(n) (0x80 + 4 * (n)) > + > +#define X1600_PWM_CCFG_PRESCALE GENMASK(3, 0) /* divide by 2^PRESCALE */ > +#define X1600_PWM_WCFG_HIGH GENMASK(31, 16) > +#define X1600_PWM_WCFG_LOW GENMASK(15, 0) > + > +#define X1600_PWM_NUM 8 > +#define X1600_PWM_MAX_COUNT 0xffff This can be determined from X1600_PWM_WCFG_HIGH and/or X1600_PWM_WCFG_LOW using FIELD_MAX(). > +#define X1600_PWM_MAX_PRESCALE 7 Why are there 4 register bits to store a value in the range [0 .. 7]? > +struct x1600_pwm { > + void __iomem *base; > + struct clk *clk; > +}; > + > +struct x1600_pwm_wfhw { > + u32 wcfg; > + u8 prescale; > + bool enabled; > + bool inl; > + bool idl; > +}; > + > +static u64 x1600_pwm_counts(u64 ns, unsigned long rate, unsigned int prescale) > +{ > + return mul_u64_u64_div_u64(ns, rate, (u64)NSEC_PER_SEC << prescale); > +} > + > +static u64 x1600_pwm_ns(u64 counts, unsigned long rate, unsigned int prescale) > +{ Please add a comment about the ranges of `counts` and `prescale` here to make it clear that there are no problems with overflows. > + return DIV64_U64_ROUND_UP((counts * NSEC_PER_SEC) << prescale, rate); > +} > + > +static int x1600_pwm_round_waveform_tohw(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_waveform *wf, void *_wfhw) > +{ > + struct x1600_pwm *x = pwmchip_get_drvdata(chip); > + unsigned long rate = clk_get_rate(x->clk); > + struct x1600_pwm_wfhw *wfhw = _wfhw; > + u64 period, high, offset; > + unsigned int prescale; > + int ret = 0; > + > + *wfhw = (struct x1600_pwm_wfhw){ }; > + if (!wf->period_length_ns) > + return 0; > + > + if (!rate) > + return -EINVAL; > + > + for (prescale = 0; prescale < X1600_PWM_MAX_PRESCALE; prescale++) > + if (x1600_pwm_counts(wf->period_length_ns, rate, prescale) <= > + X1600_PWM_MAX_COUNT) > + break; I'm sure prescale can be calculated without a loop. > + period = min(x1600_pwm_counts(wf->period_length_ns, rate, prescale), > + X1600_PWM_MAX_COUNT); After determining prescale, you might already have calculated period. > + if (period < 2) { > + period = 2; > + ret = 1; > + } > + high = min(x1600_pwm_counts(wf->duty_length_ns, rate, prescale), period); > + offset = x1600_pwm_counts(wf->duty_offset_ns, rate, prescale); > + > + wfhw->prescale = prescale; > + wfhw->wcfg = FIELD_PREP(X1600_PWM_WCFG_HIGH, high) | > + FIELD_PREP(X1600_PWM_WCFG_LOW, period - high); > + wfhw->enabled = high && high < period; > + /* Low first only if the requested offset reaches period - duty. */ > + wfhw->inl = !wfhw->enabled || offset < period - high; > + wfhw->idl = high == period; inl and idl are the levels before and after the (potential) mid-period edge, right? Doesn't idl need to depend on offset, too? > + return ret; > +} > + > +static int x1600_pwm_round_waveform_fromhw(struct pwm_chip *chip, struct pwm_device *pwm, > + const void *_wfhw, struct pwm_waveform *wf) > +{ > + struct x1600_pwm *x = pwmchip_get_drvdata(chip); > + unsigned long rate = clk_get_rate(x->clk); > + const struct x1600_pwm_wfhw *wfhw = _wfhw; > + u32 high = FIELD_GET(X1600_PWM_WCFG_HIGH, wfhw->wcfg); > + u32 low = FIELD_GET(X1600_PWM_WCFG_LOW, wfhw->wcfg); > + > + *wf = (struct pwm_waveform){ }; > + > + /* > + * A channel that is off drives its idle level. Both counts set means > + * it was stopped with a waveform still loaded: it is off. One count > + * zero is how 0 and 100% are written, and the idle level says which. > + */ > + if (!wfhw->enabled && high && low) > + return 0; The comment doesn't really match the code, does it? The return value only ever says 0% relative duty, but the comment also talks about 100%. > + if (!rate) > + return -EINVAL; > + > + wf->period_length_ns = x1600_pwm_ns(high + low, rate, wfhw->prescale); > + if (!wfhw->enabled) { > + wf->duty_length_ns = wfhw->idl ? wf->period_length_ns : 0; That is wrong if idl = inl? > + return 0; > + } > + > + wf->duty_length_ns = x1600_pwm_ns(high, rate, wfhw->prescale); > + wf->duty_offset_ns = wfhw->inl ? 0 : x1600_pwm_ns(low, rate, wfhw->prescale); > + > + return 0; > +} > + > [...] > +static int x1600_pwm_write_waveform(struct pwm_chip *chip, struct pwm_device *pwm, > + const void *_wfhw) > +{ > + struct x1600_pwm *x = pwmchip_get_drvdata(chip); > + const struct x1600_pwm_wfhw *wfhw = _wfhw; > + unsigned long rate = clk_get_rate(x->clk); > + struct x1600_pwm_wfhw cur; > + unsigned int ch = pwm->hwpwm; > + u32 busy; > + > + if (!rate) > + return -EINVAL; > + > + x1600_pwm_read_waveform(chip, pwm, &cur); > + > + /* > + * Running with the same divider and initial level: load the new word, > + * which the channel takes at the end of its period. > + */ > + if (cur.enabled && wfhw->enabled && cur.prescale == wfhw->prescale && > + cur.inl == wfhw->inl) { You also need cur.idl = wfhw->idl I think. > + writel(wfhw->wcfg, x->base + X1600_PWM_WCFG(ch)); > + writel(BIT(ch), x->base + X1600_PWM_UPT); > + return readl_poll_timeout(x->base + X1600_PWM_BUSY, busy, > + !(busy & BIT(ch)), 10, > + DIV_ROUND_UP_ULL(2ULL * USEC_PER_SEC * > + (X1600_PWM_MAX_COUNT << cur.prescale), > + rate)); What is the rationale here? You can return without waiting here even if the setting isn't active yet. But maybe you have to delay writing if the busy bit is still set on the next invocation of .write_waveform(). > + } > + > + /* The idle level first, so that stopping holds the right level. */ > + x1600_pwm_set_bit(x, X1600_PWM_IDL, ch, wfhw->idl); > + if (cur.enabled) > + writel(BIT(ch), x->base + X1600_PWM_ENC); > + > + /* The prescaler only changes while the channel is off. */ > + writel(FIELD_PREP(X1600_PWM_CCFG_PRESCALE, wfhw->prescale), > + x->base + X1600_PWM_CCFG(ch)); > + x1600_pwm_set_bit(x, X1600_PWM_INL, ch, wfhw->inl); > + writel(wfhw->wcfg, x->base + X1600_PWM_WCFG(ch)); > + x1600_pwm_set_bit(x, X1600_PWM_MS, ch, false); > + > + if (wfhw->enabled) > + writel(BIT(ch), x->base + X1600_PWM_ENS); > + > + return 0; > +} > + > +static const struct pwm_ops x1600_pwm_ops = { > + .sizeof_wfhw = sizeof(struct x1600_pwm_wfhw), > + .round_waveform_tohw = x1600_pwm_round_waveform_tohw, > + .round_waveform_fromhw = x1600_pwm_round_waveform_fromhw, > + .read_waveform = x1600_pwm_read_waveform, > + .write_waveform = x1600_pwm_write_waveform, > +}; > + > +static int x1600_pwm_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct pwm_chip *chip; > + struct x1600_pwm *x; > + > + chip = devm_pwmchip_alloc(dev, X1600_PWM_NUM, sizeof(*x)); > + if (IS_ERR(chip)) > + return PTR_ERR(chip); > + x = pwmchip_get_drvdata(chip); > + > + x->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(x->base)) > + return PTR_ERR(x->base); > + > + x->clk = devm_clk_get_enabled(dev, NULL); > + if (IS_ERR(x->clk)) > + return dev_err_probe(dev, PTR_ERR(x->clk), "Unable to get clock\n"); Please make use of clk_rate_exclusive_get(). If you can write a new configuration even if the busy bit is active you can also call clk_get_rate() here, cache the result and then set chip->atomic. > + > + chip->ops = &x1600_pwm_ops; > + return devm_pwmchip_add(dev, chip); > +} > + > +static const struct of_device_id x1600_pwm_of_match[] = { > + { .compatible = "ingenic,x1600-pwm" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, x1600_pwm_of_match); > + > +static struct platform_driver x1600_pwm_driver = { > + .driver = { > + .name = "ingenic-x1600-pwm", > + .of_match_table = x1600_pwm_of_match, > + }, > + .probe = x1600_pwm_probe, > +}; > +module_platform_driver(x1600_pwm_driver); > + > +MODULE_AUTHOR("Olivier Louvignes <olivier@mgcrea.io>"); > +MODULE_DESCRIPTION("Ingenic X1600 PWM driver"); > +MODULE_LICENSE("GPL"); > -- > 2.39.5 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-27 18:44 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-25 19:16 [PATCH v2 0/2] pwm: Add the Ingenic X1600 PWM controller Olivier Louvignes 2026-10-25 19:16 ` [PATCH v2 1/2] dt-bindings: " Olivier Louvignes 2026-10-27 18:44 ` Krzysztof Kozlowski 2026-10-25 19:16 ` [PATCH v2 2/2] pwm: Add a driver for " Olivier Louvignes 2026-10-26 8:04 ` Uwe Kleine-König
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®