Re: [PATCH v3 4/8] pwm: stm32: add support for stm32mp25

[Fabrice Gasnier <fabrice.gasnier@foss.st.com>]

On 1/10/25 10:19, Fabrice Gasnier wrote:
> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
> new features along with registers and bits diversity.
> The MFD part of the driver fills in ipidr, so it is used to check the
> hardware configuration register, when available to gather the number
> of PWM channels and complementary outputs.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> ---
> Changes in v2:
> Address Uwe review comments:
> - Make MAX_PWM_OUTPUT definition less generic: STM32_PWM_MAX_OUTPUT
> - No need to initialize 'npwm'
> - refactor code, for *num_enabled to use same code path
> ---
>  drivers/pwm/pwm-stm32.c | 42 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 7 deletions(-)

Hi Uwe,

I think this patch still miss some reviews.
The first patches of this series have been merged.

Is it ok for you to merge, or shall I resend separately ?

Please advise,
BR,
Fabrice

> 
> diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
> index 17e591f61efb..b9aadc473280 100644
> --- a/drivers/pwm/pwm-stm32.c
> +++ b/drivers/pwm/pwm-stm32.c
> @@ -19,6 +19,7 @@
>  #define CCMR_CHANNEL_SHIFT 8
>  #define CCMR_CHANNEL_MASK  0xFF
>  #define MAX_BREAKINPUT 2
> +#define STM32_MAX_PWM_OUTPUT 4
>  
>  struct stm32_breakinput {
>  	u32 index;
> @@ -775,10 +776,19 @@ static int stm32_pwm_probe_breakinputs(struct stm32_pwm *priv,
>  	return stm32_pwm_apply_breakinputs(priv);
>  }
>  
> -static void stm32_pwm_detect_complementary(struct stm32_pwm *priv)
> +static void stm32_pwm_detect_complementary(struct stm32_pwm *priv, struct stm32_timers *ddata)
>  {
>  	u32 ccer;
>  
> +	if (ddata->ipidr) {
> +		u32 val;
> +
> +		/* Simply read from HWCFGR the number of complementary outputs (MP25). */
> +		regmap_read(priv->regmap, TIM_HWCFGR1, &val);
> +		priv->have_complementary_output = !!FIELD_GET(TIM_HWCFGR1_NB_OF_DT, val);
> +		return;
> +	}
> +
>  	/*
>  	 * If complementary bit doesn't exist writing 1 will have no
>  	 * effect so we can detect it.
> @@ -790,22 +800,39 @@ static void stm32_pwm_detect_complementary(struct stm32_pwm *priv)
>  	priv->have_complementary_output = (ccer != 0);
>  }
>  
> -static unsigned int stm32_pwm_detect_channels(struct regmap *regmap,
> +static unsigned int stm32_pwm_detect_channels(struct stm32_timers *ddata,
>  					      unsigned int *num_enabled)
>  {
> +	struct regmap *regmap = ddata->regmap;
>  	u32 ccer, ccer_backup;
>  
> +	regmap_read(regmap, TIM_CCER, &ccer_backup);
> +	*num_enabled = hweight32(ccer_backup & TIM_CCER_CCXE);
> +
> +	if (ddata->ipidr) {
> +		u32 hwcfgr;
> +		unsigned int npwm;
> +
> +		/* Deduce from HWCFGR the number of outputs (MP25). */
> +		regmap_read(regmap, TIM_HWCFGR1, &hwcfgr);
> +
> +		/*
> +		 * Timers may have more capture/compare channels than the
> +		 * actual number of PWM channel outputs (e.g. TIM_CH[1..4]).
> +		 */
> +		npwm = FIELD_GET(TIM_HWCFGR1_NB_OF_CC, hwcfgr);
> +
> +		return npwm < STM32_MAX_PWM_OUTPUT ? npwm : STM32_MAX_PWM_OUTPUT;
> +	}
> +
>  	/*
>  	 * If channels enable bits don't exist writing 1 will have no
>  	 * effect so we can detect and count them.
>  	 */
> -	regmap_read(regmap, TIM_CCER, &ccer_backup);
>  	regmap_set_bits(regmap, TIM_CCER, TIM_CCER_CCXE);
>  	regmap_read(regmap, TIM_CCER, &ccer);
>  	regmap_write(regmap, TIM_CCER, ccer_backup);
>  
> -	*num_enabled = hweight32(ccer_backup & TIM_CCER_CCXE);
> -
>  	return hweight32(ccer & TIM_CCER_CCXE);
>  }
>  
> @@ -820,7 +847,7 @@ static int stm32_pwm_probe(struct platform_device *pdev)
>  	unsigned int i;
>  	int ret;
>  
> -	npwm = stm32_pwm_detect_channels(ddata->regmap, &num_enabled);
> +	npwm = stm32_pwm_detect_channels(ddata, &num_enabled);
>  
>  	chip = devm_pwmchip_alloc(dev, npwm, sizeof(*priv));
>  	if (IS_ERR(chip))
> @@ -841,7 +868,7 @@ static int stm32_pwm_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, ret,
>  				     "Failed to configure breakinputs\n");
>  
> -	stm32_pwm_detect_complementary(priv);
> +	stm32_pwm_detect_complementary(priv, ddata);
>  
>  	ret = devm_clk_rate_exclusive_get(dev, priv->clk);
>  	if (ret)
> @@ -911,6 +938,7 @@ static DEFINE_SIMPLE_DEV_PM_OPS(stm32_pwm_pm_ops, stm32_pwm_suspend, stm32_pwm_r
>  
>  static const struct of_device_id stm32_pwm_of_match[] = {
>  	{ .compatible = "st,stm32-pwm",	},
> +	{ .compatible = "st,stm32mp25-pwm", },
>  	{ /* end node */ },
>  };
>  MODULE_DEVICE_TABLE(of, stm32_pwm_of_match);