From eb18504ca5cf1e6a76a752b73daf0ef51de3551b Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Fri, 20 Sep 2024 10:58:01 +0200 Subject: pwm: axi-pwmgen: Implementation of the waveform callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the axi-pwmgen driver to use the new callbacks for hardware programming. Signed-off-by: Uwe Kleine-König Tested-by: Trevor Gamblin Link: https://lore.kernel.org/r/922277f07b1d1fb9c9cd915b1ec3fdeec888a916.1726819463.git.u.kleine-koenig@baylibre.com Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-axi-pwmgen.c | 148 ++++++++++++++++++++++++++++++------------- 1 file changed, 105 insertions(+), 43 deletions(-) (limited to 'drivers/pwm/pwm-axi-pwmgen.c') diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c index b5477659ba18..39d184417c7c 100644 --- a/drivers/pwm/pwm-axi-pwmgen.c +++ b/drivers/pwm/pwm-axi-pwmgen.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -53,81 +54,142 @@ static const struct regmap_config axi_pwmgen_regmap_config = { .max_register = 0xFC, }; -static int axi_pwmgen_apply(struct pwm_chip *chip, struct pwm_device *pwm, - const struct pwm_state *state) +/* This represents a hardware configuration for one channel */ +struct axi_pwmgen_waveform { + u32 period_cnt; + u32 duty_cycle_cnt; + u32 duty_offset_cnt; +}; + +static int axi_pwmgen_round_waveform_tohw(struct pwm_chip *chip, + struct pwm_device *pwm, + const struct pwm_waveform *wf, + void *_wfhw) { + struct axi_pwmgen_waveform *wfhw = _wfhw; struct axi_pwmgen_ddata *ddata = pwmchip_get_drvdata(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 (wf->period_length_ns == 0) { + *wfhw = (struct axi_pwmgen_waveform){ + .period_cnt = 0, + .duty_cycle_cnt = 0, + .duty_offset_cnt = 0, + }; + } else { + /* With ddata->clk_rate_hz < NSEC_PER_SEC this won't overflow. */ + wfhw->period_cnt = min_t(u64, + mul_u64_u32_div(wf->period_length_ns, ddata->clk_rate_hz, NSEC_PER_SEC), + U32_MAX); + + if (wfhw->period_cnt == 0) { + /* + * The specified period is too short for the hardware. + * Let's round .duty_cycle down to 0 to get a (somewhat) + * valid result. + */ + wfhw->period_cnt = 1; + wfhw->duty_cycle_cnt = 0; + wfhw->duty_offset_cnt = 0; + } else { + wfhw->duty_cycle_cnt = min_t(u64, + mul_u64_u32_div(wf->duty_length_ns, ddata->clk_rate_hz, NSEC_PER_SEC), + U32_MAX); + wfhw->duty_offset_cnt = min_t(u64, + mul_u64_u32_div(wf->duty_offset_ns, ddata->clk_rate_hz, NSEC_PER_SEC), + U32_MAX); + } + } - if (state->enabled) { - period_cnt = mul_u64_u64_div_u64(state->period, ddata->clk_rate_hz, NSEC_PER_SEC); - if (period_cnt > UINT_MAX) - period_cnt = UINT_MAX; + dev_dbg(&chip->dev, "pwm#%u: %lld/%lld [+%lld] @%lu -> PERIOD: %08x, DUTY: %08x, OFFSET: %08x\n", + pwm->hwpwm, wf->duty_length_ns, wf->period_length_ns, wf->duty_offset_ns, + ddata->clk_rate_hz, wfhw->period_cnt, wfhw->duty_cycle_cnt, wfhw->duty_offset_cnt); - if (period_cnt == 0) - return -EINVAL; + return 0; +} - ret = regmap_write(regmap, AXI_PWMGEN_CHX_PERIOD(ch), period_cnt); - if (ret) - return ret; +static int axi_pwmgen_round_waveform_fromhw(struct pwm_chip *chip, struct pwm_device *pwm, + const void *_wfhw, struct pwm_waveform *wf) +{ + const struct axi_pwmgen_waveform *wfhw = _wfhw; + struct axi_pwmgen_ddata *ddata = pwmchip_get_drvdata(chip); - duty_cnt = mul_u64_u64_div_u64(state->duty_cycle, ddata->clk_rate_hz, NSEC_PER_SEC); - if (duty_cnt > UINT_MAX) - duty_cnt = UINT_MAX; + wf->period_length_ns = DIV64_U64_ROUND_UP((u64)wfhw->period_cnt * NSEC_PER_SEC, + ddata->clk_rate_hz); - ret = regmap_write(regmap, AXI_PWMGEN_CHX_DUTY(ch), duty_cnt); - if (ret) - return ret; - } else { - ret = regmap_write(regmap, AXI_PWMGEN_CHX_PERIOD(ch), 0); - if (ret) - return ret; + wf->duty_length_ns = DIV64_U64_ROUND_UP((u64)wfhw->duty_cycle_cnt * NSEC_PER_SEC, + ddata->clk_rate_hz); - ret = regmap_write(regmap, AXI_PWMGEN_CHX_DUTY(ch), 0); - if (ret) - return ret; - } + wf->duty_offset_ns = DIV64_U64_ROUND_UP((u64)wfhw->duty_offset_cnt * NSEC_PER_SEC, + ddata->clk_rate_hz); - return regmap_write(regmap, AXI_PWMGEN_REG_CONFIG, AXI_PWMGEN_LOAD_CONFIG); + return 0; } -static int axi_pwmgen_get_state(struct pwm_chip *chip, struct pwm_device *pwm, - struct pwm_state *state) +static int axi_pwmgen_write_waveform(struct pwm_chip *chip, + struct pwm_device *pwm, + const void *_wfhw) { + const struct axi_pwmgen_waveform *wfhw = _wfhw; struct axi_pwmgen_ddata *ddata = pwmchip_get_drvdata(chip); struct regmap *regmap = ddata->regmap; unsigned int ch = pwm->hwpwm; - u32 cnt; int ret; - ret = regmap_read(regmap, AXI_PWMGEN_CHX_PERIOD(ch), &cnt); + ret = regmap_write(regmap, AXI_PWMGEN_CHX_PERIOD(ch), wfhw->period_cnt); if (ret) return ret; - state->enabled = cnt != 0; + ret = regmap_write(regmap, AXI_PWMGEN_CHX_DUTY(ch), wfhw->duty_cycle_cnt); + if (ret) + return ret; - state->period = DIV_ROUND_UP_ULL((u64)cnt * NSEC_PER_SEC, ddata->clk_rate_hz); + ret = regmap_write(regmap, AXI_PWMGEN_CHX_OFFSET(ch), wfhw->duty_offset_cnt); + if (ret) + return ret; - ret = regmap_read(regmap, AXI_PWMGEN_CHX_DUTY(ch), &cnt); + return regmap_write(regmap, AXI_PWMGEN_REG_CONFIG, AXI_PWMGEN_LOAD_CONFIG); +} + +static int axi_pwmgen_read_waveform(struct pwm_chip *chip, + struct pwm_device *pwm, + void *_wfhw) +{ + struct axi_pwmgen_waveform *wfhw = _wfhw; + struct axi_pwmgen_ddata *ddata = pwmchip_get_drvdata(chip); + struct regmap *regmap = ddata->regmap; + unsigned int ch = pwm->hwpwm; + int ret; + + ret = regmap_read(regmap, AXI_PWMGEN_CHX_PERIOD(ch), &wfhw->period_cnt); + if (ret) + return ret; + + ret = regmap_read(regmap, AXI_PWMGEN_CHX_DUTY(ch), &wfhw->duty_cycle_cnt); if (ret) return ret; - state->duty_cycle = DIV_ROUND_UP_ULL((u64)cnt * NSEC_PER_SEC, ddata->clk_rate_hz); + ret = regmap_read(regmap, AXI_PWMGEN_CHX_OFFSET(ch), &wfhw->duty_offset_cnt); + if (ret) + return ret; - state->polarity = PWM_POLARITY_NORMAL; + if (wfhw->duty_cycle_cnt > wfhw->period_cnt) + wfhw->duty_cycle_cnt = wfhw->period_cnt; + + /* XXX: is this the actual behaviour of the hardware? */ + if (wfhw->duty_offset_cnt >= wfhw->period_cnt) { + wfhw->duty_cycle_cnt = 0; + wfhw->duty_offset_cnt = 0; + } return 0; } static const struct pwm_ops axi_pwmgen_pwm_ops = { - .apply = axi_pwmgen_apply, - .get_state = axi_pwmgen_get_state, + .sizeof_wfhw = sizeof(struct axi_pwmgen_waveform), + .round_waveform_tohw = axi_pwmgen_round_waveform_tohw, + .round_waveform_fromhw = axi_pwmgen_round_waveform_fromhw, + .read_waveform = axi_pwmgen_read_waveform, + .write_waveform = axi_pwmgen_write_waveform, }; static int axi_pwmgen_setup(struct regmap *regmap, struct device *dev) -- cgit From 22f032c7900ca457f4facdd711eee61c9648c7c1 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Mon, 23 Sep 2024 14:54:17 +0200 Subject: pwm: axi-pwmgen: Create a dedicated function for getting driver data from a chip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compared to direct calls to pwmchip_get_drvdata() a dedicated function has two upsides: A better name and the right type. So the code becomes easier to read and the new function is harder to use wrongly. Another side effect (which is the secret motivation for this patch, but shhh) is that the driver becomes a bit easier to backport to kernel versions that don't have devm_pwmchip_alloc() yet. Signed-off-by: Uwe Kleine-König Reviewed-by: Trevor Gamblin Link: https://lore.kernel.org/r/20240923125418.16558-2-u.kleine-koenig@baylibre.com [ukleinek: added an * to the new function's prototype to make the compiler happy] Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-axi-pwmgen.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/pwm/pwm-axi-pwmgen.c') diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c index b5477659ba18..5cee5645fe80 100644 --- a/drivers/pwm/pwm-axi-pwmgen.c +++ b/drivers/pwm/pwm-axi-pwmgen.c @@ -53,10 +53,15 @@ static const struct regmap_config axi_pwmgen_regmap_config = { .max_register = 0xFC, }; +static struct axi_pwmgen_ddata *axi_pwmgen_ddata_from_chip(struct pwm_chip *chip) +{ + return pwmchip_get_drvdata(chip); +} + static int axi_pwmgen_apply(struct pwm_chip *chip, struct pwm_device *pwm, const struct pwm_state *state) { - struct axi_pwmgen_ddata *ddata = pwmchip_get_drvdata(chip); + struct axi_pwmgen_ddata *ddata = axi_pwmgen_ddata_from_chip(chip); unsigned int ch = pwm->hwpwm; struct regmap *regmap = ddata->regmap; u64 period_cnt, duty_cnt; @@ -100,7 +105,7 @@ static int axi_pwmgen_apply(struct pwm_chip *chip, struct pwm_device *pwm, static int axi_pwmgen_get_state(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state) { - struct axi_pwmgen_ddata *ddata = pwmchip_get_drvdata(chip); + struct axi_pwmgen_ddata *ddata = axi_pwmgen_ddata_from_chip(chip); struct regmap *regmap = ddata->regmap; unsigned int ch = pwm->hwpwm; u32 cnt; -- cgit From 2e82d58c7ba8f7b4bb273ca5859b682b65654f9e Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 9 Oct 2024 16:11:49 -0500 Subject: pwm: axi-pwmgen: Rename 0x10 register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the 0x10 register from REG_CONFIG to REG_RSTN. Also rename the associated bit macros accordingly. While touching this, move the bit macros close to the register address macro for better organization. According to [1], the name of the 0x10 register is REG_RSTN, and there is a different register named REG_CONFIG (0x18). So we should not be using REG_CONFIG for the 0x10 register to avoid confusion. [1]: http://analogdevicesinc.github.io/hdl/library/axi_pwm_gen/index.html Signed-off-by: David Lechner Reviewed-by: Nuno Sa Link: https://lore.kernel.org/r/20241009-pwm-axi-pwmgen-enable-force_align-v1-1-5d6ad8cbf5b4@baylibre.com Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-axi-pwmgen.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers/pwm/pwm-axi-pwmgen.c') diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c index 6e56ceb23d18..e1ddeaa4998b 100644 --- a/drivers/pwm/pwm-axi-pwmgen.c +++ b/drivers/pwm/pwm-axi-pwmgen.c @@ -9,7 +9,7 @@ * * Limitations: * - The writes to registers for period and duty are shadowed until - * LOAD_CONFIG is written to AXI_PWMGEN_REG_CONFIG, at which point + * LOAD_CONFIG is written to AXI_PWMGEN_REG_RSTN, at which point * they take effect. * - Writing LOAD_CONFIG also has the effect of re-synchronizing all * enabled channels, which could cause glitching on other channels. It @@ -33,14 +33,14 @@ #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_RSTN 0x10 +#define AXI_PWMGEN_REG_RSTN_LOAD_CONFIG BIT(1) +#define AXI_PWMGEN_REG_RSTN_RESET BIT(0) #define AXI_PWMGEN_REG_NPWM 0x14 #define AXI_PWMGEN_CHX_PERIOD(ch) (0x40 + (4 * (ch))) #define AXI_PWMGEN_CHX_DUTY(ch) (0x80 + (4 * (ch))) #define AXI_PWMGEN_CHX_OFFSET(ch) (0xC0 + (4 * (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_REG_CONFIG_RESET BIT(0) struct axi_pwmgen_ddata { struct regmap *regmap; @@ -152,7 +152,7 @@ static int axi_pwmgen_write_waveform(struct pwm_chip *chip, if (ret) return ret; - return regmap_write(regmap, AXI_PWMGEN_REG_CONFIG, AXI_PWMGEN_LOAD_CONFIG); + return regmap_write(regmap, AXI_PWMGEN_REG_RSTN, AXI_PWMGEN_REG_RSTN_LOAD_CONFIG); } static int axi_pwmgen_read_waveform(struct pwm_chip *chip, @@ -223,7 +223,7 @@ static int axi_pwmgen_setup(struct regmap *regmap, struct device *dev) } /* Enable the core */ - ret = regmap_clear_bits(regmap, AXI_PWMGEN_REG_CONFIG, AXI_PWMGEN_REG_CONFIG_RESET); + ret = regmap_clear_bits(regmap, AXI_PWMGEN_REG_RSTN, AXI_PWMGEN_REG_RSTN_RESET); if (ret) return ret; -- cgit From 15effedc481e69a23b374ba516d73a2bc665abe6 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 9 Oct 2024 16:11:50 -0500 Subject: pwm: axi-pwmgen: Enable FORCE_ALIGN by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enable the FORCE_ALIGN flag by default in the AXI PWMGEN driver. This flag makes the behavior of the PWM output consistent with the description at the top of the driver file. * Limitations: * - The writes to registers for period and duty are shadowed until * LOAD_CONFIG is written to AXI_PWMGEN_REG_RSTN, at which point * they take effect. * - 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. Without this flag, the PWM output does not change until the period of all PWM output channels has run out, which makes the PWM impossible to use in some cases because it takes too long to change the output. Signed-off-by: David Lechner Reviewed-by: Nuno Sa Link: https://lore.kernel.org/r/20241009-pwm-axi-pwmgen-enable-force_align-v1-2-5d6ad8cbf5b4@baylibre.com Signed-off-by: Uwe Kleine-König --- drivers/pwm/pwm-axi-pwmgen.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'drivers/pwm/pwm-axi-pwmgen.c') diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c index e1ddeaa4998b..4259a0db9ff4 100644 --- a/drivers/pwm/pwm-axi-pwmgen.c +++ b/drivers/pwm/pwm-axi-pwmgen.c @@ -37,6 +37,8 @@ #define AXI_PWMGEN_REG_RSTN_LOAD_CONFIG BIT(1) #define AXI_PWMGEN_REG_RSTN_RESET BIT(0) #define AXI_PWMGEN_REG_NPWM 0x14 +#define AXI_PWMGEN_REG_CONFIG 0x18 +#define AXI_PWMGEN_REG_CONFIG_FORCE_ALIGN BIT(1) #define AXI_PWMGEN_CHX_PERIOD(ch) (0x40 + (4 * (ch))) #define AXI_PWMGEN_CHX_DUTY(ch) (0x80 + (4 * (ch))) #define AXI_PWMGEN_CHX_OFFSET(ch) (0xC0 + (4 * (ch))) @@ -227,6 +229,16 @@ static int axi_pwmgen_setup(struct regmap *regmap, struct device *dev) if (ret) return ret; + /* + * Enable force align so that changes to PWM period and duty cycle take + * effect immediately. Otherwise, the effect of the change is delayed + * until the period of all channels run out, which can be long after the + * apply function returns. + */ + ret = regmap_set_bits(regmap, AXI_PWMGEN_REG_CONFIG, AXI_PWMGEN_REG_CONFIG_FORCE_ALIGN); + if (ret) + return ret; + ret = regmap_read(regmap, AXI_PWMGEN_REG_NPWM, &val); if (ret) return ret; -- cgit