Re: [PATCH v3 2/4] pwm: sun50i: Add H616 PWM support
From: Richard GENOUD
Date: Fri Feb 27 2026 - 11:22:40 EST
Le 26/02/2026 à 15:12, Philipp Zabel a écrit :
On Fr, 2026-01-23 at 10:33 +0100, Richard Genoud wrote:Ah! You're right!
Add driver for Allwinner H616 PWM controller, supporting up to 6[...]
channels.
Those channels output can be either a PWM signal output or a clock
output, thanks to the bypass.
The channels are paired (0/1, 2/3 and 4/5) and each pair has a
prescaler/mux/gate.
Moreover, each channel has its own prescaler and bypass.
The clock provider part of this driver is needed not only because the
H616 PWM controller provides also clocks when bypass is enabled, but
really because pwm-clock isn't fit to handle all cases here.
pwm-clock would work if the 100MHz clock is requested, but if a lower
clock is requested (like 24MHz), it will request a 42ns period to the
PWM driver which will happily serve, with the 100MHz clock as input a
25MHz frequency and a duty cycle adjustable in the range [0-4]/4,
because that is a sane thing to do for a PWM.
The information missing is that a real clock is resquested, not a PWM.
Signed-off-by: Richard Genoud <richard.genoud@xxxxxxxxxxx>
---
drivers/pwm/Kconfig | 12 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-sun50i-h616.c | 959 ++++++++++++++++++++++++++++++++++
3 files changed, 972 insertions(+)
create mode 100644 drivers/pwm/pwm-sun50i-h616.c
diff --git a/drivers/pwm/pwm-sun50i-h616.c b/drivers/pwm/pwm-sun50i-h616.c[...]
new file mode 100644
index 000000000000..02a8e2d39f86
--- /dev/null
+++ b/drivers/pwm/pwm-sun50i-h616.c
@@ -0,0 +1,959 @@
+static int h616_pwm_init_clocks(struct platform_device *pdev,
+ struct h616_pwm_chip *h616chip)
+{
+ struct clk_pwm_pdata *pdata;
+ struct device *dev = &pdev->dev;
+ int num_clocks = 0;
+ int ret;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return dev_err_probe(dev, -ENOMEM,
+ "Failed to allocate clk_pwm_pdata\n");
+
+ while (pwmcc_data[num_clocks].name)
+ num_clocks++;
+
+ pdata->hw_data = devm_kzalloc(dev, struct_size(pdata->hw_data, hws, num_clocks),
+ GFP_KERNEL);
+ if (!pdata->hw_data)
+ return dev_err_probe(dev, -ENOMEM,
+ "Failed to allocate hw clocks\n");
+
+ pdata->hw_data->num = num_clocks;
+ pdata->reg = h616chip->base;
+
+ spin_lock_init(&pdata->lock);
+
+ for (int i = 0; i < num_clocks; i++) {
+ struct clk_hw **hw = &pdata->hw_data->hws[i];
+
+ ret = h616_add_composite_clk(&pwmcc_data[i], pdata->reg,
+ &pdata->lock, dev, hw);
+ if (ret) {
+ dev_err_probe(dev, ret,
+ "Failed to register hw clock %s\n",
+ pwmcc_data[i].name);
+ for (i--; i >= 0; i--)
+ clk_hw_unregister_composite(pdata->hw_data->hws[i]);
+ return ret;
+ }
+ }
+
+ h616chip->clk_pdata = pdata;
+
+ return 0;
+}
+
+static int h616_pwm_probe(struct platform_device *pdev)
+{
+ const struct h616_pwm_data *data;
+ struct device *dev = &pdev->dev;
+ struct h616_pwm_chip *h616chip;
+ struct pwm_chip *chip;
+ int ret;
+
+ data = of_device_get_match_data(dev);
+ if (!data)
+ return dev_err_probe(dev, -ENODEV,
+ "Missing specific data structure\n");
+
+ chip = devm_pwmchip_alloc(dev, data->npwm, sizeof(*h616chip));
+ if (IS_ERR(chip))
+ return dev_err_probe(dev, PTR_ERR(chip),
+ "Failed to allocate pwmchip\n");
+
+ h616chip = h616_pwm_from_chip(chip);
+ h616chip->data = data;
+ h616chip->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(h616chip->base))
+ return dev_err_probe(dev, PTR_ERR(h616chip->base),
+ "Failed to get PWM base address\n");
+
+ h616chip->bus_clk = devm_clk_get_enabled(dev, "bus");
+ if (IS_ERR(h616chip->bus_clk))
+ return dev_err_probe(dev, PTR_ERR(h616chip->bus_clk),
+ "Failed to get bus clock\n");
+
+ h616chip->channels = devm_kmalloc_array(dev, data->npwm,
+ sizeof(*(h616chip->channels)),
+ GFP_KERNEL);
+ if (!h616chip->channels)
+ return dev_err_probe(dev, -ENOMEM,
+ "Failed to allocate %d channels array\n",
+ data->npwm);
+
+ h616chip->rst = devm_reset_control_get_shared(dev, NULL);
+ if (IS_ERR(h616chip->rst))
+ return dev_err_probe(dev, PTR_ERR(h616chip->rst),
+ "Failed to get reset control\n");
+
+ chip->ops = &h616_pwm_ops;
+
+ ret = h616_pwm_init_clocks(pdev, h616chip);
+ if (ret)
+ return ret;
+
+ for (unsigned int i = 0; i < data->npwm; i++) {
+ struct h616_pwm_channel *chan = &h616chip->channels[i];
+ struct clk_hw **hw = &h616chip->clk_pdata->hw_data->hws[i];
+
+ chan->pwm_clk = devm_clk_hw_get_clk(dev, *hw, NULL);
+ if (IS_ERR(chan->pwm_clk)) {
+ ret = dev_err_probe(dev, PTR_ERR(chan->pwm_clk),
+ "Failed to register PWM clock %d\n", i);
+ goto err_get_clk;
+ }
+ chan->mode = H616_PWM_MODE_NONE;
+ }
+
+ ret = devm_of_clk_add_hw_provider(dev, h616_pwm_of_clk_get, h616chip);
+ if (ret) {
+ dev_err_probe(dev, ret, "Failed to add HW clock provider\n");
+ goto err_add_clk_provider;
+ }
+
+ /* Deassert reset */
+ ret = reset_control_deassert(h616chip->rst);
+ if (ret) {
+ dev_err_probe(dev, ret, "Cannot deassert reset control\n");
+ goto err_ctrl_deassert;
+ }
+
+ ret = pwmchip_add(chip);
+ if (ret < 0) {
+ dev_err_probe(dev, ret, "Failed to add PWM chip\n");
+ goto err_pwm_add;
+ }
+
+ platform_set_drvdata(pdev, chip);
+
+ return 0;
+
+err_pwm_add:
+ reset_control_assert(h616chip->rst);
+
+err_ctrl_deassert:
+err_add_clk_provider:
+err_get_clk:
+ for (unsigned int i = 0; i < h616chip->clk_pdata->hw_data->num; i++)
+ clk_hw_unregister_composite(h616chip->clk_pdata->hw_data->hws[i]);
Won't this try to unregister the clk_hw before the pwm_clk derived from
it? You could place this in a devres action to correct the cleanup
order and get rid of the duplicated cleanup in h616_pwm_remove().
Nice!
I think you could even use devm_pwmchip_add() and
devm_reset_control_get_shared_deasserted() and remove h616_pwm_remove
entirely.
I'll do that.
Thanks!
Regards,
Richard
regards
Philipp