[PATCH v3 0/2] Add PWM support Amlogic S7 S7D S6

[PATCH v3 0/2] Add PWM support Amlogic S7 S7D S6

[Xianwei Zhao via B4 Relay]

Add bindings and driver support Amlogic S7/S7D/S6 SoCs.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Changes in v3:
- Use npwm instead of single_pwm in priv_data and format adjustments.
- Link to v2: https://lore.kernel.org/r/20260402-s6-s7-pwm-v2-0-657dce040956@amlogic.com

Changes in v2:
- Simpler s7 clock desc, and drop the example in bindings.
- Make minor changes to probe based on Martin's suggestion.
- Link to v1: https://lore.kernel.org/r/20260326-s6-s7-pwm-v1-0-67e2f72b98bc@amlogic.com

---
Junyi Zhao (1):
      dt-bindings: pwm: amlogic: Add new bindings for S6 S7 S7D

Xianwei Zhao (1):
      pwm: meson: Add support for Amlogic S7

 .../devicetree/bindings/pwm/pwm-amlogic.yaml       | 19 ++++++++++
 drivers/pwm/pwm-meson.c                            | 41 ++++++++++++++++++++--
 2 files changed, 57 insertions(+), 3 deletions(-)
---
base-commit: b1385d0c7b7c633e55adcf4a7c1ef46c43a84a4a
change-id: 20260325-s6-s7-pwm-281658b88736

Best regards,
-- 
Xianwei Zhao <xianwei.zhao@amlogic.com>

[PATCH v3 1/2] dt-bindings: pwm: amlogic: Add new bindings for S6 S7 S7D

[Xianwei Zhao via B4 Relay]

From: Junyi Zhao <junyi.zhao@amlogic.com>

Amlogic S7/S7D/S6 different from the previous SoCs, a controller
includes one pwm, at the same time, the controller has only one
input clock source.

Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 .../devicetree/bindings/pwm/pwm-amlogic.yaml          | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
index c337d85da40f..93fa97f4011b 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
+++ b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
@@ -37,6 +37,7 @@ properties:
       - enum:
           - amlogic,meson8-pwm-v2
           - amlogic,meson-s4-pwm
+          - amlogic,s7-pwm
       - items:
           - enum:
               - amlogic,a4-pwm
@@ -45,6 +46,11 @@ properties:
               - amlogic,t7-pwm
               - amlogic,meson-a1-pwm
           - const: amlogic,meson-s4-pwm
+      - items:
+          - enum:
+              - amlogic,s6-pwm
+              - amlogic,s7d-pwm
+          - const: amlogic,s7-pwm
       - items:
           - enum:
               - amlogic,meson8b-pwm-v2
@@ -146,6 +152,19 @@ allOf:
         clock-names: false
       required:
         - clocks
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,s7-pwm
+    then:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names: false
+      required:
+        - clocks
 
   - if:
       properties:

-- 
2.52.0

Re: [PATCH v3 1/2] dt-bindings: pwm: amlogic: Add new bindings for S6 S7 S7D

[Alexandre Mergnat]

On Thu, 21 May 2026 08:26:58 +0000, Xianwei Zhao <xianwei.zhao@amlogic.com> wrote:
> Amlogic S7/S7D/S6 different from the previous SoCs, a controller
> includes one pwm, at the same time, the controller has only one
> input clock source.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

-- 
Alexandre Mergnat <amergnat@baylibre.com>

[PATCH v3 2/2] pwm: meson: Add support for Amlogic S7

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add support for Amlogic S7 PWM. Amlogic S7 different from the
previous SoCs, a controller includes one pwm, at the same time,
the controller has only one input clock source.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/pwm/pwm-meson.c | 41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 8c6bf3d49753..66c41bf036de 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -113,6 +113,7 @@ struct meson_pwm_data {
 	int (*channels_init)(struct pwm_chip *chip);
 	bool has_constant;
 	bool has_polarity;
+	u8 npwm;
 };
 
 struct meson_pwm {
@@ -503,6 +504,18 @@ static void meson_pwm_s4_put_clk(void *data)
 	clk_put(clk);
 }
 
