[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<TY3PR01MB11346477DC214777A4D94045F86DCA@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Fri, 28 Nov 2025 10:37:17 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: biju.das.au <biju.das.au@...il.com>, Uwe Kleine-König
<ukleinek@...nel.org>
CC: "linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Geert
Uytterhoeven <geert+renesas@...der.be>, Prabhakar Mahadev Lad
<prabhakar.mahadev-lad.rj@...renesas.com>, biju.das.au
<biju.das.au@...il.com>, "linux-renesas-soc@...r.kernel.org"
<linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH v4] pwm: rzg2l-gpt: Implementation of the waveform
callbacks
Hi All,
> -----Original Message-----
> From: Biju <biju.das.au@...il.com>
> Sent: 28 November 2025 10:32
> Subject: [PATCH v4] pwm: rzg2l-gpt: Implementation of the waveform callbacks
>
> From: Biju Das <biju.das.jz@...renesas.com>
>
> Convert the rzg2l-gpt driver to use the new callbacks for hardware programming.
>
> Signed-off-by: Biju Das <biju.das.jz@...renesas.com>
> ---
> v3->v4:
> * Rebased the patch.
> * rzg2l_gpt_is_ch_enabled() fit into 80-character limit for consistency
> with other functions.
> v2->v3:
> * Prepared as a series.
> * Created separate patch for fix for reinitialization of the cache
> variable in disable().
> * .round_waveform_tohw() do not fail if the requested period is too small
> but use the smallest possible value.
> * Added lock in rzg2l_gpt_read_waveform().
> * wfhw is reinitialized in rzg2l_gpt_read_waveform if channel is disabled
> * Optimizated rzg2l_gpt_is_ch_enabled() to avoid redundant reads to
> RZG2L_GTCR(ch).
> * .write_waveform() returns error incase of invalid period for second
> channel.
> v1->v2:
> * Dropped modifing hardware from .round_waveform_tohw() callback.
> v1:
> * https://lore.kernel.org/all/20250805065032.193766-1-biju.das.jz@bp.renesas.com/
> ---
> drivers/pwm/pwm-rzg2l-gpt.c | 192 ++++++++++++++++++++++--------------
> 1 file changed, 118 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/pwm/pwm-rzg2l-gpt.c b/drivers/pwm/pwm-rzg2l-gpt.c index
> 4856af080e8e..2f424a7b05cc 100644
> --- a/drivers/pwm/pwm-rzg2l-gpt.c
> +++ b/drivers/pwm/pwm-rzg2l-gpt.c
> @@ -86,6 +86,13 @@ struct rzg2l_gpt_chip {
> u32 channel_enable_count[RZG2L_MAX_HW_CHANNELS];
> };
>
> +/* This represents a hardware configuration for one channel */ struct
> +rzg2l_gpt_waveform {
> + u32 gtpr;
> + u32 gtccr;
> + u8 prescale;
> +};
> +
> static inline struct rzg2l_gpt_chip *to_rzg2l_gpt_chip(struct pwm_chip *chip) {
> return pwmchip_get_drvdata(chip);
> @@ -153,7 +160,8 @@ static void rzg2l_gpt_free(struct pwm_chip *chip, struct pwm_device *pwm)
> rzg2l_gpt->channel_request_count[ch]--;
> }
>
> -static bool rzg2l_gpt_is_ch_enabled(struct rzg2l_gpt_chip *rzg2l_gpt, u8 hwpwm)
> +static bool rzg2l_gpt_is_ch_enabled(struct rzg2l_gpt_chip *rzg2l_gpt, u8 hwpwm,
> + u32 *gtcr)
> {
> u8 ch = RZG2L_GET_CH(hwpwm);
> u32 val;
> @@ -162,6 +170,9 @@ static bool rzg2l_gpt_is_ch_enabled(struct rzg2l_gpt_chip *rzg2l_gpt, u8 hwpwm)
> if (!(val & RZG2L_GTCR_CST))
> return false;
>
> + if (gtcr)
> + *gtcr = val;
> +
> val = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTIOR(ch));
>
> return val & RZG2L_GTIOR_OxE(rzg2l_gpt_subchannel(hwpwm));
> @@ -220,54 +231,38 @@ static u64 rzg2l_gpt_calculate_period_or_duty(struct rzg2l_gpt_chip *rzg2l_gpt,
> return DIV64_U64_ROUND_UP(tmp, rzg2l_gpt->rate_khz); }
>
> -static int rzg2l_gpt_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> - struct pwm_state *state)
> -{
> - struct rzg2l_gpt_chip *rzg2l_gpt = to_rzg2l_gpt_chip(chip);
> -
> - state->enabled = rzg2l_gpt_is_ch_enabled(rzg2l_gpt, pwm->hwpwm);
> - if (state->enabled) {
> - u32 sub_ch = rzg2l_gpt_subchannel(pwm->hwpwm);
> - u32 ch = RZG2L_GET_CH(pwm->hwpwm);
> - u8 prescale;
> - u32 val;
> -
> - val = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCR(ch));
> - prescale = FIELD_GET(RZG2L_GTCR_TPCS, val);
> -
> - val = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTPR(ch));
> - state->period = rzg2l_gpt_calculate_period_or_duty(rzg2l_gpt, val, prescale);
> -
> - val = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCCR(ch, sub_ch));
> - state->duty_cycle = rzg2l_gpt_calculate_period_or_duty(rzg2l_gpt, val, prescale);
> - if (state->duty_cycle > state->period)
> - state->duty_cycle = state->period;
> - }
> -
> - state->polarity = PWM_POLARITY_NORMAL;
> -
> - return 0;
> -}
> -
> static u32 rzg2l_gpt_calculate_pv_or_dc(u64 period_or_duty_cycle, u8 prescale) {
> return min_t(u64, DIV_ROUND_DOWN_ULL(period_or_duty_cycle, 1 << (2 * prescale)),
> U32_MAX);
> }
>
> -/* Caller holds the lock while calling rzg2l_gpt_config() */ -static int rzg2l_gpt_config(struct
> pwm_chip *chip, struct pwm_device *pwm,
> - const struct pwm_state *state)
> +static int rzg2l_gpt_round_waveform_tohw(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + const struct pwm_waveform *wf,
> + void *_wfhw)
> +
> {
> struct rzg2l_gpt_chip *rzg2l_gpt = to_rzg2l_gpt_chip(chip);
> - u8 sub_ch = rzg2l_gpt_subchannel(pwm->hwpwm);
> + struct rzg2l_gpt_waveform *wfhw = _wfhw;
> + bool is_small_second_period = false;
> u8 ch = RZG2L_GET_CH(pwm->hwpwm);
> u64 period_ticks, duty_ticks;
> unsigned long pv, dc;
> - u8 prescale;
> +
> + guard(mutex)(&rzg2l_gpt->lock);
> + if (wf->period_length_ns == 0) {
> + *wfhw = (struct rzg2l_gpt_waveform){
> + .gtpr = 0,
> + .gtccr = 0,
> + .prescale = 0,
> + };
> +
> + return 0;
> + }
>
> /* Limit period/duty cycle to max value supported by the HW */
> - period_ticks = mul_u64_u64_div_u64(state->period, rzg2l_gpt->rate_khz, USEC_PER_SEC);
> + period_ticks = mul_u64_u64_div_u64(wf->period_length_ns,
> +rzg2l_gpt->rate_khz, USEC_PER_SEC);
> if (period_ticks > RZG2L_MAX_TICKS)
> period_ticks = RZG2L_MAX_TICKS;
> /*
> @@ -278,21 +273,25 @@ static int rzg2l_gpt_config(struct pwm_chip *chip, struct pwm_device *pwm,
> if (rzg2l_gpt->channel_request_count[ch] > 1) {
> u8 sibling_ch = rzg2l_gpt_sibling(pwm->hwpwm);
>
> - if (rzg2l_gpt_is_ch_enabled(rzg2l_gpt, sibling_ch)) {
> + if (rzg2l_gpt_is_ch_enabled(rzg2l_gpt, sibling_ch, NULL)) {
> if (period_ticks < rzg2l_gpt->period_ticks[ch])
> - return -EBUSY;
> + is_small_second_period = true;
>
> period_ticks = rzg2l_gpt->period_ticks[ch];
> }
> }
>
> - prescale = rzg2l_gpt_calculate_prescale(rzg2l_gpt, period_ticks);
> - pv = rzg2l_gpt_calculate_pv_or_dc(period_ticks, prescale);
> + wfhw->prescale = rzg2l_gpt_calculate_prescale(rzg2l_gpt, period_ticks);
> + pv = rzg2l_gpt_calculate_pv_or_dc(period_ticks, wfhw->prescale);
> + wfhw->gtpr = pv;
> + if (is_small_second_period)
> + return 1;
>
> - duty_ticks = mul_u64_u64_div_u64(state->duty_cycle, rzg2l_gpt->rate_khz, USEC_PER_SEC);
> + duty_ticks = mul_u64_u64_div_u64(wf->duty_length_ns,
> +rzg2l_gpt->rate_khz, USEC_PER_SEC);
> if (duty_ticks > period_ticks)
> duty_ticks = period_ticks;
> - dc = rzg2l_gpt_calculate_pv_or_dc(duty_ticks, prescale);
> + dc = rzg2l_gpt_calculate_pv_or_dc(duty_ticks, wfhw->prescale);
> + wfhw->gtccr = dc;
>
> /*
> * GPT counter is shared by multiple channels, we cache the period ticks @@ -301,6 +300,61 @@
> static int rzg2l_gpt_config(struct pwm_chip *chip, struct pwm_device *pwm,
> */
> rzg2l_gpt->period_ticks[ch] = period_ticks;
>
> + return 0;
> +}
> +
> +static int rzg2l_gpt_round_waveform_fromhw(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + const void *_wfhw,
> + struct pwm_waveform *wf)
> +{
> + struct rzg2l_gpt_chip *rzg2l_gpt = to_rzg2l_gpt_chip(chip);
> + const struct rzg2l_gpt_waveform *wfhw = _wfhw;
> +
> + wf->period_length_ns = rzg2l_gpt_calculate_period_or_duty(rzg2l_gpt, wfhw->gtpr,
> + wfhw->prescale);
> + wf->duty_length_ns = rzg2l_gpt_calculate_period_or_duty(rzg2l_gpt, wfhw->gtccr,
> + wfhw->prescale);
> + wf->duty_offset_ns = 0;
> +
> + return 0;
> +}
> +
> +static int rzg2l_gpt_read_waveform(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + void *_wfhw)
> +{
> + struct rzg2l_gpt_chip *rzg2l_gpt = to_rzg2l_gpt_chip(chip);
> + struct rzg2l_gpt_waveform *wfhw = _wfhw;
> + u32 sub_ch = rzg2l_gpt_subchannel(pwm->hwpwm);
> + u32 ch = RZG2L_GET_CH(pwm->hwpwm);
> + u32 gtcr;
> +
> + guard(mutex)(&rzg2l_gpt->lock);
> + if (rzg2l_gpt_is_ch_enabled(rzg2l_gpt, pwm->hwpwm, >cr)) {
> + wfhw->prescale = FIELD_GET(RZG2L_GTCR_TPCS, gtcr);
> + wfhw->gtpr = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTPR(ch));
> + wfhw->gtccr = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTCCR(ch, sub_ch));
> + if (wfhw->gtccr > wfhw->gtpr)
> + wfhw->gtccr = wfhw->gtpr;
> + } else {
> + *wfhw = (struct rzg2l_gpt_waveform) { };
> + }
> +
> + return 0;
> +}
> +
> +static int rzg2l_gpt_write_waveform(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + const void *_wfhw)
> +{
> + struct rzg2l_gpt_chip *rzg2l_gpt = to_rzg2l_gpt_chip(chip);
> + const struct rzg2l_gpt_waveform *wfhw = _wfhw;
> + u8 sub_ch = rzg2l_gpt_subchannel(pwm->hwpwm);
> + u8 ch = RZG2L_GET_CH(pwm->hwpwm);
> + u32 gptr;
> +
> + guard(mutex)(&rzg2l_gpt->lock);
> /*
> * Counter must be stopped before modifying mode, prescaler, timer
> * counter and buffer enable registers. These registers are shared @@ -319,14 +373,20 @@ static
> int rzg2l_gpt_config(struct pwm_chip *chip, struct pwm_device *pwm,
>
> /* Select count clock */
> rzg2l_gpt_modify(rzg2l_gpt, RZG2L_GTCR(ch), RZG2L_GTCR_TPCS,
> - FIELD_PREP(RZG2L_GTCR_TPCS, prescale));
> + FIELD_PREP(RZG2L_GTCR_TPCS, wfhw->prescale));
>
> /* Set period */
> - rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTPR(ch), pv);
> + rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTPR(ch), wfhw->gtpr);
> + } else {
> + if (wfhw->gtpr) {
> + gptr = rzg2l_gpt_read(rzg2l_gpt, RZG2L_GTPR(ch));
> + if (wfhw->gtpr < gptr)
> + return -EBUSY;
> + }
> }
>
> /* Set duty cycle */
> - rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTCCR(ch, sub_ch), dc);
> + rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTCCR(ch, sub_ch), wfhw->gtccr);
>
> if (rzg2l_gpt->channel_enable_count[ch] <= 1) {
> /* Set initial value for counter */
> @@ -335,44 +395,28 @@ static int rzg2l_gpt_config(struct pwm_chip *chip, struct pwm_device *pwm,
> /* Set no buffer operation */
> rzg2l_gpt_write(rzg2l_gpt, RZG2L_GTBER(ch), 0);
>
> - /* Restart the counter after updating the registers */
> - rzg2l_gpt_modify(rzg2l_gpt, RZG2L_GTCR(ch),
> - RZG2L_GTCR_CST, RZG2L_GTCR_CST);
> - }
> -
> - return 0;
> -}
> -
> -static int rzg2l_gpt_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> - const struct pwm_state *state)
> -{
> - struct rzg2l_gpt_chip *rzg2l_gpt = to_rzg2l_gpt_chip(chip);
> - bool enabled = pwm->state.enabled;
> - int ret;
> -
> - if (state->polarity != PWM_POLARITY_NORMAL)
> - return -EINVAL;
> -
> - guard(mutex)(&rzg2l_gpt->lock);
> - if (!state->enabled) {
> - if (enabled)
> - rzg2l_gpt_disable(rzg2l_gpt, pwm);
> -
> - return 0;
> + if (wfhw->gtpr)
> + /* Restart the counter after updating the registers */
> + rzg2l_gpt_modify(rzg2l_gpt, RZG2L_GTCR(ch),
> + RZG2L_GTCR_CST, RZG2L_GTCR_CST);
> }
>
> - ret = rzg2l_gpt_config(chip, pwm, state);
> - if (!ret && !enabled)
> + if (wfhw->gtpr && !rzg2l_gpt_is_ch_enabled(rzg2l_gpt, pwm->hwpwm,
> +NULL))
> rzg2l_gpt_enable(rzg2l_gpt, pwm);
> + else if (!wfhw->gtpr && rzg2l_gpt_is_ch_enabled(rzg2l_gpt, pwm->hwpwm, NULL))
> + rzg2l_gpt_disable(rzg2l_gpt, pwm);
>
> - return ret;
> + return 0;
> }
>
> static const struct pwm_ops rzg2l_gpt_ops = {
> .request = rzg2l_gpt_request,
> .free = rzg2l_gpt_free,
> - .get_state = rzg2l_gpt_get_state,
> - .apply = rzg2l_gpt_apply,
> + .sizeof_wfhw = sizeof(struct rzg2l_gpt_waveform),
> + .round_waveform_tohw = rzg2l_gpt_round_waveform_tohw,
> + .round_waveform_fromhw = rzg2l_gpt_round_waveform_fromhw,
> + .read_waveform = rzg2l_gpt_read_waveform,
> + .write_waveform = rzg2l_gpt_write_waveform,
> };
>
> static int rzg2l_gpt_probe(struct platform_device *pdev)
>
> base-commit: 7d31f578f3230f3b7b33b0930b08f9afd8429817
> prerequisite-patch-id: 994e29857383f8aa61bdc53df997f6e28f5cb723
> prerequisite-patch-id: c122f15fbeb0c18317e59981bcad9831f472bb57
> prerequisite-patch-id: 069b26e8ef7fc1d961488b3c9fee3cf0bf82d44f
> prerequisite-patch-id: 63bfa4ff3912cdb0f3a897b1f0286fe7eb3ca08d
> prerequisite-patch-id: 81a4395c1935f9cc757743c5de646bf17a1bcfa0
> prerequisite-patch-id: 34306f03faabbe6a0c238225e748b1d76726fd2c
> prerequisite-patch-id: 500209adf95b4f460e2a6264e2b9c5f3a3ad4640
> prerequisite-patch-id: 51945f93d2fd9c9433b62d847430d4bd21d0eafe
Oops, my script does not removed the unwanted patches.
I have resend[1] manually removing prerequisite-patch-id's.
[1] https://lore.kernel.org/all/20251128103431.161591-1-biju.das.jz@bp.renesas.com/
Cheers,
Biju
Powered by blists - more mailing lists