On Tue, 2024-01-23 at 17:05 -0500, Trevor Gamblin wrote: > From: Drew Fustini <dfustini@xxxxxxxxxxxx> > > Add support for the Analog Devices AXI PWM Generator. This device is an > FPGA-implemented peripheral used as PWM signal generator and can be > interfaced with AXI4. The register map of this peripheral makes it > possible to configure the period and duty cycle of the output signal. > > Link: https://wiki.analog.com/resources/fpga/docs/axi_pwm_gen > Co-developed-by: Sergiu Cuciurean <sergiu.cuciurean@xxxxxxxxxx> > Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@xxxxxxxxxx> > Co-developed-by: David Lechner <dlechner@xxxxxxxxxxxx> > Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx> > Signed-off-by: Drew Fustini <dfustini@xxxxxxxxxxxx> > Co-developed-by: Trevor Gamblin <tgamblin@xxxxxxxxxxxx> > Signed-off-by: Trevor Gamblin <tgamblin@xxxxxxxxxxxx> > > --- One minor nitpick below (but no need to re-spin just for that). Anyways, from a code point of view, LGTM: Acked-by: Nuno Sa <nuno.sa@xxxxxxxxxx> > v2 changes: > * Address feedback for driver and device tree in v1: > * Use more reasonable Kconfig approach > * Use common prefixes for all functions > * Rename axi_pwmgen struct to axi_pwmgen_ddata > * Change use of "pwm" to "ddata" > * Set and check state->polarity > * Multiply safely with mul_u64_u64_div_u64() > * Improve handling of max and zero periods > * Error if clk_rate_hz > NSEC_PER_SEC > * Add "Limitations" section at top of pwm-axi-pwmgen.c > * Don't disable outputs by default > * Remove unnecessary macros for period, duty, offset > * Fix axi_pwmgen_ddata alignment > * Don't artificially limit npwm to four > * Use clk_rate_exclusive_get(), balance with clk_rate_exclusive_put() > * Cache clk rate in axi_pwmgen_ddata > * Don't assign pwm->chip.base, do assign pwm->chip.atomic > * Remove redundant calls to clk_get_rate > * Test contents of AXI_PWMGEN_REG_CORE_MAGIC instead of > arbitrary AXI_PWMGEN_TEST_DATA in AXI_PWMGEN_REG_SCRATCHPAD > * Remove redundant clk struct from axi_pwmgen_ddata > * Add self as module author > * Add major version check for IP core > > --- > MAINTAINERS | 1 + > drivers/pwm/Kconfig | 13 ++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-axi-pwmgen.c | 246 +++++++++++++++++++++++++++++++++++ > 4 files changed, 261 insertions(+) > create mode 100644 drivers/pwm/pwm-axi-pwmgen.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8a4ed5545680..2baa7a0a1c8c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3438,6 +3438,7 @@ L: linux-pwm@xxxxxxxxxxxxxxx > S: Supported > W: https://ez.analog.com/linux-software-drivers > F: Documentation/devicetree/bindings/pwm/adi,axi-pwmgen.yaml > +F: drivers/pwm/pwm-axi-pwmgen.c > > AXXIA I2C CONTROLLER > M: Krzysztof Adamski <krzysztof.adamski@xxxxxxxxx> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 4b956d661755..d44b0e86adee 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -98,6 +98,19 @@ config PWM_ATMEL_TCB > To compile this driver as a module, choose M here: the module > will be called pwm-atmel-tcb. > > +config PWM_AXI_PWMGEN > + tristate "Analog Devices AXI PWM generator" > + depends on MICROBLAZE || NIOS2 || ARCH_ZYNQ || ARCH_ZYNQMP || > ARCH_INTEL_SOCFPGA || COMPILE_TEST > + select REGMAP_MMIO > + help > + This enables support for the Analog Devices AXI PWM generator. > + > + This is a configurable PWM generator with variable pulse width and > + period. > + > + To compile this driver as a module, choose M here: the module will > be > + called pwm-axi-pwmgen. > + > config PWM_BCM_IPROC > tristate "iProc PWM support" > depends on ARCH_BCM_IPROC || COMPILE_TEST > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index c5ec9e168ee7..8322089954e9 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -6,6 +6,7 @@ obj-$(CONFIG_PWM_APPLE) += pwm-apple.o > obj-$(CONFIG_PWM_ATMEL) += pwm-atmel.o > obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM) += pwm-atmel-hlcdc.o > obj-$(CONFIG_PWM_ATMEL_TCB) += pwm-atmel-tcb.o > +obj-$(CONFIG_PWM_AXI_PWMGEN) += pwm-axi-pwmgen.o > obj-$(CONFIG_PWM_BCM_IPROC) += pwm-bcm-iproc.o > obj-$(CONFIG_PWM_BCM_KONA) += pwm-bcm-kona.o > obj-$(CONFIG_PWM_BCM2835) += pwm-bcm2835.o > diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c > new file mode 100644 > index 000000000000..39d2c7be0cb4 > --- /dev/null > +++ b/drivers/pwm/pwm-axi-pwmgen.c > @@ -0,0 +1,246 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Analog Devices AXI PWM generator > + * > + * Copyright 2024 Analog Devices Inc. > + * Copyright 2024 Baylibre SAS > + * > + * Limitations: > + * - The writes to registers for period and duty are shadowed until > + * LOAD_CONFIG is written to AXI_PWMGEN_REG_CONFIG at the end of the > + * current period. > + * - Writing LOAD_CONFIG also has the effect of re-synchronizing all > + * enabled channels, which could cause glitching on other channels. It > + * is therefore expected that channels are assigned harmonic periods > + * and all have a single user coordinating this. > + * - Supports normal polarity. Does not support changing polarity. > + */ > +#include <linux/bits.h> > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/pwm.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +#define AXI_PWMGEN_VERSION_MAJOR(x) (((x) >> 16) & 0xff) > +#define AXI_PWMGEN_VERSION_MINOR(x) (((x) >> 8) & 0xff) > +#define AXI_PWMGEN_VERSION_PATCH(x) ((x) & 0xff) > + > +#define AXI_PWMGEN_REG_CORE_VERSION 0x00 > +#define AXI_PWMGEN_REG_ID 0x04 > +#define AXI_PWMGEN_REG_SCRATCHPAD 0x08 > +#define AXI_PWMGEN_REG_CORE_MAGIC 0x0C > +#define AXI_PWMGEN_REG_CONFIG 0x10 > +#define AXI_PWMGEN_REG_NPWM 0x14 > +#define AXI_PWMGEN_CHX_PERIOD(ch) (0x40 + (12 * (ch))) > +#define AXI_PWMGEN_CHX_DUTY(ch) (0x44 + (12 * (ch))) > +#define AXI_PWMGEN_CHX_OFFSET(ch) (0x48 + (12 * (ch))) > +#define AXI_PWMGEN_REG_CORE_MAGIC_VAL 0x601A3471 /* Identification number > to test during setup */ > +#define AXI_PWMGEN_LOAD_CONFIG BIT(1) > +#define AXI_PWMGEN_RESET BIT(0) > + > +struct axi_pwmgen_ddata { > + struct pwm_chip chip; > + struct regmap *regmap; > + unsigned long clk_rate_hz; > +}; > + > +static const struct regmap_config axi_pwmgen_regmap_config = { > + .reg_bits = 32, > + .reg_stride = 4, > + .val_bits = 32, > +}; > + > +static struct axi_pwmgen_ddata *axi_pwmgen_from_chip(struct pwm_chip *chip) > +{ > + return container_of(chip, struct axi_pwmgen_ddata, chip); > +} > + > +static int axi_pwmgen_apply(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + struct axi_pwmgen_ddata *ddata = axi_pwmgen_from_chip(chip); > + unsigned int ch = pwm->hwpwm; > + struct regmap *regmap = ddata->regmap; > + u64 period_cnt, duty_cnt; > + int ret; > + > + if (state->polarity != PWM_POLARITY_NORMAL) > + return -EINVAL; > + > + if (state->enabled) { > + no need for extra line in here... at the very least, you should be coherent in both your code paths... - Nuno Sá
