[PATCH 0/8] Add STM32MP25 LPTIM support: MFD, PWM, IIO, counter, clocksource

[PATCH 0/8] Add STM32MP25 LPTIM support: MFD, PWM, IIO, counter, clocksource

[Fabrice Gasnier]

This series adds support for STM32MP25 to MFD PWM, IIO, counter and
clocksource low-power timer (LPTIM) drivers.
This new variant is managed by using a new DT compatible string.
It comes with a slightly updated register set, some new features and new
interconnect signals inside the SoC.
Same feature list as on STM32MP1x is supported currently.
The device tree files add all instances in stm32mp251 dtsi file.

Fabrice Gasnier (6):
  dt-bindings: mfd: stm32-lptimer: add support for stm32mp25
  mfd: stm32-lptimer: add support for stm32mp25
  pwm: stm32-lp: add support for stm32mp25
  counter: stm32-lptimer-cnt: add support for stm32mp25
  arm64: defconfig: enable STM32 LP timers drivers
  arm64: dts: st: add low-power timer nodes on stm32mp251

Olivier Moysan (1):
  iio: trigger: stm32-lptimer: add support for stm32mp25

Patrick Delaunay (1):
  clocksource: stm32-lptimer: add stm32mp25 support

 .../bindings/mfd/st,stm32-lptimer.yaml        |  23 +-
 arch/arm64/boot/dts/st/stm32mp251.dtsi        | 177 ++++++++++++++
 arch/arm64/configs/defconfig                  |   5 +
 drivers/clocksource/timer-stm32-lp.c          |   1 +
 drivers/counter/stm32-lptimer-cnt.c           |   1 +
 drivers/iio/trigger/stm32-lptimer-trigger.c   | 109 +++++++--
 drivers/mfd/stm32-lptimer.c                   |  30 ++-
 drivers/pwm/pwm-stm32-lp.c                    | 220 +++++++++++++++---
 include/linux/iio/timer/stm32-lptim-trigger.h |   9 +
 include/linux/mfd/stm32-lptimer.h             |  32 ++-
 10 files changed, 554 insertions(+), 53 deletions(-)

-- 
2.25.1

[PATCH 1/8] dt-bindings: mfd: stm32-lptimer: add support for stm32mp25

[Fabrice Gasnier]

Add a new stm32mp25 compatible to stm32-lptimer dt-bindings, to support
STM32MP25 SoC. Some features has been added to the low-power timer like
new capture compare channels (hence more PWM channels, and PWM input
capture). Some registers/bits has been revisited to support this.
So introduce a new compatible to handle this diversity.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 .../bindings/mfd/st,stm32-lptimer.yaml        | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml b/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
index d41308856408..4822ce5d0c76 100644
--- a/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
+++ b/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
@@ -21,7 +21,9 @@ maintainers:
 
 properties:
   compatible:
-    const: st,stm32-lptimer
+    enum:
+      - st,stm32-lptimer
+      - st,stm32mp25-lptimer
 
   reg:
     maxItems: 1
@@ -48,13 +50,18 @@ properties:
     minItems: 1
     maxItems: 2
 
+  power-domains:
+    maxItems: 1
+
   pwm:
     type: object
     additionalProperties: false
 
     properties:
       compatible:
-        const: st,stm32-pwm-lp
+        enum:
+          - st,stm32-pwm-lp
+          - st,stm32mp25-pwm-lp
 
       "#pwm-cells":
         const: 3
@@ -69,7 +76,9 @@ properties:
 
     properties:
       compatible:
-        const: st,stm32-lptimer-counter
+        enum:
+          - st,stm32-lptimer-counter
+          - st,stm32mp25-lptimer-counter
 
     required:
       - compatible
@@ -80,7 +89,9 @@ properties:
 
     properties:
       compatible:
-        const: st,stm32-lptimer-timer
+        enum:
+          - st,stm32-lptimer-timer
+          - st,stm32mp25-lptimer-timer
 
     required:
       - compatible
@@ -92,7 +103,9 @@ patternProperties:
 
     properties:
       compatible:
-        const: st,stm32-lptimer-trigger
+        enum:
+          - st,stm32-lptimer-trigger
+          - st,stm32mp25-lptimer-trigger
 
       reg:
         description: Identify trigger hardware block.
-- 
2.25.1

Re: [PATCH 1/8] dt-bindings: mfd: stm32-lptimer: add support for stm32mp25

[Krzysztof Kozlowski]

On Mon, Feb 24, 2025 at 07:01:43PM +0100, Fabrice Gasnier wrote:
>    pwm:
>      type: object
>      additionalProperties: false
>  
>      properties:
>        compatible:
> -        const: st,stm32-pwm-lp
> +        enum:
> +          - st,stm32-pwm-lp
> +          - st,stm32mp25-pwm-lp
>  
>        "#pwm-cells":
>          const: 3
> @@ -69,7 +76,9 @@ properties:
>  
>      properties:
>        compatible:
> -        const: st,stm32-lptimer-counter
> +        enum:
> +          - st,stm32-lptimer-counter
> +          - st,stm32mp25-lptimer-counter

Driver changes suggest many of these are compatible. Why isn't this expressed?

Best regards,
Krzysztof

Re: [PATCH 1/8] dt-bindings: mfd: stm32-lptimer: add support for stm32mp25

[Fabrice Gasnier]

On 2/25/25 13:02, Krzysztof Kozlowski wrote:
> On Mon, Feb 24, 2025 at 07:01:43PM +0100, Fabrice Gasnier wrote:
>>    pwm:
>>      type: object
>>      additionalProperties: false
>>  
>>      properties:
>>        compatible:
>> -        const: st,stm32-pwm-lp
>> +        enum:
>> +          - st,stm32-pwm-lp
>> +          - st,stm32mp25-pwm-lp
>>  
>>        "#pwm-cells":
>>          const: 3
>> @@ -69,7 +76,9 @@ properties:
>>  
>>      properties:
>>        compatible:
>> -        const: st,stm32-lptimer-counter
>> +        enum:
>> +          - st,stm32-lptimer-counter
>> +          - st,stm32mp25-lptimer-counter
> 
> Driver changes suggest many of these are compatible. Why isn't this expressed?

Hi Krzysztof,

The Low Power Timer (LPTIM) hardware isn't fully backward compatible.

At driver level, as indicated in the cover-letter, same feature list as
on STM32MP1x is supported currently. This is probably what makes it look
like it's compatible, but it's not fully compatible.

The hardware controller is a bit different. Some registers/bits has been
revisited among other things. This is the purpose for these new compatibles.

Please find driver-related answers each driver part.

Best regards,
Fabrice

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/8] dt-bindings: mfd: stm32-lptimer: add support for stm32mp25

[Krzysztof Kozlowski]

On 25/02/2025 15:57, Fabrice Gasnier wrote:
> On 2/25/25 13:02, Krzysztof Kozlowski wrote:
>> On Mon, Feb 24, 2025 at 07:01:43PM +0100, Fabrice Gasnier wrote:
>>>    pwm:
>>>      type: object
>>>      additionalProperties: false
>>>  
>>>      properties:
>>>        compatible:
>>> -        const: st,stm32-pwm-lp
>>> +        enum:
>>> +          - st,stm32-pwm-lp
>>> +          - st,stm32mp25-pwm-lp
>>>  
>>>        "#pwm-cells":
>>>          const: 3
>>> @@ -69,7 +76,9 @@ properties:
>>>  
>>>      properties:
>>>        compatible:
>>> -        const: st,stm32-lptimer-counter
>>> +        enum:
>>> +          - st,stm32-lptimer-counter
>>> +          - st,stm32mp25-lptimer-counter
>>
>> Driver changes suggest many of these are compatible. Why isn't this expressed?
> 
> Hi Krzysztof,
> 
> The Low Power Timer (LPTIM) hardware isn't fully backward compatible.
> 
> At driver level, as indicated in the cover-letter, same feature list as
> on STM32MP1x is supported currently. This is probably what makes it look
> like it's compatible, but it's not fully compatible.

I don't understand. Same feature list is supported means fully
compatible, but you say not fully compatible. You are aware that
compatible means not the same?

> 
> The hardware controller is a bit different. Some registers/bits has been
> revisited among other things. This is the purpose for these new compatibles.

We do not discuss new compatibles. We discuss lack of compatibility. If
registers/bits are changed, how existing driver can work with same ID table?



Best regards,
Krzysztof

Re: [PATCH 1/8] dt-bindings: mfd: stm32-lptimer: add support for stm32mp25

[Fabrice Gasnier]

On 2/26/25 08:51, Krzysztof Kozlowski wrote:
> On 25/02/2025 15:57, Fabrice Gasnier wrote:
>> On 2/25/25 13:02, Krzysztof Kozlowski wrote:
>>> On Mon, Feb 24, 2025 at 07:01:43PM +0100, Fabrice Gasnier wrote:
>>>>    pwm:
>>>>      type: object
>>>>      additionalProperties: false
>>>>  
>>>>      properties:
>>>>        compatible:
>>>> -        const: st,stm32-pwm-lp
>>>> +        enum:
>>>> +          - st,stm32-pwm-lp
>>>> +          - st,stm32mp25-pwm-lp
>>>>  
>>>>        "#pwm-cells":
>>>>          const: 3
>>>> @@ -69,7 +76,9 @@ properties:
>>>>  
>>>>      properties:
>>>>        compatible:
>>>> -        const: st,stm32-lptimer-counter
>>>> +        enum:
>>>> +          - st,stm32-lptimer-counter
>>>> +          - st,stm32mp25-lptimer-counter
>>>
>>> Driver changes suggest many of these are compatible. Why isn't this expressed?
>>
>> Hi Krzysztof,
>>
>> The Low Power Timer (LPTIM) hardware isn't fully backward compatible.
>>
>> At driver level, as indicated in the cover-letter, same feature list as
>> on STM32MP1x is supported currently. This is probably what makes it look
>> like it's compatible, but it's not fully compatible.
> 
> I don't understand. Same feature list is supported means fully
> compatible, but you say not fully compatible. You are aware that
> compatible means not the same?
> 
>>
>> The hardware controller is a bit different. Some registers/bits has been
>> revisited among other things. This is the purpose for these new compatibles.
> 
> We do not discuss new compatibles. We discuss lack of compatibility. If
> registers/bits are changed, how existing driver can work with same ID table?

