| Message ID | 20240212210652.368680-3-fabrizio.castro.jz@renesas.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add RZ/V2{M, MA} PWM driver support | expand |
On Mon, Feb 12, 2024 at 09:06:50PM +0000, Fabrizio Castro wrote: > From: Biju Das <biju.das.jz@bp.renesas.com> > > The RZ/V2{M, MA} PWM Timer supports the following functions: > > * The PWM has 24-bit counters which operate at PWM_CLK (48 MHz). > * The frequency division ratio for internal counter operation is > selectable as PWM_CLK divided by 1, 16, 256, or 2048. > * The period as well as the duty cycle is adjustable. > * The low-level and high-level order of the PWM signals can be > inverted. > * The duty cycle of the PWM signal is selectable in the range from > 0 to 100%. > * The minimum resolution is 20.83 ns. > * Three interrupt sources: Rising and falling edges of the PWM signal > and clearing of the counter > * Counter operation and the bus interface are asynchronous and both > can operate independently of the magnitude relationship of the > respective clock periods. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com> > --- > > v6->v7: > * Addressed the build issue reported by the kernel test robot. > * Added include math64.h. > * Reworked rzv2m_pwm_mul_u64_u64_div_u64_roundup to make use of > div64_u64 and to get rid of % while keeping the same formula. > * Added rzv2m_pwm_mul_u64_u64_div_u64_rounddown. > * Replaced / with div64_u64 wherever necessary. > v5->v6: > * Added Fab's Signed-off-by. > * Updated copyright year to 2024. > * Added include of limits.h. > * Added variable max_period to rzv2m_pwm_chip. > * Simplified the calculations by calculating max_period during probe, > based on the numerical limits of the formula and the u64 data type. > * Added rzv2m_pwm_mul_u64_u64_div_u64_roundup. > * Added rzv2m_pwm_prescale_to_shift to fix the calculation of the > frequency divider. > * Improved the calculations and the variable names of > rzv2m_pwm_get_state. > * Improved the calculations of rzv2m_pwm_config. > * Removed .owner from rzv2m_pwm_ops. > * Improved rzv2m_pwm_pm_runtime_resume and renamed its err variable to > ret. > * Removed of_match_ptr. > * Added Fab as module author. > v4->v5: > * Sorted KConfig file > * Sorted Make file > * Updated copyright header 2022->2023. > * Updated limitation section. > * Replaced the variable chip->rzv2m_pwm in rzv2m_pwm_wait_delay() > * Replaced polarity logic as per HW manual dutycycle = Ton/Ton+Toff, so > eventhough native polarity is inverted from period point of view it > is correct. > * Added logic for supporting 0% , 100% and remaining duty cycle. > * On config() replaced > pm_runtime_resume_and_get()->pm_runtime_get_sync() > * Counter is stopped while updating period/polarity to avoid glitches. > * Added error check for clk_prepare_enable() > * Introduced is_ch_enabled variable to cache channel enable status. > * clk_get_rate is called after enabling the clock and > clk_rate_exclusive_get() > * Added comment for delay > * Replaced 1000000000UL->NSEC_PER_SEC. > * Improved error handling in probe(). > v3->v4: > * Documented the hardware properties in "Limitations" section > * Dropped the macros F2CYCLE_NSEC, U24_MASK and U24_MAX. > * Added RZV2M_PWMCYC_PERIOD macro for U24_MAX > * Dropped rzv2m_pwm_freq_div variable and started using 1 << (4 * i) > for calculating divider as it is power of 16. > * Reordered the functions to have rzv2m_pwm_config() directly before > rzv2m_pwm_apply(). > * Improved the logic for calculating period and duty cycle in config() > * Merged multiple RZV2M_PWMCTR register writes to a single write in > * config() > * replaced pwm_is_enabled()->pwm->state.enabled > * Avoided assigning bit value as enum pwm_polarity instead used enum > * constant. > * Fixed various issues in probe error path. > * Updated the logic for PWM cycle setting register > * A 100% duty cycle is only possible with PWMLOW > PWMCYC. So > restricting PWMCYC values < 0xffffff > * The native polarity of the hardware is inverted (i.e. it starts with > the low part). So switched the inversion bit handling. > v2->v3: > * Added return code for rzv2m_pwm_get_state() > * Added comment in rzv2m_pwm_reset_assert_pm_disable() > v1->v2: > * Replaced > devm_reset_control_get_optional_shared->devm_reset_control_get_shared > > drivers/pwm/Kconfig | 11 + > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-rzv2m.c | 480 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 492 insertions(+) > create mode 100644 drivers/pwm/pwm-rzv2m.c > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 4b956d661755..55d46e6183a2 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -524,6 +524,17 @@ config PWM_RZ_MTU3 > To compile this driver as a module, choose M here: the module > will be called pwm-rz-mtu3. > > +config PWM_RZV2M > + tristate "Renesas RZ/V2M PWM support" > + depends on ARCH_R9A09G011 || COMPILE_TEST > + depends on HAS_IOMEM > + help > + This driver exposes the PWM controller found in Renesas > + RZ/V2M like chips through the PWM API. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-rzv2m. > + > config PWM_SAMSUNG > tristate "Samsung PWM support" > depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index c5ec9e168ee7..cf5a4a1c3b1a 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -48,6 +48,7 @@ obj-$(CONFIG_PWM_RCAR) += pwm-rcar.o > obj-$(CONFIG_PWM_RENESAS_TPU) += pwm-renesas-tpu.o > obj-$(CONFIG_PWM_ROCKCHIP) += pwm-rockchip.o > obj-$(CONFIG_PWM_RZ_MTU3) += pwm-rz-mtu3.o > +obj-$(CONFIG_PWM_RZV2M) += pwm-rzv2m.o > obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o > obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o > obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o > diff --git a/drivers/pwm/pwm-rzv2m.c b/drivers/pwm/pwm-rzv2m.c > new file mode 100644 > index 000000000000..eb9062293590 > --- /dev/null > +++ b/drivers/pwm/pwm-rzv2m.c > @@ -0,0 +1,480 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Renesas RZ/V2M PWM Timer (PWM) driver > + * > + * Copyright (C) 2024 Renesas Electronics Corporation > + * > + * Hardware manual for this IP can be found here > + * https://www.renesas.com/in/en/document/mah/rzv2m-users-manual-hardware?language=en > + * > + * Limitations: > + * - Changes to the duty cycle configuration get effective only after the next > + * period end. > + * - The duty cycle can be changed only by modifying the PWMLOW register > + * value and changing the pulse width at low level. The duty cycle becomes > + * 0% for the low width when the value of the PWMLOW register is 0x0h > + * and 100% for the low width when the value of the PWMLOW > PWMCYC. If polarity or period is changed, the hardware has to be stopped, to this yields glitches. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/io.h> > +#include <linux/limits.h> > +#include <linux/math64.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/pwm.h> > +#include <linux/reset.h> > +#include <linux/time.h> > + > +#define RZV2M_PWMCTR 0x0 > +#define RZV2M_PWMCYC 0x4 > +#define RZV2M_PWMLOW 0x8 > +#define RZV2M_PWMCNT 0xc > + > +#define RZV2M_PWMCTR_PWMPS GENMASK(17, 16) > +#define RZV2M_PWMCTR_PWMHL BIT(3) > +#define RZV2M_PWMCTR_PWMTM BIT(2) > +#define RZV2M_PWMCTR_PWME BIT(1) > + > +#define RZV2M_PWMCYC_PERIOD GENMASK(23, 0) > +#define RZV2M_PWMLOW_PERIOD GENMASK(23, 0) > + > +struct rzv2m_pwm_chip { > + u64 max_period; > + struct pwm_chip chip; > + void __iomem *mmio; > + struct reset_control *rstc; > + struct clk *apb_clk; > + struct clk *pwm_clk; > + unsigned long rate; > + unsigned long delay; > + unsigned long pwm_cyc; > + enum pwm_polarity polarity; > + bool is_ch_enabled; > +}; > + > +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_roundup(u64 a, u64 b, u64 c) > +{ > + u64 ab = a * b; a * b might overflow?! > + u64 d = div64_u64(ab, c); > + u64 e = d * c; > + > + return d + ((ab - e) ? 1 : 0); > +} > + > +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_rounddown(u64 a, u64 b, u64 c) > +{ > + return div64_u64(a * b, c); ditto. This is the same function as mul_u64_u64_div_u64() isn't it? > +} > + > +static inline struct rzv2m_pwm_chip *to_rzv2m_pwm_chip(struct pwm_chip *chip) > +{ > + return container_of(chip, struct rzv2m_pwm_chip, chip); > +} > + > +static void rzv2m_pwm_wait_delay(struct rzv2m_pwm_chip *rzv2m_pwm) > +{ > + /* delay timer when change the setting register */ > + ndelay(rzv2m_pwm->delay); > +} > + > +static void rzv2m_pwm_write(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg, u32 data) > +{ > + writel(data, rzv2m_pwm->mmio + reg); > +} > + > +static u32 rzv2m_pwm_read(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg) > +{ > + return readl(rzv2m_pwm->mmio + reg); > +} > + > +static void rzv2m_pwm_modify(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg, u32 clr, > + u32 set) > +{ > + rzv2m_pwm_write(rzv2m_pwm, reg, > + (rzv2m_pwm_read(rzv2m_pwm, reg) & ~clr) | set); > +} > + > +static u8 rzv2m_pwm_calculate_prescale(struct rzv2m_pwm_chip *rzv2m_pwm, > + u64 period_cycles) > +{ > + u32 prescaled_period_cycles; > + u8 prescale; > + > + prescaled_period_cycles = period_cycles >> 24; > + if (prescaled_period_cycles >= 256) > + prescale = 3; > + else > + prescale = (fls(prescaled_period_cycles) + 3) / 4; > + > + return prescale; > +} > + > +static inline int rzv2m_pwm_prescale_to_shift(u8 prescale) > +{ > + return prescale == 3 ? 11 : prescale * 4; > +} > + > +static int rzv2m_pwm_enable(struct rzv2m_pwm_chip *rzv2m_pwm) > +{ > + int rc; > + > + rc = pm_runtime_resume_and_get(rzv2m_pwm->chip.dev); > + if (rc) > + return rc; > + > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, > + RZV2M_PWMCTR_PWME); > + rzv2m_pwm_wait_delay(rzv2m_pwm); > + rzv2m_pwm->is_ch_enabled = true; > + > + return 0; > +} > + > +static void rzv2m_pwm_disable(struct rzv2m_pwm_chip *rzv2m_pwm) > +{ > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, 0); > + rzv2m_pwm_wait_delay(rzv2m_pwm); > + pm_runtime_put_sync(rzv2m_pwm->chip.dev); > + rzv2m_pwm->is_ch_enabled = false; > +} > + > +static int rzv2m_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > + struct pwm_state *state) > +{ > + struct rzv2m_pwm_chip *rzv2m_pwm = to_rzv2m_pwm_chip(chip); > + u16 frequency_divisor; > + u32 ctr, cyc, low; > + u8 prescale; > + > + pm_runtime_get_sync(chip->dev); > + ctr = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCTR); > + state->enabled = FIELD_GET(RZV2M_PWMCTR_PWME, ctr); > + state->polarity = FIELD_GET(RZV2M_PWMCTR_PWMHL, ctr) ? > + PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; > + prescale = FIELD_GET(RZV2M_PWMCTR_PWMPS, ctr); > + frequency_divisor = 1 << rzv2m_pwm_prescale_to_shift(prescale); Giving a shift is cheaper than a multiplication, I suggest to do frequency_divisor = rzv2m_pwm_prescale_to_shift(prescale); and instead of multiply by frequency_divisor, use a left-shift operation. > + cyc = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCYC); > + state->period = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1, > + NSEC_PER_SEC * frequency_divisor, > + rzv2m_pwm->rate); > + > + low = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMLOW); > + state->duty_cycle = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1 - low, > + NSEC_PER_SEC * frequency_divisor, > + rzv2m_pwm->rate); The register semantic makes me wonder if each period starts with the low part. In that case the hardware called "normal" what is called inverted in the pwm framework?! > + return pm_runtime_put(chip->dev); If you evaluate the return value of pm_runtime_put() maybe check pm_runtime_get_sync() for symmetry, too? > +} > + > +static int rzv2m_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + struct rzv2m_pwm_chip *rzv2m_pwm = to_rzv2m_pwm_chip(chip); > + u64 period = state->period, duty_cycle = state->duty_cycle; > + u16 frequency_divisor; > + u64 pwm_cyc, pwm_low; > + u8 prescale; > + u32 pwm_ctr; > + > + /* > + * Clamp period and duty cycle to their maximum values for our current > + * configuration rather than letting our calculations overflow. > + */ > + if (period > rzv2m_pwm->max_period) { > + period = rzv2m_pwm->max_period; > + if (duty_cycle > rzv2m_pwm->max_period) > + duty_cycle = period; > + } > + > + /* > + * Formula for calculating PWM Cycle Setting Register: > + * PWM cycle = (PWM period(ns) / (PWM_CLK period(ns) × Div ratio)) - 1 > + */ > + pwm_cyc = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(period, > + rzv2m_pwm->rate, > + NSEC_PER_SEC); > + pwm_cyc = pwm_cyc ? pwm_cyc : 1; if pwm_cyc is 0 here, the period is too short to be realized, right? That's an error condition. > + prescale = rzv2m_pwm_calculate_prescale(rzv2m_pwm, pwm_cyc - 1); > + frequency_divisor = 1 << rzv2m_pwm_prescale_to_shift(prescale); > + if (frequency_divisor > 1) { > + pwm_cyc = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(period, > + rzv2m_pwm->rate, > + NSEC_PER_SEC * frequency_divisor); > + pwm_cyc = pwm_cyc ? pwm_cyc : 1; > + } > + > + if (pwm_cyc && !FIELD_FIT(RZV2M_PWMCYC_PERIOD, pwm_cyc - 1)) > + pwm_cyc = RZV2M_PWMCYC_PERIOD + 1; I don't understand the relevance of FIELD_FIT(RZV2M_PWMCYC_PERIOD, pwm_cyc - 1). > + > + /* > + * Formula for calculating PWMLOW register: > + * PWMLOW register = PWM cycle * Low pulse width ratio (%) > + */ > + pwm_low = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(duty_cycle, > + rzv2m_pwm->rate, NSEC_PER_SEC * frequency_divisor); > + > + pwm_low = pwm_cyc - pwm_low; Either the old or the new value of pwm_low doesn't match the variable's name. Please add another variable for the wrong one. The compiler should optimize that out and the reader can more easily understand the code. > + if (!FIELD_FIT(RZV2M_PWMLOW_PERIOD, pwm_low)) > + pwm_low = RZV2M_PWMLOW_PERIOD; > + > + pwm_cyc--; > + > + /* > + * If the PWM channel is disabled, make sure to turn on the clock > + * before writing the register. > + */ > + if (!pwm->state.enabled) > + pm_runtime_get_sync(rzv2m_pwm->chip.dev); > + > + /* > + * To change the setting value of the PWM cycle setting register > + * (PWMm_PWMCYC) or polarity, set the PWME bit of the PWM control > + * register (PWMm_PWMCTR) to 0b and stop the counter operation. > + */ > + if (rzv2m_pwm->polarity != state->polarity || rzv2m_pwm->pwm_cyc != pwm_cyc) { > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, 0); > + rzv2m_pwm_wait_delay(rzv2m_pwm); > + } > + > + rzv2m_pwm_write(rzv2m_pwm, RZV2M_PWMCYC, pwm_cyc); > + rzv2m_pwm_write(rzv2m_pwm, RZV2M_PWMLOW, pwm_low); > + > + pwm_ctr = FIELD_PREP(RZV2M_PWMCTR_PWMPS, prescale); > + if (state->polarity == PWM_POLARITY_INVERSED) > + pwm_ctr |= RZV2M_PWMCTR_PWMHL; > + > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWMTM | > + RZV2M_PWMCTR_PWMPS | RZV2M_PWMCTR_PWMHL, pwm_ctr); > + > + if (rzv2m_pwm->polarity != state->polarity || rzv2m_pwm->pwm_cyc != pwm_cyc) { > + rzv2m_pwm->polarity = state->polarity; > + rzv2m_pwm->pwm_cyc = pwm_cyc; > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, > + RZV2M_PWMCTR_PWME); > + } > + > + rzv2m_pwm_wait_delay(rzv2m_pwm); > + > + /* If the PWM is not enabled, turn the clock off again to save power. */ > + if (!pwm->state.enabled) > + pm_runtime_put(rzv2m_pwm->chip.dev); > + > + return 0; > +} > [...] > +static int rzv2m_pwm_probe(struct platform_device *pdev) > +{ > + struct rzv2m_pwm_chip *rzv2m_pwm; > + unsigned long apb_clk_rate; > + int ret; > + > + rzv2m_pwm = devm_kzalloc(&pdev->dev, sizeof(*rzv2m_pwm), GFP_KERNEL); > + if (!rzv2m_pwm) > + return -ENOMEM; > + > + rzv2m_pwm->mmio = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(rzv2m_pwm->mmio)) > + return PTR_ERR(rzv2m_pwm->mmio); > + > + rzv2m_pwm->apb_clk = devm_clk_get(&pdev->dev, "apb"); > + if (IS_ERR(rzv2m_pwm->apb_clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->apb_clk), > + "cannot get apb clock\n"); > + > + rzv2m_pwm->pwm_clk = devm_clk_get(&pdev->dev, "pwm"); > + if (IS_ERR(rzv2m_pwm->pwm_clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->pwm_clk), > + "cannot get pwm clock\n"); > + > + rzv2m_pwm->rstc = devm_reset_control_get_shared(&pdev->dev, NULL); > + if (IS_ERR(rzv2m_pwm->rstc)) > + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->rstc), > + "get reset failed\n"); > + > + platform_set_drvdata(pdev, rzv2m_pwm); > + ret = reset_control_deassert(rzv2m_pwm->rstc); > + if (ret) { > + return dev_err_probe(&pdev->dev, ret, > + "cannot deassert reset control\n"); > + } > + > + ret = clk_prepare_enable(rzv2m_pwm->apb_clk); > + if (ret < 0) > + goto err_reset; > + > + ret = clk_prepare_enable(rzv2m_pwm->pwm_clk); > + if (ret < 0) > + goto disable_apb_clk; > + > + clk_rate_exclusive_get(rzv2m_pwm->apb_clk); There is a devm_clk_rate_exclusive_get() in next starting from tomorrow I hope. Using it should simplify the driver. Best regards Uwe
Hi Uwe, Thanks for your feedback! > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Sent: Thursday, February 29, 2024 4:42 PM > To: Fabrizio Castro <fabrizio.castro.jz@renesas.com> > Subject: Re: [PATCH v7 2/4] pwm: Add support for RZ/V2M PWM driver > > On Mon, Feb 12, 2024 at 09:06:50PM +0000, Fabrizio Castro wrote: > > From: Biju Das <biju.das.jz@bp.renesas.com> > > > > The RZ/V2{M, MA} PWM Timer supports the following functions: > > > > * The PWM has 24-bit counters which operate at PWM_CLK (48 MHz). > > * The frequency division ratio for internal counter operation is > > selectable as PWM_CLK divided by 1, 16, 256, or 2048. > > * The period as well as the duty cycle is adjustable. > > * The low-level and high-level order of the PWM signals can be > > inverted. > > * The duty cycle of the PWM signal is selectable in the range from > > 0 to 100%. > > * The minimum resolution is 20.83 ns. > > * Three interrupt sources: Rising and falling edges of the PWM signal > > and clearing of the counter > > * Counter operation and the bus interface are asynchronous and both > > can operate independently of the magnitude relationship of the > > respective clock periods. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com> > > --- > > > > v6->v7: > > * Addressed the build issue reported by the kernel test robot. > > * Added include math64.h. > > * Reworked rzv2m_pwm_mul_u64_u64_div_u64_roundup to make use of > > div64_u64 and to get rid of % while keeping the same formula. > > * Added rzv2m_pwm_mul_u64_u64_div_u64_rounddown. > > * Replaced / with div64_u64 wherever necessary. > > v5->v6: > > * Added Fab's Signed-off-by. > > * Updated copyright year to 2024. > > * Added include of limits.h. > > * Added variable max_period to rzv2m_pwm_chip. > > * Simplified the calculations by calculating max_period during probe, > > based on the numerical limits of the formula and the u64 data type. > > * Added rzv2m_pwm_mul_u64_u64_div_u64_roundup. > > * Added rzv2m_pwm_prescale_to_shift to fix the calculation of the > > frequency divider. > > * Improved the calculations and the variable names of > > rzv2m_pwm_get_state. > > * Improved the calculations of rzv2m_pwm_config. > > * Removed .owner from rzv2m_pwm_ops. > > * Improved rzv2m_pwm_pm_runtime_resume and renamed its err variable to > > ret. > > * Removed of_match_ptr. > > * Added Fab as module author. > > v4->v5: > > * Sorted KConfig file > > * Sorted Make file > > * Updated copyright header 2022->2023. > > * Updated limitation section. > > * Replaced the variable chip->rzv2m_pwm in rzv2m_pwm_wait_delay() > > * Replaced polarity logic as per HW manual dutycycle = Ton/Ton+Toff, so > > eventhough native polarity is inverted from period point of view it > > is correct. > > * Added logic for supporting 0% , 100% and remaining duty cycle. > > * On config() replaced > > pm_runtime_resume_and_get()->pm_runtime_get_sync() > > * Counter is stopped while updating period/polarity to avoid glitches. > > * Added error check for clk_prepare_enable() > > * Introduced is_ch_enabled variable to cache channel enable status. > > * clk_get_rate is called after enabling the clock and > > clk_rate_exclusive_get() > > * Added comment for delay > > * Replaced 1000000000UL->NSEC_PER_SEC. > > * Improved error handling in probe(). > > v3->v4: > > * Documented the hardware properties in "Limitations" section > > * Dropped the macros F2CYCLE_NSEC, U24_MASK and U24_MAX. > > * Added RZV2M_PWMCYC_PERIOD macro for U24_MAX > > * Dropped rzv2m_pwm_freq_div variable and started using 1 << (4 * i) > > for calculating divider as it is power of 16. > > * Reordered the functions to have rzv2m_pwm_config() directly before > > rzv2m_pwm_apply(). > > * Improved the logic for calculating period and duty cycle in config() > > * Merged multiple RZV2M_PWMCTR register writes to a single write in > > * config() > > * replaced pwm_is_enabled()->pwm->state.enabled > > * Avoided assigning bit value as enum pwm_polarity instead used enum > > * constant. > > * Fixed various issues in probe error path. > > * Updated the logic for PWM cycle setting register > > * A 100% duty cycle is only possible with PWMLOW > PWMCYC. So > > restricting PWMCYC values < 0xffffff > > * The native polarity of the hardware is inverted (i.e. it starts with > > the low part). So switched the inversion bit handling. > > v2->v3: > > * Added return code for rzv2m_pwm_get_state() > > * Added comment in rzv2m_pwm_reset_assert_pm_disable() > > v1->v2: > > * Replaced > > devm_reset_control_get_optional_shared->devm_reset_control_get_shared > > > > drivers/pwm/Kconfig | 11 + > > drivers/pwm/Makefile | 1 + > > drivers/pwm/pwm-rzv2m.c | 480 ++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 492 insertions(+) > > create mode 100644 drivers/pwm/pwm-rzv2m.c > > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > > index 4b956d661755..55d46e6183a2 100644 > > --- a/drivers/pwm/Kconfig > > +++ b/drivers/pwm/Kconfig > > @@ -524,6 +524,17 @@ config PWM_RZ_MTU3 > > To compile this driver as a module, choose M here: the module > > will be called pwm-rz-mtu3. > > > > +config PWM_RZV2M > > + tristate "Renesas RZ/V2M PWM support" > > + depends on ARCH_R9A09G011 || COMPILE_TEST > > + depends on HAS_IOMEM > > + help > > + This driver exposes the PWM controller found in Renesas > > + RZ/V2M like chips through the PWM API. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called pwm-rzv2m. > > + > > config PWM_SAMSUNG > > tristate "Samsung PWM support" > > depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || > COMPILE_TEST > > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > > index c5ec9e168ee7..cf5a4a1c3b1a 100644 > > --- a/drivers/pwm/Makefile > > +++ b/drivers/pwm/Makefile > > @@ -48,6 +48,7 @@ obj-$(CONFIG_PWM_RCAR) += pwm-rcar.o > > obj-$(CONFIG_PWM_RENESAS_TPU) += pwm-renesas-tpu.o > > obj-$(CONFIG_PWM_ROCKCHIP) += pwm-rockchip.o > > obj-$(CONFIG_PWM_RZ_MTU3) += pwm-rz-mtu3.o > > +obj-$(CONFIG_PWM_RZV2M) += pwm-rzv2m.o > > obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o > > obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o > > obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o > > diff --git a/drivers/pwm/pwm-rzv2m.c b/drivers/pwm/pwm-rzv2m.c > > new file mode 100644 > > index 000000000000..eb9062293590 > > --- /dev/null > > +++ b/drivers/pwm/pwm-rzv2m.c > > @@ -0,0 +1,480 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Renesas RZ/V2M PWM Timer (PWM) driver > > + * > > + * Copyright (C) 2024 Renesas Electronics Corporation > > + * > > + * Hardware manual for this IP can be found here > > + * https://www.renesas.com/in/en/document/mah/rzv2m-users-manual- > hardware?language=en > > + * > > + * Limitations: > > + * - Changes to the duty cycle configuration get effective only after > the next > > + * period end. > > + * - The duty cycle can be changed only by modifying the PWMLOW register > > + * value and changing the pulse width at low level. The duty cycle > becomes > > + * 0% for the low width when the value of the PWMLOW register is 0x0h > > + * and 100% for the low width when the value of the PWMLOW > PWMCYC. > > If polarity or period is changed, the hardware has to be stopped, to > this yields glitches. Thanks for the suggestion, I'll add it. > > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/clk.h> > > +#include <linux/delay.h> > > +#include <linux/io.h> > > +#include <linux/limits.h> > > +#include <linux/math64.h> > > +#include <linux/platform_device.h> > > +#include <linux/pm_runtime.h> > > +#include <linux/pwm.h> > > +#include <linux/reset.h> > > +#include <linux/time.h> > > + > > +#define RZV2M_PWMCTR 0x0 > > +#define RZV2M_PWMCYC 0x4 > > +#define RZV2M_PWMLOW 0x8 > > +#define RZV2M_PWMCNT 0xc > > + > > +#define RZV2M_PWMCTR_PWMPS GENMASK(17, 16) > > +#define RZV2M_PWMCTR_PWMHL BIT(3) > > +#define RZV2M_PWMCTR_PWMTM BIT(2) > > +#define RZV2M_PWMCTR_PWME BIT(1) > > + > > +#define RZV2M_PWMCYC_PERIOD GENMASK(23, 0) > > +#define RZV2M_PWMLOW_PERIOD GENMASK(23, 0) > > + > > +struct rzv2m_pwm_chip { > > + u64 max_period; > > + struct pwm_chip chip; > > + void __iomem *mmio; > > + struct reset_control *rstc; > > + struct clk *apb_clk; > > + struct clk *pwm_clk; > > + unsigned long rate; > > + unsigned long delay; > > + unsigned long pwm_cyc; > > + enum pwm_polarity polarity; > > + bool is_ch_enabled; > > +}; > > + > > +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_roundup(u64 a, u64 b, > u64 c) > > +{ > > + u64 ab = a * b; > > a * b might overflow?! In the context of this driver, this cannot overflow. The 2 formulas the above is needed for are: 1) period = (cyc + 1)*(NSEC_PER_SEC * frequency_divisor)/rate 2) duty_cycle = (cyc + 1 - low)*(NSEC_PER_SEC * frequency_divisor)/rate With respect to 1), the dividend overflows when period * rate also overflows (its product is calculated in rzv2m_pwm_config). However, limiting the period to a maximum value of U64_MAX / rate prevents the calculations from overflowing (in both directions, from period to cyc, and from cyc to period). v6 introduced max_period for this. The situation for 2) is very similar to 1), with duty_cycle<=period, therefore limiting period to a max value (and clamping the duty cycle accordingly) will ensure that the calculation for duty_cycle won't overflow, either. > > > + u64 d = div64_u64(ab, c); > > + u64 e = d * c; > > + > > + return d + ((ab - e) ? 1 : 0); > > +} > > + > > +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_rounddown(u64 a, u64 b, > u64 c) > > +{ > > + return div64_u64(a * b, c); > > ditto. This is the same function as mul_u64_u64_div_u64() isn't it? Since a * b cannot overflow in the case of this driver, I believe the above to be a better option than mul_u64_u64_div_u64. > > > +} > > + > > +static inline struct rzv2m_pwm_chip *to_rzv2m_pwm_chip(struct pwm_chip > *chip) > > +{ > > + return container_of(chip, struct rzv2m_pwm_chip, chip); > > +} > > + > > +static void rzv2m_pwm_wait_delay(struct rzv2m_pwm_chip *rzv2m_pwm) > > +{ > > + /* delay timer when change the setting register */ > > + ndelay(rzv2m_pwm->delay); > > +} > > + > > +static void rzv2m_pwm_write(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg, > u32 data) > > +{ > > + writel(data, rzv2m_pwm->mmio + reg); > > +} > > + > > +static u32 rzv2m_pwm_read(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg) > > +{ > > + return readl(rzv2m_pwm->mmio + reg); > > +} > > + > > +static void rzv2m_pwm_modify(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg, > u32 clr, > > + u32 set) > > +{ > > + rzv2m_pwm_write(rzv2m_pwm, reg, > > + (rzv2m_pwm_read(rzv2m_pwm, reg) & ~clr) | set); > > +} > > + > > +static u8 rzv2m_pwm_calculate_prescale(struct rzv2m_pwm_chip *rzv2m_pwm, > > + u64 period_cycles) > > +{ > > + u32 prescaled_period_cycles; > > + u8 prescale; > > + > > + prescaled_period_cycles = period_cycles >> 24; > > + if (prescaled_period_cycles >= 256) > > + prescale = 3; > > + else > > + prescale = (fls(prescaled_period_cycles) + 3) / 4; > > + > > + return prescale; > > +} > > + > > +static inline int rzv2m_pwm_prescale_to_shift(u8 prescale) > > +{ > > + return prescale == 3 ? 11 : prescale * 4; > > +} > > + > > +static int rzv2m_pwm_enable(struct rzv2m_pwm_chip *rzv2m_pwm) > > +{ > > + int rc; > > + > > + rc = pm_runtime_resume_and_get(rzv2m_pwm->chip.dev); > > + if (rc) > > + return rc; > > + > > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, > > + RZV2M_PWMCTR_PWME); > > + rzv2m_pwm_wait_delay(rzv2m_pwm); > > + rzv2m_pwm->is_ch_enabled = true; > > + > > + return 0; > > +} > > + > > +static void rzv2m_pwm_disable(struct rzv2m_pwm_chip *rzv2m_pwm) > > +{ > > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, 0); > > + rzv2m_pwm_wait_delay(rzv2m_pwm); > > + pm_runtime_put_sync(rzv2m_pwm->chip.dev); > > + rzv2m_pwm->is_ch_enabled = false; > > +} > > + > > +static int rzv2m_pwm_get_state(struct pwm_chip *chip, struct pwm_device > *pwm, > > + struct pwm_state *state) > > +{ > > + struct rzv2m_pwm_chip *rzv2m_pwm = to_rzv2m_pwm_chip(chip); > > + u16 frequency_divisor; > > + u32 ctr, cyc, low; > > + u8 prescale; > > + > > + pm_runtime_get_sync(chip->dev); > > + ctr = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCTR); > > + state->enabled = FIELD_GET(RZV2M_PWMCTR_PWME, ctr); > > + state->polarity = FIELD_GET(RZV2M_PWMCTR_PWMHL, ctr) ? > > + PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; > > + prescale = FIELD_GET(RZV2M_PWMCTR_PWMPS, ctr); > > + frequency_divisor = 1 << rzv2m_pwm_prescale_to_shift(prescale); > > Giving a shift is cheaper than a multiplication, I suggest to do > > frequency_divisor = rzv2m_pwm_prescale_to_shift(prescale); > > and instead of multiply by frequency_divisor, use a left-shift > operation. Will do, thanks for the suggestion. > > > + cyc = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCYC); > > + state->period = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1, > > + NSEC_PER_SEC * frequency_divisor, > > + rzv2m_pwm->rate); > > + > > + low = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMLOW); > > + state->duty_cycle = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1 - > low, > > + NSEC_PER_SEC * frequency_divisor, > > + rzv2m_pwm->rate); > > The register semantic makes me wonder if each period starts with the low > part. In that case the hardware called "normal" what is called inverted > in the pwm framework?! My understanding is that the PWM framework defines "normal" polarity a signal that starts high (and stays high) for the duration of the duty cycle, and goes low for the remainder of the period. Conversely, a signal with "inversed" polarity starts low (and stays low) for the duration of the duty cycle and goes high for the remainder of the period. This IP _does_ start low, but it _doesn't_ stay low for the duration of the duty cycle, as it then goes high for the duration of the duty cycle, therefore this IP doesn't perfectly fit either ("normal" or "inverted") definitions. I think you can say that the "normal" signal is _shifted_ in phase for this IP, rather than being "inverted". > > > + return pm_runtime_put(chip->dev); > > If you evaluate the return value of pm_runtime_put() maybe check > pm_runtime_get_sync() for symmetry, too? Or I could just discard it and return 0? I am fine with either, what's your preference? > > > +} > > + > > +static int rzv2m_pwm_config(struct pwm_chip *chip, struct pwm_device > *pwm, > > + const struct pwm_state *state) > > +{ > > + struct rzv2m_pwm_chip *rzv2m_pwm = to_rzv2m_pwm_chip(chip); > > + u64 period = state->period, duty_cycle = state->duty_cycle; > > + u16 frequency_divisor; > > + u64 pwm_cyc, pwm_low; > > + u8 prescale; > > + u32 pwm_ctr; > > + > > + /* > > + * Clamp period and duty cycle to their maximum values for our > current > > + * configuration rather than letting our calculations overflow. > > + */ > > + if (period > rzv2m_pwm->max_period) { > > + period = rzv2m_pwm->max_period; > > + if (duty_cycle > rzv2m_pwm->max_period) > > + duty_cycle = period; > > + } > > + > > + /* > > + * Formula for calculating PWM Cycle Setting Register: > > + * PWM cycle = (PWM period(ns) / (PWM_CLK period(ns) × Div ratio)) - > 1 > > + */ > > + pwm_cyc = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(period, > > + rzv2m_pwm->rate, > > + NSEC_PER_SEC); > > + pwm_cyc = pwm_cyc ? pwm_cyc : 1; > > if pwm_cyc is 0 here, the period is too short to be realized, right? > That's an error condition. I will return an error instead. > > > + prescale = rzv2m_pwm_calculate_prescale(rzv2m_pwm, pwm_cyc - 1); > > + frequency_divisor = 1 << rzv2m_pwm_prescale_to_shift(prescale); > > + if (frequency_divisor > 1) { > > + pwm_cyc = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(period, > > + rzv2m_pwm->rate, > > + NSEC_PER_SEC * frequency_divisor); > > + pwm_cyc = pwm_cyc ? pwm_cyc : 1; > > + } > > + > > + if (pwm_cyc && !FIELD_FIT(RZV2M_PWMCYC_PERIOD, pwm_cyc - 1)) > > + pwm_cyc = RZV2M_PWMCYC_PERIOD + 1; > > I don't understand the relevance of FIELD_FIT(RZV2M_PWMCYC_PERIOD, > pwm_cyc - 1). CYC is only made of 24 bits, therefore this is to make sure we don't go beyond a 24-bit representation. > > > + > > + /* > > + * Formula for calculating PWMLOW register: > > + * PWMLOW register = PWM cycle * Low pulse width ratio (%) > > + */ > > + pwm_low = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(duty_cycle, > > + rzv2m_pwm->rate, NSEC_PER_SEC * frequency_divisor); > > + > > + pwm_low = pwm_cyc - pwm_low; > > Either the old or the new value of pwm_low doesn't match the variable's > name. Please add another variable for the wrong one. The compiler should > optimize that out and the reader can more easily understand the code. You are right, I'll use an intermediate variable rather than leveraging pwm_low, as it's confusing. > > > + if (!FIELD_FIT(RZV2M_PWMLOW_PERIOD, pwm_low)) > > + pwm_low = RZV2M_PWMLOW_PERIOD; > > + > > + pwm_cyc--; > > + > > + /* > > + * If the PWM channel is disabled, make sure to turn on the clock > > + * before writing the register. > > + */ > > + if (!pwm->state.enabled) > > + pm_runtime_get_sync(rzv2m_pwm->chip.dev); > > + > > + /* > > + * To change the setting value of the PWM cycle setting register > > + * (PWMm_PWMCYC) or polarity, set the PWME bit of the PWM control > > + * register (PWMm_PWMCTR) to 0b and stop the counter operation. > > + */ > > + if (rzv2m_pwm->polarity != state->polarity || rzv2m_pwm->pwm_cyc != > pwm_cyc) { > > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, > 0); > > + rzv2m_pwm_wait_delay(rzv2m_pwm); > > + } > > + > > + rzv2m_pwm_write(rzv2m_pwm, RZV2M_PWMCYC, pwm_cyc); > > + rzv2m_pwm_write(rzv2m_pwm, RZV2M_PWMLOW, pwm_low); > > + > > + pwm_ctr = FIELD_PREP(RZV2M_PWMCTR_PWMPS, prescale); > > + if (state->polarity == PWM_POLARITY_INVERSED) > > + pwm_ctr |= RZV2M_PWMCTR_PWMHL; > > + > > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWMTM | > > + RZV2M_PWMCTR_PWMPS | RZV2M_PWMCTR_PWMHL, pwm_ctr); > > + > > + if (rzv2m_pwm->polarity != state->polarity || rzv2m_pwm->pwm_cyc != > pwm_cyc) { > > + rzv2m_pwm->polarity = state->polarity; > > + rzv2m_pwm->pwm_cyc = pwm_cyc; > > + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, > > + RZV2M_PWMCTR_PWME); > > + } > > + > > + rzv2m_pwm_wait_delay(rzv2m_pwm); > > + > > + /* If the PWM is not enabled, turn the clock off again to save power. > */ > > + if (!pwm->state.enabled) > > + pm_runtime_put(rzv2m_pwm->chip.dev); > > + > > + return 0; > > +} > > [...] > > +static int rzv2m_pwm_probe(struct platform_device *pdev) > > +{ > > + struct rzv2m_pwm_chip *rzv2m_pwm; > > + unsigned long apb_clk_rate; > > + int ret; > > + > > + rzv2m_pwm = devm_kzalloc(&pdev->dev, sizeof(*rzv2m_pwm), GFP_KERNEL); > > + if (!rzv2m_pwm) > > + return -ENOMEM; > > + > > + rzv2m_pwm->mmio = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(rzv2m_pwm->mmio)) > > + return PTR_ERR(rzv2m_pwm->mmio); > > + > > + rzv2m_pwm->apb_clk = devm_clk_get(&pdev->dev, "apb"); > > + if (IS_ERR(rzv2m_pwm->apb_clk)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->apb_clk), > > + "cannot get apb clock\n"); > > + > > + rzv2m_pwm->pwm_clk = devm_clk_get(&pdev->dev, "pwm"); > > + if (IS_ERR(rzv2m_pwm->pwm_clk)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->pwm_clk), > > + "cannot get pwm clock\n"); > > + > > + rzv2m_pwm->rstc = devm_reset_control_get_shared(&pdev->dev, NULL); > > + if (IS_ERR(rzv2m_pwm->rstc)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->rstc), > > + "get reset failed\n"); > > + > > + platform_set_drvdata(pdev, rzv2m_pwm); > > + ret = reset_control_deassert(rzv2m_pwm->rstc); > > + if (ret) { > > + return dev_err_probe(&pdev->dev, ret, > > + "cannot deassert reset control\n"); > > + } > > + > > + ret = clk_prepare_enable(rzv2m_pwm->apb_clk); > > + if (ret < 0) > > + goto err_reset; > > + > > + ret = clk_prepare_enable(rzv2m_pwm->pwm_clk); > > + if (ret < 0) > > + goto disable_apb_clk; > > + > > + clk_rate_exclusive_get(rzv2m_pwm->apb_clk); > > There is a devm_clk_rate_exclusive_get() in next starting from tomorrow > I hope. Using it should simplify the driver. I'll have a look and adjust accordingly. Thanks! Cheers, Fab > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello Fabrizio, your MUA introduces strange line breaks. You could do every reader of you mails a favour and fix that. I fixed it up for my reply. On Thu, Feb 29, 2024 at 10:45:01PM +0000, Fabrizio Castro wrote: > > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Sent: Thursday, February 29, 2024 4:42 PM > > To: Fabrizio Castro <fabrizio.castro.jz@renesas.com> > > Subject: Re: [PATCH v7 2/4] pwm: Add support for RZ/V2M PWM driver > > > > On Mon, Feb 12, 2024 at 09:06:50PM +0000, Fabrizio Castro wrote: > > > +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_roundup(u64 a, u64 b, > > u64 c) > > > +{ > > > + u64 ab = a * b; > > > > a * b might overflow?! > > In the context of this driver, this cannot overflow. > The 2 formulas the above is needed for are: > 1) period = (cyc + 1)*(NSEC_PER_SEC * frequency_divisor)/rate > 2) duty_cycle = (cyc + 1 - low)*(NSEC_PER_SEC * frequency_divisor)/rate > > With respect to 1), the dividend overflows when period * rate also > overflows (its product is calculated in rzv2m_pwm_config). > However, limiting the period to a maximum value of U64_MAX / rate > prevents the calculations from overflowing (in both directions, from period to cyc, and from cyc to period). v6 introduced max_period for this. > The situation for 2) is very similar to 1), with duty_cycle<=period, > therefore limiting period to a max value (and clamping the duty cycle > accordingly) will ensure that the calculation for duty_cycle won't > overflow, either. OK, so it might be right from a technical POV. From a maintainer POV this is still bad. Authors for other drivers might copy it, or the driver might be changed and there is no indication that the the function relies on only be called with certain parameters. > > > + u64 d = div64_u64(ab, c); > > > + u64 e = d * c; > > > + > > > + return d + ((ab - e) ? 1 : 0); > > > +} > > > + > > > +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_rounddown(u64 a, u64 b, > > u64 c) > > > +{ > > > + return div64_u64(a * b, c); > > > > ditto. This is the same function as mul_u64_u64_div_u64() isn't it? > > Since a * b cannot overflow in the case of this driver, I believe the > above to be a better option than mul_u64_u64_div_u64. Same technical POV vs maintainer POV as above. Plus: Even if mul_u64_u64_div_u64 is a tad slower, reusing it has some benefits nevertheless. > > > [...] > > > + cyc = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCYC); > > > + state->period = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1, > > > + NSEC_PER_SEC * frequency_divisor, > > > + rzv2m_pwm->rate); > > > + > > > + low = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMLOW); > > > + state->duty_cycle = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1 - low, > > > + NSEC_PER_SEC * frequency_divisor, > > > + rzv2m_pwm->rate); > > > > The register semantic makes me wonder if each period starts with the low > > part. In that case the hardware called "normal" what is called inverted > > in the pwm framework?! > > My understanding is that the PWM framework defines "normal" polarity a > signal that starts high (and stays high) for the duration of the duty cycle, > and goes low for the remainder of the period. Conversely, a signal with > "inversed" polarity starts low (and stays low) for the duration of the duty > cycle and goes high for the remainder of the period. Ack. > This IP _does_ start low, but it _doesn't_ stay low for the duration of the > duty cycle, as it then goes high for the duration of the duty cycle, > therefore this IP doesn't perfectly fit either ("normal" or "inverted") > definitions. > I think you can say that the "normal" signal is _shifted_ in phase for this > IP, rather than being "inverted". Alternatively (and a better match): What you describe is an inverted wave form with duty_cycle = period - duty_cycle. > > > + return pm_runtime_put(chip->dev); > > > > If you evaluate the return value of pm_runtime_put() maybe check > > pm_runtime_get_sync() for symmetry, too? > > Or I could just discard it and return 0? > I am fine with either, what's your preference? My preference would be to always check the return value, but given that many drivers don't care for that, I agree to accept never checking it. So choose one option and do it consistently please. > > > + if (pwm_cyc && !FIELD_FIT(RZV2M_PWMCYC_PERIOD, pwm_cyc - 1)) > > > + pwm_cyc = RZV2M_PWMCYC_PERIOD + 1; > > > > I don't understand the relevance of FIELD_FIT(RZV2M_PWMCYC_PERIOD, > > pwm_cyc - 1). > > CYC is only made of 24 bits, therefore this is to make sure we don't > go beyond a 24-bit representation. I would have understood: if (FIELD_FIT(RZV2M_PWMCYC_PERIOD, pwm_cyc + 1)) pwm_cyc = RZV2M_PWMCYC_PERIOD + 1; Notice there are three changes compared to your variant: - drop pwm_cyc != 0 check - drop ! from FIELD_FIT - pwm_cyc + 1 instead of pwm_cyc - 1 Best regards Uwe
Hello Uwe, Thanks for your feedback! > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > Sent: Friday, March 1, 2024 9:15 AM > To: Fabrizio Castro <fabrizio.castro.jz@renesas.com> > Subject: Re: [PATCH v7 2/4] pwm: Add support for RZ/V2M PWM driver > > Hello Fabrizio, > > your MUA introduces strange line breaks. You could do every reader of you mails a favour and fix that. I think I understand what you mean, long lines would be wrapped onto new lines, which are then indented incorrectly? Hopefully it's been fixed now. Thanks for highlighting this, as it has been going on for some time, and was never noticed before. > I fixed it up for my reply. Thanks for that. > > On Thu, Feb 29, 2024 at 10:45:01PM +0000, Fabrizio Castro wrote: > > > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > Sent: Thursday, February 29, 2024 4:42 PM > > > To: Fabrizio Castro <fabrizio.castro.jz@renesas.com> > > > Subject: Re: [PATCH v7 2/4] pwm: Add support for RZ/V2M PWM driver > > > > > > On Mon, Feb 12, 2024 at 09:06:50PM +0000, Fabrizio Castro wrote: > > > > +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_roundup(u64 a, > > > > +u64 b, > > > u64 c) > > > > +{ > > > > + u64 ab = a * b; > > > > > > a * b might overflow?! > > > > In the context of this driver, this cannot overflow. > > The 2 formulas the above is needed for are: > > 1) period = (cyc + 1)*(NSEC_PER_SEC * frequency_divisor)/rate > > 2) duty_cycle = (cyc + 1 - low)*(NSEC_PER_SEC * > > frequency_divisor)/rate > > > > With respect to 1), the dividend overflows when period * rate also > > overflows (its product is calculated in rzv2m_pwm_config). > > However, limiting the period to a maximum value of U64_MAX / rate > > prevents the calculations from overflowing (in both directions, from period to cyc, and from cyc to > period). v6 introduced max_period for this. > > The situation for 2) is very similar to 1), with duty_cycle<=period, > > therefore limiting period to a max value (and clamping the duty cycle > > accordingly) will ensure that the calculation for duty_cycle won't > > overflow, either. > > OK, so it might be right from a technical POV. From a maintainer POV this is still bad. Authors for > other drivers might copy it, or the driver might be changed and there is no indication that the the > function relies on only be called with certain parameters. I could add comments to clarify this, or checks to make sure the parameters are passed as expected, or both? Or if you have a better suggestion? I would still like to be able to use the below formula if possible, as it allows for the smallest restriction on the period: (a * b) / c + ( (a * b) - (((a * b) / c) * c) ? 1 : 0 ) > > > > > + u64 d = div64_u64(ab, c); > > > > + u64 e = d * c; > > > > + > > > > + return d + ((ab - e) ? 1 : 0); > > > > +} > > > > + > > > > +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_rounddown(u64 a, > > > > +u64 b, > > > u64 c) > > > > +{ > > > > + return div64_u64(a * b, c); > > > > > > ditto. This is the same function as mul_u64_u64_div_u64() isn't it? > > > > Since a * b cannot overflow in the case of this driver, I believe the > > above to be a better option than mul_u64_u64_div_u64. > > Same technical POV vs maintainer POV as above. Plus: Even if > mul_u64_u64_div_u64 is a tad slower, reusing it has some benefits nevertheless. I'll use mul_u64_u64_div_u64 instead. > > > > > [...] > > > > + cyc = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCYC); > > > > + state->period = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1, > > > > + NSEC_PER_SEC * frequency_divisor, > > > > + rzv2m_pwm->rate); > > > > + > > > > + low = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMLOW); > > > > + state->duty_cycle = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1 - low, > > > > + NSEC_PER_SEC * frequency_divisor, > > > > + rzv2m_pwm->rate); > > > > > > The register semantic makes me wonder if each period starts with the > > > low part. In that case the hardware called "normal" what is called > > > inverted in the pwm framework?! > > > > My understanding is that the PWM framework defines "normal" polarity a > > signal that starts high (and stays high) for the duration of the duty > > cycle, and goes low for the remainder of the period. Conversely, a > > signal with "inversed" polarity starts low (and stays low) for the > > duration of the duty cycle and goes high for the remainder of the period. > > Ack. > > > This IP _does_ start low, but it _doesn't_ stay low for the duration > > of the duty cycle, as it then goes high for the duration of the duty > > cycle, therefore this IP doesn't perfectly fit either ("normal" or > > "inverted") definitions. > > I think you can say that the "normal" signal is _shifted_ in phase for > > this IP, rather than being "inverted". > > Alternatively (and a better match): What you describe is an inverted wave form with duty_cycle = period > - duty_cycle. That is also true. Also, it'll have the benefit of getting the first period out of the door without a shift in phase. I'll adjust accordingly. > > > > > + return pm_runtime_put(chip->dev); > > > > > > If you evaluate the return value of pm_runtime_put() maybe check > > > pm_runtime_get_sync() for symmetry, too? > > > > Or I could just discard it and return 0? > > I am fine with either, what's your preference? > > My preference would be to always check the return value, but given that many drivers don't care for > that, I agree to accept never checking it. > So choose one option and do it consistently please. Thanks. > > > > > + if (pwm_cyc && !FIELD_FIT(RZV2M_PWMCYC_PERIOD, pwm_cyc - 1)) > > > > + pwm_cyc = RZV2M_PWMCYC_PERIOD + 1; > > > > > > I don't understand the relevance of FIELD_FIT(RZV2M_PWMCYC_PERIOD, > > > pwm_cyc - 1). > > > > CYC is only made of 24 bits, therefore this is to make sure we don't > > go beyond a 24-bit representation. > > I would have understood: > > if (FIELD_FIT(RZV2M_PWMCYC_PERIOD, pwm_cyc + 1)) > pwm_cyc = RZV2M_PWMCYC_PERIOD + 1; By the time the above test is run, pwm_cyc is incremented by 1, therefore it needs to be decremented in order to be tested, and incremented when re-assigned. pwm_cyc was left incremented to simplify the pwm_low calculation, but I can see it's very confusing, therefore I'll improve this in v8. Cheers, Fab > > Notice there are three changes compared to your variant: > - drop pwm_cyc != 0 check > - drop ! from FIELD_FIT > - pwm_cyc + 1 instead of pwm_cyc - 1 > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 4b956d661755..55d46e6183a2 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -524,6 +524,17 @@ config PWM_RZ_MTU3 To compile this driver as a module, choose M here: the module will be called pwm-rz-mtu3. +config PWM_RZV2M + tristate "Renesas RZ/V2M PWM support" + depends on ARCH_R9A09G011 || COMPILE_TEST + depends on HAS_IOMEM + help + This driver exposes the PWM controller found in Renesas + RZ/V2M like chips through the PWM API. + + To compile this driver as a module, choose M here: the module + will be called pwm-rzv2m. + config PWM_SAMSUNG tristate "Samsung PWM support" depends on PLAT_SAMSUNG || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index c5ec9e168ee7..cf5a4a1c3b1a 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -48,6 +48,7 @@ obj-$(CONFIG_PWM_RCAR) += pwm-rcar.o obj-$(CONFIG_PWM_RENESAS_TPU) += pwm-renesas-tpu.o obj-$(CONFIG_PWM_ROCKCHIP) += pwm-rockchip.o obj-$(CONFIG_PWM_RZ_MTU3) += pwm-rz-mtu3.o +obj-$(CONFIG_PWM_RZV2M) += pwm-rzv2m.o obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o diff --git a/drivers/pwm/pwm-rzv2m.c b/drivers/pwm/pwm-rzv2m.c new file mode 100644 index 000000000000..eb9062293590 --- /dev/null +++ b/drivers/pwm/pwm-rzv2m.c @@ -0,0 +1,480 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Renesas RZ/V2M PWM Timer (PWM) driver + * + * Copyright (C) 2024 Renesas Electronics Corporation + * + * Hardware manual for this IP can be found here + * https://www.renesas.com/in/en/document/mah/rzv2m-users-manual-hardware?language=en + * + * Limitations: + * - Changes to the duty cycle configuration get effective only after the next + * period end. + * - The duty cycle can be changed only by modifying the PWMLOW register + * value and changing the pulse width at low level. The duty cycle becomes + * 0% for the low width when the value of the PWMLOW register is 0x0h + * and 100% for the low width when the value of the PWMLOW > PWMCYC. + */ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/io.h> +#include <linux/limits.h> +#include <linux/math64.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/pwm.h> +#include <linux/reset.h> +#include <linux/time.h> + +#define RZV2M_PWMCTR 0x0 +#define RZV2M_PWMCYC 0x4 +#define RZV2M_PWMLOW 0x8 +#define RZV2M_PWMCNT 0xc + +#define RZV2M_PWMCTR_PWMPS GENMASK(17, 16) +#define RZV2M_PWMCTR_PWMHL BIT(3) +#define RZV2M_PWMCTR_PWMTM BIT(2) +#define RZV2M_PWMCTR_PWME BIT(1) + +#define RZV2M_PWMCYC_PERIOD GENMASK(23, 0) +#define RZV2M_PWMLOW_PERIOD GENMASK(23, 0) + +struct rzv2m_pwm_chip { + u64 max_period; + struct pwm_chip chip; + void __iomem *mmio; + struct reset_control *rstc; + struct clk *apb_clk; + struct clk *pwm_clk; + unsigned long rate; + unsigned long delay; + unsigned long pwm_cyc; + enum pwm_polarity polarity; + bool is_ch_enabled; +}; + +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_roundup(u64 a, u64 b, u64 c) +{ + u64 ab = a * b; + u64 d = div64_u64(ab, c); + u64 e = d * c; + + return d + ((ab - e) ? 1 : 0); +} + +static inline u64 rzv2m_pwm_mul_u64_u64_div_u64_rounddown(u64 a, u64 b, u64 c) +{ + return div64_u64(a * b, c); +} + +static inline struct rzv2m_pwm_chip *to_rzv2m_pwm_chip(struct pwm_chip *chip) +{ + return container_of(chip, struct rzv2m_pwm_chip, chip); +} + +static void rzv2m_pwm_wait_delay(struct rzv2m_pwm_chip *rzv2m_pwm) +{ + /* delay timer when change the setting register */ + ndelay(rzv2m_pwm->delay); +} + +static void rzv2m_pwm_write(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg, u32 data) +{ + writel(data, rzv2m_pwm->mmio + reg); +} + +static u32 rzv2m_pwm_read(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg) +{ + return readl(rzv2m_pwm->mmio + reg); +} + +static void rzv2m_pwm_modify(struct rzv2m_pwm_chip *rzv2m_pwm, u32 reg, u32 clr, + u32 set) +{ + rzv2m_pwm_write(rzv2m_pwm, reg, + (rzv2m_pwm_read(rzv2m_pwm, reg) & ~clr) | set); +} + +static u8 rzv2m_pwm_calculate_prescale(struct rzv2m_pwm_chip *rzv2m_pwm, + u64 period_cycles) +{ + u32 prescaled_period_cycles; + u8 prescale; + + prescaled_period_cycles = period_cycles >> 24; + if (prescaled_period_cycles >= 256) + prescale = 3; + else + prescale = (fls(prescaled_period_cycles) + 3) / 4; + + return prescale; +} + +static inline int rzv2m_pwm_prescale_to_shift(u8 prescale) +{ + return prescale == 3 ? 11 : prescale * 4; +} + +static int rzv2m_pwm_enable(struct rzv2m_pwm_chip *rzv2m_pwm) +{ + int rc; + + rc = pm_runtime_resume_and_get(rzv2m_pwm->chip.dev); + if (rc) + return rc; + + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, + RZV2M_PWMCTR_PWME); + rzv2m_pwm_wait_delay(rzv2m_pwm); + rzv2m_pwm->is_ch_enabled = true; + + return 0; +} + +static void rzv2m_pwm_disable(struct rzv2m_pwm_chip *rzv2m_pwm) +{ + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, 0); + rzv2m_pwm_wait_delay(rzv2m_pwm); + pm_runtime_put_sync(rzv2m_pwm->chip.dev); + rzv2m_pwm->is_ch_enabled = false; +} + +static int rzv2m_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct rzv2m_pwm_chip *rzv2m_pwm = to_rzv2m_pwm_chip(chip); + u16 frequency_divisor; + u32 ctr, cyc, low; + u8 prescale; + + pm_runtime_get_sync(chip->dev); + ctr = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCTR); + state->enabled = FIELD_GET(RZV2M_PWMCTR_PWME, ctr); + state->polarity = FIELD_GET(RZV2M_PWMCTR_PWMHL, ctr) ? + PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; + prescale = FIELD_GET(RZV2M_PWMCTR_PWMPS, ctr); + frequency_divisor = 1 << rzv2m_pwm_prescale_to_shift(prescale); + + cyc = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCYC); + state->period = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1, + NSEC_PER_SEC * frequency_divisor, + rzv2m_pwm->rate); + + low = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMLOW); + state->duty_cycle = rzv2m_pwm_mul_u64_u64_div_u64_roundup(cyc + 1 - low, + NSEC_PER_SEC * frequency_divisor, + rzv2m_pwm->rate); + + return pm_runtime_put(chip->dev); +} + +static int rzv2m_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct rzv2m_pwm_chip *rzv2m_pwm = to_rzv2m_pwm_chip(chip); + u64 period = state->period, duty_cycle = state->duty_cycle; + u16 frequency_divisor; + u64 pwm_cyc, pwm_low; + u8 prescale; + u32 pwm_ctr; + + /* + * Clamp period and duty cycle to their maximum values for our current + * configuration rather than letting our calculations overflow. + */ + if (period > rzv2m_pwm->max_period) { + period = rzv2m_pwm->max_period; + if (duty_cycle > rzv2m_pwm->max_period) + duty_cycle = period; + } + + /* + * Formula for calculating PWM Cycle Setting Register: + * PWM cycle = (PWM period(ns) / (PWM_CLK period(ns) × Div ratio)) - 1 + */ + pwm_cyc = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(period, + rzv2m_pwm->rate, + NSEC_PER_SEC); + pwm_cyc = pwm_cyc ? pwm_cyc : 1; + + prescale = rzv2m_pwm_calculate_prescale(rzv2m_pwm, pwm_cyc - 1); + frequency_divisor = 1 << rzv2m_pwm_prescale_to_shift(prescale); + if (frequency_divisor > 1) { + pwm_cyc = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(period, + rzv2m_pwm->rate, + NSEC_PER_SEC * frequency_divisor); + pwm_cyc = pwm_cyc ? pwm_cyc : 1; + } + + if (pwm_cyc && !FIELD_FIT(RZV2M_PWMCYC_PERIOD, pwm_cyc - 1)) + pwm_cyc = RZV2M_PWMCYC_PERIOD + 1; + + /* + * Formula for calculating PWMLOW register: + * PWMLOW register = PWM cycle * Low pulse width ratio (%) + */ + pwm_low = rzv2m_pwm_mul_u64_u64_div_u64_rounddown(duty_cycle, + rzv2m_pwm->rate, NSEC_PER_SEC * frequency_divisor); + + pwm_low = pwm_cyc - pwm_low; + if (!FIELD_FIT(RZV2M_PWMLOW_PERIOD, pwm_low)) + pwm_low = RZV2M_PWMLOW_PERIOD; + + pwm_cyc--; + + /* + * If the PWM channel is disabled, make sure to turn on the clock + * before writing the register. + */ + if (!pwm->state.enabled) + pm_runtime_get_sync(rzv2m_pwm->chip.dev); + + /* + * To change the setting value of the PWM cycle setting register + * (PWMm_PWMCYC) or polarity, set the PWME bit of the PWM control + * register (PWMm_PWMCTR) to 0b and stop the counter operation. + */ + if (rzv2m_pwm->polarity != state->polarity || rzv2m_pwm->pwm_cyc != pwm_cyc) { + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, 0); + rzv2m_pwm_wait_delay(rzv2m_pwm); + } + + rzv2m_pwm_write(rzv2m_pwm, RZV2M_PWMCYC, pwm_cyc); + rzv2m_pwm_write(rzv2m_pwm, RZV2M_PWMLOW, pwm_low); + + pwm_ctr = FIELD_PREP(RZV2M_PWMCTR_PWMPS, prescale); + if (state->polarity == PWM_POLARITY_INVERSED) + pwm_ctr |= RZV2M_PWMCTR_PWMHL; + + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWMTM | + RZV2M_PWMCTR_PWMPS | RZV2M_PWMCTR_PWMHL, pwm_ctr); + + if (rzv2m_pwm->polarity != state->polarity || rzv2m_pwm->pwm_cyc != pwm_cyc) { + rzv2m_pwm->polarity = state->polarity; + rzv2m_pwm->pwm_cyc = pwm_cyc; + rzv2m_pwm_modify(rzv2m_pwm, RZV2M_PWMCTR, RZV2M_PWMCTR_PWME, + RZV2M_PWMCTR_PWME); + } + + rzv2m_pwm_wait_delay(rzv2m_pwm); + + /* If the PWM is not enabled, turn the clock off again to save power. */ + if (!pwm->state.enabled) + pm_runtime_put(rzv2m_pwm->chip.dev); + + return 0; +} + +static int rzv2m_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct rzv2m_pwm_chip *rzv2m_pwm = to_rzv2m_pwm_chip(chip); + bool enabled = pwm->state.enabled; + int ret; + + if (!state->enabled) { + if (enabled) + rzv2m_pwm_disable(rzv2m_pwm); + + return 0; + } + + ret = rzv2m_pwm_config(chip, pwm, state); + if (ret) + return ret; + + if (!enabled) + ret = rzv2m_pwm_enable(rzv2m_pwm); + + return ret; +} + +static const struct pwm_ops rzv2m_pwm_ops = { + .get_state = rzv2m_pwm_get_state, + .apply = rzv2m_pwm_apply, +}; + +static int rzv2m_pwm_pm_runtime_suspend(struct device *dev) +{ + struct rzv2m_pwm_chip *rzv2m_pwm = dev_get_drvdata(dev); + + clk_disable_unprepare(rzv2m_pwm->pwm_clk); + clk_disable_unprepare(rzv2m_pwm->apb_clk); + + return 0; +} + +static int rzv2m_pwm_pm_runtime_resume(struct device *dev) +{ + struct rzv2m_pwm_chip *rzv2m_pwm = dev_get_drvdata(dev); + int ret; + + ret = clk_prepare_enable(rzv2m_pwm->apb_clk); + if (ret) + return ret; + + ret = clk_prepare_enable(rzv2m_pwm->pwm_clk); + if (ret) + clk_disable_unprepare(rzv2m_pwm->apb_clk); + + return ret; +} + +static DEFINE_RUNTIME_DEV_PM_OPS(rzv2m_pwm_pm_ops, + rzv2m_pwm_pm_runtime_suspend, + rzv2m_pwm_pm_runtime_resume, NULL); + +static void rzv2m_pwm_reset_assert_pm_disable(void *data) +{ + struct rzv2m_pwm_chip *rzv2m_pwm = data; + + /* + * The below check is for making balanced PM usage count in probe/remove + * eg: boot loader is turning on PWM and probe increments the PM usage + * count. Before apply, if there is unbind/remove callback we need to + * decrement the PM usage count. + */ + if (rzv2m_pwm->is_ch_enabled) + pm_runtime_put(rzv2m_pwm->chip.dev); + + clk_rate_exclusive_put(rzv2m_pwm->pwm_clk); + clk_rate_exclusive_put(rzv2m_pwm->apb_clk); + pm_runtime_disable(rzv2m_pwm->chip.dev); + pm_runtime_set_suspended(rzv2m_pwm->chip.dev); + reset_control_assert(rzv2m_pwm->rstc); +} + +static int rzv2m_pwm_probe(struct platform_device *pdev) +{ + struct rzv2m_pwm_chip *rzv2m_pwm; + unsigned long apb_clk_rate; + int ret; + + rzv2m_pwm = devm_kzalloc(&pdev->dev, sizeof(*rzv2m_pwm), GFP_KERNEL); + if (!rzv2m_pwm) + return -ENOMEM; + + rzv2m_pwm->mmio = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(rzv2m_pwm->mmio)) + return PTR_ERR(rzv2m_pwm->mmio); + + rzv2m_pwm->apb_clk = devm_clk_get(&pdev->dev, "apb"); + if (IS_ERR(rzv2m_pwm->apb_clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->apb_clk), + "cannot get apb clock\n"); + + rzv2m_pwm->pwm_clk = devm_clk_get(&pdev->dev, "pwm"); + if (IS_ERR(rzv2m_pwm->pwm_clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->pwm_clk), + "cannot get pwm clock\n"); + + rzv2m_pwm->rstc = devm_reset_control_get_shared(&pdev->dev, NULL); + if (IS_ERR(rzv2m_pwm->rstc)) + return dev_err_probe(&pdev->dev, PTR_ERR(rzv2m_pwm->rstc), + "get reset failed\n"); + + platform_set_drvdata(pdev, rzv2m_pwm); + ret = reset_control_deassert(rzv2m_pwm->rstc); + if (ret) { + return dev_err_probe(&pdev->dev, ret, + "cannot deassert reset control\n"); + } + + ret = clk_prepare_enable(rzv2m_pwm->apb_clk); + if (ret < 0) + goto err_reset; + + ret = clk_prepare_enable(rzv2m_pwm->pwm_clk); + if (ret < 0) + goto disable_apb_clk; + + clk_rate_exclusive_get(rzv2m_pwm->apb_clk); + clk_rate_exclusive_get(rzv2m_pwm->pwm_clk); + apb_clk_rate = clk_get_rate(rzv2m_pwm->apb_clk); + if (!apb_clk_rate) + goto err_rate_put; + + rzv2m_pwm->rate = clk_get_rate(rzv2m_pwm->pwm_clk); + if (!rzv2m_pwm->rate) + goto err_rate_put; + rzv2m_pwm->max_period = div64_u64(U64_MAX, rzv2m_pwm->rate); + + /* + * The registers other than the PWM interrupt register (PWMINT) are + * always synchronized with PWM_CLK at regular intervals. It takes some + * time (Min: 2 × PCLK + 4 × PWM_CLK to Max: 6 × PCLK + 9 × PWM_CLK) for + * the value set in the register to be reflected in the PWM circuit + * because there is a synchronizer between the register and the PWM + * circuit. + */ + rzv2m_pwm->delay = 6 * DIV_ROUND_UP(NSEC_PER_SEC, apb_clk_rate) + + 9 * DIV_ROUND_UP(NSEC_PER_SEC, rzv2m_pwm->rate); + + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->dev); + + /* + * We need to keep the clock on, in case the bootloader has enabled the + * PWM and is running during probe(). + */ + if (!!(rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCTR) & RZV2M_PWMCTR_PWME)) { + u32 val; + + pm_runtime_get_sync(&pdev->dev); + rzv2m_pwm->is_ch_enabled = true; + rzv2m_pwm->pwm_cyc = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCYC); + val = rzv2m_pwm_read(rzv2m_pwm, RZV2M_PWMCTR); + rzv2m_pwm->polarity = FIELD_GET(RZV2M_PWMCTR_PWMHL, val) ? + PWM_POLARITY_NORMAL : PWM_POLARITY_INVERSED; + } + + rzv2m_pwm->chip.dev = &pdev->dev; + ret = devm_add_action_or_reset(&pdev->dev, + rzv2m_pwm_reset_assert_pm_disable, + rzv2m_pwm); + if (ret) + return ret; + + rzv2m_pwm->chip.ops = &rzv2m_pwm_ops; + rzv2m_pwm->chip.npwm = 1; + ret = devm_pwmchip_add(&pdev->dev, &rzv2m_pwm->chip); + if (ret) + return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n"); + + pm_runtime_idle(&pdev->dev); + + return 0; + +err_rate_put: + clk_rate_exclusive_put(rzv2m_pwm->pwm_clk); + clk_rate_exclusive_put(rzv2m_pwm->apb_clk); + clk_disable_unprepare(rzv2m_pwm->pwm_clk); +disable_apb_clk: + clk_disable_unprepare(rzv2m_pwm->apb_clk); +err_reset: + reset_control_assert(rzv2m_pwm->rstc); + return ret; +} + +static const struct of_device_id rzv2m_pwm_of_table[] = { + { .compatible = "renesas,rzv2m-pwm", }, + { /* Sentinel */ } +}; +MODULE_DEVICE_TABLE(of, rzv2m_pwm_of_table); + +static struct platform_driver rzv2m_pwm_driver = { + .driver = { + .name = "pwm-rzv2m", + .pm = pm_ptr(&rzv2m_pwm_pm_ops), + .of_match_table = rzv2m_pwm_of_table, + }, + .probe = rzv2m_pwm_probe, +}; +module_platform_driver(rzv2m_pwm_driver); + +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>"); +MODULE_AUTHOR("Fabrizio Castro <fabrizio.castro.jz@renesas.com>"); +MODULE_DESCRIPTION("Renesas RZ/V2M PWM Timer Driver"); +MODULE_LICENSE("GPL");