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

["Uwe Kleine-König" <ukleinek@kernel.org>]

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

Hello,

On Thu, Apr 02, 2026 at 02:40:16AM +0000, Xianwei Zhao via B4 Relay wrote:
> 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 | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
> index 8c6bf3d49753..7a43c42ef3d6 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;
> +	bool single_pwm;

Conceptually I'd prefer a `npwm` field here. That doesn't take more
space in memory and simplifies the logic a bit. (At the cost of having
to adapt all already existing meson_pwm_data instances, but that's fine
in my book.)

>  };
>  
>  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);
> @@ -592,6 +605,13 @@ static const struct meson_pwm_data pwm_s4_data = {
>  	.has_polarity = true,
>  };
>  
> +static const struct meson_pwm_data pwm_s7_data = {
> +	.channels_init = meson_pwm_init_channels_s7,
> +	.has_constant = true,
> +	.has_polarity = true,
> +	.single_pwm = true,
> +};
> +
>  static const struct of_device_id meson_pwm_matches[] = {
>  	{
>  		.compatible = "amlogic,meson8-pwm-v2",
> @@ -642,6 +662,10 @@ 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
> +	},
>  	{},

If you touch that array in the next revision, please make this line:

	{ }

(I.e. add a space and drop the comma.)

Best regards
Uwe

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