Hi Krzysztof,

To summarize on dt-bindings side, here is my view, following your
comments on per driver basis of the compatible usage.

Let's keep these new compatibles:
- "st,stm32mp25-lptimer-trigger"
- "st,stm32mp25-lptimer-counter"

Both reflect not only LPTimer hardware update, but also specific
interconnect to other hardware blocks throughout all the STM32MP25 SoC.

Other compatible strings may be dropped. Reading the
revision/identification register of the LPTimer should be enough for
compatibility in the related drivers.

I'll update this in next revision of the series.

Thanks for reviewing,
Best Regards,
Fabrice

> 
> 
> 
> Best regards,
> Krzysztof

[PATCH 2/8] mfd: stm32-lptimer: add support for stm32mp25

[Fabrice Gasnier]

Add support for STM32MP25 SoC. Use newly introduced compatible, to handle
new features along with registers and bits diversity.

A new hardware configuration register (HWCFGR2) has been added, to gather
number of capture/compare channels, autonomous mode and input capture
capability. The full feature set is implemented in LPTIM1/2/3/4. LPTIM5
supports a smaller set of features. This can now be read from HWCFGR
registers.

Add new registers to the stm32-lptimer.h: CCMR1, CCR2 and HWCFGR1/2.
Update the stm32_lptimer data struct so signal the number of
capture/compare channels to the child devices.
Also Remove some unused bit masks (CMPOK_ARROK / CMPOKCF_ARROKCF).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/mfd/stm32-lptimer.c       | 30 ++++++++++++++++++++++++++++-
 include/linux/mfd/stm32-lptimer.h | 32 +++++++++++++++++++++++++++++--
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/stm32-lptimer.c b/drivers/mfd/stm32-lptimer.c
index b2704a9809c7..e5a9ceb78c10 100644
--- a/drivers/mfd/stm32-lptimer.c
+++ b/drivers/mfd/stm32-lptimer.c
@@ -6,6 +6,7 @@
  * Inspired by Benjamin Gaignard's stm32-timers driver
  */
 
+#include <linux/bitfield.h>
 #include <linux/mfd/stm32-lptimer.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
@@ -49,6 +50,32 @@ static int stm32_lptimer_detect_encoder(struct stm32_lptimer *ddata)
 	return 0;
 }
 
+static int stm32_lptimer_detect_hwcfgr(struct stm32_lptimer *ddata)
+{
+	u32 val;
+	int ret;
+
+	/* Try to guess parameters from HWCFGR: e.g. encodrer mode (STM32MP15) */
+	ret = regmap_read(ddata->regmap, STM32MP15_LPTIM_HWCFGR, &val);
+	if (ret)
+		return ret;
+
+	/* Fallback to legacy init if HWCFGR isn't present */
+	if (!val)
+		return stm32_lptimer_detect_encoder(ddata);
+
+	ddata->has_encoder = FIELD_GET(STM32MP15_LPTIM_HWCFGR_ENCODER, val);
+
+	ret = regmap_read(ddata->regmap, STM32MP25_LPTIM_HWCFGR2, &val);
+	if (ret)
+		return ret;
+
+	/* Number of capture/compare channels */
+	ddata->num_cc_chans = FIELD_GET(STM32MP25_LPTIM_HWCFGR2_CHAN_NUM, val);
+
+	return 0;
+}
+
 static int stm32_lptimer_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -73,7 +100,7 @@ static int stm32_lptimer_probe(struct platform_device *pdev)
 	if (IS_ERR(ddata->clk))
 		return PTR_ERR(ddata->clk);
 
-	ret = stm32_lptimer_detect_encoder(ddata);
+	ret = stm32_lptimer_detect_hwcfgr(ddata);
 	if (ret)
 		return ret;
 
