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