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

[Xianwei Zhao <xianwei.zhao@amlogic.com>]

Hi Uwe,
    Thanks for your review.

On 2026/5/18 01:12, Uwe Kleine-König wrote:
> 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.)
> 

I will use npwm(u8) instead of single_pwm.

>>   };
>>   
>>   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.)
>

Will do.

> Best regards
> Uwe