@@ -84,6 +111,7 @@ static int stm32_lptimer_probe(struct platform_device *pdev)
 
 static const struct of_device_id stm32_lptimer_of_match[] = {
 	{ .compatible = "st,stm32-lptimer", },
+	{ .compatible = "st,stm32mp25-lptimer", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, stm32_lptimer_of_match);
diff --git a/include/linux/mfd/stm32-lptimer.h b/include/linux/mfd/stm32-lptimer.h
index 06d3f11dc3c9..b9da7d0d2a36 100644
--- a/include/linux/mfd/stm32-lptimer.h
+++ b/include/linux/mfd/stm32-lptimer.h
@@ -17,18 +17,26 @@
 #define STM32_LPTIM_IER		0x08	/* Interrupt Enable Reg      */
 #define STM32_LPTIM_CFGR	0x0C	/* Configuration Reg         */
 #define STM32_LPTIM_CR		0x10	/* Control Reg               */
-#define STM32_LPTIM_CMP		0x14	/* Compare Reg               */
+#define STM32_LPTIM_CMP		0x14	/* Compare Reg (CCR1 on mp25) */
 #define STM32_LPTIM_ARR		0x18	/* Autoreload Reg            */
 #define STM32_LPTIM_CNT		0x1C	/* Counter Reg               */
+#define STM32MP25_LPTIM_CCMR1	0x2C	/* Capture/Compare Mode Reg  */
+#define STM32MP25_LPTIM_CCR2	0x34	/* Compare Reg2              */
+
+#define STM32MP25_LPTIM_HWCFGR2	0x3EC	/* Hardware configuration register 2 */
+#define STM32MP15_LPTIM_HWCFGR	0x3F0	/* Hardware configuration register 1 */
+#define STM32MP15_LPTIM_VERR	0x3F4	/* Version identification register   */
 
 /* STM32_LPTIM_ISR - bit fields */
+#define STM32_LPTIM_CMP2_ARROK		(BIT(19) | BIT(4))
 #define STM32_LPTIM_CMPOK_ARROK		GENMASK(4, 3)
 #define STM32_LPTIM_ARROK		BIT(4)
 #define STM32_LPTIM_CMPOK		BIT(3)
 
 /* STM32_LPTIM_ICR - bit fields */
-#define STM32_LPTIM_ARRMCF		BIT(1)
+#define STM32_LPTIM_CMP2OKCF_ARROKCF	(BIT(19) | BIT(4))
 #define STM32_LPTIM_CMPOKCF_ARROKCF	GENMASK(4, 3)
+#define STM32_LPTIM_ARRMCF		BIT(1)
 
 /* STM32_LPTIM_IER - bit flieds */
 #define STM32_LPTIM_ARRMIE	BIT(1)
@@ -53,16 +61,36 @@
 /* STM32_LPTIM_ARR */
 #define STM32_LPTIM_MAX_ARR	0xFFFF
 
+/* STM32MP25_LPTIM_CCMR1 */
+#define STM32MP25_LPTIM_CC2P	GENMASK(19, 18)
+#define STM32MP25_LPTIM_CC2E	BIT(17)
+#define STM32MP25_LPTIM_CC2SEL	BIT(16)
+#define STM32MP25_LPTIM_CC1P	GENMASK(3, 2)
+#define STM32MP25_LPTIM_CC1E	BIT(1)
+#define STM32MP25_LPTIM_CC1SEL	BIT(0)
+
+/* STM32MP15_LPTIM_HWCFGR */
+#define STM32MP15_LPTIM_HWCFGR_ENCODER	BIT(16)
+
+/* STM32MP25_LPTIM_HWCFGR2 */
+#define STM32MP25_LPTIM_HWCFGR2_CHAN_NUM	GENMASK(3, 0)
+
+/* STM32MP15_LPTIM_VERR */
+#define STM32MP15_MINREV_MASK		GENMASK(3, 0)
+#define STM32MP15_MAJREV_MASK		GENMASK(7, 4)
+
 /**
  * struct stm32_lptimer - STM32 Low-Power Timer data assigned by parent device
  * @clk: clock reference for this instance
  * @regmap: register map reference for this instance
  * @has_encoder: indicates this Low-Power Timer supports encoder mode
+ * @num_cc_chans: indicates the number of capture/compare channels
  */
 struct stm32_lptimer {
 	struct clk *clk;
 	struct regmap *regmap;
 	bool has_encoder;
+	unsigned int num_cc_chans;
 };
 
 #endif
-- 
2.25.1

[PATCH 3/8] iio: trigger: stm32-lptimer: add support for stm32mp25

[Fabrice Gasnier]

From: Olivier Moysan <olivier.moysan@foss.st.com>

Add support for STM32MP25 SoC. Use newly introduced compatible to handle
this new HW variant. Add new trigger definitions that can be used by the
stm32 analog-to-digital converter. Use compatible data to identify them.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/iio/trigger/stm32-lptimer-trigger.c   | 109 +++++++++++++++---
 include/linux/iio/timer/stm32-lptim-trigger.h |   9 ++
 2 files changed, 99 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/trigger/stm32-lptimer-trigger.c b/drivers/iio/trigger/stm32-lptimer-trigger.c
index f1e18913236a..995234c1e7d5 100644
--- a/drivers/iio/trigger/stm32-lptimer-trigger.c
+++ b/drivers/iio/trigger/stm32-lptimer-trigger.c
@@ -16,16 +16,44 @@
 #include <linux/platform_device.h>
 #include <linux/property.h>
 
-/* List Low-Power Timer triggers */
-static const char * const stm32_lptim_triggers[] = {
-	LPTIM1_OUT,
-	LPTIM2_OUT,
-	LPTIM3_OUT,
+/* Maximum triggers + one trailing null entry to indicate the end of array */
+#define MAX_TRIGGERS 3
+
+struct stm32_lptim_cfg {
+	const char * const (*triggers)[MAX_TRIGGERS];
+	unsigned int nb_triggers;
+};
+
+/* List Low-Power Timer triggers for H7, MP13, MP15 */
+static const char * const stm32_lptim_triggers[][MAX_TRIGGERS] = {
+	{ LPTIM1_OUT,},
+	{ LPTIM2_OUT,},
+	{ LPTIM3_OUT,},
+};
+
+/* List Low-Power Timer triggers for STM32MP25 */
+static const char * const stm32mp25_lptim_triggers[][MAX_TRIGGERS] = {
+	{ LPTIM1_CH1, LPTIM1_CH2, },
+	{ LPTIM2_CH1, LPTIM2_CH2, },
+	{ LPTIM3_CH1,},
+	{ LPTIM4_CH1,},
+	{ LPTIM5_OUT,},
+};
+
+static const struct stm32_lptim_cfg stm32mp15_lptim_cfg = {
+	.triggers = stm32_lptim_triggers,
+	.nb_triggers = ARRAY_SIZE(stm32_lptim_triggers),
+};
+
+static const struct stm32_lptim_cfg stm32mp25_lptim_cfg = {
+	.triggers = stm32mp25_lptim_triggers,
+	.nb_triggers = ARRAY_SIZE(stm32mp25_lptim_triggers),
 };
 
 struct stm32_lptim_trigger {
 	struct device *dev;
-	const char *trg;
+	const char * const *triggers;
+	struct list_head tr_list;
 };
 
 static int stm32_lptim_validate_device(struct iio_trigger *trig,
@@ -54,25 +82,49 @@ bool is_stm32_lptim_trigger(struct iio_trigger *trig)
 }
 EXPORT_SYMBOL(is_stm32_lptim_trigger);
 
-static int stm32_lptim_setup_trig(struct stm32_lptim_trigger *priv)
+static void stm32_lptim_unregister_triggers(struct stm32_lptim_trigger *priv)
 {
-	struct iio_trigger *trig;
+	struct iio_trigger *tr;
 
-	trig = devm_iio_trigger_alloc(priv->dev, "%s", priv->trg);
-	if  (!trig)
-		return -ENOMEM;
+	list_for_each_entry(tr, &priv->tr_list, alloc_list)
+		iio_trigger_unregister(tr);
+}
+
+static int stm32_lptim_register_triggers(struct stm32_lptim_trigger *priv)
+{
+	const char * const *cur = priv->triggers;
+	int ret;
 
-	trig->dev.parent = priv->dev->parent;
-	trig->ops = &stm32_lptim_trigger_ops;
-	iio_trigger_set_drvdata(trig, priv);
+	INIT_LIST_HEAD(&priv->tr_list);
 
-	return devm_iio_trigger_register(priv->dev, trig);
+	while (cur && *cur) {
+		struct iio_trigger *trig;
+
+		trig = devm_iio_trigger_alloc(priv->dev, "%s", *cur);
+		if  (!trig)
+			return -ENOMEM;
+
+		trig->dev.parent = priv->dev->parent;
+		trig->ops = &stm32_lptim_trigger_ops;
+		iio_trigger_set_drvdata(trig, priv);
+
+		ret = iio_trigger_register(trig);
+		if (ret)
+			return ret;
+
+		list_add_tail(&trig->alloc_list, &priv->tr_list);
+		cur++;
+	}
+
+	return 0;
 }
 
 static int stm32_lptim_trigger_probe(struct platform_device *pdev)
 {
 	struct stm32_lptim_trigger *priv;
+	struct stm32_lptim_cfg const *lptim_cfg;
 	u32 index;
+	int ret;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -81,23 +133,42 @@ static int stm32_lptim_trigger_probe(struct platform_device *pdev)
 	if (device_property_read_u32(&pdev->dev, "reg", &index))
 		return -EINVAL;
 
-	if (index >= ARRAY_SIZE(stm32_lptim_triggers))
+	lptim_cfg = device_get_match_data(&pdev->dev);
+
+	if (index >= lptim_cfg->nb_triggers)
 		return -EINVAL;
 
 	priv->dev = &pdev->dev;
-	priv->trg = stm32_lptim_triggers[index];
+	priv->triggers = lptim_cfg->triggers[index];
+
+	ret = stm32_lptim_register_triggers(priv);
+	if (ret) {
+		stm32_lptim_unregister_triggers(priv);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static void stm32_lptim_trigger_remove(struct platform_device *pdev)
+{
+	struct stm32_lptim_trigger *priv = platform_get_drvdata(pdev);
 
-	return stm32_lptim_setup_trig(priv);
+	stm32_lptim_unregister_triggers(priv);
 }
 
 static const struct of_device_id stm32_lptim_trig_of_match[] = {
-	{ .compatible = "st,stm32-lptimer-trigger", },
+	{ .compatible = "st,stm32-lptimer-trigger", .data = (void *)&stm32mp15_lptim_cfg },
+	{ .compatible = "st,stm32mp25-lptimer-trigger", .data = (void *)&stm32mp25_lptim_cfg},
 	{},
 };
 MODULE_DEVICE_TABLE(of, stm32_lptim_trig_of_match);
 
 static struct platform_driver stm32_lptim_trigger_driver = {
 	.probe = stm32_lptim_trigger_probe,
+	.remove = stm32_lptim_trigger_remove,
 	.driver = {
 		.name = "stm32-lptimer-trigger",
 		.of_match_table = stm32_lptim_trig_of_match,
diff --git a/include/linux/iio/timer/stm32-lptim-trigger.h b/include/linux/iio/timer/stm32-lptim-trigger.h
index a34dcf6a6001..ce3cf0addb2e 100644
--- a/include/linux/iio/timer/stm32-lptim-trigger.h
+++ b/include/linux/iio/timer/stm32-lptim-trigger.h
@@ -14,6 +14,15 @@
 #define LPTIM1_OUT	"lptim1_out"
 #define LPTIM2_OUT	"lptim2_out"
 #define LPTIM3_OUT	"lptim3_out"
+#define LPTIM4_OUT	"lptim4_out"
+#define LPTIM5_OUT	"lptim5_out"
+
+#define LPTIM1_CH1	"lptim1_ch1"
+#define LPTIM1_CH2	"lptim1_ch2"
+#define LPTIM2_CH1	"lptim2_ch1"
+#define LPTIM2_CH2	"lptim2_ch2"
+#define LPTIM3_CH1	"lptim3_ch1"
+#define LPTIM4_CH1	"lptim4_ch1"
 
 #if IS_REACHABLE(CONFIG_IIO_STM32_LPTIMER_TRIGGER)
 bool is_stm32_lptim_trigger(struct iio_trigger *trig);
-- 
2.25.1

Re: [PATCH 3/8] iio: trigger: stm32-lptimer: add support for stm32mp25

[Jonathan Cameron]

On Mon, 24 Feb 2025 19:01:45 +0100
Fabrice Gasnier <fabrice.gasnier@foss.st.com> wrote:

> From: Olivier Moysan <olivier.moysan@foss.st.com>
> 
> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
> this new HW variant. Add new trigger definitions that can be used by the
> stm32 analog-to-digital converter. Use compatible data to identify them.
> 
> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Hi. I'm not really following why you can't use devm calls for the
trigger probe path and hence why you need the explicit remove.
Feels like a lot of infrastructure and I can't see why we need it.

Jonathan


> @@ -54,25 +82,49 @@ bool is_stm32_lptim_trigger(struct iio_trigger *trig)
>  }
>  EXPORT_SYMBOL(is_stm32_lptim_trigger);
>  
> -static int stm32_lptim_setup_trig(struct stm32_lptim_trigger *priv)
> +static void stm32_lptim_unregister_triggers(struct stm32_lptim_trigger *priv)
>  {
> -	struct iio_trigger *trig;
> +	struct iio_trigger *tr;
>  
> -	trig = devm_iio_trigger_alloc(priv->dev, "%s", priv->trg);
> -	if  (!trig)
> -		return -ENOMEM;
> +	list_for_each_entry(tr, &priv->tr_list, alloc_list)
> +		iio_trigger_unregister(tr);
> +}
> +
> +static int stm32_lptim_register_triggers(struct stm32_lptim_trigger *priv)
> +{
> +	const char * const *cur = priv->triggers;
> +	int ret;
>  
> -	trig->dev.parent = priv->dev->parent;
> -	trig->ops = &stm32_lptim_trigger_ops;
> -	iio_trigger_set_drvdata(trig, priv);
> +	INIT_LIST_HEAD(&priv->tr_list);
>  
> -	return devm_iio_trigger_register(priv->dev, trig);
> +	while (cur && *cur) {
> +		struct iio_trigger *trig;
> +
> +		trig = devm_iio_trigger_alloc(priv->dev, "%s", *cur);
> +		if  (!trig)
> +			return -ENOMEM;
> +
> +		trig->dev.parent = priv->dev->parent;
> +		trig->ops = &stm32_lptim_trigger_ops;
> +		iio_trigger_set_drvdata(trig, priv);
> +
> +		ret = iio_trigger_register(trig);

I'm not really following why you can't use devm_iio_trigger_register() here
and avoid your own tracking with the list below.


> +		if (ret)
> +			return ret;
> +
> +		list_add_tail(&trig->alloc_list, &priv->tr_list);
> +		cur++;
> +	}
> +
> +	return 0;
>  }
>  
>  static int stm32_lptim_trigger_probe(struct platform_device *pdev)
>  {
>  	struct stm32_lptim_trigger *priv;
> +	struct stm32_lptim_cfg const *lptim_cfg;
>  	u32 index;
> +	int ret;
>  
>  	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>  	if (!priv)
> @@ -81,23 +133,42 @@ static int stm32_lptim_trigger_probe(struct platform_device *pdev)
>  	if (device_property_read_u32(&pdev->dev, "reg", &index))
>  		return -EINVAL;
>  
> -	if (index >= ARRAY_SIZE(stm32_lptim_triggers))
> +	lptim_cfg = device_get_match_data(&pdev->dev);
> +
> +	if (index >= lptim_cfg->nb_triggers)
>  		return -EINVAL;
>  
>  	priv->dev = &pdev->dev;
> -	priv->trg = stm32_lptim_triggers[index];
> +	priv->triggers = lptim_cfg->triggers[index];
> +
> +	ret = stm32_lptim_register_triggers(priv);
> +	if (ret) {
> +		stm32_lptim_unregister_triggers(priv);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return 0;
> +}
> +
> +static void stm32_lptim_trigger_remove(struct platform_device *pdev)
> +{
> +	struct stm32_lptim_trigger *priv = platform_get_drvdata(pdev);
>  
> -	return stm32_lptim_setup_trig(priv);
> +	stm32_lptim_unregister_triggers(priv);

Why not a devm_add_action_or_reset?
or for that matter a devm_iio_trigger_register() in the first place.

>  }
>  
>  static const struct of_device_id stm32_lptim_trig_of_match[] = {
> -	{ .compatible = "st,stm32-lptimer-trigger", },
> +	{ .compatible = "st,stm32-lptimer-trigger", .data = (void *)&stm32mp15_lptim_cfg },
> +	{ .compatible = "st,stm32mp25-lptimer-trigger", .data = (void *)&stm32mp25_lptim_cfg},
Why cast away a const then pass it to a const void *?

That is I don't think the casts are needed.

>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, stm32_lptim_trig_of_match);
>  
>  static struct platform_driver stm32_lptim_trigger_driver = {
>  	.probe = stm32_lptim_trigger_probe,
> +	.remove = stm32_lptim_trigger_remove,
>  	.driver = {
>  		.name = "stm32-lptimer-trigger",
>  		.of_match_table = stm32_lptim_trig_of_match,
> diff --git a/include/linux/iio/timer/stm32-lptim-trigger.h b/include/linux/iio/timer/stm32-lptim-trigger.h
> index a34dcf6a6001..ce3cf0addb2e 100644
> --- a/include/linux/iio/timer/stm32-lptim-trigger.h
> +++ b/include/linux/iio/timer/stm32-lptim-trigger.h
> @@ -14,6 +14,15 @@
>  #define LPTIM1_OUT	"lptim1_out"
>  #define LPTIM2_OUT	"lptim2_out"
>  #define LPTIM3_OUT	"lptim3_out"
> +#define LPTIM4_OUT	"lptim4_out"
> +#define LPTIM5_OUT	"lptim5_out"
> +
> +#define LPTIM1_CH1	"lptim1_ch1"
> +#define LPTIM1_CH2	"lptim1_ch2"
> +#define LPTIM2_CH1	"lptim2_ch1"
> +#define LPTIM2_CH2	"lptim2_ch2"
> +#define LPTIM3_CH1	"lptim3_ch1"
> +#define LPTIM4_CH1	"lptim4_ch1"
>  
>  #if IS_REACHABLE(CONFIG_IIO_STM32_LPTIMER_TRIGGER)
>  bool is_stm32_lptim_trigger(struct iio_trigger *trig);

[PATCH 4/8] clocksource: stm32-lptimer: add stm32mp25 support

[Fabrice Gasnier]

From: Patrick Delaunay <patrick.delaunay@foss.st.com>

Add the support of the new compatible for STM32MP25 SoC in driver, as
described in Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
and used in arch/arm64/boot/dts/st/stm32mp251.dtsi.

Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/clocksource/timer-stm32-lp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/timer-stm32-lp.c b/drivers/clocksource/timer-stm32-lp.c
index a4c95161cb22..db055348e2cc 100644
--- a/drivers/clocksource/timer-stm32-lp.c
+++ b/drivers/clocksource/timer-stm32-lp.c
@@ -197,6 +197,7 @@ static int stm32_clkevent_lp_probe(struct platform_device *pdev)
 
 static const struct of_device_id stm32_clkevent_lp_of_match[] = {
 	{ .compatible = "st,stm32-lptimer-timer", },
+	{ .compatible = "st,stm32mp25-lptimer-timer", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, stm32_clkevent_lp_of_match);
-- 
2.25.1

Re: [PATCH 4/8] clocksource: stm32-lptimer: add stm32mp25 support

[Krzysztof Kozlowski]

On Mon, Feb 24, 2025 at 07:01:46PM +0100, Fabrice Gasnier wrote:
> From: Patrick Delaunay <patrick.delaunay@foss.st.com>
> 
> Add the support of the new compatible for STM32MP25 SoC in driver, as
> described in Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
> and used in arch/arm64/boot/dts/st/stm32mp251.dtsi.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> ---
>  drivers/clocksource/timer-stm32-lp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clocksource/timer-stm32-lp.c b/drivers/clocksource/timer-stm32-lp.c
> index a4c95161cb22..db055348e2cc 100644
> --- a/drivers/clocksource/timer-stm32-lp.c
> +++ b/drivers/clocksource/timer-stm32-lp.c
> @@ -197,6 +197,7 @@ static int stm32_clkevent_lp_probe(struct platform_device *pdev)
>  
>  static const struct of_device_id stm32_clkevent_lp_of_match[] = {
>  	{ .compatible = "st,stm32-lptimer-timer", },
> +	{ .compatible = "st,stm32mp25-lptimer-timer", },
>  	{},

Same question.

Best regards,
Krzysztof

Re: [PATCH 4/8] clocksource: stm32-lptimer: add stm32mp25 support

[Fabrice Gasnier]

On 2/25/25 13:02, Krzysztof Kozlowski wrote:
> On Mon, Feb 24, 2025 at 07:01:46PM +0100, Fabrice Gasnier wrote:
>> From: Patrick Delaunay <patrick.delaunay@foss.st.com>
>>
>> Add the support of the new compatible for STM32MP25 SoC in driver, as
>> described in Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
>> and used in arch/arm64/boot/dts/st/stm32mp251.dtsi.
>>
>> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>> ---
>>  drivers/clocksource/timer-stm32-lp.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/clocksource/timer-stm32-lp.c b/drivers/clocksource/timer-stm32-lp.c
>> index a4c95161cb22..db055348e2cc 100644
>> --- a/drivers/clocksource/timer-stm32-lp.c
>> +++ b/drivers/clocksource/timer-stm32-lp.c
>> @@ -197,6 +197,7 @@ static int stm32_clkevent_lp_probe(struct platform_device *pdev)
>>  
>>  static const struct of_device_id stm32_clkevent_lp_of_match[] = {
>>  	{ .compatible = "st,stm32-lptimer-timer", },
>> +	{ .compatible = "st,stm32mp25-lptimer-timer", },
>>  	{},
> 
> Same question.

Oops, I just figured out I have missed a change to this driver, to
enable interrupts, in order to comply with the LPTimer spec, starting
with STM32MP25.

E.g. with earlier STM32MP13, STM32MP15 or even STM32H7:
* The LPTIM_IER register must only be modified when the LPTIM is
disabled (ENABLE bit reset to ‘0’)

On STM32MP25:
* The LPTIMx_DIER register must only be modified when the LPTIM is
enabled (ENABLE bit set to 1)

I'll add this as compatible data in next revision.

Best regards,
Fabrice

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 4/8] clocksource: stm32-lptimer: add stm32mp25 support

[Fabrice Gasnier]

On 2/25/25 15:57, Fabrice Gasnier wrote:
> On 2/25/25 13:02, Krzysztof Kozlowski wrote:
>> On Mon, Feb 24, 2025 at 07:01:46PM +0100, Fabrice Gasnier wrote:
>>> From: Patrick Delaunay <patrick.delaunay@foss.st.com>
>>>
>>> Add the support of the new compatible for STM32MP25 SoC in driver, as
>>> described in Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
>>> and used in arch/arm64/boot/dts/st/stm32mp251.dtsi.
>>>
>>> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>>> ---
>>>  drivers/clocksource/timer-stm32-lp.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/clocksource/timer-stm32-lp.c b/drivers/clocksource/timer-stm32-lp.c
>>> index a4c95161cb22..db055348e2cc 100644
>>> --- a/drivers/clocksource/timer-stm32-lp.c
>>> +++ b/drivers/clocksource/timer-stm32-lp.c
>>> @@ -197,6 +197,7 @@ static int stm32_clkevent_lp_probe(struct platform_device *pdev)
>>>  
>>>  static const struct of_device_id stm32_clkevent_lp_of_match[] = {
>>>  	{ .compatible = "st,stm32-lptimer-timer", },
>>> +	{ .compatible = "st,stm32mp25-lptimer-timer", },
>>>  	{},
>>
>> Same question.
> 
> Oops, I just figured out I have missed a change to this driver, to
> enable interrupts, in order to comply with the LPTimer spec, starting
> with STM32MP25.
> 
> E.g. with earlier STM32MP13, STM32MP15 or even STM32H7:
> * The LPTIM_IER register must only be modified when the LPTIM is
> disabled (ENABLE bit reset to ‘0’)
> 
> On STM32MP25:
> * The LPTIMx_DIER register must only be modified when the LPTIM is
> enabled (ENABLE bit set to 1)
> 
> I'll add this as compatible data in next revision.

This is specific behavior, to the new revision of the LPTimer hardware.
It can be read from revision/identification registers. So this new
compatible can be dropped.

I'll remove "st,stm32mp25-lptimer-timer" compatible string in next revision.

Thanks for reviewing,
BR,
Fabrice

> 
> Best regards,
> Fabrice
> 
>>
>> Best regards,
>> Krzysztof
>>

[PATCH 5/8] pwm: stm32-lp: add support for stm32mp25

[Fabrice Gasnier]

Add support for STM32MP25 SoC. Use newly introduced compatible to handle
new features along with registers and bits diversity.

New dedicated capture/compare channels has been added: e.g. a new compare
register for channel 2. Some controls (polarity / cc channel enable) are
handled in CCMR register on this new variant (instead of wavepol bit).

So, Low-power timer can now have up to two PWM outputs. Use device data
from the MFD parent to configure the number of PWM channels e.g. 'npwm'.

Update current get_state() and apply() ops to support either:
- one PWM channel (as on older revision, or LPTIM5 on STM32MP25)
- two PWM channels (e.g. LPTIM1/2/3/4 on STM32MP25 that has the full
  feature set)
Introduce new routines to manage common prescaler, reload register and
global enable bit.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/pwm/pwm-stm32-lp.c | 220 ++++++++++++++++++++++++++++++++-----
 1 file changed, 194 insertions(+), 26 deletions(-)

diff --git a/drivers/pwm/pwm-stm32-lp.c b/drivers/pwm/pwm-stm32-lp.c
index 5832dce8ed9d..cdbc31bedfcf 100644
--- a/drivers/pwm/pwm-stm32-lp.c
+++ b/drivers/pwm/pwm-stm32-lp.c
@@ -20,6 +20,7 @@
 struct stm32_pwm_lp {
 	struct clk *clk;
 	struct regmap *regmap;
+	unsigned int num_cc_chans;
 };
 
 static inline struct stm32_pwm_lp *to_stm32_pwm_lp(struct pwm_chip *chip)
@@ -30,13 +31,101 @@ static inline struct stm32_pwm_lp *to_stm32_pwm_lp(struct pwm_chip *chip)
 /* STM32 Low-Power Timer is preceded by a configurable power-of-2 prescaler */
 #define STM32_LPTIM_MAX_PRESCALER	128
 
+static int stm32_pwm_lp_update_allowed(struct stm32_pwm_lp *priv, int channel)
+{
+	int ret;
+	u32 ccmr1;
+	unsigned long ccmr;
+
+	/* Only one PWM on this LPTIMER: enable, prescaler and reload value can be changed */
+	if (!priv->num_cc_chans)
+		return true;
+
+	ret = regmap_read(priv->regmap, STM32MP25_LPTIM_CCMR1, &ccmr1);
+	if (ret)
+		return ret;
+	ccmr = ccmr1 & (STM32MP25_LPTIM_CC1E | STM32MP25_LPTIM_CC2E);
+
+	/* More than one channel enabled: enable, prescaler or ARR value can't be changed */
+	if (bitmap_weight(&ccmr, sizeof(u32) * BITS_PER_BYTE) > 1)
+		return false;
+
+	/*
+	 * Only one channel is enabled (or none): check status on the other channel, to
+	 * report if enable, prescaler or ARR value can be changed.
+	 */
+	if (channel)
+		return !(ccmr1 & STM32MP25_LPTIM_CC1E);
+	else
+		return !(ccmr1 & STM32MP25_LPTIM_CC2E);
+}
+
+static int stm32_pwm_lp_compare_channel_apply(struct stm32_pwm_lp *priv, int channel,
+					      bool enable, enum pwm_polarity polarity)
+{
+	u32 ccmr1, val, mask;
+	bool reenable;
+	int ret;
+
+	/* No dedicated CC channel: nothing to do */
+	if (!priv->num_cc_chans)
+		return 0;
+
+	ret = regmap_read(priv->regmap, STM32MP25_LPTIM_CCMR1, &ccmr1);
+	if (ret)
+		return ret;
+
+	if (channel) {
+		/* Must disable CC channel (CCxE) to modify polarity (CCxP), then re-enable */
+		reenable = (enable && FIELD_GET(STM32MP25_LPTIM_CC2E, ccmr1)) &&
+			(polarity != FIELD_GET(STM32MP25_LPTIM_CC2P, ccmr1));
+
+		mask = STM32MP25_LPTIM_CC2SEL | STM32MP25_LPTIM_CC2E | STM32MP25_LPTIM_CC2P;
+		val = FIELD_PREP(STM32MP25_LPTIM_CC2P, polarity);
+		val |= FIELD_PREP(STM32MP25_LPTIM_CC2E, enable);
+	} else {
+		reenable = (enable && FIELD_GET(STM32MP25_LPTIM_CC1E, ccmr1)) &&
+			(polarity != FIELD_GET(STM32MP25_LPTIM_CC1P, ccmr1));
+
+		mask = STM32MP25_LPTIM_CC1SEL | STM32MP25_LPTIM_CC1E | STM32MP25_LPTIM_CC1P;
+		val = FIELD_PREP(STM32MP25_LPTIM_CC1P, polarity);
+		val |= FIELD_PREP(STM32MP25_LPTIM_CC1E, enable);
+	}
+
+	if (reenable) {
+		u32 cfgr, presc;
+		unsigned long rate;
+		unsigned int delay_us;
+
+		ret = regmap_update_bits(priv->regmap, STM32MP25_LPTIM_CCMR1,
+					 channel ? STM32MP25_LPTIM_CC2E : STM32MP25_LPTIM_CC1E, 0);
+		if (ret)
+			return ret;
+		/*
+		 * After a write to the LPTIM_CCMRx register, a new write operation can only be
+		 * performed after a delay of at least (PRESC × 3) clock cycles
+		 */
+		ret = regmap_read(priv->regmap, STM32_LPTIM_CFGR, &cfgr);
+		if (ret)
+			return ret;
+		presc = FIELD_GET(STM32_LPTIM_PRESC, cfgr);
+		rate = clk_get_rate(priv->clk) >> presc;
+		if (!rate)
+			return -EINVAL;
+		delay_us = 3 * DIV_ROUND_UP(USEC_PER_SEC, rate);
+		usleep_range(delay_us, delay_us * 2);
+	}
+
+	return regmap_update_bits(priv->regmap, STM32MP25_LPTIM_CCMR1, mask, val);
+}
+
 static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			      const struct pwm_state *state)
 {
 	struct stm32_pwm_lp *priv = to_stm32_pwm_lp(chip);
 	unsigned long long prd, div, dty;
 	struct pwm_state cstate;
-	u32 val, mask, cfgr, presc = 0;
+	u32 arr, val, mask, cfgr, presc = 0;
 	bool reenable;
 	int ret;
 
@@ -45,10 +134,28 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	if (!state->enabled) {
 		if (cstate.enabled) {
-			/* Disable LP timer */
-			ret = regmap_write(priv->regmap, STM32_LPTIM_CR, 0);
+			/* Disable CC channel if any */
+			ret = stm32_pwm_lp_compare_channel_apply(priv, pwm->hwpwm, false,
+								 state->polarity);
 			if (ret)
 				return ret;
+			ret = regmap_write(priv->regmap, pwm->hwpwm ?
+					   STM32MP25_LPTIM_CCR2 : STM32_LPTIM_CMP, 0);
+			if (ret)
+				return ret;
+
+			/* Check if the timer can be disabled */
+			ret = stm32_pwm_lp_update_allowed(priv, pwm->hwpwm);
+			if (ret < 0)
+				return ret;
+
+			if (ret) {
+				/* Disable LP timer */
+				ret = regmap_write(priv->regmap, STM32_LPTIM_CR, 0);
+				if (ret)
+					return ret;
+			}
+
 			/* disable clock to PWM counter */
 			clk_disable(priv->clk);
 		}
@@ -79,6 +186,23 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	dty = prd * state->duty_cycle;
 	do_div(dty, state->period);
 
+	ret = regmap_read(priv->regmap, STM32_LPTIM_CFGR, &cfgr);
+	if (ret)
+		return ret;
+
+	/*
+	 * When there are several channels, they share the same prescaler and reload value.
+	 * Check if this can be changed, or the values are the same for all channels.
+	 */
+	if (!stm32_pwm_lp_update_allowed(priv, pwm->hwpwm)) {
+		ret = regmap_read(priv->regmap, STM32_LPTIM_ARR, &arr);
+		if (ret)
+			return ret;
+
+		if ((FIELD_GET(STM32_LPTIM_PRESC, cfgr) != presc) || (arr != prd - 1))
+			return -EBUSY;
+	}
+
 	if (!cstate.enabled) {
 		/* enable clock to drive PWM counter */
 		ret = clk_enable(priv->clk);
@@ -86,15 +210,20 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			return ret;
 	}
 
-	ret = regmap_read(priv->regmap, STM32_LPTIM_CFGR, &cfgr);
-	if (ret)
-		goto err;
-
 	if ((FIELD_GET(STM32_LPTIM_PRESC, cfgr) != presc) ||
-	    (FIELD_GET(STM32_LPTIM_WAVPOL, cfgr) != state->polarity)) {
+	    ((FIELD_GET(STM32_LPTIM_WAVPOL, cfgr) != state->polarity) && !priv->num_cc_chans)) {
 		val = FIELD_PREP(STM32_LPTIM_PRESC, presc);
-		val |= FIELD_PREP(STM32_LPTIM_WAVPOL, state->polarity);
-		mask = STM32_LPTIM_PRESC | STM32_LPTIM_WAVPOL;
+		mask = STM32_LPTIM_PRESC;
+
+		if (!priv->num_cc_chans) {
+			/*
+			 * WAVPOL bit is only available when no capature compare channel is used,
+			 * e.g. on LPTIMER instances that have only one output channel. CCMR1 is
+			 * used otherwise.
+			 */
+			val |= FIELD_PREP(STM32_LPTIM_WAVPOL, state->polarity);
+			mask |= STM32_LPTIM_WAVPOL;
+		}
 
 		/* Must disable LP timer to modify CFGR */
 		reenable = true;
@@ -120,20 +249,27 @@ static int stm32_pwm_lp_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	if (ret)
 		goto err;
 
-	ret = regmap_write(priv->regmap, STM32_LPTIM_CMP, prd - (1 + dty));
+	/* Write CMP/CCRx register and ensure it's been properly written */
+	ret = regmap_write(priv->regmap, pwm->hwpwm ? STM32MP25_LPTIM_CCR2 : STM32_LPTIM_CMP,
+			   prd - (1 + dty));
 	if (ret)
 		goto err;
 
-	/* ensure CMP & ARR registers are properly written */
-	ret = regmap_read_poll_timeout(priv->regmap, STM32_LPTIM_ISR, val,
+	/* ensure ARR and CMP/CCRx registers are properly written */
+	ret = regmap_read_poll_timeout(priv->regmap, STM32_LPTIM_ISR, val, pwm->hwpwm ?
+				       (val & STM32_LPTIM_CMP2_ARROK) == STM32_LPTIM_CMP2_ARROK :
 				       (val & STM32_LPTIM_CMPOK_ARROK) == STM32_LPTIM_CMPOK_ARROK,
 				       100, 1000);
 	if (ret) {
 		dev_err(pwmchip_parent(chip), "ARR/CMP registers write issue\n");
 		goto err;
 	}
-	ret = regmap_write(priv->regmap, STM32_LPTIM_ICR,
-			   STM32_LPTIM_CMPOKCF_ARROKCF);
+	ret = regmap_write(priv->regmap, STM32_LPTIM_ICR, pwm->hwpwm ?
+			   STM32_LPTIM_CMP2OKCF_ARROKCF : STM32_LPTIM_CMPOKCF_ARROKCF);
+	if (ret)
+		goto err;
+
+	ret = stm32_pwm_lp_compare_channel_apply(priv, pwm->hwpwm, true, state->polarity);
 	if (ret)
 		goto err;
 
@@ -161,11 +297,22 @@ static int stm32_pwm_lp_get_state(struct pwm_chip *chip,
 {
 	struct stm32_pwm_lp *priv = to_stm32_pwm_lp(chip);
 	unsigned long rate = clk_get_rate(priv->clk);
-	u32 val, presc, prd;
+	u32 val, presc, prd, ccmr1;
+	bool enabled;
 	u64 tmp;
 
 	regmap_read(priv->regmap, STM32_LPTIM_CR, &val);
-	state->enabled = !!FIELD_GET(STM32_LPTIM_ENABLE, val);
+	enabled = !!FIELD_GET(STM32_LPTIM_ENABLE, val);
+	if (priv->num_cc_chans) {
+		/* There's a CC chan, need to also check if it's enabled */
+		regmap_read(priv->regmap, STM32MP25_LPTIM_CCMR1, &ccmr1);
+		if (pwm->hwpwm)
+			enabled &= !!FIELD_GET(STM32MP25_LPTIM_CC2E, ccmr1);
+		else
+			enabled &= !!FIELD_GET(STM32MP25_LPTIM_CC1E, ccmr1);
+	}
+	state->enabled = enabled;
+
 	/* Keep PWM counter clock refcount in sync with PWM initial state */
 	if (state->enabled) {
 		int ret = clk_enable(priv->clk);
@@ -176,14 +323,21 @@ static int stm32_pwm_lp_get_state(struct pwm_chip *chip,
 
 	regmap_read(priv->regmap, STM32_LPTIM_CFGR, &val);
 	presc = FIELD_GET(STM32_LPTIM_PRESC, val);
-	state->polarity = FIELD_GET(STM32_LPTIM_WAVPOL, val);
+	if (priv->num_cc_chans) {
+		if (pwm->hwpwm)
+			state->polarity = FIELD_GET(STM32MP25_LPTIM_CC2P, ccmr1);
+		else
+			state->polarity = FIELD_GET(STM32MP25_LPTIM_CC1P, ccmr1);
+	} else {
+		state->polarity = FIELD_GET(STM32_LPTIM_WAVPOL, val);
+	}
 
 	regmap_read(priv->regmap, STM32_LPTIM_ARR, &prd);
 	tmp = prd + 1;
 	tmp = (tmp << presc) * NSEC_PER_SEC;
 	state->period = DIV_ROUND_CLOSEST_ULL(tmp, rate);
 
-	regmap_read(priv->regmap, STM32_LPTIM_CMP, &val);
+	regmap_read(priv->regmap, pwm->hwpwm ? STM32MP25_LPTIM_CCR2 : STM32_LPTIM_CMP, &val);
 	tmp = prd - val;
 	tmp = (tmp << presc) * NSEC_PER_SEC;
 	state->duty_cycle = DIV_ROUND_CLOSEST_ULL(tmp, rate);
@@ -201,15 +355,25 @@ static int stm32_pwm_lp_probe(struct platform_device *pdev)
 	struct stm32_lptimer *ddata = dev_get_drvdata(pdev->dev.parent);
 	struct stm32_pwm_lp *priv;
 	struct pwm_chip *chip;
+	unsigned int npwm;
 	int ret;
 
-	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*priv));
+	if (!ddata->num_cc_chans) {
+		/* No dedicated CC channel, so there's only one PWM channel */
+		npwm = 1;
+	} else {
+		/* There are dedicated CC channels, each with one PWM output */
+		npwm = ddata->num_cc_chans;
+	}
+
+	chip = devm_pwmchip_alloc(&pdev->dev, npwm, sizeof(*priv));
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
 	priv = to_stm32_pwm_lp(chip);
 
 	priv->regmap = ddata->regmap;
 	priv->clk = ddata->clk;
+	priv->num_cc_chans = ddata->num_cc_chans;
 	chip->ops = &stm32_pwm_lp_ops;
 
 	ret = devm_pwmchip_add(&pdev->dev, chip);
@@ -225,12 +389,15 @@ static int stm32_pwm_lp_suspend(struct device *dev)
 {
 	struct pwm_chip *chip = dev_get_drvdata(dev);
 	struct pwm_state state;
-
-	pwm_get_state(&chip->pwms[0], &state);
-	if (state.enabled) {
-		dev_err(dev, "The consumer didn't stop us (%s)\n",
-			chip->pwms[0].label);
-		return -EBUSY;
+	unsigned int i;
+
+	for (i = 0; i < chip->npwm; i++) {
+		pwm_get_state(&chip->pwms[i], &state);
+		if (state.enabled) {
+			dev_err(dev, "The consumer didn't stop us (%s)\n",
+				chip->pwms[i].label);
+			return -EBUSY;
+		}
 	}
 
 	return pinctrl_pm_select_sleep_state(dev);
@@ -246,6 +413,7 @@ static DEFINE_SIMPLE_DEV_PM_OPS(stm32_pwm_lp_pm_ops, stm32_pwm_lp_suspend,
 
 static const struct of_device_id stm32_pwm_lp_of_match[] = {
 	{ .compatible = "st,stm32-pwm-lp", },
+	{ .compatible = "st,stm32mp25-pwm-lp", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, stm32_pwm_lp_of_match);
-- 
2.25.1

Re: [PATCH 5/8] pwm: stm32-lp: add support for stm32mp25

[Krzysztof Kozlowski]

On Mon, Feb 24, 2025 at 07:01:47PM +0100, Fabrice Gasnier wrote:
>  	}
>  
>  	return pinctrl_pm_select_sleep_state(dev);
> @@ -246,6 +413,7 @@ static DEFINE_SIMPLE_DEV_PM_OPS(stm32_pwm_lp_pm_ops, stm32_pwm_lp_suspend,
>  
>  static const struct of_device_id stm32_pwm_lp_of_match[] = {
>  	{ .compatible = "st,stm32-pwm-lp", },
> +	{ .compatible = "st,stm32mp25-pwm-lp", },

No driver data suggests device is backwards compatible. Commit msg
suggests not, so that's confusing.

Best regards,
Krzysztof

Re: [PATCH 5/8] pwm: stm32-lp: add support for stm32mp25

[Fabrice Gasnier]

On 2/25/25 13:04, Krzysztof Kozlowski wrote:
> On Mon, Feb 24, 2025 at 07:01:47PM +0100, Fabrice Gasnier wrote:
>>  	}
>>  
>>  	return pinctrl_pm_select_sleep_state(dev);
>> @@ -246,6 +413,7 @@ static DEFINE_SIMPLE_DEV_PM_OPS(stm32_pwm_lp_pm_ops, stm32_pwm_lp_suspend,
>>  
>>  static const struct of_device_id stm32_pwm_lp_of_match[] = {
>>  	{ .compatible = "st,stm32-pwm-lp", },
>> +	{ .compatible = "st,stm32mp25-pwm-lp", },
> 
> No driver data suggests device is backwards compatible. Commit msg
> suggests not, so that's confusing.


The LPTimer PWM driver takes benefit of the MFD parent driver to feed in
data, e.g. 'num_cc_chans'. Number of channels is now variable, on
STM32MP25 (e.g. not a single channel). But it can't be hard-coded as
compatible data. (there's only 1 channel on earlier LP Timer hardware
revision).

The hardware controller is a bit different, hence the new compatible.

Best regards,
Fabrice

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 5/8] pwm: stm32-lp: add support for stm32mp25

[Krzysztof Kozlowski]

On 25/02/2025 15:58, Fabrice Gasnier wrote:
> 
> 
> On 2/25/25 13:04, Krzysztof Kozlowski wrote:
>> On Mon, Feb 24, 2025 at 07:01:47PM +0100, Fabrice Gasnier wrote:
>>>  	}
>>>  
>>>  	return pinctrl_pm_select_sleep_state(dev);
>>> @@ -246,6 +413,7 @@ static DEFINE_SIMPLE_DEV_PM_OPS(stm32_pwm_lp_pm_ops, stm32_pwm_lp_suspend,
>>>  
>>>  static const struct of_device_id stm32_pwm_lp_of_match[] = {
>>>  	{ .compatible = "st,stm32-pwm-lp", },
>>> +	{ .compatible = "st,stm32mp25-pwm-lp", },
>>
>> No driver data suggests device is backwards compatible. Commit msg
>> suggests not, so that's confusing.
> 
> 
> The LPTimer PWM driver takes benefit of the MFD parent driver to feed in
> data, e.g. 'num_cc_chans'. Number of channels is now variable, on

This means this ID table is useless. You do the matching via parent
device, so stop growing the table and call it deprecated or something.

> STM32MP25 (e.g. not a single channel). But it can't be hard-coded as
> compatible data. (there's only 1 channel on earlier LP Timer hardware
> revision).
> 
> The hardware controller is a bit different, hence the new compatible

If it works with old compatible, it's an easy proof that it is
compatible, so please counter argument that with something specific.
What is different that driver cannot work with new device using old
interface or old features?


Best regards,
Krzysztof

Re: [PATCH 5/8] pwm: stm32-lp: add support for stm32mp25

[Fabrice Gasnier]

On 2/26/25 08:54, Krzysztof Kozlowski wrote:
> On 25/02/2025 15:58, Fabrice Gasnier wrote:
>>
>>
>> On 2/25/25 13:04, Krzysztof Kozlowski wrote:
>>> On Mon, Feb 24, 2025 at 07:01:47PM +0100, Fabrice Gasnier wrote:
>>>>  	}
>>>>  
>>>>  	return pinctrl_pm_select_sleep_state(dev);
>>>> @@ -246,6 +413,7 @@ static DEFINE_SIMPLE_DEV_PM_OPS(stm32_pwm_lp_pm_ops, stm32_pwm_lp_suspend,
>>>>  
>>>>  static const struct of_device_id stm32_pwm_lp_of_match[] = {
>>>>  	{ .compatible = "st,stm32-pwm-lp", },
>>>> +	{ .compatible = "st,stm32mp25-pwm-lp", },
>>>
>>> No driver data suggests device is backwards compatible. Commit msg
>>> suggests not, so that's confusing.
>>
>>
>> The LPTimer PWM driver takes benefit of the MFD parent driver to feed in
>> data, e.g. 'num_cc_chans'. Number of channels is now variable, on
> 
> This means this ID table is useless. You do the matching via parent
> device, so stop growing the table and call it deprecated or something.
> 
>> STM32MP25 (e.g. not a single channel). But it can't be hard-coded as
>> compatible data. (there's only 1 channel on earlier LP Timer hardware
>> revision).
>>
>> The hardware controller is a bit different, hence the new compatible
> 
> If it works with old compatible, it's an easy proof that it is
> compatible, so please counter argument that with something specific.

Ack, I'll drop the "st,stm32mp25-pwm-lp" compatible, as match through
the parent device is achieved here.
Alternatively, reading directly the hardware configuration register
could be used to retrieve the 'num_cc_chans'.

Thanks for reviewing,
Best regards,
Fabrice
> What is different that driver cannot work with new device using old
> interface or old features?
> 
> 
> Best regards,
> Krzysztof

[PATCH 6/8] counter: stm32-lptimer-cnt: add support for stm32mp25

[Fabrice Gasnier]

Add support for STM32MP25 SoC. Use newly introduced compatible to handle
this new HW variant, even if no major change is expected on the counter
driver.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 drivers/counter/stm32-lptimer-cnt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
index b249c8647639..a5dce017c37b 100644
--- a/drivers/counter/stm32-lptimer-cnt.c
+++ b/drivers/counter/stm32-lptimer-cnt.c
@@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
 
 static const struct of_device_id stm32_lptim_cnt_of_match[] = {
 	{ .compatible = "st,stm32-lptimer-counter", },
+	{ .compatible = "st,stm32mp25-lptimer-counter", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, stm32_lptim_cnt_of_match);
-- 
2.25.1

Re: [PATCH 6/8] counter: stm32-lptimer-cnt: add support for stm32mp25

[Krzysztof Kozlowski]

On Mon, Feb 24, 2025 at 07:01:48PM +0100, Fabrice Gasnier wrote:
> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
> this new HW variant, even if no major change is expected on the counter
> driver.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> ---
>  drivers/counter/stm32-lptimer-cnt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
> index b249c8647639..a5dce017c37b 100644
> --- a/drivers/counter/stm32-lptimer-cnt.c
> +++ b/drivers/counter/stm32-lptimer-cnt.c
> @@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
>  
>  static const struct of_device_id stm32_lptim_cnt_of_match[] = {
>  	{ .compatible = "st,stm32-lptimer-counter", },
> +	{ .compatible = "st,stm32mp25-lptimer-counter", },

So fully compatible? Why this change then?

Best regards,
Krzysztof

Re: [PATCH 6/8] counter: stm32-lptimer-cnt: add support for stm32mp25

[Fabrice Gasnier]

On 2/25/25 13:02, Krzysztof Kozlowski wrote:
> On Mon, Feb 24, 2025 at 07:01:48PM +0100, Fabrice Gasnier wrote:
>> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
>> this new HW variant, even if no major change is expected on the counter
>> driver.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>> ---
>>  drivers/counter/stm32-lptimer-cnt.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
>> index b249c8647639..a5dce017c37b 100644
>> --- a/drivers/counter/stm32-lptimer-cnt.c
>> +++ b/drivers/counter/stm32-lptimer-cnt.c
>> @@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
>>  
>>  static const struct of_device_id stm32_lptim_cnt_of_match[] = {
>>  	{ .compatible = "st,stm32-lptimer-counter", },
>> +	{ .compatible = "st,stm32mp25-lptimer-counter", },
> 
> So fully compatible? Why this change then?

Hi Krzysztof,

I should have mentioned it in the commit message:

Currently, same feature list as on STM32MP1x is supported.
New capture input stage is now available in the hardware. It's not added
yet to the driver.

The Low Power Timer (LPTIM) hardware isn't fully backward compatible,
hence add a new compatible to properly support it.

Best regards,
Fabrice

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 6/8] counter: stm32-lptimer-cnt: add support for stm32mp25

[Krzysztof Kozlowski]

On 25/02/2025 15:58, Fabrice Gasnier wrote:
> On 2/25/25 13:02, Krzysztof Kozlowski wrote:
>> On Mon, Feb 24, 2025 at 07:01:48PM +0100, Fabrice Gasnier wrote:
>>> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
>>> this new HW variant, even if no major change is expected on the counter
>>> driver.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>>> ---
>>>  drivers/counter/stm32-lptimer-cnt.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
>>> index b249c8647639..a5dce017c37b 100644
>>> --- a/drivers/counter/stm32-lptimer-cnt.c
>>> +++ b/drivers/counter/stm32-lptimer-cnt.c
>>> @@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
>>>  
>>>  static const struct of_device_id stm32_lptim_cnt_of_match[] = {
>>>  	{ .compatible = "st,stm32-lptimer-counter", },
>>> +	{ .compatible = "st,stm32mp25-lptimer-counter", },
>>
>> So fully compatible? Why this change then?
> 
> Hi Krzysztof,
> 
> I should have mentioned it in the commit message:
> 
> Currently, same feature list as on STM32MP1x is supported.
> New capture input stage is now available in the hardware. It's not added
> yet to the driver.

Which is the exact meaning of compatible. Express it properly in
bindings and driver.

> 
> The Low Power Timer (LPTIM) hardware isn't fully backward compatible,

How so? How can it work then with above ID table?


Best regards,
Krzysztof

Re: [PATCH 6/8] counter: stm32-lptimer-cnt: add support for stm32mp25

[Fabrice Gasnier]

On 2/26/25 08:49, Krzysztof Kozlowski wrote:
> On 25/02/2025 15:58, Fabrice Gasnier wrote:
>> On 2/25/25 13:02, Krzysztof Kozlowski wrote:
>>> On Mon, Feb 24, 2025 at 07:01:48PM +0100, Fabrice Gasnier wrote:
>>>> Add support for STM32MP25 SoC. Use newly introduced compatible to handle
>>>> this new HW variant, even if no major change is expected on the counter
>>>> driver.
>>>>
>>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>>>> ---
>>>>  drivers/counter/stm32-lptimer-cnt.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
>>>> index b249c8647639..a5dce017c37b 100644
>>>> --- a/drivers/counter/stm32-lptimer-cnt.c
>>>> +++ b/drivers/counter/stm32-lptimer-cnt.c
>>>> @@ -508,6 +508,7 @@ static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
>>>>  
>>>>  static const struct of_device_id stm32_lptim_cnt_of_match[] = {
>>>>  	{ .compatible = "st,stm32-lptimer-counter", },
>>>> +	{ .compatible = "st,stm32mp25-lptimer-counter", },
>>>
>>> So fully compatible? Why this change then?
>>
>> Hi Krzysztof,
>>
>> I should have mentioned it in the commit message:
>>
>> Currently, same feature list as on STM32MP1x is supported.
>> New capture input stage is now available in the hardware. It's not added
>> yet to the driver.
> 
> Which is the exact meaning of compatible. Express it properly in
> bindings and driver.

Could you please just clarify here, do you suggest to better explain it
in the commit message, both in bindings and driver ?
Or do you mean something else ?

There's one more thing here, I haven't mentioned earlier, which is
specific to the STM32MP25 SoC family (not only to the LPTimer revision).

The LPTimer has interconnect signals throughout all the SoC. It's listed
in the "Peripheral interconnects" section of the Reference Manual (RM0457).
Here, simply reading the LPTimer revision/identification register
wouldn't be enough, as it specific to the SoC. Upcoming STM32MP21 SoC
family will have a different list of inputs/outpus internally in the SoC
(but same controller revision).

So here I intend to keep the specific compatible
"st,stm32mp25-lptimer-counter", to be used even if not yet implemented
by the driver.

I'll try to explain it better in next revision.

Thanks for reviewing,
Best Regards,
Fabrice

> 
>>
>> The Low Power Timer (LPTIM) hardware isn't fully backward compatible,
> 
> How so? How can it work then with above ID table?
> 
> 
> Best regards,
> Krzysztof

[PATCH 7/8] arm64: defconfig: enable STM32 LP timers drivers

[Fabrice Gasnier]

Enable the STM32 timer drivers: MFD, counter, PWM and trigger as modules.
Clocksource is a bool, hence set to y. These drivers can be used on
STM32MP25.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 arch/arm64/configs/defconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 1f25423de383..952eee30f21c 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -775,6 +775,7 @@ CONFIG_MFD_TI_LP873X=m
 CONFIG_MFD_TPS65219=y
 CONFIG_MFD_TPS6594_I2C=m
 CONFIG_MFD_ROHM_BD718XX=y
+CONFIG_MFD_STM32_LPTIMER=m
 CONFIG_MFD_WCD934X=m
 CONFIG_MFD_KHADAS_MCU=m
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
@@ -1401,6 +1402,7 @@ CONFIG_CLK_RENESAS_VBATTB=m
 CONFIG_HWSPINLOCK=y
 CONFIG_HWSPINLOCK_QCOM=y
 CONFIG_TEGRA186_TIMER=y
+CONFIG_CLKSRC_STM32_LP=y
 CONFIG_RENESAS_OSTM=y
 CONFIG_ARM_MHU=y
 CONFIG_IMX_MBOX=y
@@ -1526,6 +1528,7 @@ CONFIG_IIO_CROS_EC_LIGHT_PROX=m
 CONFIG_SENSORS_ISL29018=m
 CONFIG_VCNL4000=m
 CONFIG_IIO_ST_MAGN_3AXIS=m
+CONFIG_IIO_STM32_LPTIMER_TRIGGER=m
 CONFIG_IIO_CROS_EC_BARO=m
 CONFIG_MPL3115=m
 CONFIG_PWM=y
@@ -1543,6 +1546,7 @@ CONFIG_PWM_ROCKCHIP=y
 CONFIG_PWM_RZ_MTU3=m
 CONFIG_PWM_SAMSUNG=y
 CONFIG_PWM_SL28CPLD=m
+CONFIG_PWM_STM32_LP=m
 CONFIG_PWM_SUN4I=m
 CONFIG_PWM_TEGRA=m
 CONFIG_PWM_TIECAP=m
@@ -1682,6 +1686,7 @@ CONFIG_INTERCONNECT_QCOM_SM8750=y
 CONFIG_INTERCONNECT_QCOM_X1E80100=y
 CONFIG_COUNTER=m
 CONFIG_RZ_MTU3_CNT=m
+CONFIG_STM32_LPTIMER_CNT=m
 CONFIG_HTE=y
 CONFIG_HTE_TEGRA194=y
 CONFIG_HTE_TEGRA194_TEST=m
-- 
2.25.1

Re: [PATCH 7/8] arm64: defconfig: enable STM32 LP timers drivers

[Krzysztof Kozlowski]

On 24/02/2025 19:01, Fabrice Gasnier wrote:
> Enable the STM32 timer drivers: MFD, counter, PWM and trigger as modules.
> Clocksource is a bool, hence set to y. These drivers can be used on
> STM32MP25.


Which upstream board? If you do not have upstream board, the defconfig
is pointless for us. It's not defconfig for your downstream forks.


Best regards,
Krzysztof

Re: [PATCH 7/8] arm64: defconfig: enable STM32 LP timers drivers

[Fabrice Gasnier]

On 2/25/25 08:48, Krzysztof Kozlowski wrote:
> On 24/02/2025 19:01, Fabrice Gasnier wrote:
>> Enable the STM32 timer drivers: MFD, counter, PWM and trigger as modules.
>> Clocksource is a bool, hence set to y. These drivers can be used on
>> STM32MP25.
> 
> 
> Which upstream board? If you do not have upstream board, the defconfig
> is pointless for us. It's not defconfig for your downstream forks.

Hi Krzysztof,

It's going to be used on stm32mp257f-dk and stm32mp257f-ev1 boards.
I can add the relevant DT for the(se) board(s) in next revision.

Best regards,
Fabrice

> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 7/8] arm64: defconfig: enable STM32 LP timers drivers

[Krzysztof Kozlowski]

On Tue, Feb 25, 2025 at 09:43:58AM +0100, Fabrice Gasnier wrote:
> On 2/25/25 08:48, Krzysztof Kozlowski wrote:
> > On 24/02/2025 19:01, Fabrice Gasnier wrote:
> >> Enable the STM32 timer drivers: MFD, counter, PWM and trigger as modules.
> >> Clocksource is a bool, hence set to y. These drivers can be used on
> >> STM32MP25.
> > 
> > 
> > Which upstream board? If you do not have upstream board, the defconfig
> > is pointless for us. It's not defconfig for your downstream forks.
> 
> Hi Krzysztof,
> 
> It's going to be used on stm32mp257f-dk and stm32mp257f-ev1 boards.

The defconfig commit should mention which upstream products use it.

> I can add the relevant DT for the(se) board(s) in next revision.

Does this mean there are no upstream users of this? Then this defconfig
change is no-op. We do not add enable things in defconfig just because
there is some downstream/fork using something. Defconfig is for our
(upstream) boards.

Best regards,
Krzysztof

[PATCH 8/8] arm64: dts: st: add low-power timer nodes on stm32mp251

[Fabrice Gasnier]

Add low-power timer (LPTimer) support on STM32MP25 SoC.
The full feature set is implemented in LPTIM1/2/3/4. LPTIM5 supports a
smaller set of features (no capture/compare) channel. Still, LPTIM5 can
be used as single PWM, counter, trigger or timer.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp251.dtsi | 177 +++++++++++++++++++++++++
 1 file changed, 177 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index f3c6cdfd7008..742367e4f16d 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -238,6 +238,78 @@ rifsc: bus@42080000 {
 			#access-controller-cells = <1>;
 			ranges;
 
+			lptimer1: timer@40090000 {
+				compatible = "st,stm32mp25-lptimer";
+				reg = <0x40090000 0x400>;
+				interrupts-extended = <&exti1 47 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&rcc CK_KER_LPTIM1>;
+				clock-names = "mux";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				access-controllers = <&rifsc 17>;
+				power-domains = <&RET_PD>;
+				wakeup-source;
+				status = "disabled";
+
+				counter {
+					compatible = "st,stm32mp25-lptimer-counter";
+					status = "disabled";
+				};
+
+				pwm {
+					compatible = "st,stm32mp25-pwm-lp";
+					#pwm-cells = <3>;
+					status = "disabled";
+				};
+
+				timer {
+					compatible = "st,stm32mp25-lptimer-timer";
+					status = "disabled";
+				};
+
+				trigger@0 {
+					compatible = "st,stm32mp25-lptimer-trigger";
+					reg = <0>;
+					status = "disabled";
+				};
+			};
+
+			lptimer2: timer@400a0000 {
+				compatible = "st,stm32mp25-lptimer";
+				reg = <0x400a0000 0x400>;
+				interrupts-extended = <&exti1 48 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&rcc CK_KER_LPTIM2>;
+				clock-names = "mux";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				access-controllers = <&rifsc 18>;
+				power-domains = <&RET_PD>;
+				wakeup-source;
+				status = "disabled";
+
+				counter {
+					compatible = "st,stm32mp25-lptimer-counter";
+					status = "disabled";
+				};
+
+				pwm {
+					compatible = "st,stm32mp25-pwm-lp";
+					#pwm-cells = <3>;
+					status = "disabled";
+				};
+
+				timer {
+					compatible = "st,stm32mp25-lptimer-timer";
+					status = "disabled";
+				};
+
+				trigger@1 {
+					compatible = "st,stm32mp25-lptimer-trigger";
+					reg = <1>;
+					status = "disabled";
+				};
+			};
+
 			i2s2: audio-controller@400b0000 {
 				compatible = "st,stm32mp25-i2s";
 				reg = <0x400b0000 0x400>;
@@ -799,6 +871,111 @@ i2c8: i2c@46040000 {
 				status = "disabled";
 			};
 
+			lptimer3: timer@46050000 {
+				compatible = "st,stm32mp25-lptimer";
+				reg = <0x46050000 0x400>;
+				interrupts-extended = <&exti2 29 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&rcc CK_KER_LPTIM3>;
+				clock-names = "mux";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				access-controllers = <&rifsc 19>;
+				wakeup-source;
+				status = "disabled";
+
+				counter {
+					compatible = "st,stm32mp25-lptimer-counter";
+					status = "disabled";
+				};
+
+				pwm {
+					compatible = "st,stm32mp25-pwm-lp";
+					#pwm-cells = <3>;
+					status = "disabled";
+				};
+
+				timer {
+					compatible = "st,stm32mp25-lptimer-timer";
+					status = "disabled";
+				};
+
+				trigger@2 {
+					compatible = "st,stm32mp25-lptimer-trigger";
+					reg = <2>;
+					status = "disabled";
+				};
+			};
+
+			lptimer4: timer@46060000 {
+				compatible = "st,stm32mp25-lptimer";
+				reg = <0x46060000 0x400>;
+				interrupts-extended = <&exti2 30 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&rcc CK_KER_LPTIM4>;
+				clock-names = "mux";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				access-controllers = <&rifsc 20>;
+				wakeup-source;
+				status = "disabled";
+
+				counter {
+					compatible = "st,stm32mp25-lptimer-counter";
+					status = "disabled";
+				};
+
+				pwm {
+					compatible = "st,stm32mp25-pwm-lp";
+					#pwm-cells = <3>;
+					status = "disabled";
+				};
+
+				timer {
+					compatible = "st,stm32mp25-lptimer-timer";
+					status = "disabled";
+				};
+
+				trigger@3 {
+					compatible = "st,stm32mp25-lptimer-trigger";
+					reg = <3>;
+					status = "disabled";
+				};
+			};
+
+			lptimer5: timer@46070000 {
+				compatible = "st,stm32mp25-lptimer";
+				reg = <0x46070000 0x400>;
+				interrupts-extended = <&exti2 31 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&rcc CK_KER_LPTIM5>;
+				clock-names = "mux";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				access-controllers = <&rifsc 21>;
+				wakeup-source;
+				status = "disabled";
+
+				counter {
+					compatible = "st,stm32mp25-lptimer-counter";
+					status = "disabled";
+				};
+
+				pwm {
+					compatible = "st,stm32mp25-pwm-lp";
+					#pwm-cells = <3>;
+					status = "disabled";
+				};
+
+				timer {
+					compatible = "st,stm32mp25-lptimer-timer";
+					status = "disabled";
+				};
+
+				trigger@4 {
+					compatible = "st,stm32mp25-lptimer-trigger";
+					reg = <4>;
+					status = "disabled";
+				};
+			};
+
 			csi: csi@48020000 {
 				compatible = "st,stm32mp25-csi";
 				reg = <0x48020000 0x2000>;
-- 
2.25.1

Re: [PATCH 0/8] Add STM32MP25 LPTIM support: MFD, PWM, IIO, counter, clocksource

[Rob Herring (Arm)]

On Mon, 24 Feb 2025 19:01:42 +0100, Fabrice Gasnier wrote:
> This series adds support for STM32MP25 to MFD PWM, IIO, counter and
> clocksource low-power timer (LPTIM) drivers.
> This new variant is managed by using a new DT compatible string.
> It comes with a slightly updated register set, some new features and new
> interconnect signals inside the SoC.
> Same feature list as on STM32MP1x is supported currently.
> The device tree files add all instances in stm32mp251 dtsi file.
> 
> Fabrice Gasnier (6):
>   dt-bindings: mfd: stm32-lptimer: add support for stm32mp25
>   mfd: stm32-lptimer: add support for stm32mp25
>   pwm: stm32-lp: add support for stm32mp25
>   counter: stm32-lptimer-cnt: add support for stm32mp25
>   arm64: defconfig: enable STM32 LP timers drivers
>   arm64: dts: st: add low-power timer nodes on stm32mp251
> 
> Olivier Moysan (1):
>   iio: trigger: stm32-lptimer: add support for stm32mp25
> 
> Patrick Delaunay (1):
>   clocksource: stm32-lptimer: add stm32mp25 support
> 
>  .../bindings/mfd/st,stm32-lptimer.yaml        |  23 +-
>  arch/arm64/boot/dts/st/stm32mp251.dtsi        | 177 ++++++++++++++
>  arch/arm64/configs/defconfig                  |   5 +
>  drivers/clocksource/timer-stm32-lp.c          |   1 +
>  drivers/counter/stm32-lptimer-cnt.c           |   1 +
>  drivers/iio/trigger/stm32-lptimer-trigger.c   | 109 +++++++--
>  drivers/mfd/stm32-lptimer.c                   |  30 ++-
>  drivers/pwm/pwm-stm32-lp.c                    | 220 +++++++++++++++---
>  include/linux/iio/timer/stm32-lptim-trigger.h |   9 +
>  include/linux/mfd/stm32-lptimer.h             |  32 ++-
>  10 files changed, 554 insertions(+), 53 deletions(-)
> 
> --
> 2.25.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/st/' for 20250224180150.3689638-1-fabrice.gasnier@foss.st.com:

arch/arm64/boot/dts/st/stm32mp257f-ev1.dtb: timer@46060000: trigger@3:reg:0:0: 3 is greater than the maximum of 2
	from schema $id: http://devicetree.org/schemas/mfd/st,stm32-lptimer.yaml#
arch/arm64/boot/dts/st/stm32mp257f-ev1.dtb: timer@46070000: trigger@4:reg:0:0: 4 is greater than the maximum of 2
	from schema $id: http://devicetree.org/schemas/mfd/st,stm32-lptimer.yaml#