Re: [PATCH 2/9] mfd: stm32-timers: add support for stm32mp25

Lee Jones
2025-01-09T10:50:02Z

On Wed, 18 Dec 2024, Fabrice Gasnier wrote:

> Add support for STM32MP25 SoC. Use newly introduced compatible, to handle
> new features.
> Identification and hardware configuration registers allow to read the
> timer version and capabilities (counter width, number of channels...).
> So, rework the probe to avoid touching ARR register by simply read the
> counter width when available. This may avoid messing with a possibly
> running timer.
> Also add useful bit fields to stm32-timers header file.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> ---
> drivers/mfd/stm32-timers.c | 32 +++++++++++++++++++++++++++++++-
> include/linux/mfd/stm32-timers.h | 9 +++++++++
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> index 650724e19b88..6f217c32482c 100644
> --- a/drivers/mfd/stm32-timers.c
> +++ b/drivers/mfd/stm32-timers.c
> @@ -9,6 +9,7 @@
> #include <linux/module.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> +#include <linux/property.h>
> #include <linux/reset.h>
>
> #define STM32_TIMERS_MAX_REGISTERS 0x3fc
> @@ -173,6 +174,32 @@ static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
> regmap_write(ddata->regmap, TIM_ARR, arr);
> }
>
> +static int stm32_timers_probe_hwcfgr(struct device *dev, struct stm32_timers *ddata)
> +{
> + u32 val;
> +
> + ddata->ipidr = (uintptr_t)device_get_match_data(dev);

Are you sure this cast is needed?

> + if (!ddata->ipidr) {
> + /* fallback to legacy method for probing counter width */

Sentences start with uppercase chars.

> + stm32_timers_get_arr_size(ddata);
> + return 0;
> + }
> +
> + regmap_read(ddata->regmap, TIM_IPIDR, &val);
> + /* Sanity check on IP identification register */

This seems obvious, thus superfluous.

> + if (val != ddata->ipidr) {
> + dev_err(dev, "Unexpected identification: %u\n", val);

"Unexpected model number"?
"Unsupported device detected"?

> + return -EINVAL;
> + }
> +
> + regmap_read(ddata->regmap, TIM_HWCFGR2, &val);

'/n' here.

> + /* Counter width in bits, max reload value is BIT(width) - 1 */
> + ddata->max_arr = BIT(FIELD_GET(TIM_HWCFGR2_CNT_WIDTH, val)) - 1;
> + dev_dbg(dev, "TIM width: %ld\n", FIELD_GET(TIM_HWCFGR2_CNT_WIDTH, val));

How useful is this now the driver has been developed?

> + return 0;
> +}
> +
> static int stm32_timers_dma_probe(struct device *dev,
> struct stm32_timers *ddata)
> {
> @@ -285,7 +312,9 @@ static int stm32_timers_probe(struct platform_device *pdev)
> if (IS_ERR(ddata->clk))
> return PTR_ERR(ddata->clk);
>
> - stm32_timers_get_arr_size(ddata);
> + ret = stm32_timers_probe_hwcfgr(dev, ddata);
> + if (ret)
> + return ret;
>
> ret = stm32_timers_irq_probe(pdev, ddata);
> if (ret)
> @@ -320,6 +349,7 @@ static void stm32_timers_remove(struct platform_device *pdev)
>
> static const struct of_device_id stm32_timers_of_match[] = {
> { .compatible = "st,stm32-timers", },
> + { .compatible = "st,stm32mp25-timers", .data = (void *)STM32MP25_TIM_IPIDR },
> { /* end node */ },
> };
> MODULE_DEVICE_TABLE(of, stm32_timers_of_match);
> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> index f09ba598c97a..23b0cae4a9f8 100644
> --- a/include/linux/mfd/stm32-timers.h
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -33,6 +33,9 @@
> #define TIM_DCR 0x48 /* DMA control register */
> #define TIM_DMAR 0x4C /* DMA register for transfer */
> #define TIM_TISEL 0x68 /* Input Selection */
> +#define TIM_HWCFGR2 0x3EC /* hardware configuration 2 Reg (MP25) */
> +#define TIM_HWCFGR1 0x3F0 /* hardware configuration 1 Reg (MP25) */
> +#define TIM_IPIDR 0x3F8 /* IP identification Reg (MP25) */
>
> #define TIM_CR1_CEN BIT(0) /* Counter Enable */
> #define TIM_CR1_DIR BIT(4) /* Counter Direction */
> @@ -100,6 +103,9 @@
> #define TIM_BDTR_BKF(x) (0xf << (16 + (x) * 4))
> #define TIM_DCR_DBA GENMASK(4, 0) /* DMA base addr */
> #define TIM_DCR_DBL GENMASK(12, 8) /* DMA burst len */
> +#define TIM_HWCFGR1_NB_OF_CC GENMASK(3, 0) /* Capture/compare channels */
> +#define TIM_HWCFGR1_NB_OF_DT GENMASK(7, 4) /* Complementary outputs & dead-time generators */
> +#define TIM_HWCFGR2_CNT_WIDTH GENMASK(15, 8) /* Counter width */
>
> #define MAX_TIM_PSC 0xFFFF
> #define MAX_TIM_ICPSC 0x3
> @@ -113,6 +119,8 @@
> #define TIM_BDTR_BKF_MASK 0xF
> #define TIM_BDTR_BKF_SHIFT(x) (16 + (x) * 4)
>
> +#define STM32MP25_TIM_IPIDR 0x00120002
> +
> enum stm32_timers_dmas {
> STM32_TIMERS_DMA_CH1,
> STM32_TIMERS_DMA_CH2,
> @@ -151,6 +159,7 @@ struct stm32_timers_dma {
>
> struct stm32_timers {
> struct clk *clk;
> + u32 ipidr;
> struct regmap *regmap;
> u32 max_arr;
> struct stm32_timers_dma dma; /* Only to be used by the parent */
> --
> 2.25.1
>

--
Lee Jones [李琼斯]