When we created the driver for the AXI PWMGEN IP block, we overlooked the fact that it can optionally be configured to use an external clock in addition to the AXI bus clock. This is easy to miss in testing because the bus clock is always on because it is driving other peripherals as well.
Up to now, users were specifying the external clock if there was one and the AXI bus clock otherwise. But the proper way to do this is to would be to always specify the bus clock and only specify the external clock if the IP block has been configured to use it.
To fix this, we add clock-names to the devicetree bindings and change clocks to allow 1 or 2 clocks.
--- Changes in v3: - Fixed clock-names DT property restrictions (was failing dt_binding_check) - Added Cc: stable - Picked up trailers - Link to v2: https://lore.kernel.org/r/20250522-pwm-axi-pwmgen-add-external-clock-v2-0-08...
Changes in v2: - Consider this a fix rather than a new feature. - Make clock-names required. - Simplify the logic in the pwm driver to avoid needing to test if clock-names is present in old dtbs that used the broken binding. - Link to v1: https://lore.kernel.org/r/20250520-pwm-axi-pwmgen-add-external-clock-v1-0-6c...
--- David Lechner (3): dt-bindings: pwm: adi,axi-pwmgen: update documentation link dt-bindings: pwm: adi,axi-pwmgen: fix clocks pwm: axi-pwmgen: fix missing separate external clock
.../devicetree/bindings/pwm/adi,axi-pwmgen.yaml | 15 +++++++++++--- drivers/pwm/pwm-axi-pwmgen.c | 23 +++++++++++++++++++--- 2 files changed, 32 insertions(+), 6 deletions(-) --- base-commit: 484803582c77061b470ac64a634f25f89715be3f change-id: 20250515-pwm-axi-pwmgen-add-external-clock-0364fbdf809b
Best regards,
Fix a shortcoming in the bindings that doesn't allow for a separate external clock.
The AXI PWMGEN IP block has a compile option ASYNC_CLK_EN that allows the use of an external clock for the PWM output separate from the AXI clock that runs the peripheral.
This was missed in the original bindings and so users were writing dts files where the one and only clock specified would be the external clock, if there was one, incorrectly missing the separate AXI clock.
The correct bindings are that the AXI clock is always required and the external clock is optional (must be given only when HDL compile option ASYNC_CLK_EN=1).
Cc: stable@vger.kernel.org Fixes: 1edf2c2a2841 ("dt-bindings: pwm: Add AXI PWM generator") Signed-off-by: David Lechner dlechner@baylibre.com --- Documentation/devicetree/bindings/pwm/adi,axi-pwmgen.yaml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pwm/adi,axi-pwmgen.yaml b/Documentation/devicetree/bindings/pwm/adi,axi-pwmgen.yaml index bc44381692054f647a160a6573dae4cff2ee3f31..e4c2d5186dedb18701af74bbc957b82a2b0f8737 100644 --- a/Documentation/devicetree/bindings/pwm/adi,axi-pwmgen.yaml +++ b/Documentation/devicetree/bindings/pwm/adi,axi-pwmgen.yaml @@ -30,11 +30,19 @@ properties: const: 3
clocks: - maxItems: 1 + minItems: 1 + maxItems: 2 + + clock-names: + minItems: 1 + items: + - const: axi + - const: ext
required: - reg - clocks + - clock-names
unevaluatedProperties: false
@@ -43,6 +51,7 @@ examples: pwm@44b00000 { compatible = "adi,axi-pwmgen-2.00.a"; reg = <0x44b00000 0x1000>; - clocks = <&spi_clk>; + clocks = <&fpga_clk>, <&spi_clk>; + clock-names = "axi", "ext"; #pwm-cells = <3>; };
Add proper support for external clock to the AXI PWM generator driver.
In most cases, the HDL for this IP block is compiled with the default ASYNC_CLK_EN=1. With this option, there is a separate external clock that drives the PWM output separate from the peripheral clock. So the driver should be enabling the "axi" clock to power the peripheral and the "ext" clock to drive the PWM output.
When ASYNC_CLK_EN=0, the "axi" clock is also used to drive the PWM output and there is no "ext" clock.
Previously, if there was a separate external clock, users had to specify only the external clock and (incorrectly) omit the AXI clock in order to get the correct operating frequency for the PWM output.
The devicetree bindings are updated to fix this shortcoming and this patch changes the driver to match the new bindings. To preserve compatibility with any existing dtbs that specify only one clock, we don't require the clock name on the first clock.
Cc: stable@vger.kernel.org Fixes: 41814fe5c782 ("pwm: Add driver for AXI PWM generator") Acked-by: Nuno Sá nuno.sa@analog.com Reviewed-by: Trevor Gamblin tgamblin@baylibre.com Signed-off-by: David Lechner dlechner@baylibre.com --- drivers/pwm/pwm-axi-pwmgen.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c index 4337c8f5acf055fc87dc134f2a70b99b0cb5ede6..60dcd354237316bced2d951b7f0b116c8291bb0d 100644 --- a/drivers/pwm/pwm-axi-pwmgen.c +++ b/drivers/pwm/pwm-axi-pwmgen.c @@ -257,7 +257,7 @@ static int axi_pwmgen_probe(struct platform_device *pdev) struct regmap *regmap; struct pwm_chip *chip; struct axi_pwmgen_ddata *ddata; - struct clk *clk; + struct clk *axi_clk, *clk; void __iomem *io_base; int ret;
@@ -280,9 +280,26 @@ static int axi_pwmgen_probe(struct platform_device *pdev) ddata = pwmchip_get_drvdata(chip); ddata->regmap = regmap;
- clk = devm_clk_get_enabled(dev, NULL); + /* + * Using NULL here instead of "axi" for backwards compatibility. There + * are some dtbs that don't give clock-names and have the "ext" clock + * as the one and only clock (due to mistake in the original bindings). + */ + axi_clk = devm_clk_get_enabled(dev, NULL); + if (IS_ERR(axi_clk)) + return dev_err_probe(dev, PTR_ERR(axi_clk), "failed to get axi clock\n"); + + clk = devm_clk_get_optional_enabled(dev, "ext"); if (IS_ERR(clk)) - return dev_err_probe(dev, PTR_ERR(clk), "failed to get clock\n"); + return dev_err_probe(dev, PTR_ERR(clk), "failed to get ext clock\n"); + + /* + * If there is no "ext" clock, it means the HDL was compiled with + * ASYNC_CLK_EN=0. In this case, the AXI clock is also used for the + * PWM output clock. + */ + if (!clk) + clk = axi_clk;
ret = devm_clk_rate_exclusive_get(dev, clk); if (ret)
linux-stable-mirror@lists.linaro.org
-
David Lechner