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

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

 



Hi Biju,

On Fri, Oct 28, 2022 at 12:42 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>
> ---
> v8->v9:
>  * deassert after devm_clk_get() to avoid reset stays deasserted,in case
>    clk_get() fails.
>  * Removed ch_offs from struct rzg2l_gpt_chip and use macro instead.
>  * Removed clk_disable_unprepare() from probe as it is giving
>    gpt_pclk already disabled warning in the error path.
>         [    0.915664]  clk_core_disable+0x25c/0x274
>         [    0.915754]  clk_disable+0x2c/0x44
>         [    0.915833]  rzg2l_gpt_pm_runtime_suspend+0x1c/0x34
>         [    0.915938]  pm_generic_runtime_suspend+0x28/0x40
>         [    0.916042]  genpd_runtime_suspend+0xa8/0x2b0
>         [    0.916136]  __rpm_callback+0x44/0x13c
>         [    0.916218]  rpm_callback+0x64/0x70
>         [    0.916296]  rpm_suspend+0x104/0x630
>         [    0.916374]  pm_runtime_work+0xb4/0xbc
>         [    0.916456]  process_one_work+0x288/0x6a

Thanks for the update!

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

> +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)
> +};
> +
> +static void rzg2l_gpt_reset_assert_pm_disable(void *data)
> +{
> +       struct rzg2l_gpt_chip *rzg2l_gpt = data;
> +
> +       pm_runtime_disable(rzg2l_gpt->chip.dev);
> +       pm_runtime_set_suspended(rzg2l_gpt->chip.dev);
> +       reset_control_assert(rzg2l_gpt->rstc);
> +}
> +
> +static int rzg2l_gpt_probe(struct platform_device *pdev)
> +{
> +       bool ch_en[RZG2L_MAX_PWM_CHANNELS];
> +       struct rzg2l_gpt_chip *rzg2l_gpt;
> +       int ret;
> +       u32 i;
> +
> +       rzg2l_gpt = devm_kzalloc(&pdev->dev, sizeof(*rzg2l_gpt), GFP_KERNEL);
> +       if (!rzg2l_gpt)
> +               return -ENOMEM;
> +
> +       rzg2l_gpt->mmio = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(rzg2l_gpt->mmio))
> +               return PTR_ERR(rzg2l_gpt->mmio);
> +
> +       rzg2l_gpt->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +       if (IS_ERR(rzg2l_gpt->rstc))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt->rstc),
> +                                    "get reset failed\n");
> +
> +       rzg2l_gpt->clk = devm_clk_get(&pdev->dev, NULL);
> +       if (IS_ERR(rzg2l_gpt->clk))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(rzg2l_gpt->clk),
> +                                    "cannot get clock\n");
> +
> +       ret = reset_control_deassert(rzg2l_gpt->rstc);
> +       if (ret)
> +               return dev_err_probe(&pdev->dev, ret,
> +                                    "cannot deassert reset control\n");
> +
> +       rzg2l_gpt->rate = clk_get_rate(rzg2l_gpt->clk);
> +
> +       clk_prepare_enable(rzg2l_gpt->clk);

So you enable the clock in .probe()...

> +       pm_runtime_set_active(&pdev->dev);
> +       pm_runtime_enable(&pdev->dev);
> +       ret = devm_add_action_or_reset(&pdev->dev,
> +                                      rzg2l_gpt_reset_assert_pm_disable,

... and rely on Runtime PM to disable the clock on error/remove?
Does that actually work?

> +                                      rzg2l_gpt);
> +       if (ret < 0)
> +               return ret;
> +
> +       mutex_init(&rzg2l_gpt->lock);
> +       platform_set_drvdata(pdev, rzg2l_gpt);
> +
> +       /*
> +        *  We need to keep the clock on, in case the bootloader has enabled the
> +        *  PWM and is running during probe().
> +        */
> +       for (i = 0; i < RZG2L_MAX_PWM_CHANNELS; i++) {
> +               ch_en[i] = rzg2l_gpt_is_ch_enabled(rzg2l_gpt, i);
> +               if (ch_en[i])
> +                       pm_runtime_get_sync(&pdev->dev);
> +       }
> +
> +       rzg2l_gpt->chip.dev = &pdev->dev;
> +       rzg2l_gpt->chip.ops = &rzg2l_gpt_ops;
> +       rzg2l_gpt->chip.npwm = RZG2L_MAX_PWM_CHANNELS;
> +
> +       ret = devm_pwmchip_add(&pdev->dev, &rzg2l_gpt->chip);
> +       if (ret) {
> +               for (i = 0; i < RZG2L_MAX_PWM_CHANNELS; i++) {
> +                       if (ch_en[i])
> +                               pm_runtime_put(&pdev->dev);
> +               }
> +
> +               dev_err_probe(&pdev->dev, ret, "failed to add PWM chip\n");
> +       }
> +
> +       return ret;
> +}

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