Re: [PATCH v12 2/2] pwm: Add support for RZ/G2L GPT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Biju,

On Wed, Dec 14, 2022 at 2:22 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> RZ/G2L General PWM Timer (GPT) composed of 8 channels with 32-bit timer
> (GPT32E). It supports the following functions
>  * 32 bits × 8 channels
>  * Up-counting or down-counting (saw waves) or up/down-counting
>    (triangle waves) for each counter.
>  * Clock sources independently selectable for each channel
>  * Two I/O pins per channel
>  * Two output compare/input capture registers per channel
>  * For the two output compare/input capture registers of each channel,
>    four registers are provided as buffer registers and are capable of
>    operating as comparison registers when buffering is not in use.
>  * In output compare operation, buffer switching can be at crests or
>    troughs, enabling the generation of laterally asymmetric PWM waveforms.
>  * Registers for setting up frame cycles in each channel (with capability
>    for generating interrupts at overflow or underflow)
>  * Generation of dead times in PWM operation
>  * Synchronous starting, stopping and clearing counters for arbitrary
>    channels
>  * Starting, stopping, clearing and up/down counters in response to input
>    level comparison
>  * Starting, clearing, stopping and up/down counters in response to a
>    maximum of four external triggers
>  * Output pin disable function by dead time error and detected
>    short-circuits between output pins
>  * A/D converter start triggers can be generated (GPT32E0 to GPT32E3)
>  * Enables the noise filter for input capture and external trigger
>    operation
>
> This patch adds basic pwm support for RZ/G2L GPT driver by creating
> separate logical channels for each IOs.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v11->v12:
>  * Added return code for get_state()
>  * Cache duty cycle/prescale as the driver cannot read the current duty
>    cycle/prescale from the hardware if the hardware is disabled. Cache the
>    last programmed duty cycle/prescale value to return in that case.
>  * Updated rzg2l_gpt_enable to enable the clocks.
>  * Updated rzg2l_gpt_disable to disable the clocks.
>  * Updated rzg2l_gpt_config() to cache duty cucle/prescale value
>  * Updated rzg2l_gpt_get_state to use cached value of duty cycle/prescale,If the PWM
>    is disabled.
>  * Simplified rzg2l_gpt_apply()
>  * Added comments in rzg2l_gpt_reset_assert_pm_disable()

Thanks for the update!

> --- /dev/null
> +++ b/drivers/pwm/pwm-rzg2l-gpt.c

> +struct rzg2l_gpt_chip {
> +       struct pwm_chip chip;
> +       void __iomem *mmio;
> +       struct reset_control *rstc;
> +       struct clk *clk;
> +       struct mutex lock;
> +       unsigned long rate;
> +       u32 state_period[RZG2L_MAX_HW_CHANNELS];
> +       u32 user_count[RZG2L_MAX_HW_CHANNELS];
> +       /*
> +        * The driver cannot read the current duty cycle/prescale from the
> +        * hardware if the hardware is disabled. Cache the last programmed
> +        * duty cycle/prescale value to return in that case.
> +        */
> +       u8 prescale[RZG2L_MAX_HW_CHANNELS];
> +       unsigned int duty_cycle[RZG2L_MAX_PWM_CHANNELS];

u32? The maximum value stored is U32_MAX.

> +};

> +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);
> +       u32 ch_index = RZG2L_GET_CH_INDEX(pwm->hwpwm);
> +       u32 offs = RZG2L_GET_CH_OFFS(ch_index);
> +       u8 prescale;
> +       u64 tmp;
> +       u32 val;
> +
> +       pm_runtime_get_sync(chip->dev);
> +       val = rzg2l_gpt_read(rzg2l_gpt, offs + RZG2L_GTCR);
> +       state->enabled = rzg2l_gpt_is_ch_enabled(rzg2l_gpt, pwm->hwpwm);
> +       if (state->enabled) {
> +               prescale = FIELD_GET(RZG2L_GTCR_TPCS, val);
> +
> +               val = rzg2l_gpt_read(rzg2l_gpt, offs + RZG2L_GTPR);
> +               tmp = NSEC_PER_SEC * (u64)val;
> +               state->period = DIV_ROUND_UP_ULL(tmp, rzg2l_gpt->rate) << (2 * prescale);
> +
> +               val = rzg2l_gpt_read(rzg2l_gpt,
> +                                    offs + RZG2L_GTCCR(RZG2L_IS_IOB(pwm->hwpwm)));
> +               tmp = NSEC_PER_SEC * (u64)val;
> +       } else {
> +               /* If the PWM is disabled, use the cached value. */
> +               prescale = rzg2l_gpt->prescale[ch_index];
> +               tmp = NSEC_PER_SEC * (u64)rzg2l_gpt->duty_cycle[pwm->hwpwm];

Nit: Just set "val = rzg2l_gpt->duty_cycle[pwm->hwpwm];", and factor
"tmp = NSEC_PER_SEC * (u64)val;" out of the if-statement.

> +       }
> +
> +       state->duty_cycle = DIV_ROUND_UP_ULL(tmp, rzg2l_gpt->rate) << (2 * prescale);
> +       state->polarity = PWM_POLARITY_NORMAL;
> +       pm_runtime_put(chip->dev);
> +
> +       return 0;
> +}

> +static int __maybe_unused rzg2l_gpt_pm_runtime_suspend(struct device *dev)
> +{
> +       struct rzg2l_gpt_chip *rzg2l_gpt = dev_get_drvdata(dev);
> +
> +       clk_disable_unprepare(rzg2l_gpt->clk);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused rzg2l_gpt_pm_runtime_resume(struct device *dev)
> +{
> +       struct rzg2l_gpt_chip *rzg2l_gpt = dev_get_drvdata(dev);
> +
> +       clk_prepare_enable(rzg2l_gpt->clk);
> +
> +       return 0;
> +}
> +
> +static const struct dev_pm_ops rzg2l_gpt_pm_ops = {
> +       SET_RUNTIME_PM_OPS(rzg2l_gpt_pm_runtime_suspend, rzg2l_gpt_pm_runtime_resume, NULL)
> +};

DEFINE_RUNTIME_DEV_PM_OPS(), so you can drop the __maybe_unused
from the callbacks.

> +static struct platform_driver rzg2l_gpt_driver = {
> +       .driver = {
> +               .name = "pwm-rzg2l-gpt",
> +               .pm = &rzg2l_gpt_pm_ops,

pm_ptr(&rzg2l_gpt_pm_ops)?

> +               .of_match_table = of_match_ptr(rzg2l_gpt_of_table),
> +       },
> +       .probe = rzg2l_gpt_probe,
> +};
> +module_platform_driver(rzg2l_gpt_driver);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux