| Message ID | 20230310170654.268047-7-biju.das.jz@bp.renesas.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
Hi Thierry and Uwe, Gentle ping Cheers, Biju > -----Original Message----- > From: Biju Das <biju.das.jz@bp.renesas.com> > Sent: Friday, March 10, 2023 5:07 PM > To: Thierry Reding <thierry.reding@gmail.com> > Cc: Biju Das <biju.das.jz@bp.renesas.com>; Uwe Kleine-König <u.kleine- > koenig@pengutronix.de>; linux-pwm@vger.kernel.org; Lee Jones > <lee@kernel.org>; Daniel Lezcano <daniel.lezcano@linaro.org>; William > Breathitt Gray <william.gray@linaro.org>; Geert Uytterhoeven > <geert+renesas@glider.be>; Chris Paterson <Chris.Paterson2@renesas.com>; > Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; linux- > renesas-soc@vger.kernel.org > Subject: [PATCH v14 6/6] pwm: Add Renesas RZ/G2L MTU3a PWM driver > > The RZ/G2L Multi-Function Timer Pulse Unit 3 (a.k.a MTU3a) uses one counter > and two match components to configure duty_cycle and period to generate PWM > output waveform. > > Add basic support for RZ/G2L MTU3a PWM driver by creating separate PWM > channels for each IOs. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v13->v14: > * Updated commit description > * Updated Limitations section. > * Replaced the macros RZ_MTU*->RZ_MTU3_CHAN_* in probe() > * Fixed a kernel crash in error path by moving rz_mtu3_pwm->chip.dev before > devm_add_action_or_reset() > * Added pm_runtime_idle() and simplified error paths for > devm_add_action_or_reset() > and devm_pwmchip_add(). > > v12->v13: > * Updated commit description > * Moved RZ_MTU3_TMDR1_MD_* macros to rz_mtu3.h > * Updated Limitations section. > * Removed PWM mode1 references from the driver. > * Dropped prescale and duty_cycle from struct rz_mtu3_pwm_chip. > * Replaced rz_mtu3_pwm_mode1_num_ios->rz_mtu3_hw_channel_ios. > * Avoided race condition in rz_mtu3_pwm_request()/rz_mtu3_pwm_free(). > * Updated get_state() by adding dc > pv check and added a comment about > overflow condition. > * Moved overflow condition check from config->probe() > * Replaced pm_runtime_resume_and_get with unconditional > pm_runtime_get_sync() > in config() > * Added error check for clk_prepare_enable() in probe() and propagating > error > to the caller for pm_runtime_resume() > * clk_get_rate() is called after enabling the clock and > clk_rate_exclusive_put() > v11->v12: > * Updated header file to <linux/mfd/rz-mtu3.h> as core driver is in MFD. > * Reordered get_state() > v10->v11: > * No change. > v9->v10: > * No change. > v8->v9: > * Added prescale/duty_cycle variables to struct rz_mtu3_pwm_chip and > cached this values in rz_mtu3_pwm_config and used this cached values > in get_state(), if PWM is disabled. > * Added return code for get_state() > v7->v8: > * Simplified rz_mtu3_pwm_request by calling rz_mtu3_request_channel() > * Simplified rz_mtu3_pwm_free by calling rz_mtu3_release_channel() > v6->v7: > * Added channel specific mutex lock to avoid race between counter > device and rz_mtu3_pwm_{request,free} > * Added pm_runtime_resume_and_get in rz_mtu3_pwm_enable() > * Added pm_runtime_put_sync in rz_mtu3_pwm_disable() > * Updated rz_mtu3_pwm_config() > * Updated rz_mtu3_pwm_apply() > v5->v6: > * Updated commit and Kconfig description > * Sorted the header > * Replaced dev_get_drvdata from rz_mtu3_pwm_pm_disable() > * Replaced SET_RUNTIME_PM_OPS->DEFINE_RUNTIME_DEV_PM_OPS and removed > __maybe_unused from suspend/resume() > v4->v5: > * pwm device is instantiated by mtu3a core driver. > v3->v4: > * There is no resource associated with "rz-mtu3-pwm" compatible > and moved the code to mfd subsystem as it binds against "rz-mtu". > * Removed struct platform_driver rz_mtu3_pwm_driver. > v2->v3: > * No change. > v1->v2: > * Modelled as a single PWM device handling multiple channles. > * Used PM framework to manage the clocks. > --- > drivers/pwm/Kconfig | 11 + > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-rz-mtu3.c | 482 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 494 insertions(+) > create mode 100644 drivers/pwm/pwm-rz-mtu3.c > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index > dae023d783a2..ccc0299fd0dd 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -481,6 +481,17 @@ config PWM_ROCKCHIP > Generic PWM framework driver for the PWM controller found on > Rockchip SoCs. > > +config PWM_RZ_MTU3 > + tristate "Renesas RZ/G2L MTU3a PWM Timer support" > + depends on RZ_MTU3 || COMPILE_TEST > + depends on HAS_IOMEM > + help > + This driver exposes the MTU3a PWM Timer controller found in Renesas > + RZ/G2L like chips through the PWM API. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-rz-mtu3. > + > 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 > 7bf1a29f02b8..b85fc9fba326 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -44,6 +44,7 @@ obj-$(CONFIG_PWM_RASPBERRYPI_POE) += pwm-raspberrypi- > poe.o > 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_SAMSUNG) += pwm-samsung.o > obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o > obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o > diff --git a/drivers/pwm/pwm-rz-mtu3.c b/drivers/pwm/pwm-rz-mtu3.c new file > mode 100644 index 000000000000..146948656755 > --- /dev/null > +++ b/drivers/pwm/pwm-rz-mtu3.c > @@ -0,0 +1,482 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Renesas RZ/G2L MTU3a PWM Timer driver > + * > + * Copyright (C) 2022 Renesas Electronics Corporation > + * > + * Hardware manual for this IP can be found here > + * > +https://www.renesas.com/eu/en/document/mah/rzg2l-group-rzg2lc-group-use > +rs-manual-hardware-0?language=en > + * > + * Limitations: > + * - When PWM is disabled, the output is driven to Hi-Z. > + * - While the hardware supports both polarities, the driver (for now) > + * only handles normal polarity. > + * - HW uses one counter and two match components to configure duty_cycle > + * and period. > + * - Multi-Function Timer Pulse Unit(a.k.a MTU) has 7 HW channels for PWM > + * operations(The channels are MTU{0..4, 6, 7}). > + * - MTU{1, 2} channels have a single IO, whereas all other HW channels > have > + * 2 IOs. > + * - Each IO is modelled as an independent PWM channel. > + * - rz_mtu3_hw_channel_ios table is used to map the PWM channel to the > + * corresponding HW channel as there are difference in number of IOs > + * between HW channels. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/limits.h> > +#include <linux/mfd/rz-mtu3.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/pwm.h> > +#include <linux/time.h> > + > +#define RZ_MTU3_TIOR_OC_RETAIN (0) > +#define RZ_MTU3_TIOR_OC_0_H_COMP_MATCH (2) > +#define RZ_MTU3_TIOR_OC_1_TOGGLE (7) > +#define RZ_MTU3_TIOR_OC_IOA GENMASK(3, 0) > + > +#define RZ_MTU3_TCR_CCLR_TGRC (5 << 5) > +#define RZ_MTU3_TCR_CKEG_RISING (0 << 3) > + > +#define RZ_MTU3_TCR_TPCS GENMASK(2, 0) > + > +#define RZ_MTU3_MAX_PWM_CHANNELS (12) > + > +#define RZ_MTU3_MAX_HW_CHANNELS (7) > + > +/* The table represents the number of IOs on each MTU HW channel. */ > +static const u8 rz_mtu3_hw_channel_ios[] = { 2, 1, 1, 2, 2, 2, 2 }; > + > +/** > + * struct rz_mtu3_pwm_chip - MTU3 pwm private data > + * > + * @chip: MTU3 pwm chip data > + * @clk: MTU3 module clock > + * @lock: Lock to prevent concurrent access for usage count > + * @rate: MTU3 clock rate > + * @user_count: MTU3 usage count > + * @rz_mtu3_channel: HW channels for the PWM */ > + > +struct rz_mtu3_pwm_chip { > + struct pwm_chip chip; > + struct clk *clk; > + struct mutex lock; > + unsigned long rate; > + u32 user_count[RZ_MTU3_MAX_HW_CHANNELS]; > + struct rz_mtu3_channel *ch[RZ_MTU3_MAX_HW_CHANNELS]; }; > + > +static inline struct rz_mtu3_pwm_chip *to_rz_mtu3_pwm_chip(struct > +pwm_chip *chip) { > + return container_of(chip, struct rz_mtu3_pwm_chip, chip); } > + > +static u8 rz_mtu3_pwm_calculate_prescale(struct rz_mtu3_pwm_chip *rz_mtu3, > + u64 period_cycles) > +{ > + u32 prescaled_period_cycles; > + u8 prescale; > + > + prescaled_period_cycles = period_cycles >> 16; > + if (prescaled_period_cycles >= 16) > + prescale = 3; > + else > + prescale = (fls(prescaled_period_cycles) + 1) / 2; > + > + return prescale; > +} > + > +static struct rz_mtu3_channel * > +rz_mtu3_get_hw_channel(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, u32 > +channel) { > + unsigned int i, ch_index = 0; > + > + for (i = 0; i < ARRAY_SIZE(rz_mtu3_hw_channel_ios); i++) { > + ch_index += rz_mtu3_hw_channel_ios[i]; > + > + if (ch_index > channel) > + break; > + } > + > + return rz_mtu3_pwm->ch[i]; > +} > + > +static u32 rz_mtu3_get_hw_channel_index(struct rz_mtu3_pwm_chip > *rz_mtu3_pwm, > + struct rz_mtu3_channel *ch) > +{ > + u32 i; > + > + for (i = 0; i < ARRAY_SIZE(rz_mtu3_hw_channel_ios); i++) { > + if (ch == rz_mtu3_pwm->ch[i]) > + break; > + } > + > + return i; > +} > + > +static bool rz_mtu3_pwm_is_second_channel(u32 ch_index, u32 hwpwm) { > + u32 i, pwm_ch_index = 0; > + > + for (i = 0; i < ch_index; i++) > + pwm_ch_index += rz_mtu3_hw_channel_ios[i]; > + > + return pwm_ch_index != hwpwm; > +} > + > +static bool rz_mtu3_pwm_is_ch_enabled(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > + u32 hwpwm) > +{ > + struct rz_mtu3_channel *ch; > + bool is_channel_en; > + u32 ch_index; > + u8 val; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + is_channel_en = rz_mtu3_is_enabled(ch); > + > + if (rz_mtu3_pwm_is_second_channel(ch_index, hwpwm)) > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORL); > + else > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORH); > + > + return (is_channel_en && (val & RZ_MTU3_TIOR_OC_IOA)); } > + > +static int rz_mtu3_pwm_request(struct pwm_chip *chip, struct pwm_device > +*pwm) { > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + struct rz_mtu3_channel *ch; > + u32 ch_index; > + > + mutex_lock(&rz_mtu3_pwm->lock); > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + if (!rz_mtu3_pwm->user_count[ch_index] && > !rz_mtu3_request_channel(ch)) { > + mutex_unlock(&rz_mtu3_pwm->lock); > + return -EBUSY; > + } > + > + rz_mtu3_pwm->user_count[ch_index]++; > + mutex_unlock(&rz_mtu3_pwm->lock); > + > + return 0; > +} > + > +static void rz_mtu3_pwm_free(struct pwm_chip *chip, struct pwm_device > +*pwm) { > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + struct rz_mtu3_channel *ch; > + u32 ch_index; > + > + mutex_lock(&rz_mtu3_pwm->lock); > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + > + rz_mtu3_pwm->user_count[ch_index]--; > + if (!rz_mtu3_pwm->user_count[ch_index]) > + rz_mtu3_release_channel(ch); > + > + mutex_unlock(&rz_mtu3_pwm->lock); > +} > + > +static int rz_mtu3_pwm_enable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > + struct pwm_device *pwm) > +{ > + struct rz_mtu3_channel *ch; > + u32 ch_index; > + u8 val; > + int rc; > + > + rc = pm_runtime_resume_and_get(rz_mtu3_pwm->chip.dev); > + if (rc) > + return rc; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + val = (RZ_MTU3_TIOR_OC_1_TOGGLE << 4) | > +RZ_MTU3_TIOR_OC_0_H_COMP_MATCH; > + > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_MD_PWMMODE1); > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, val); > + else > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, val); > + > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > + rz_mtu3_enable(ch); > + > + return 0; > +} > + > +static void rz_mtu3_pwm_disable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > + struct pwm_device *pwm) > +{ > + struct rz_mtu3_channel *ch; > + u32 ch_index; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + > + /* Return to normal mode and disable output pins of MTU3 channel */ > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, > RZ_MTU3_TMDR1_MD_NORMAL); > + > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, > RZ_MTU3_TIOR_OC_RETAIN); > + else > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, > RZ_MTU3_TIOR_OC_RETAIN); > + > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > + rz_mtu3_disable(ch); > + > + pm_runtime_put_sync(rz_mtu3_pwm->chip.dev); > +} > + > +static int rz_mtu3_pwm_get_state(struct pwm_chip *chip, struct pwm_device > *pwm, > + struct pwm_state *state) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + struct rz_mtu3_channel *ch; > + u8 prescale, val; > + u32 ch_index; > + u16 dc, pv; > + u64 tmp; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + pm_runtime_get_sync(chip->dev); > + state->enabled = rz_mtu3_pwm_is_ch_enabled(rz_mtu3_pwm, pwm->hwpwm); > + if (state->enabled) { > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TCR); > + prescale = FIELD_GET(RZ_MTU3_TCR_TPCS, val); > + > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) { > + dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRD); > + pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRC); > + } else { > + dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRB); > + pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRA); > + } > + > + /* With prescale <= 7 and pv <= 0xffff this doesn't overflow. > */ > + tmp = NSEC_PER_SEC * (u64)pv << (2 * prescale); > + state->period = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate); > + tmp = NSEC_PER_SEC * (u64)dc << (2 * prescale); > + state->duty_cycle = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate); > + } > + > + if (state->duty_cycle > state->period) > + state->duty_cycle = state->period; > + > + state->polarity = PWM_POLARITY_NORMAL; > + pm_runtime_put(chip->dev); > + > + return 0; > +} > + > +static int rz_mtu3_pwm_config(struct pwm_chip *chip, struct pwm_device > *pwm, > + const struct pwm_state *state) { > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + struct rz_mtu3_channel *ch; > + unsigned long pv, dc; > + u64 period_cycles; > + u64 duty_cycles; > + u32 ch_index; > + u8 prescale; > + u8 val; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + period_cycles = mul_u64_u32_div(state->period, rz_mtu3_pwm->rate, > + NSEC_PER_SEC); > + prescale = rz_mtu3_pwm_calculate_prescale(rz_mtu3_pwm, period_cycles); > + > + if (period_cycles >> (2 * prescale) <= U16_MAX) > + pv = period_cycles >> (2 * prescale); > + else > + pv = U16_MAX; > + > + duty_cycles = mul_u64_u32_div(state->duty_cycle, rz_mtu3_pwm->rate, > + NSEC_PER_SEC); > + if (duty_cycles >> (2 * prescale) <= U16_MAX) > + dc = duty_cycles >> (2 * prescale); > + else > + dc = U16_MAX; > + > + /* > + * 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(chip->dev); > + > + val = RZ_MTU3_TCR_CKEG_RISING | prescale; > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) { > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, > + RZ_MTU3_TCR_CCLR_TGRC | val); > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRD, dc); > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRC, pv); > + } else { > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, > + RZ_MTU3_TCR_CCLR_TGRA | val); > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRB, dc); > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRA, pv); > + } > + > + /* If the PWM is not enabled, turn the clock off again to save power. > */ > + if (!pwm->state.enabled) > + pm_runtime_put(chip->dev); > + > + return 0; > +} > + > +static int rz_mtu3_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + bool enabled = pwm->state.enabled; > + int ret; > + > + if (state->polarity != PWM_POLARITY_NORMAL) > + return -EINVAL; > + > + if (!state->enabled) { > + if (enabled) > + rz_mtu3_pwm_disable(rz_mtu3_pwm, pwm); > + > + return 0; > + } > + > + ret = rz_mtu3_pwm_config(chip, pwm, state); > + if (ret) > + return ret; > + > + if (!enabled) > + ret = rz_mtu3_pwm_enable(rz_mtu3_pwm, pwm); > + > + return ret; > +} > + > +static const struct pwm_ops rz_mtu3_pwm_ops = { > + .request = rz_mtu3_pwm_request, > + .free = rz_mtu3_pwm_free, > + .get_state = rz_mtu3_pwm_get_state, > + .apply = rz_mtu3_pwm_apply, > + .owner = THIS_MODULE, > +}; > + > +static int rz_mtu3_pwm_pm_runtime_suspend(struct device *dev) { > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = dev_get_drvdata(dev); > + > + clk_disable_unprepare(rz_mtu3_pwm->clk); > + > + return 0; > +} > + > +static int rz_mtu3_pwm_pm_runtime_resume(struct device *dev) { > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = dev_get_drvdata(dev); > + > + return clk_prepare_enable(rz_mtu3_pwm->clk); > +} > + > +static DEFINE_RUNTIME_DEV_PM_OPS(rz_mtu3_pwm_pm_ops, > + rz_mtu3_pwm_pm_runtime_suspend, > + rz_mtu3_pwm_pm_runtime_resume, NULL); > + > +static void rz_mtu3_pwm_pm_disable(void *data) { > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = data; > + > + clk_rate_exclusive_put(rz_mtu3_pwm->clk); > + pm_runtime_disable(rz_mtu3_pwm->chip.dev); > + pm_runtime_set_suspended(rz_mtu3_pwm->chip.dev); > +} > + > +static int rz_mtu3_pwm_probe(struct platform_device *pdev) { > + struct rz_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent); > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm; > + struct device *dev = &pdev->dev; > + int num_pwm_hw_ch = 0; > + unsigned int i; > + int ret; > + > + rz_mtu3_pwm = devm_kzalloc(&pdev->dev, sizeof(*rz_mtu3_pwm), > GFP_KERNEL); > + if (!rz_mtu3_pwm) > + return -ENOMEM; > + > + rz_mtu3_pwm->clk = ddata->clk; > + > + for (i = 0; i < RZ_MTU_NUM_CHANNELS; i++) { > + if (i == RZ_MTU3_CHAN_5 || i == RZ_MTU3_CHAN_8) > + continue; > + > + rz_mtu3_pwm->ch[num_pwm_hw_ch] = &ddata->channels[i]; > + rz_mtu3_pwm->ch[num_pwm_hw_ch]->dev = dev; > + num_pwm_hw_ch++; > + } > + > + mutex_init(&rz_mtu3_pwm->lock); > + platform_set_drvdata(pdev, rz_mtu3_pwm); > + ret = clk_prepare_enable(rz_mtu3_pwm->clk); > + if (ret) > + return dev_err_probe(dev, ret, "Clock enable failed\n"); > + > + clk_rate_exclusive_get(rz_mtu3_pwm->clk); > + > + rz_mtu3_pwm->rate = clk_get_rate(rz_mtu3_pwm->clk); > + /* > + * Refuse clk rates > 1 GHz to prevent overflow later for computing > + * period and duty cycle. > + */ > + if (rz_mtu3_pwm->rate > NSEC_PER_SEC) { > + ret = -EINVAL; > + clk_rate_exclusive_put(rz_mtu3_pwm->clk); > + goto disable_clock; > + } > + > + pm_runtime_set_active(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + rz_mtu3_pwm->chip.dev = &pdev->dev; > + ret = devm_add_action_or_reset(&pdev->dev, > + rz_mtu3_pwm_pm_disable, > + rz_mtu3_pwm); > + if (ret < 0) > + return ret; > + > + rz_mtu3_pwm->chip.ops = &rz_mtu3_pwm_ops; > + rz_mtu3_pwm->chip.npwm = RZ_MTU3_MAX_PWM_CHANNELS; > + ret = devm_pwmchip_add(&pdev->dev, &rz_mtu3_pwm->chip); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to add PWM > chip\n"); > + > + pm_runtime_idle(&pdev->dev); > + > + return 0; > + > +disable_clock: > + clk_disable_unprepare(rz_mtu3_pwm->clk); > + return ret; > +} > + > +static struct platform_driver rz_mtu3_pwm_driver = { > + .driver = { > + .name = "pwm-rz-mtu3", > + .pm = pm_ptr(&rz_mtu3_pwm_pm_ops), > + }, > + .probe = rz_mtu3_pwm_probe, > +}; > +module_platform_driver(rz_mtu3_pwm_driver); > + > +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>"); > +MODULE_ALIAS("platform:pwm-rz-mtu3"); > +MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a PWM Timer Driver"); > +MODULE_LICENSE("GPL"); > -- > 2.25.1
Hello, I have the feeling this driver is more complicated than necessary. On Fri, Mar 10, 2023 at 05:06:54PM +0000, Biju Das wrote: > The RZ/G2L Multi-Function Timer Pulse Unit 3 (a.k.a MTU3a) uses > one counter and two match components to configure duty_cycle > and period to generate PWM output waveform. > > Add basic support for RZ/G2L MTU3a PWM driver by creating separate > PWM channels for each IOs. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v13->v14: > * Updated commit description > * Updated Limitations section. > * Replaced the macros RZ_MTU*->RZ_MTU3_CHAN_* in probe() > * Fixed a kernel crash in error path by moving rz_mtu3_pwm->chip.dev before > devm_add_action_or_reset() > * Added pm_runtime_idle() and simplified error paths for devm_add_action_or_reset() > and devm_pwmchip_add(). > > v12->v13: > * Updated commit description > * Moved RZ_MTU3_TMDR1_MD_* macros to rz_mtu3.h > * Updated Limitations section. > * Removed PWM mode1 references from the driver. > * Dropped prescale and duty_cycle from struct rz_mtu3_pwm_chip. > * Replaced rz_mtu3_pwm_mode1_num_ios->rz_mtu3_hw_channel_ios. > * Avoided race condition in rz_mtu3_pwm_request()/rz_mtu3_pwm_free(). > * Updated get_state() by adding dc > pv check and added a comment about > overflow condition. > * Moved overflow condition check from config->probe() > * Replaced pm_runtime_resume_and_get with unconditional pm_runtime_get_sync() > in config() > * Added error check for clk_prepare_enable() in probe() and propagating error > to the caller for pm_runtime_resume() > * clk_get_rate() is called after enabling the clock and clk_rate_exclusive_put() > v11->v12: > * Updated header file to <linux/mfd/rz-mtu3.h> as core driver is in MFD. > * Reordered get_state() > v10->v11: > * No change. > v9->v10: > * No change. > v8->v9: > * Added prescale/duty_cycle variables to struct rz_mtu3_pwm_chip and > cached this values in rz_mtu3_pwm_config and used this cached values > in get_state(), if PWM is disabled. > * Added return code for get_state() > v7->v8: > * Simplified rz_mtu3_pwm_request by calling rz_mtu3_request_channel() > * Simplified rz_mtu3_pwm_free by calling rz_mtu3_release_channel() > v6->v7: > * Added channel specific mutex lock to avoid race between counter > device and rz_mtu3_pwm_{request,free} > * Added pm_runtime_resume_and_get in rz_mtu3_pwm_enable() > * Added pm_runtime_put_sync in rz_mtu3_pwm_disable() > * Updated rz_mtu3_pwm_config() > * Updated rz_mtu3_pwm_apply() > v5->v6: > * Updated commit and Kconfig description > * Sorted the header > * Replaced dev_get_drvdata from rz_mtu3_pwm_pm_disable() > * Replaced SET_RUNTIME_PM_OPS->DEFINE_RUNTIME_DEV_PM_OPS and removed > __maybe_unused from suspend/resume() > v4->v5: > * pwm device is instantiated by mtu3a core driver. > v3->v4: > * There is no resource associated with "rz-mtu3-pwm" compatible > and moved the code to mfd subsystem as it binds against "rz-mtu". > * Removed struct platform_driver rz_mtu3_pwm_driver. > v2->v3: > * No change. > v1->v2: > * Modelled as a single PWM device handling multiple channles. > * Used PM framework to manage the clocks. > --- > drivers/pwm/Kconfig | 11 + > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-rz-mtu3.c | 482 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 494 insertions(+) > create mode 100644 drivers/pwm/pwm-rz-mtu3.c > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index dae023d783a2..ccc0299fd0dd 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -481,6 +481,17 @@ config PWM_ROCKCHIP > Generic PWM framework driver for the PWM controller found on > Rockchip SoCs. > > +config PWM_RZ_MTU3 > + tristate "Renesas RZ/G2L MTU3a PWM Timer support" > + depends on RZ_MTU3 || COMPILE_TEST > + depends on HAS_IOMEM > + help > + This driver exposes the MTU3a PWM Timer controller found in Renesas > + RZ/G2L like chips through the PWM API. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-rz-mtu3. > + > 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 7bf1a29f02b8..b85fc9fba326 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -44,6 +44,7 @@ obj-$(CONFIG_PWM_RASPBERRYPI_POE) += pwm-raspberrypi-poe.o > 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_SAMSUNG) += pwm-samsung.o > obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o > obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o > diff --git a/drivers/pwm/pwm-rz-mtu3.c b/drivers/pwm/pwm-rz-mtu3.c > new file mode 100644 > index 000000000000..146948656755 > --- /dev/null > +++ b/drivers/pwm/pwm-rz-mtu3.c > @@ -0,0 +1,482 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Renesas RZ/G2L MTU3a PWM Timer driver > + * > + * Copyright (C) 2022 Renesas Electronics Corporation 2023? > + * > + * Hardware manual for this IP can be found here > + * https://www.renesas.com/eu/en/document/mah/rzg2l-group-rzg2lc-group-users-manual-hardware-0?language=en > + * > + * Limitations: > + * - When PWM is disabled, the output is driven to Hi-Z. > + * - While the hardware supports both polarities, the driver (for now) > + * only handles normal polarity. > + * - HW uses one counter and two match components to configure duty_cycle > + * and period. > + * - Multi-Function Timer Pulse Unit(a.k.a MTU) has 7 HW channels for PWM s/t(/t (/ > + * operations(The channels are MTU{0..4, 6, 7}). s/s(/s. (/; s/)./.)/ > + * - MTU{1, 2} channels have a single IO, whereas all other HW channels have > + * 2 IOs. > + * - Each IO is modelled as an independent PWM channel. > + * - rz_mtu3_hw_channel_ios table is used to map the PWM channel to the > + * corresponding HW channel as there are difference in number of IOs > + * between HW channels. > + */ > + > +#include <linux/bitfield.h> > +#include <linux/clk.h> > +#include <linux/limits.h> > +#include <linux/mfd/rz-mtu3.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/pwm.h> > +#include <linux/time.h> > + > +#define RZ_MTU3_TIOR_OC_RETAIN (0) > +#define RZ_MTU3_TIOR_OC_0_H_COMP_MATCH (2) > +#define RZ_MTU3_TIOR_OC_1_TOGGLE (7) > +#define RZ_MTU3_TIOR_OC_IOA GENMASK(3, 0) > + > +#define RZ_MTU3_TCR_CCLR_TGRC (5 << 5) > +#define RZ_MTU3_TCR_CKEG_RISING (0 << 3) > + > +#define RZ_MTU3_TCR_TPCS GENMASK(2, 0) I assume the other register definitions are in <linux/mfd/rz-mtu3.h>, I suggest to move these there, too. > + > +#define RZ_MTU3_MAX_PWM_CHANNELS (12) > + > +#define RZ_MTU3_MAX_HW_CHANNELS (7) > + > +/* The table represents the number of IOs on each MTU HW channel. */ > +static const u8 rz_mtu3_hw_channel_ios[] = { 2, 1, 1, 2, 2, 2, 2 }; OK, so you have twelve PWM outputs, each one is driven by a "channel". Each channel drives one or two PWMs according to the above array, right? > + > +/** > + * struct rz_mtu3_pwm_chip - MTU3 pwm private data > + * > + * @chip: MTU3 pwm chip data > + * @clk: MTU3 module clock > + * @lock: Lock to prevent concurrent access for usage count > + * @rate: MTU3 clock rate > + * @user_count: MTU3 usage count > + * @rz_mtu3_channel: HW channels for the PWM > + */ > + > +struct rz_mtu3_pwm_chip { > + struct pwm_chip chip; > + struct clk *clk; > + struct mutex lock; > + unsigned long rate; > + u32 user_count[RZ_MTU3_MAX_HW_CHANNELS]; > + struct rz_mtu3_channel *ch[RZ_MTU3_MAX_HW_CHANNELS]; > +}; > + > +static inline struct rz_mtu3_pwm_chip *to_rz_mtu3_pwm_chip(struct pwm_chip *chip) > +{ > + return container_of(chip, struct rz_mtu3_pwm_chip, chip); > +} > + > +static u8 rz_mtu3_pwm_calculate_prescale(struct rz_mtu3_pwm_chip *rz_mtu3, > + u64 period_cycles) > +{ > + u32 prescaled_period_cycles; > + u8 prescale; > + > + prescaled_period_cycles = period_cycles >> 16; > + if (prescaled_period_cycles >= 16) > + prescale = 3; > + else > + prescale = (fls(prescaled_period_cycles) + 1) / 2; > + > + return prescale; > +} > + > +static struct rz_mtu3_channel * > +rz_mtu3_get_hw_channel(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, u32 channel) > +{ > + unsigned int i, ch_index = 0; > + > + for (i = 0; i < ARRAY_SIZE(rz_mtu3_hw_channel_ios); i++) { > + ch_index += rz_mtu3_hw_channel_ios[i]; > + > + if (ch_index > channel) > + break; > + } > + > + return rz_mtu3_pwm->ch[i]; > +} This function determines the channel that drives pwm $channel. Maybe pick more sensible names? Something like "hwpwm" instead of "channel" and "channel" instead of "hw_channel" (and "ch"). I think the driver would be easier to understand if the naming was used in a consistent way. > +static u32 rz_mtu3_get_hw_channel_index(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > + struct rz_mtu3_channel *ch) > +{ > + u32 i; > + > + for (i = 0; i < ARRAY_SIZE(rz_mtu3_hw_channel_ios); i++) { > + if (ch == rz_mtu3_pwm->ch[i]) > + break; > + } > + > + return i; > +} This is a complicated way to express ch - rz_mtu3_pwm->ch isn't it? > + > +static bool rz_mtu3_pwm_is_second_channel(u32 ch_index, u32 hwpwm) > +{ > + u32 i, pwm_ch_index = 0; > + > + for (i = 0; i < ch_index; i++) > + pwm_ch_index += rz_mtu3_hw_channel_ios[i]; > + > + return pwm_ch_index != hwpwm; > +} I think the three functions above could be simplified (or not needed) if you represent the channel -> pwm mapping in a different way. I'd do something like: ch = rz_mtu3_pwm_get_channel(..., hwpwm); and in *ch store the index of the lowest PWM and the number of PWMs handled by this channel. > +static bool rz_mtu3_pwm_is_ch_enabled(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > + u32 hwpwm) > +{ > + struct rz_mtu3_channel *ch; > + bool is_channel_en; > + u32 ch_index; > + u8 val; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + is_channel_en = rz_mtu3_is_enabled(ch); > + > + if (rz_mtu3_pwm_is_second_channel(ch_index, hwpwm)) > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORL); > + else > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORH); > + > + return (is_channel_en && (val & RZ_MTU3_TIOR_OC_IOA)); You could exit early (and so skip reading from the hardware) if is_channel_en is false. > +} > + > +static int rz_mtu3_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + struct rz_mtu3_channel *ch; > + u32 ch_index; > + > + mutex_lock(&rz_mtu3_pwm->lock); > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); ch and ch_index can be determined before taking the lock, can it not? I'd only work with a single variable that holds the value of your ch_index, call it "ch" and use rz_mtu3_pwm->channel[ch]. This is simpler and so easier to understand for a code reader. > + if (!rz_mtu3_pwm->user_count[ch_index] && !rz_mtu3_request_channel(ch)) { > + mutex_unlock(&rz_mtu3_pwm->lock); > + return -EBUSY; > + } This would be easier to read if written as follows: /* * Each channel must be requested only once, so if the channel * serves two PWMs and the other is already requested, skip over * rz_mtu3_request_channel() */ if (!rz_mtu3_pwm->user_count[ch_index]) { ret = rz_mtu3_request_channel(ch); if (ret) return ret; /* or -EBUSY? */ } > + > + rz_mtu3_pwm->user_count[ch_index]++; > + mutex_unlock(&rz_mtu3_pwm->lock); > + > + return 0; > +} > + > +static void rz_mtu3_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + struct rz_mtu3_channel *ch; > + u32 ch_index; > + > + mutex_lock(&rz_mtu3_pwm->lock); > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + > + rz_mtu3_pwm->user_count[ch_index]--; > + if (!rz_mtu3_pwm->user_count[ch_index]) > + rz_mtu3_release_channel(ch); > + > + mutex_unlock(&rz_mtu3_pwm->lock); > +} > + > +static int rz_mtu3_pwm_enable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > + struct pwm_device *pwm) > +{ > + struct rz_mtu3_channel *ch; > + u32 ch_index; > + u8 val; > + int rc; > + > + rc = pm_runtime_resume_and_get(rz_mtu3_pwm->chip.dev); > + if (rc) > + return rc; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + val = (RZ_MTU3_TIOR_OC_1_TOGGLE << 4) | RZ_MTU3_TIOR_OC_0_H_COMP_MATCH; Introduce a symbol for the 4 above? This is the only use of RZ_MTU3_TIOR_OC_1_TOGGLE, maybe move the shift to the definition of that symbol? > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_MD_PWMMODE1); > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, val); > + else > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, val); > + > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > + rz_mtu3_enable(ch); That looks wrong. user_count signals if the channel is used for one or two PWMs, right? If both PWMs are requested before any of them is enabled, rz_mtu3_enable is never called, isn't it? > + return 0; > +} > + > +static void rz_mtu3_pwm_disable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > + struct pwm_device *pwm) > +{ > + struct rz_mtu3_channel *ch; > + u32 ch_index; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + > + /* Return to normal mode and disable output pins of MTU3 channel */ > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_MD_NORMAL); > + > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, RZ_MTU3_TIOR_OC_RETAIN); > + else > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, RZ_MTU3_TIOR_OC_RETAIN); > + > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > + rz_mtu3_disable(ch); > + > + pm_runtime_put_sync(rz_mtu3_pwm->chip.dev); > +} > + > +static int rz_mtu3_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, > + struct pwm_state *state) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + struct rz_mtu3_channel *ch; > + u8 prescale, val; > + u32 ch_index; > + u16 dc, pv; > + u64 tmp; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + pm_runtime_get_sync(chip->dev); > + state->enabled = rz_mtu3_pwm_is_ch_enabled(rz_mtu3_pwm, pwm->hwpwm); > + if (state->enabled) { Most variables can have a narrower scope and be declared here. > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TCR); > + prescale = FIELD_GET(RZ_MTU3_TCR_TPCS, val); > + > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) { > + dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRD); > + pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRC); > + } else { > + dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRB); > + pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRA); > + } > + > + /* With prescale <= 7 and pv <= 0xffff this doesn't overflow. */ > + tmp = NSEC_PER_SEC * (u64)pv << (2 * prescale); > + state->period = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate); > + tmp = NSEC_PER_SEC * (u64)dc << (2 * prescale); > + state->duty_cycle = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate); > + } > + > + if (state->duty_cycle > state->period) > + state->duty_cycle = state->period; > + > + state->polarity = PWM_POLARITY_NORMAL; > + pm_runtime_put(chip->dev); > + > + return 0; > +} > + > +static int rz_mtu3_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + struct rz_mtu3_channel *ch; > + unsigned long pv, dc; > + u64 period_cycles; > + u64 duty_cycles; > + u32 ch_index; > + u8 prescale; > + u8 val; > + > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > + period_cycles = mul_u64_u32_div(state->period, rz_mtu3_pwm->rate, > + NSEC_PER_SEC); > + prescale = rz_mtu3_pwm_calculate_prescale(rz_mtu3_pwm, period_cycles); > + > + if (period_cycles >> (2 * prescale) <= U16_MAX) > + pv = period_cycles >> (2 * prescale); > + else > + pv = U16_MAX; > + > + duty_cycles = mul_u64_u32_div(state->duty_cycle, rz_mtu3_pwm->rate, > + NSEC_PER_SEC); > + if (duty_cycles >> (2 * prescale) <= U16_MAX) > + dc = duty_cycles >> (2 * prescale); > + else > + dc = U16_MAX; > + > + /* > + * 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(chip->dev); > + > + val = RZ_MTU3_TCR_CKEG_RISING | prescale; > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) { > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, > + RZ_MTU3_TCR_CCLR_TGRC | val); > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRD, dc); > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRC, pv); > + } else { > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, > + RZ_MTU3_TCR_CCLR_TGRA | val); > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRB, dc); > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRA, pv); > + } > + > + /* If the PWM is not enabled, turn the clock off again to save power. */ > + if (!pwm->state.enabled) > + pm_runtime_put(chip->dev); > + > + return 0; > +} > + > +static int rz_mtu3_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > + bool enabled = pwm->state.enabled; > + int ret; > + > + if (state->polarity != PWM_POLARITY_NORMAL) > + return -EINVAL; > + > + if (!state->enabled) { > + if (enabled) > + rz_mtu3_pwm_disable(rz_mtu3_pwm, pwm); > + > + return 0; > + } > + > + ret = rz_mtu3_pwm_config(chip, pwm, state); > + if (ret) > + return ret; > + > + if (!enabled) > + ret = rz_mtu3_pwm_enable(rz_mtu3_pwm, pwm); > + > + return ret; > +} > + > +static const struct pwm_ops rz_mtu3_pwm_ops = { > + .request = rz_mtu3_pwm_request, > + .free = rz_mtu3_pwm_free, > + .get_state = rz_mtu3_pwm_get_state, > + .apply = rz_mtu3_pwm_apply, > + .owner = THIS_MODULE, > +}; > + > +static int rz_mtu3_pwm_pm_runtime_suspend(struct device *dev) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = dev_get_drvdata(dev); > + > + clk_disable_unprepare(rz_mtu3_pwm->clk); > + > + return 0; > +} > + > +static int rz_mtu3_pwm_pm_runtime_resume(struct device *dev) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = dev_get_drvdata(dev); > + > + return clk_prepare_enable(rz_mtu3_pwm->clk); > +} > + > +static DEFINE_RUNTIME_DEV_PM_OPS(rz_mtu3_pwm_pm_ops, > + rz_mtu3_pwm_pm_runtime_suspend, > + rz_mtu3_pwm_pm_runtime_resume, NULL); > + > +static void rz_mtu3_pwm_pm_disable(void *data) > +{ > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = data; > + > + clk_rate_exclusive_put(rz_mtu3_pwm->clk); > + pm_runtime_disable(rz_mtu3_pwm->chip.dev); > + pm_runtime_set_suspended(rz_mtu3_pwm->chip.dev); > +} > + > +static int rz_mtu3_pwm_probe(struct platform_device *pdev) > +{ > + struct rz_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent); > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm; > + struct device *dev = &pdev->dev; > + int num_pwm_hw_ch = 0; > + unsigned int i; > + int ret; > + > + rz_mtu3_pwm = devm_kzalloc(&pdev->dev, sizeof(*rz_mtu3_pwm), GFP_KERNEL); > + if (!rz_mtu3_pwm) > + return -ENOMEM; > + > + rz_mtu3_pwm->clk = ddata->clk; > + > + for (i = 0; i < RZ_MTU_NUM_CHANNELS; i++) { > + if (i == RZ_MTU3_CHAN_5 || i == RZ_MTU3_CHAN_8) > + continue; > + > + rz_mtu3_pwm->ch[num_pwm_hw_ch] = &ddata->channels[i]; > + rz_mtu3_pwm->ch[num_pwm_hw_ch]->dev = dev; > + num_pwm_hw_ch++; > + } > + > + mutex_init(&rz_mtu3_pwm->lock); > + platform_set_drvdata(pdev, rz_mtu3_pwm); > + ret = clk_prepare_enable(rz_mtu3_pwm->clk); > + if (ret) > + return dev_err_probe(dev, ret, "Clock enable failed\n"); > + > + clk_rate_exclusive_get(rz_mtu3_pwm->clk); > + > + rz_mtu3_pwm->rate = clk_get_rate(rz_mtu3_pwm->clk); > + /* > + * Refuse clk rates > 1 GHz to prevent overflow later for computing > + * period and duty cycle. > + */ > + if (rz_mtu3_pwm->rate > NSEC_PER_SEC) { > + ret = -EINVAL; > + clk_rate_exclusive_put(rz_mtu3_pwm->clk); > + goto disable_clock; > + } > + > + pm_runtime_set_active(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + rz_mtu3_pwm->chip.dev = &pdev->dev; > + ret = devm_add_action_or_reset(&pdev->dev, > + rz_mtu3_pwm_pm_disable, > + rz_mtu3_pwm); > + if (ret < 0) > + return ret; > + > + rz_mtu3_pwm->chip.ops = &rz_mtu3_pwm_ops; > + rz_mtu3_pwm->chip.npwm = RZ_MTU3_MAX_PWM_CHANNELS; > + ret = devm_pwmchip_add(&pdev->dev, &rz_mtu3_pwm->chip); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n"); > + > + pm_runtime_idle(&pdev->dev); > + > + return 0; > + > +disable_clock: > + clk_disable_unprepare(rz_mtu3_pwm->clk); > + return ret; > +} .probe() enables rz_mtu3_pwm->clk, but there is no .remove() that cares about disabling it again. > + > +static struct platform_driver rz_mtu3_pwm_driver = { > + .driver = { > + .name = "pwm-rz-mtu3", > + .pm = pm_ptr(&rz_mtu3_pwm_pm_ops), > + }, > + .probe = rz_mtu3_pwm_probe, > +}; > +module_platform_driver(rz_mtu3_pwm_driver); > + > +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>"); > +MODULE_ALIAS("platform:pwm-rz-mtu3"); > +MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a PWM Timer Driver"); > +MODULE_LICENSE("GPL"); Best regards Uwe
Hi Uwe, Thanks for the feedback. > Subject: Re: [PATCH v14 6/6] pwm: Add Renesas RZ/G2L MTU3a PWM driver > > Hello, > > I have the feeling this driver is more complicated than necessary. I agree, your suggestions made it less complex. > > On Fri, Mar 10, 2023 at 05:06:54PM +0000, Biju Das wrote: > > The RZ/G2L Multi-Function Timer Pulse Unit 3 (a.k.a MTU3a) uses one > > counter and two match components to configure duty_cycle and period to > > generate PWM output waveform. > > > > Add basic support for RZ/G2L MTU3a PWM driver by creating separate PWM > > channels for each IOs. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v13->v14: > > * Updated commit description > > * Updated Limitations section. > > * Replaced the macros RZ_MTU*->RZ_MTU3_CHAN_* in probe() > > * Fixed a kernel crash in error path by moving rz_mtu3_pwm->chip.dev > before > > devm_add_action_or_reset() > > * Added pm_runtime_idle() and simplified error paths for > devm_add_action_or_reset() > > and devm_pwmchip_add(). > > > > v12->v13: > > * Updated commit description > > * Moved RZ_MTU3_TMDR1_MD_* macros to rz_mtu3.h > > * Updated Limitations section. > > * Removed PWM mode1 references from the driver. > > * Dropped prescale and duty_cycle from struct rz_mtu3_pwm_chip. > > * Replaced rz_mtu3_pwm_mode1_num_ios->rz_mtu3_hw_channel_ios. > > * Avoided race condition in rz_mtu3_pwm_request()/rz_mtu3_pwm_free(). > > * Updated get_state() by adding dc > pv check and added a comment about > > overflow condition. > > * Moved overflow condition check from config->probe() > > * Replaced pm_runtime_resume_and_get with unconditional > pm_runtime_get_sync() > > in config() > > * Added error check for clk_prepare_enable() in probe() and propagating > error > > to the caller for pm_runtime_resume() > > * clk_get_rate() is called after enabling the clock and > > clk_rate_exclusive_put() > > v11->v12: > > * Updated header file to <linux/mfd/rz-mtu3.h> as core driver is in MFD. > > * Reordered get_state() > > v10->v11: > > * No change. > > v9->v10: > > * No change. > > v8->v9: > > * Added prescale/duty_cycle variables to struct rz_mtu3_pwm_chip and > > cached this values in rz_mtu3_pwm_config and used this cached values > > in get_state(), if PWM is disabled. > > * Added return code for get_state() > > v7->v8: > > * Simplified rz_mtu3_pwm_request by calling rz_mtu3_request_channel() > > * Simplified rz_mtu3_pwm_free by calling rz_mtu3_release_channel() > > v6->v7: > > * Added channel specific mutex lock to avoid race between counter > > device and rz_mtu3_pwm_{request,free} > > * Added pm_runtime_resume_and_get in rz_mtu3_pwm_enable() > > * Added pm_runtime_put_sync in rz_mtu3_pwm_disable() > > * Updated rz_mtu3_pwm_config() > > * Updated rz_mtu3_pwm_apply() > > v5->v6: > > * Updated commit and Kconfig description > > * Sorted the header > > * Replaced dev_get_drvdata from rz_mtu3_pwm_pm_disable() > > * Replaced SET_RUNTIME_PM_OPS->DEFINE_RUNTIME_DEV_PM_OPS and removed > > __maybe_unused from suspend/resume() > > v4->v5: > > * pwm device is instantiated by mtu3a core driver. > > v3->v4: > > * There is no resource associated with "rz-mtu3-pwm" compatible > > and moved the code to mfd subsystem as it binds against "rz-mtu". > > * Removed struct platform_driver rz_mtu3_pwm_driver. > > v2->v3: > > * No change. > > v1->v2: > > * Modelled as a single PWM device handling multiple channles. > > * Used PM framework to manage the clocks. > > --- > > drivers/pwm/Kconfig | 11 + > > drivers/pwm/Makefile | 1 + > > drivers/pwm/pwm-rz-mtu3.c | 482 > > ++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 494 insertions(+) > > create mode 100644 drivers/pwm/pwm-rz-mtu3.c > > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index > > dae023d783a2..ccc0299fd0dd 100644 > > --- a/drivers/pwm/Kconfig > > +++ b/drivers/pwm/Kconfig > > @@ -481,6 +481,17 @@ config PWM_ROCKCHIP > > Generic PWM framework driver for the PWM controller found on > > Rockchip SoCs. > > > > +config PWM_RZ_MTU3 > > + tristate "Renesas RZ/G2L MTU3a PWM Timer support" > > + depends on RZ_MTU3 || COMPILE_TEST > > + depends on HAS_IOMEM > > + help > > + This driver exposes the MTU3a PWM Timer controller found in Renesas > > + RZ/G2L like chips through the PWM API. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called pwm-rz-mtu3. > > + > > 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 7bf1a29f02b8..b85fc9fba326 100644 > > --- a/drivers/pwm/Makefile > > +++ b/drivers/pwm/Makefile > > @@ -44,6 +44,7 @@ obj-$(CONFIG_PWM_RASPBERRYPI_POE) += pwm-raspberrypi- > poe.o > > 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_SAMSUNG) += pwm-samsung.o > > obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o > > obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o > > diff --git a/drivers/pwm/pwm-rz-mtu3.c b/drivers/pwm/pwm-rz-mtu3.c new > > file mode 100644 index 000000000000..146948656755 > > --- /dev/null > > +++ b/drivers/pwm/pwm-rz-mtu3.c > > @@ -0,0 +1,482 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Renesas RZ/G2L MTU3a PWM Timer driver > > + * > > + * Copyright (C) 2022 Renesas Electronics Corporation > > 2023? OK. > > > + * > > + * Hardware manual for this IP can be found here > > + * > > + https://www.renesas.com/eu/en/document/mah/rzg2l-group-rzg2lc-group- > > + users-manual-hardware-0?language=en > > + * > > + * Limitations: > > + * - When PWM is disabled, the output is driven to Hi-Z. > > + * - While the hardware supports both polarities, the driver (for now) > > + * only handles normal polarity. > > + * - HW uses one counter and two match components to configure duty_cycle > > + * and period. > > + * - Multi-Function Timer Pulse Unit(a.k.a MTU) has 7 HW channels for > > + PWM > > s/t(/t (/ Done. > > > + * operations(The channels are MTU{0..4, 6, 7}). > > s/s(/s. (/; s/)./.)/ Done. > > > + * - MTU{1, 2} channels have a single IO, whereas all other HW channels > have > > + * 2 IOs. > > + * - Each IO is modelled as an independent PWM channel. > > + * - rz_mtu3_hw_channel_ios table is used to map the PWM channel to the > > + * corresponding HW channel as there are difference in number of IOs > > + * between HW channels. > > + */ > > + > > +#include <linux/bitfield.h> > > +#include <linux/clk.h> > > +#include <linux/limits.h> > > +#include <linux/mfd/rz-mtu3.h> > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/pm_runtime.h> > > +#include <linux/pwm.h> > > +#include <linux/time.h> > > + > > +#define RZ_MTU3_TIOR_OC_RETAIN (0) > > +#define RZ_MTU3_TIOR_OC_0_H_COMP_MATCH (2) > > +#define RZ_MTU3_TIOR_OC_1_TOGGLE (7) > > +#define RZ_MTU3_TIOR_OC_IOA GENMASK(3, 0) > > + > > +#define RZ_MTU3_TCR_CCLR_TGRC (5 << 5) > > +#define RZ_MTU3_TCR_CKEG_RISING (0 << 3) > > + > > +#define RZ_MTU3_TCR_TPCS GENMASK(2, 0) > > I assume the other register definitions are in <linux/mfd/rz-mtu3.h>, I > suggest to move these there, too. Yep moved the definitions. /* Macros for setting registers */ +#define RZ_MTU3_TCR_CCLR GENMASK(7, 5) +#define RZ_MTU3_TCR_CKEG GENMASK(4, 3) +#define RZ_MTU3_TCR_TPCS GENMASK(2, 0) #define RZ_MTU3_TCR_CCLR_TGRA BIT(5) +#define RZ_MTU3_TCR_CCLR_TGRC FIELD_PREP(RZ_MTU3_TCR_CCLR, 5) +#define RZ_MTU3_TCR_CKEG_RISING FIELD_PREP(RZ_MTU3_TCR_CKEG, 0) + +#define RZ_MTU3_TIOR_IOB GENMASK(7, 4) +#define RZ_MTU3_TIOR_IOA GENMASK(3, 0) +#define RZ_MTU3_TIOR_OC_RETAIN (0) +#define RZ_MTU3_TIOR_OC_INIT_OUT_LO_HI_OUT (2) +#define RZ_MTU3_TIOR_OC_INIT_OUT_HI_TOGGLE_OUT (7) + +#define RZ_MTU3_TIOR_OC_IOA_H_COMP_MATCH \ + FIELD_PREP(RZ_MTU3_TIOR_IOA, RZ_MTU3_TIOR_OC_INIT_OUT_LO_HI_OUT) +#define RZ_MTU3_TIOR_OC_IOB_TOGGLE \ + FIELD_PREP(RZ_MTU3_TIOR_IOB, RZ_MTU3_TIOR_OC_INIT_OUT_HI_TOGGLE_OUT) > > > + > > +#define RZ_MTU3_MAX_PWM_CHANNELS (12) > > + > > +#define RZ_MTU3_MAX_HW_CHANNELS (7) > > + > > +/* The table represents the number of IOs on each MTU HW channel. */ > > +static const u8 rz_mtu3_hw_channel_ios[] = { 2, 1, 1, 2, 2, 2, 2 }; > > OK, so you have twelve PWM outputs, each one is driven by a "channel". > Each channel drives one or two PWMs according to the above array, right? Yes that is correct. Now as per your suggestion, it is mapped differently. +/** + * struct rz_mtu3_channel_io_map - MTU3 channel io map + * + * @pwm_index: Index of the lowest pwm channel + * @num: number of IOs on the channel. + */ + +struct rz_mtu3_channel_io_map { + u8 pwm_index; + u8 num; +}; + +static const struct rz_mtu3_channel_io_map channel_map[] = + { { 0, 2 }, { 2, 1 }, { 3, 1 }, { 4 , 2 }, { 6 ,2 } , { 8, 2 }, { 10, 2 } }; > > > + > > +/** > > + * struct rz_mtu3_pwm_chip - MTU3 pwm private data > > + * > > + * @chip: MTU3 pwm chip data > > + * @clk: MTU3 module clock > > + * @lock: Lock to prevent concurrent access for usage count > > + * @rate: MTU3 clock rate > > + * @user_count: MTU3 usage count > > + * @rz_mtu3_channel: HW channels for the PWM */ > > + > > +struct rz_mtu3_pwm_chip { > > + struct pwm_chip chip; > > + struct clk *clk; > > + struct mutex lock; > > + unsigned long rate; > > + u32 user_count[RZ_MTU3_MAX_HW_CHANNELS]; > > + struct rz_mtu3_channel *ch[RZ_MTU3_MAX_HW_CHANNELS]; }; > > + > > +static inline struct rz_mtu3_pwm_chip *to_rz_mtu3_pwm_chip(struct > > +pwm_chip *chip) { > > + return container_of(chip, struct rz_mtu3_pwm_chip, chip); } > > + > > +static u8 rz_mtu3_pwm_calculate_prescale(struct rz_mtu3_pwm_chip > *rz_mtu3, > > + u64 period_cycles) > > +{ > > + u32 prescaled_period_cycles; > > + u8 prescale; > > + > > + prescaled_period_cycles = period_cycles >> 16; > > + if (prescaled_period_cycles >= 16) > > + prescale = 3; > > + else > > + prescale = (fls(prescaled_period_cycles) + 1) / 2; > > + > > + return prescale; > > +} > > + > > +static struct rz_mtu3_channel * > > +rz_mtu3_get_hw_channel(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, u32 > > +channel) { > > + unsigned int i, ch_index = 0; > > + > > + for (i = 0; i < ARRAY_SIZE(rz_mtu3_hw_channel_ios); i++) { > > + ch_index += rz_mtu3_hw_channel_ios[i]; > > + > > + if (ch_index > channel) > > + break; > > + } > > + > > + return rz_mtu3_pwm->ch[i]; > > +} > > This function determines the channel that drives pwm $channel. Maybe pick > more sensible names? Something like "hwpwm" instead of "channel" > and "channel" instead of "hw_channel" (and "ch"). I think the driver would > be easier to understand if the naming was used in a consistent way. OK. > > > +static u32 rz_mtu3_get_hw_channel_index(struct rz_mtu3_pwm_chip > *rz_mtu3_pwm, > > + struct rz_mtu3_channel *ch) > > +{ > > + u32 i; > > + > > + for (i = 0; i < ARRAY_SIZE(rz_mtu3_hw_channel_ios); i++) { > > + if (ch == rz_mtu3_pwm->ch[i]) > > + break; > > + } > > + > > + return i; > > +} > > This is a complicated way to express > > ch - rz_mtu3_pwm->ch > > isn't it? Agreed. > > > + > > +static bool rz_mtu3_pwm_is_second_channel(u32 ch_index, u32 hwpwm) { > > + u32 i, pwm_ch_index = 0; > > + > > + for (i = 0; i < ch_index; i++) > > + pwm_ch_index += rz_mtu3_hw_channel_ios[i]; > > + > > + return pwm_ch_index != hwpwm; > > +} > > I think the three functions above could be simplified (or not needed) if you > represent the channel -> pwm mapping in a different way. > > I'd do something like: > > ch = rz_mtu3_pwm_get_channel(..., hwpwm); > > and in *ch store the index of the lowest PWM and the number of PWMs handled > by this channel. Agreed defined below structure to take care this. +/** + * struct rz_mtu3_pwm_channel - MTU3 pwm channel + * + * @rz_mtu3_channel: HW channel for the PWM + * @pwm_index: Index of the pwm channel + * @num: number of IOs on the channel. + */ +struct rz_mtu3_pwm_channel { + struct rz_mtu3_channel *mtu; + u8 pwm_index; + u8 num; +}; @@ -67,9 +67,26 @@ struct rz_mtu3_pwm_chip { struct mutex lock; unsigned long rate; u32 user_count[RZ_MTU3_MAX_HW_CHANNELS]; - struct rz_mtu3_channel *ch[RZ_MTU3_MAX_HW_CHANNELS]; + struct rz_mtu3_pwm_channel channel[RZ_MTU3_MAX_HW_CHANNELS]; }; static struct rz_mtu3_pwm_channel * +rz_mtu3_get_channel(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, u32 hwpwm) { + unsigned int i; + for (i = 0; i < RZ_MTU3_MAX_HW_CHANNELS; i++) { + if (rz_mtu3_pwm->channel[i].pwm_index + rz_mtu3_pwm->channel[i].num > hwpwm) break; } + return &rz_mtu3_pwm->channel[i]; } > > > +static bool rz_mtu3_pwm_is_ch_enabled(struct rz_mtu3_pwm_chip > *rz_mtu3_pwm, > > + u32 hwpwm) > > +{ > > + struct rz_mtu3_channel *ch; > > + bool is_channel_en; > > + u32 ch_index; > > + u8 val; > > + > > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, hwpwm); > > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > > + is_channel_en = rz_mtu3_is_enabled(ch); > > + > > + if (rz_mtu3_pwm_is_second_channel(ch_index, hwpwm)) > > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORL); > > + else > > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORH); > > + > > + return (is_channel_en && (val & RZ_MTU3_TIOR_OC_IOA)); > > You could exit early (and so skip reading from the hardware) if > is_channel_en is false. Agreed. > > > +} > > + > > +static int rz_mtu3_pwm_request(struct pwm_chip *chip, struct > > +pwm_device *pwm) { > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > > + struct rz_mtu3_channel *ch; > > + u32 ch_index; > > + > > + mutex_lock(&rz_mtu3_pwm->lock); > > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > > ch and ch_index can be determined before taking the lock, can it not? Agreed. > > I'd only work with a single variable that holds the value of your ch_index, > call it "ch" and use rz_mtu3_pwm->channel[ch]. This is simpler and so easier > to understand for a code reader. > > > + if (!rz_mtu3_pwm->user_count[ch_index] && > !rz_mtu3_request_channel(ch)) { > > + mutex_unlock(&rz_mtu3_pwm->lock); > > + return -EBUSY; > > + } > > This would be easier to read if written as follows: > > /* > * Each channel must be requested only once, so if the channel > * serves two PWMs and the other is already requested, skip over > * rz_mtu3_request_channel() > */ > if (!rz_mtu3_pwm->user_count[ch_index]) { > ret = rz_mtu3_request_channel(ch); > if (ret) > return ret; /* or -EBUSY? */ > } Ok, it is rewritten as + if (!rz_mtu3_pwm->user_count[index]) { + ch_idle = rz_mtu3_request_channel(ch->mtu); + if (!ch_idle) { + mutex_unlock(&rz_mtu3_pwm->lock); + return -EBUSY; + } } Where index = ch - rz_mtu3_pwm->channel; > > > + > > + rz_mtu3_pwm->user_count[ch_index]++; > > + mutex_unlock(&rz_mtu3_pwm->lock); > > + > > + return 0; > > +} > > + > > +static void rz_mtu3_pwm_free(struct pwm_chip *chip, struct pwm_device > > +*pwm) { > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > > + struct rz_mtu3_channel *ch; > > + u32 ch_index; > > + > > + mutex_lock(&rz_mtu3_pwm->lock); > > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > > + > > + rz_mtu3_pwm->user_count[ch_index]--; > > + if (!rz_mtu3_pwm->user_count[ch_index]) > > + rz_mtu3_release_channel(ch); > > + > > + mutex_unlock(&rz_mtu3_pwm->lock); > > +} > > + > > +static int rz_mtu3_pwm_enable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > > + struct pwm_device *pwm) > > +{ > > + struct rz_mtu3_channel *ch; > > + u32 ch_index; > > + u8 val; > > + int rc; > > + > > + rc = pm_runtime_resume_and_get(rz_mtu3_pwm->chip.dev); > > + if (rc) > > + return rc; > > + > > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > > + val = (RZ_MTU3_TIOR_OC_1_TOGGLE << 4) | > > +RZ_MTU3_TIOR_OC_0_H_COMP_MATCH; > > Introduce a symbol for the 4 above? This is the only use of > RZ_MTU3_TIOR_OC_1_TOGGLE, maybe move the shift to the definition of that > symbol? Done. + val = RZ_MTU3_TIOR_OC_IOB_TOGGLE | RZ_MTU3_TIOR_OC_IOA_H_COMP_MATCH; > > > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_MD_PWMMODE1); > > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) > > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, val); > > + else > > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, val); > > + > > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > > + rz_mtu3_enable(ch); > > That looks wrong. user_count signals if the channel is used for one or two > PWMs, right? If both PWMs are requested before any of them is enabled, > rz_mtu3_enable is never called, isn't it? OK, Introduced enable_count to take care this in enable/disable function. @@ -67,9 +67,26 @@ struct rz_mtu3_pwm_chip { struct mutex lock; unsigned long rate; u32 user_count[RZ_MTU3_MAX_HW_CHANNELS]; + u32 enable_count[RZ_MTU3_MAX_HW_CHANNELS]; }; Enable: + mutex_lock(&rz_mtu3_pwm->lock); + if (!rz_mtu3_pwm->enable_count[index]) + rz_mtu3_enable(ch->mtu); + + rz_mtu3_pwm->enable_count[index]++; + mutex_unlock(&rz_mtu3_pwm->lock); Disable: + mutex_lock(&rz_mtu3_pwm->lock); + rz_mtu3_pwm->enable_count[index]--; + if (!rz_mtu3_pwm->enable_count[index]) + rz_mtu3_disable(ch->mtu); + mutex_unlock(&rz_mtu3_pwm->lock); > > > + return 0; > > +} > > + > > +static void rz_mtu3_pwm_disable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, > > + struct pwm_device *pwm) > > +{ > > + struct rz_mtu3_channel *ch; > > + u32 ch_index; > > + > > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > > + > > + /* Return to normal mode and disable output pins of MTU3 channel */ > > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, > RZ_MTU3_TMDR1_MD_NORMAL); > > + > > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) > > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, > RZ_MTU3_TIOR_OC_RETAIN); > > + else > > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, > RZ_MTU3_TIOR_OC_RETAIN); > > + > > + if (rz_mtu3_pwm->user_count[ch_index] <= 1) > > + rz_mtu3_disable(ch); > > + > > + pm_runtime_put_sync(rz_mtu3_pwm->chip.dev); > > +} > > + > > +static int rz_mtu3_pwm_get_state(struct pwm_chip *chip, struct pwm_device > *pwm, > > + struct pwm_state *state) > > +{ > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > > + struct rz_mtu3_channel *ch; > > + u8 prescale, val; > > + u32 ch_index; > > + u16 dc, pv; > > + u64 tmp; > > + > > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > > + pm_runtime_get_sync(chip->dev); > > + state->enabled = rz_mtu3_pwm_is_ch_enabled(rz_mtu3_pwm, pwm->hwpwm); > > + if (state->enabled) { > > Most variables can have a narrower scope and be declared here. OK. > > > + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TCR); > > + prescale = FIELD_GET(RZ_MTU3_TCR_TPCS, val); > > + > > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) { > > + dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRD); > > + pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRC); > > + } else { > > + dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRB); > > + pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRA); > > + } > > + > > + /* With prescale <= 7 and pv <= 0xffff this doesn't overflow. > */ > > + tmp = NSEC_PER_SEC * (u64)pv << (2 * prescale); > > + state->period = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate); > > + tmp = NSEC_PER_SEC * (u64)dc << (2 * prescale); > > + state->duty_cycle = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate); > > + } > > + > > + if (state->duty_cycle > state->period) > > + state->duty_cycle = state->period; > > + > > + state->polarity = PWM_POLARITY_NORMAL; > > + pm_runtime_put(chip->dev); > > + > > + return 0; > > +} > > + > > +static int rz_mtu3_pwm_config(struct pwm_chip *chip, struct pwm_device > *pwm, > > + const struct pwm_state *state) { > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > > + struct rz_mtu3_channel *ch; > > + unsigned long pv, dc; > > + u64 period_cycles; > > + u64 duty_cycles; > > + u32 ch_index; > > + u8 prescale; > > + u8 val; > > + > > + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); > > + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); > > + period_cycles = mul_u64_u32_div(state->period, rz_mtu3_pwm->rate, > > + NSEC_PER_SEC); > > + prescale = rz_mtu3_pwm_calculate_prescale(rz_mtu3_pwm, > > +period_cycles); > > + > > + if (period_cycles >> (2 * prescale) <= U16_MAX) > > + pv = period_cycles >> (2 * prescale); > > + else > > + pv = U16_MAX; > > + > > + duty_cycles = mul_u64_u32_div(state->duty_cycle, rz_mtu3_pwm->rate, > > + NSEC_PER_SEC); > > + if (duty_cycles >> (2 * prescale) <= U16_MAX) > > + dc = duty_cycles >> (2 * prescale); > > + else > > + dc = U16_MAX; > > + > > + /* > > + * 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(chip->dev); > > + > > + val = RZ_MTU3_TCR_CKEG_RISING | prescale; > > + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) { > > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, > > + RZ_MTU3_TCR_CCLR_TGRC | val); > > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRD, dc); > > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRC, pv); > > + } else { > > + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, > > + RZ_MTU3_TCR_CCLR_TGRA | val); > > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRB, dc); > > + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRA, pv); > > + } > > + > > + /* If the PWM is not enabled, turn the clock off again to save power. > */ > > + if (!pwm->state.enabled) > > + pm_runtime_put(chip->dev); > > + > > + return 0; > > +} > > + > > +static int rz_mtu3_pwm_apply(struct pwm_chip *chip, struct pwm_device > *pwm, > > + const struct pwm_state *state) { > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); > > + bool enabled = pwm->state.enabled; > > + int ret; > > + > > + if (state->polarity != PWM_POLARITY_NORMAL) > > + return -EINVAL; > > + > > + if (!state->enabled) { > > + if (enabled) > > + rz_mtu3_pwm_disable(rz_mtu3_pwm, pwm); > > + > > + return 0; > > + } > > + > > + ret = rz_mtu3_pwm_config(chip, pwm, state); > > + if (ret) > > + return ret; > > + > > + if (!enabled) > > + ret = rz_mtu3_pwm_enable(rz_mtu3_pwm, pwm); > > + > > + return ret; > > +} > > + > > +static const struct pwm_ops rz_mtu3_pwm_ops = { > > + .request = rz_mtu3_pwm_request, > > + .free = rz_mtu3_pwm_free, > > + .get_state = rz_mtu3_pwm_get_state, > > + .apply = rz_mtu3_pwm_apply, > > + .owner = THIS_MODULE, > > +}; > > + > > +static int rz_mtu3_pwm_pm_runtime_suspend(struct device *dev) { > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = dev_get_drvdata(dev); > > + > > + clk_disable_unprepare(rz_mtu3_pwm->clk); > > + > > + return 0; > > +} > > + > > +static int rz_mtu3_pwm_pm_runtime_resume(struct device *dev) { > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = dev_get_drvdata(dev); > > + > > + return clk_prepare_enable(rz_mtu3_pwm->clk); > > +} > > + > > +static DEFINE_RUNTIME_DEV_PM_OPS(rz_mtu3_pwm_pm_ops, > > + rz_mtu3_pwm_pm_runtime_suspend, > > + rz_mtu3_pwm_pm_runtime_resume, NULL); > > + > > +static void rz_mtu3_pwm_pm_disable(void *data) { > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = data; > > + > > + clk_rate_exclusive_put(rz_mtu3_pwm->clk); > > + pm_runtime_disable(rz_mtu3_pwm->chip.dev); > > + pm_runtime_set_suspended(rz_mtu3_pwm->chip.dev); > > +} > > + > > +static int rz_mtu3_pwm_probe(struct platform_device *pdev) { > > + struct rz_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent); > > + struct rz_mtu3_pwm_chip *rz_mtu3_pwm; > > + struct device *dev = &pdev->dev; > > + int num_pwm_hw_ch = 0; > > + unsigned int i; > > + int ret; > > + > > + rz_mtu3_pwm = devm_kzalloc(&pdev->dev, sizeof(*rz_mtu3_pwm), > GFP_KERNEL); > > + if (!rz_mtu3_pwm) > > + return -ENOMEM; > > + > > + rz_mtu3_pwm->clk = ddata->clk; > > + > > + for (i = 0; i < RZ_MTU_NUM_CHANNELS; i++) { > > + if (i == RZ_MTU3_CHAN_5 || i == RZ_MTU3_CHAN_8) > > + continue; > > + > > + rz_mtu3_pwm->ch[num_pwm_hw_ch] = &ddata->channels[i]; > > + rz_mtu3_pwm->ch[num_pwm_hw_ch]->dev = dev; > > + num_pwm_hw_ch++; > > + } > > + > > + mutex_init(&rz_mtu3_pwm->lock); > > + platform_set_drvdata(pdev, rz_mtu3_pwm); > > + ret = clk_prepare_enable(rz_mtu3_pwm->clk); > > + if (ret) > > + return dev_err_probe(dev, ret, "Clock enable failed\n"); > > + > > + clk_rate_exclusive_get(rz_mtu3_pwm->clk); > > + > > + rz_mtu3_pwm->rate = clk_get_rate(rz_mtu3_pwm->clk); > > + /* > > + * Refuse clk rates > 1 GHz to prevent overflow later for computing > > + * period and duty cycle. > > + */ > > + if (rz_mtu3_pwm->rate > NSEC_PER_SEC) { > > + ret = -EINVAL; > > + clk_rate_exclusive_put(rz_mtu3_pwm->clk); > > + goto disable_clock; > > + } > > + > > + pm_runtime_set_active(&pdev->dev); > > + pm_runtime_enable(&pdev->dev); > > + rz_mtu3_pwm->chip.dev = &pdev->dev; > > + ret = devm_add_action_or_reset(&pdev->dev, > > + rz_mtu3_pwm_pm_disable, > > + rz_mtu3_pwm); > > + if (ret < 0) > > + return ret; > > + > > + rz_mtu3_pwm->chip.ops = &rz_mtu3_pwm_ops; > > + rz_mtu3_pwm->chip.npwm = RZ_MTU3_MAX_PWM_CHANNELS; > > + ret = devm_pwmchip_add(&pdev->dev, &rz_mtu3_pwm->chip); > > + if (ret) > > + return dev_err_probe(&pdev->dev, ret, "failed to add PWM > chip\n"); > > + > > + pm_runtime_idle(&pdev->dev); > > + > > + return 0; > > + > > +disable_clock: > > + clk_disable_unprepare(rz_mtu3_pwm->clk); > > + return ret; > > +} > > .probe() enables rz_mtu3_pwm->clk, but there is no .remove() that cares > about disabling it again. I believe it is ok. I have added WARN_ON for double checking. Please correct me, if it is wrong. Probe: Clock enable by "clk_prepare_enable" [ 6.923881] rzg2l_mod_clock_endisable+0x130/0x180 [ 6.929295] rzg2l_mod_clock_enable+0x54/0x70 [ 6.934229] clk_core_enable+0xdc/0x278 [ 6.938599] clk_enable+0x28/0x44 [ 6.942403] rz_mtu3_pwm_probe+0xf0/0x230 [ 6.946944] platform_probe+0x64/0xcc Clock disable by " pm_runtime_idle" [ 7.283961] rzg2l_mod_clock_endisable+0x130/0x180 [ 7.289383] rzg2l_mod_clock_disable+0x50/0x5c [ 7.294415] clk_core_disable+0x8c/0xd8 [ 7.298787] clk_disable+0x2c/0x44 [ 7.302685] rz_mtu3_pwm_pm_runtime_suspend+0x1c/0x34 [ 7.308361] pm_generic_runtime_suspend+0x28/0x3c [ 7.313667] __rpm_callback+0x44/0x134 [ 7.317936] rpm_callback+0x64/0x70 [ 7.321920] rpm_suspend+0x130/0x628 [ 7.325999] rpm_idle+0x118/0x2c0 [ 7.329796] __pm_runtime_idle+0x4c/0x84 [ 7.334250] rz_mtu3_pwm_probe+0x1b8/0x230 Unbind: cd /sys/bus/platform/drivers/pwm-rz-mtu3/ echo pwm-rz-mtu3.0 > unbind Clock enable by "__pm_runtime_resume" [ 263.003787] rzg2l_mod_clock_endisable+0x130/0x180 [ 263.009196] rzg2l_mod_clock_enable+0x54/0x70 [ 263.014122] clk_core_enable+0xdc/0x278 [ 263.018484] clk_enable+0x28/0x44 [ 263.022278] rz_mtu3_pwm_pm_runtime_resume+0x3c/0x5c [ 263.027850] pm_generic_runtime_resume+0x28/0x3c [ 263.033054] __rpm_callback+0x44/0x134 [ 263.037314] rpm_callback+0x64/0x70 [ 263.041287] rpm_resume+0x428/0x6a0 [ 263.045260] __pm_runtime_resume+0x50/0x74 [ 263.049893] device_release_driver_internal+0xcc/0x234 [ 263.055670] device_driver_detach+0x14/0x1c Clock disable by "__pm_runtime_idle" [ 263.304970] rzg2l_mod_clock_endisable+0x130/0x180 [ 263.309812] rzg2l_mod_clock_disable+0x50/0x5c [ 263.314301] clk_core_disable+0x8c/0xd8 [ 263.318180] clk_disable+0x2c/0x44 [ 263.321620] rz_mtu3_pwm_pm_runtime_suspend+0x1c/0x34 [ 263.326720] pm_generic_runtime_suspend+0x28/0x3c [ 263.331472] __rpm_callback+0x44/0x134 [ 263.335261] rpm_callback+0x64/0x70 [ 263.338787] rpm_suspend+0x130/0x628 [ 263.342400] rpm_idle+0x118/0x2c0 [ 263.345751] __pm_runtime_idle+0x4c/0x84 [ 263.349713] device_release_driver_internal+0x1d0/0x234 Please correct me if anything wrong or clarifications needed. Cheers, Biju > > > + > > +static struct platform_driver rz_mtu3_pwm_driver = { > > + .driver = { > > + .name = "pwm-rz-mtu3", > > + .pm = pm_ptr(&rz_mtu3_pwm_pm_ops), > > + }, > > + .probe = rz_mtu3_pwm_probe, > > +}; > > +module_platform_driver(rz_mtu3_pwm_driver); > > + > > +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>"); > > +MODULE_ALIAS("platform:pwm-rz-mtu3"); > > +MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a PWM Timer Driver"); > > +MODULE_LICENSE("GPL"); > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello Biju, On Wed, Mar 29, 2023 at 05:40:17PM +0000, Biju Das wrote: > [...] IMHO it's easier to send a v15 instead of discussing what you made of my suggestions in prosa. > > .probe() enables rz_mtu3_pwm->clk, but there is no .remove() that cares > > about disabling it again. > > I believe it is ok. I have added WARN_ON for double checking. > Please correct me, if it is wrong. I might have missed some details in the runtime pm stuff. I'll happily recheck if you resend with the other things we agreed on fixed. Best regards Uwe
Hi Uwe, Thanks for the feedback. > Subject: Re: [PATCH v14 6/6] pwm: Add Renesas RZ/G2L MTU3a PWM driver > > Hello Biju, > > On Wed, Mar 29, 2023 at 05:40:17PM +0000, Biju Das wrote: > > [...] > > IMHO it's easier to send a v15 instead of discussing what you made of my > suggestions in prosa. Ok, will send v15. > > > > .probe() enables rz_mtu3_pwm->clk, but there is no .remove() that > > > cares about disabling it again. > > > > I believe it is ok. I have added WARN_ON for double checking. > > Please correct me, if it is wrong. > > I might have missed some details in the runtime pm stuff. I'll happily > recheck if you resend with the other things we agreed on fixed. Thanks, Biju
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index dae023d783a2..ccc0299fd0dd 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -481,6 +481,17 @@ config PWM_ROCKCHIP Generic PWM framework driver for the PWM controller found on Rockchip SoCs. +config PWM_RZ_MTU3 + tristate "Renesas RZ/G2L MTU3a PWM Timer support" + depends on RZ_MTU3 || COMPILE_TEST + depends on HAS_IOMEM + help + This driver exposes the MTU3a PWM Timer controller found in Renesas + RZ/G2L like chips through the PWM API. + + To compile this driver as a module, choose M here: the module + will be called pwm-rz-mtu3. + 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 7bf1a29f02b8..b85fc9fba326 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -44,6 +44,7 @@ obj-$(CONFIG_PWM_RASPBERRYPI_POE) += pwm-raspberrypi-poe.o 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_SAMSUNG) += pwm-samsung.o obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o obj-$(CONFIG_PWM_SL28CPLD) += pwm-sl28cpld.o diff --git a/drivers/pwm/pwm-rz-mtu3.c b/drivers/pwm/pwm-rz-mtu3.c new file mode 100644 index 000000000000..146948656755 --- /dev/null +++ b/drivers/pwm/pwm-rz-mtu3.c @@ -0,0 +1,482 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Renesas RZ/G2L MTU3a PWM Timer driver + * + * Copyright (C) 2022 Renesas Electronics Corporation + * + * Hardware manual for this IP can be found here + * https://www.renesas.com/eu/en/document/mah/rzg2l-group-rzg2lc-group-users-manual-hardware-0?language=en + * + * Limitations: + * - When PWM is disabled, the output is driven to Hi-Z. + * - While the hardware supports both polarities, the driver (for now) + * only handles normal polarity. + * - HW uses one counter and two match components to configure duty_cycle + * and period. + * - Multi-Function Timer Pulse Unit(a.k.a MTU) has 7 HW channels for PWM + * operations(The channels are MTU{0..4, 6, 7}). + * - MTU{1, 2} channels have a single IO, whereas all other HW channels have + * 2 IOs. + * - Each IO is modelled as an independent PWM channel. + * - rz_mtu3_hw_channel_ios table is used to map the PWM channel to the + * corresponding HW channel as there are difference in number of IOs + * between HW channels. + */ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/limits.h> +#include <linux/mfd/rz-mtu3.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/pwm.h> +#include <linux/time.h> + +#define RZ_MTU3_TIOR_OC_RETAIN (0) +#define RZ_MTU3_TIOR_OC_0_H_COMP_MATCH (2) +#define RZ_MTU3_TIOR_OC_1_TOGGLE (7) +#define RZ_MTU3_TIOR_OC_IOA GENMASK(3, 0) + +#define RZ_MTU3_TCR_CCLR_TGRC (5 << 5) +#define RZ_MTU3_TCR_CKEG_RISING (0 << 3) + +#define RZ_MTU3_TCR_TPCS GENMASK(2, 0) + +#define RZ_MTU3_MAX_PWM_CHANNELS (12) + +#define RZ_MTU3_MAX_HW_CHANNELS (7) + +/* The table represents the number of IOs on each MTU HW channel. */ +static const u8 rz_mtu3_hw_channel_ios[] = { 2, 1, 1, 2, 2, 2, 2 }; + +/** + * struct rz_mtu3_pwm_chip - MTU3 pwm private data + * + * @chip: MTU3 pwm chip data + * @clk: MTU3 module clock + * @lock: Lock to prevent concurrent access for usage count + * @rate: MTU3 clock rate + * @user_count: MTU3 usage count + * @rz_mtu3_channel: HW channels for the PWM + */ + +struct rz_mtu3_pwm_chip { + struct pwm_chip chip; + struct clk *clk; + struct mutex lock; + unsigned long rate; + u32 user_count[RZ_MTU3_MAX_HW_CHANNELS]; + struct rz_mtu3_channel *ch[RZ_MTU3_MAX_HW_CHANNELS]; +}; + +static inline struct rz_mtu3_pwm_chip *to_rz_mtu3_pwm_chip(struct pwm_chip *chip) +{ + return container_of(chip, struct rz_mtu3_pwm_chip, chip); +} + +static u8 rz_mtu3_pwm_calculate_prescale(struct rz_mtu3_pwm_chip *rz_mtu3, + u64 period_cycles) +{ + u32 prescaled_period_cycles; + u8 prescale; + + prescaled_period_cycles = period_cycles >> 16; + if (prescaled_period_cycles >= 16) + prescale = 3; + else + prescale = (fls(prescaled_period_cycles) + 1) / 2; + + return prescale; +} + +static struct rz_mtu3_channel * +rz_mtu3_get_hw_channel(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, u32 channel) +{ + unsigned int i, ch_index = 0; + + for (i = 0; i < ARRAY_SIZE(rz_mtu3_hw_channel_ios); i++) { + ch_index += rz_mtu3_hw_channel_ios[i]; + + if (ch_index > channel) + break; + } + + return rz_mtu3_pwm->ch[i]; +} + +static u32 rz_mtu3_get_hw_channel_index(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, + struct rz_mtu3_channel *ch) +{ + u32 i; + + for (i = 0; i < ARRAY_SIZE(rz_mtu3_hw_channel_ios); i++) { + if (ch == rz_mtu3_pwm->ch[i]) + break; + } + + return i; +} + +static bool rz_mtu3_pwm_is_second_channel(u32 ch_index, u32 hwpwm) +{ + u32 i, pwm_ch_index = 0; + + for (i = 0; i < ch_index; i++) + pwm_ch_index += rz_mtu3_hw_channel_ios[i]; + + return pwm_ch_index != hwpwm; +} + +static bool rz_mtu3_pwm_is_ch_enabled(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, + u32 hwpwm) +{ + struct rz_mtu3_channel *ch; + bool is_channel_en; + u32 ch_index; + u8 val; + + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, hwpwm); + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); + is_channel_en = rz_mtu3_is_enabled(ch); + + if (rz_mtu3_pwm_is_second_channel(ch_index, hwpwm)) + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORL); + else + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TIORH); + + return (is_channel_en && (val & RZ_MTU3_TIOR_OC_IOA)); +} + +static int rz_mtu3_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm) +{ + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); + struct rz_mtu3_channel *ch; + u32 ch_index; + + mutex_lock(&rz_mtu3_pwm->lock); + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); + if (!rz_mtu3_pwm->user_count[ch_index] && !rz_mtu3_request_channel(ch)) { + mutex_unlock(&rz_mtu3_pwm->lock); + return -EBUSY; + } + + rz_mtu3_pwm->user_count[ch_index]++; + mutex_unlock(&rz_mtu3_pwm->lock); + + return 0; +} + +static void rz_mtu3_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) +{ + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); + struct rz_mtu3_channel *ch; + u32 ch_index; + + mutex_lock(&rz_mtu3_pwm->lock); + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); + + rz_mtu3_pwm->user_count[ch_index]--; + if (!rz_mtu3_pwm->user_count[ch_index]) + rz_mtu3_release_channel(ch); + + mutex_unlock(&rz_mtu3_pwm->lock); +} + +static int rz_mtu3_pwm_enable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, + struct pwm_device *pwm) +{ + struct rz_mtu3_channel *ch; + u32 ch_index; + u8 val; + int rc; + + rc = pm_runtime_resume_and_get(rz_mtu3_pwm->chip.dev); + if (rc) + return rc; + + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); + val = (RZ_MTU3_TIOR_OC_1_TOGGLE << 4) | RZ_MTU3_TIOR_OC_0_H_COMP_MATCH; + + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_MD_PWMMODE1); + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, val); + else + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, val); + + if (rz_mtu3_pwm->user_count[ch_index] <= 1) + rz_mtu3_enable(ch); + + return 0; +} + +static void rz_mtu3_pwm_disable(struct rz_mtu3_pwm_chip *rz_mtu3_pwm, + struct pwm_device *pwm) +{ + struct rz_mtu3_channel *ch; + u32 ch_index; + + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); + + /* Return to normal mode and disable output pins of MTU3 channel */ + if (rz_mtu3_pwm->user_count[ch_index] <= 1) + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TMDR1, RZ_MTU3_TMDR1_MD_NORMAL); + + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORL, RZ_MTU3_TIOR_OC_RETAIN); + else + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TIORH, RZ_MTU3_TIOR_OC_RETAIN); + + if (rz_mtu3_pwm->user_count[ch_index] <= 1) + rz_mtu3_disable(ch); + + pm_runtime_put_sync(rz_mtu3_pwm->chip.dev); +} + +static int rz_mtu3_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm, + struct pwm_state *state) +{ + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); + struct rz_mtu3_channel *ch; + u8 prescale, val; + u32 ch_index; + u16 dc, pv; + u64 tmp; + + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); + pm_runtime_get_sync(chip->dev); + state->enabled = rz_mtu3_pwm_is_ch_enabled(rz_mtu3_pwm, pwm->hwpwm); + if (state->enabled) { + val = rz_mtu3_8bit_ch_read(ch, RZ_MTU3_TCR); + prescale = FIELD_GET(RZ_MTU3_TCR_TPCS, val); + + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) { + dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRD); + pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRC); + } else { + dc = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRB); + pv = rz_mtu3_16bit_ch_read(ch, RZ_MTU3_TGRA); + } + + /* With prescale <= 7 and pv <= 0xffff this doesn't overflow. */ + tmp = NSEC_PER_SEC * (u64)pv << (2 * prescale); + state->period = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate); + tmp = NSEC_PER_SEC * (u64)dc << (2 * prescale); + state->duty_cycle = DIV_ROUND_UP_ULL(tmp, rz_mtu3_pwm->rate); + } + + if (state->duty_cycle > state->period) + state->duty_cycle = state->period; + + state->polarity = PWM_POLARITY_NORMAL; + pm_runtime_put(chip->dev); + + return 0; +} + +static int rz_mtu3_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); + struct rz_mtu3_channel *ch; + unsigned long pv, dc; + u64 period_cycles; + u64 duty_cycles; + u32 ch_index; + u8 prescale; + u8 val; + + ch = rz_mtu3_get_hw_channel(rz_mtu3_pwm, pwm->hwpwm); + ch_index = rz_mtu3_get_hw_channel_index(rz_mtu3_pwm, ch); + period_cycles = mul_u64_u32_div(state->period, rz_mtu3_pwm->rate, + NSEC_PER_SEC); + prescale = rz_mtu3_pwm_calculate_prescale(rz_mtu3_pwm, period_cycles); + + if (period_cycles >> (2 * prescale) <= U16_MAX) + pv = period_cycles >> (2 * prescale); + else + pv = U16_MAX; + + duty_cycles = mul_u64_u32_div(state->duty_cycle, rz_mtu3_pwm->rate, + NSEC_PER_SEC); + if (duty_cycles >> (2 * prescale) <= U16_MAX) + dc = duty_cycles >> (2 * prescale); + else + dc = U16_MAX; + + /* + * 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(chip->dev); + + val = RZ_MTU3_TCR_CKEG_RISING | prescale; + if (rz_mtu3_pwm_is_second_channel(ch_index, pwm->hwpwm)) { + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, + RZ_MTU3_TCR_CCLR_TGRC | val); + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRD, dc); + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRC, pv); + } else { + rz_mtu3_8bit_ch_write(ch, RZ_MTU3_TCR, + RZ_MTU3_TCR_CCLR_TGRA | val); + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRB, dc); + rz_mtu3_16bit_ch_write(ch, RZ_MTU3_TGRA, pv); + } + + /* If the PWM is not enabled, turn the clock off again to save power. */ + if (!pwm->state.enabled) + pm_runtime_put(chip->dev); + + return 0; +} + +static int rz_mtu3_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = to_rz_mtu3_pwm_chip(chip); + bool enabled = pwm->state.enabled; + int ret; + + if (state->polarity != PWM_POLARITY_NORMAL) + return -EINVAL; + + if (!state->enabled) { + if (enabled) + rz_mtu3_pwm_disable(rz_mtu3_pwm, pwm); + + return 0; + } + + ret = rz_mtu3_pwm_config(chip, pwm, state); + if (ret) + return ret; + + if (!enabled) + ret = rz_mtu3_pwm_enable(rz_mtu3_pwm, pwm); + + return ret; +} + +static const struct pwm_ops rz_mtu3_pwm_ops = { + .request = rz_mtu3_pwm_request, + .free = rz_mtu3_pwm_free, + .get_state = rz_mtu3_pwm_get_state, + .apply = rz_mtu3_pwm_apply, + .owner = THIS_MODULE, +}; + +static int rz_mtu3_pwm_pm_runtime_suspend(struct device *dev) +{ + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = dev_get_drvdata(dev); + + clk_disable_unprepare(rz_mtu3_pwm->clk); + + return 0; +} + +static int rz_mtu3_pwm_pm_runtime_resume(struct device *dev) +{ + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = dev_get_drvdata(dev); + + return clk_prepare_enable(rz_mtu3_pwm->clk); +} + +static DEFINE_RUNTIME_DEV_PM_OPS(rz_mtu3_pwm_pm_ops, + rz_mtu3_pwm_pm_runtime_suspend, + rz_mtu3_pwm_pm_runtime_resume, NULL); + +static void rz_mtu3_pwm_pm_disable(void *data) +{ + struct rz_mtu3_pwm_chip *rz_mtu3_pwm = data; + + clk_rate_exclusive_put(rz_mtu3_pwm->clk); + pm_runtime_disable(rz_mtu3_pwm->chip.dev); + pm_runtime_set_suspended(rz_mtu3_pwm->chip.dev); +} + +static int rz_mtu3_pwm_probe(struct platform_device *pdev) +{ + struct rz_mtu3 *ddata = dev_get_drvdata(pdev->dev.parent); + struct rz_mtu3_pwm_chip *rz_mtu3_pwm; + struct device *dev = &pdev->dev; + int num_pwm_hw_ch = 0; + unsigned int i; + int ret; + + rz_mtu3_pwm = devm_kzalloc(&pdev->dev, sizeof(*rz_mtu3_pwm), GFP_KERNEL); + if (!rz_mtu3_pwm) + return -ENOMEM; + + rz_mtu3_pwm->clk = ddata->clk; + + for (i = 0; i < RZ_MTU_NUM_CHANNELS; i++) { + if (i == RZ_MTU3_CHAN_5 || i == RZ_MTU3_CHAN_8) + continue; + + rz_mtu3_pwm->ch[num_pwm_hw_ch] = &ddata->channels[i]; + rz_mtu3_pwm->ch[num_pwm_hw_ch]->dev = dev; + num_pwm_hw_ch++; + } + + mutex_init(&rz_mtu3_pwm->lock); + platform_set_drvdata(pdev, rz_mtu3_pwm); + ret = clk_prepare_enable(rz_mtu3_pwm->clk); + if (ret) + return dev_err_probe(dev, ret, "Clock enable failed\n"); + + clk_rate_exclusive_get(rz_mtu3_pwm->clk); + + rz_mtu3_pwm->rate = clk_get_rate(rz_mtu3_pwm->clk); + /* + * Refuse clk rates > 1 GHz to prevent overflow later for computing + * period and duty cycle. + */ + if (rz_mtu3_pwm->rate > NSEC_PER_SEC) { + ret = -EINVAL; + clk_rate_exclusive_put(rz_mtu3_pwm->clk); + goto disable_clock; + } + + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->dev); + rz_mtu3_pwm->chip.dev = &pdev->dev; + ret = devm_add_action_or_reset(&pdev->dev, + rz_mtu3_pwm_pm_disable, + rz_mtu3_pwm); + if (ret < 0) + return ret; + + rz_mtu3_pwm->chip.ops = &rz_mtu3_pwm_ops; + rz_mtu3_pwm->chip.npwm = RZ_MTU3_MAX_PWM_CHANNELS; + ret = devm_pwmchip_add(&pdev->dev, &rz_mtu3_pwm->chip); + if (ret) + return dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n"); + + pm_runtime_idle(&pdev->dev); + + return 0; + +disable_clock: + clk_disable_unprepare(rz_mtu3_pwm->clk); + return ret; +} + +static struct platform_driver rz_mtu3_pwm_driver = { + .driver = { + .name = "pwm-rz-mtu3", + .pm = pm_ptr(&rz_mtu3_pwm_pm_ops), + }, + .probe = rz_mtu3_pwm_probe, +}; +module_platform_driver(rz_mtu3_pwm_driver); + +MODULE_AUTHOR("Biju Das <biju.das.jz@bp.renesas.com>"); +MODULE_ALIAS("platform:pwm-rz-mtu3"); +MODULE_DESCRIPTION("Renesas RZ/G2L MTU3a PWM Timer Driver"); +MODULE_LICENSE("GPL");
The RZ/G2L Multi-Function Timer Pulse Unit 3 (a.k.a MTU3a) uses one counter and two match components to configure duty_cycle and period to generate PWM output waveform. Add basic support for RZ/G2L MTU3a PWM driver by creating separate PWM channels for each IOs. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v13->v14: * Updated commit description * Updated Limitations section. * Replaced the macros RZ_MTU*->RZ_MTU3_CHAN_* in probe() * Fixed a kernel crash in error path by moving rz_mtu3_pwm->chip.dev before devm_add_action_or_reset() * Added pm_runtime_idle() and simplified error paths for devm_add_action_or_reset() and devm_pwmchip_add(). v12->v13: * Updated commit description * Moved RZ_MTU3_TMDR1_MD_* macros to rz_mtu3.h * Updated Limitations section. * Removed PWM mode1 references from the driver. * Dropped prescale and duty_cycle from struct rz_mtu3_pwm_chip. * Replaced rz_mtu3_pwm_mode1_num_ios->rz_mtu3_hw_channel_ios. * Avoided race condition in rz_mtu3_pwm_request()/rz_mtu3_pwm_free(). * Updated get_state() by adding dc > pv check and added a comment about overflow condition. * Moved overflow condition check from config->probe() * Replaced pm_runtime_resume_and_get with unconditional pm_runtime_get_sync() in config() * Added error check for clk_prepare_enable() in probe() and propagating error to the caller for pm_runtime_resume() * clk_get_rate() is called after enabling the clock and clk_rate_exclusive_put() v11->v12: * Updated header file to <linux/mfd/rz-mtu3.h> as core driver is in MFD. * Reordered get_state() v10->v11: * No change. v9->v10: * No change. v8->v9: * Added prescale/duty_cycle variables to struct rz_mtu3_pwm_chip and cached this values in rz_mtu3_pwm_config and used this cached values in get_state(), if PWM is disabled. * Added return code for get_state() v7->v8: * Simplified rz_mtu3_pwm_request by calling rz_mtu3_request_channel() * Simplified rz_mtu3_pwm_free by calling rz_mtu3_release_channel() v6->v7: * Added channel specific mutex lock to avoid race between counter device and rz_mtu3_pwm_{request,free} * Added pm_runtime_resume_and_get in rz_mtu3_pwm_enable() * Added pm_runtime_put_sync in rz_mtu3_pwm_disable() * Updated rz_mtu3_pwm_config() * Updated rz_mtu3_pwm_apply() v5->v6: * Updated commit and Kconfig description * Sorted the header * Replaced dev_get_drvdata from rz_mtu3_pwm_pm_disable() * Replaced SET_RUNTIME_PM_OPS->DEFINE_RUNTIME_DEV_PM_OPS and removed __maybe_unused from suspend/resume() v4->v5: * pwm device is instantiated by mtu3a core driver. v3->v4: * There is no resource associated with "rz-mtu3-pwm" compatible and moved the code to mfd subsystem as it binds against "rz-mtu". * Removed struct platform_driver rz_mtu3_pwm_driver. v2->v3: * No change. v1->v2: * Modelled as a single PWM device handling multiple channles. * Used PM framework to manage the clocks. --- drivers/pwm/Kconfig | 11 + drivers/pwm/Makefile | 1 + drivers/pwm/pwm-rz-mtu3.c | 482 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 494 insertions(+) create mode 100644 drivers/pwm/pwm-rz-mtu3.c