+static int meson_pwm_init_channels_s7(struct pwm_chip *chip)
+{
+	struct device *dev = pwmchip_parent(chip);
+	struct meson_pwm *meson = to_meson_pwm(chip);
+
+	meson->channels[0].clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(meson->channels[0].clk))
+		return dev_err_probe(dev, PTR_ERR(meson->channels[0].clk),
+				     "Failed to get clk\n");
+	return 0;
+}
+
 static int meson_pwm_init_channels_s4(struct pwm_chip *chip)
 {
 	struct device *dev = pwmchip_parent(chip);
@@ -530,6 +543,7 @@ static int meson_pwm_init_channels_s4(struct pwm_chip *chip)
 static const struct meson_pwm_data pwm_meson8b_data = {
 	.parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" },
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
+	.npwm = MESON_NUM_PWMS,
 };
 
 /*
@@ -539,6 +553,7 @@ static const struct meson_pwm_data pwm_meson8b_data = {
 static const struct meson_pwm_data pwm_gxbb_ao_data = {
 	.parent_names = { "xtal", "clk81", NULL, NULL },
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
+	.npwm = MESON_NUM_PWMS,
 };
 
 static const struct meson_pwm_data pwm_axg_ee_data = {
@@ -546,6 +561,7 @@ static const struct meson_pwm_data pwm_axg_ee_data = {
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
 	.has_constant = true,
 	.has_polarity = true,
+	.npwm = MESON_NUM_PWMS,
 };
 
 static const struct meson_pwm_data pwm_axg_ao_data = {
@@ -553,6 +569,7 @@ static const struct meson_pwm_data pwm_axg_ao_data = {
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
 	.has_constant = true,
 	.has_polarity = true,
+	.npwm = MESON_NUM_PWMS,
 };
 
 static const struct meson_pwm_data pwm_g12a_ee_data = {
@@ -560,6 +577,7 @@ static const struct meson_pwm_data pwm_g12a_ee_data = {
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
 	.has_constant = true,
 	.has_polarity = true,
+	.npwm = MESON_NUM_PWMS,
 };
 
 static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
@@ -567,6 +585,7 @@ static const struct meson_pwm_data pwm_g12a_ao_ab_data = {
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
 	.has_constant = true,
 	.has_polarity = true,
+	.npwm = MESON_NUM_PWMS,
 };
 
 static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
@@ -574,22 +593,33 @@ static const struct meson_pwm_data pwm_g12a_ao_cd_data = {
 	.channels_init = meson_pwm_init_channels_meson8b_legacy,
 	.has_constant = true,
 	.has_polarity = true,
+	.npwm = MESON_NUM_PWMS,
 };
 
 static const struct meson_pwm_data pwm_meson8_v2_data = {
 	.channels_init = meson_pwm_init_channels_meson8b_v2,
+	.npwm = MESON_NUM_PWMS,
 };
 
 static const struct meson_pwm_data pwm_meson_axg_v2_data = {
 	.channels_init = meson_pwm_init_channels_meson8b_v2,
 	.has_constant = true,
 	.has_polarity = true,
+	.npwm = MESON_NUM_PWMS,
 };
 
 static const struct meson_pwm_data pwm_s4_data = {
 	.channels_init = meson_pwm_init_channels_s4,
 	.has_constant = true,
 	.has_polarity = true,
+	.npwm = MESON_NUM_PWMS,
+};
+
+static const struct meson_pwm_data pwm_s7_data = {
+	.channels_init = meson_pwm_init_channels_s7,
+	.has_constant = true,
+	.has_polarity = true,
+	.npwm = 1,
 };
 
 static const struct of_device_id meson_pwm_matches[] = {
@@ -642,7 +672,11 @@ static const struct of_device_id meson_pwm_matches[] = {
 		.compatible = "amlogic,meson-s4-pwm",
 		.data = &pwm_s4_data
 	},
-	{},
+	{
+		.compatible = "amlogic,s7-pwm",
+		.data = &pwm_s7_data
+	},
+	{ }
 };
 MODULE_DEVICE_TABLE(of, meson_pwm_matches);
 
@@ -650,9 +684,10 @@ static int meson_pwm_probe(struct platform_device *pdev)
 {
 	struct pwm_chip *chip;
 	struct meson_pwm *meson;
+	const struct meson_pwm_data *pdata = of_device_get_match_data(&pdev->dev);
 	int err;
 
-	chip = devm_pwmchip_alloc(&pdev->dev, MESON_NUM_PWMS, sizeof(*meson));
+	chip = devm_pwmchip_alloc(&pdev->dev, pdata->npwm, sizeof(*meson));
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
 	meson = to_meson_pwm(chip);
@@ -664,7 +699,7 @@ static int meson_pwm_probe(struct platform_device *pdev)
 	spin_lock_init(&meson->lock);
 	chip->ops = &meson_pwm_ops;
 
-	meson->data = of_device_get_match_data(&pdev->dev);
+	meson->data = pdata;
 
 	err = meson->data->channels_init(chip);
 	if (err < 0)

-- 
2.52.0

Re: [PATCH v3 2/2] pwm: meson: Add support for Amlogic S7

[Alexandre Mergnat]

On Thu, 21 May 2026 08:26:59 +0000, Xianwei Zhao <xianwei.zhao@amlogic.com> wrote:
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 8c6bf3d49753..66c41bf036de 100644
> --- a/drivers/pwm/pwm-meson.c
> +++ b/drivers/pwm/pwm-meson.c
> @@ -503,6 +504,18 @@ static void meson_pwm_s4_put_clk(void *data)
>  	clk_put(clk);
>  }
>  
> +static int meson_pwm_init_channels_s7(struct pwm_chip *chip)
> +{
> +	struct device *dev = pwmchip_parent(chip);
> +	struct meson_pwm *meson = to_meson_pwm(chip);
> +
> +	meson->channels[0].clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(meson->channels[0].clk))
> +		return dev_err_probe(dev, PTR_ERR(meson->channels[0].clk),
> +				     "Failed to get clk\n");
> +	return 0;
> +}

This adds a fourth channels_init callback, and after the series
pwm_s7_data ends up differing from pwm_s4_data only by .npwm and by this
function. Would it make sense to share a single init between the two
instead?

What blocks reusing meson_pwm_init_channels_s4() as it stands is that it
needs an indexed clock lookup, and there is no devm variant of clk_get()
taking an index, hence the of_clk_get() + devm_add_action_or_reset()
dance. devm_clk_bulk_get_all() (include/linux/clk.h) is however both
devm-managed and index-based, and of_clk_bulk_get_all() is implemented as
clks[i].clk = of_clk_get(np, i) (drivers/clk/clk-bulk.c), so it performs
exactly the lookup the S4 path already does, only with automatic cleanup.
Something along these lines:

 static int meson_pwm_init_channels_per_channel_clk(struct pwm_chip *chip)
 {
      struct device *dev = pwmchip_parent(chip);
      struct meson_pwm *meson = to_meson_pwm(chip);
      struct clk_bulk_data *clks;
      unsigned int i;
      int num;

      num = devm_clk_bulk_get_all(dev, &clks);
      if (num < 0)
              return dev_err_probe(dev, num, "Failed to get clocks\n");

      if (num != chip->npwm)
              return dev_err_probe(dev, -EINVAL,
                                   "expected %u clocks, got %d\n",
                                   chip->npwm, num);

      for (i = 0; i < chip->npwm; i++)
              meson->channels[i].clk = clks[i].clk;

      return 0;
 }

chip->npwm is usable here because devm_pwmchip_alloc() fills it in probe
before channels_init() runs, so one function covers npwm = 2 and npwm = 1
with no variant-specific code, and meson_pwm_s4_put_clk() goes away with
it. A future variant would then only need its .npwm value.

The count check is worth keeping: of_clk_get(np, i) currently fails probe
with a clear message when the DT node has fewer clocks than expected,
while devm_clk_bulk_get_all() would simply return fewer clocks and leave
channels[i].clk NULL. As clk_prepare_enable(NULL) succeeds and
clk_round_rate(NULL, ...) returns 0, the problem would only surface later
in .apply() as a confusing "invalid source clock frequency". The check
just preserves the diagnostic you have today.

There is a similar use of this API, count check included, in
drivers/pmdomain/amlogic/meson-ee-pwrc.c, in case it is useful as a
reference.

Since this touches the existing S4 path, it would probably be easier to
review split up, roughly:

   1. dt-bindings (unchanged, keeps its Reviewed-by tags)
   2. pwm: meson: make the PWM count driver data -- add .npwm, fill the
      existing entries, use it in probe, and switch the loop in
      meson_pwm_init_clocks_meson8b() to chip->npwm. No functional change.
   3. pwm: meson: get per-channel clocks with devm_clk_bulk_get_all() --
      convert and rename meson_pwm_init_channels_s4(), drop
      meson_pwm_s4_put_clk(). No functional change.
   4. pwm: meson: Add support for Amlogic S7 -- reduced to pwm_s7_data
      with .npwm = 1 and the of_device_id entry.

That is clearly more work than what you have, so please do push back if
you think it is not worth it for this series. And if you are keeping a
separate function on purpose because devm_clk_get() is where you would
like the driver to head, that is a perfectly good answer too -- I may
well be missing context here.

> @@ -530,6 +543,7 @@ static int meson_pwm_init_channels_s4(struct pwm_chip *chip)
>  static const struct meson_pwm_data pwm_meson8b_data = {
>  	.parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" },
>  	.channels_init = meson_pwm_init_channels_meson8b_legacy,
> +	.npwm = MESON_NUM_PWMS,

I suggest use "2" instead of "MESON_NUM_PWMS"

> @@ -642,7 +672,11 @@ static const struct of_device_id meson_pwm_matches[] = {
>  		.compatible = "amlogic,meson-s4-pwm",
>  		.data = &pwm_s4_data
>  	},
> -	{},
> +	{
> +		.compatible = "amlogic,s7-pwm",
> +		.data = &pwm_s7_data
> +	},
> +	{ }

Small nit: changing the sentinel from "{}," to "{ }" is unrelated to
adding S7 support. Patches tend to be easier to review when they carry only
the functional change, so would you mind dropping it, or splitting it into
its own trivial cleanup?

-- 
Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH v3 2/2] pwm: meson: Add support for Amlogic S7

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

Hello Alexandre,

thanks for your review, that's very appreciated.

On Tue, Aug 18, 2026 at 02:53:50PM +0200, Alexandre Mergnat wrote:
> > @@ -642,7 +672,11 @@ static const struct of_device_id meson_pwm_matches[] = {
> >  		.compatible = "amlogic,meson-s4-pwm",
> >  		.data = &pwm_s4_data
> >  	},
> > -	{},
> > +	{
> > +		.compatible = "amlogic,s7-pwm",
> > +		.data = &pwm_s7_data
> > +	},
> > +	{ }
> 
> Small nit: changing the sentinel from "{}," to "{ }" is unrelated to
> adding S7 support. Patches tend to be easier to review when they carry only
> the functional change, so would you mind dropping it, or splitting it into
> its own trivial cleanup?

Just skimming your mail, only here I want to express a little
disagreement. While I'm generally a friend of "each patch does exactly
one atomic thing", IMHO such reformatting in an array that is touched
anyhow is fine, and a separate commit that just updates the terminator
in this one array is over-engineered.

Just mention it in the commit log and that's fine for me.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 0/2] Add PWM support Amlogic S7 S7D S6

[Xianwei Zhao]

Hi Uwe,
    Gentle ping. I'd appreciate it if you could take another look when 
you have time.

On 2026/5/21 16:26, Xianwei Zhao via B4 Relay wrote:
> Add bindings and driver support Amlogic S7/S7D/S6 SoCs.
> 
> Signed-off-by: Xianwei Zhao<xianwei.zhao@amlogic.com>
> ---
> Changes in v3:
> - Use npwm instead of single_pwm in priv_data and format adjustments.
> - Link to v2:https://lore.kernel.org/r/20260402-s6-s7-pwm-v2-0-657dce040956@amlogic.com
> 
> Changes in v2:
> - Simpler s7 clock desc, and drop the example in bindings.
> - Make minor changes to probe based on Martin's suggestion.
> - Link to v1:https://lore.kernel.org/r/20260326-s6-s7-pwm-v1-0-67e2f72b98bc@amlogic.com
> 
> ---
> Junyi Zhao (1):
>        dt-bindings: pwm: amlogic: Add new bindings for S6 S7 S7D
> 
> Xianwei Zhao (1):
>        pwm: meson: Add support for Amlogic S7
> 
>   .../devicetree/bindings/pwm/pwm-amlogic.yaml       | 19 ++++++++++
>   drivers/pwm/pwm-meson.c                            | 41 ++++++++++++++++++++--
>   2 files changed, 57 insertions(+), 3 deletions(-)
> ---
> base-commit: b1385d0c7b7c633e55adcf4a7c1ef46c43a84a4a
> change-id: 20260325-s6-s7-pwm-281658b88736
> 
> Best regards,
> --
> Xianwei Zhao<xianwei.zhao@amlogic.com>