[PATCH v1 0/2] Add the s32g2 and s32g3 FTM PWM support

[PATCH v1 0/2] Add the s32g2 and s32g3 FTM PWM support

[Daniel Lezcano]

The NXP Automotive platform s32g2 and s32g3 have on their board a
FlexTimer (FTM) dedicated for the PWM. The same IP is found on the
Freescale Vybrid Family and the i.MX8 SoCs. However, there is a small
difference with some registers not available on the s32g2/3 and 6
channels instead of 8.

These two patches provide the DT bindings for the s32g2/3 compatible
strings and the code to deal with the FTM difference.

Daniel Lezcano (1):
  dt: bindings: fsl,vf610-ftm-pwm: Add compatible for s32g2 and s32g3

Ghennadi Procopciuc (1):
  pwm: Add the S32G support in the Freescale FTM driver

 .../bindings/pwm/fsl,vf610-ftm-pwm.yaml       | 11 +++--
 drivers/pwm/pwm-fsl-ftm.c                     | 42 ++++++++++++++++++-
 2 files changed, 48 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v1 1/2] dt: bindings: fsl,vf610-ftm-pwm: Add compatible for s32g2 and s32g3

[Daniel Lezcano]

The S32G2 and S32G3 have a FlexTimer (FTM) available which is the same
as the one found on the Vybrid Family and the i.MX8.

Add the compatibles in the bindings

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 .../devicetree/bindings/pwm/fsl,vf610-ftm-pwm.yaml    | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/fsl,vf610-ftm-pwm.yaml b/Documentation/devicetree/bindings/pwm/fsl,vf610-ftm-pwm.yaml
index 7f9f72d95e7a..c7a10180208e 100644
--- a/Documentation/devicetree/bindings/pwm/fsl,vf610-ftm-pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/fsl,vf610-ftm-pwm.yaml
@@ -26,9 +26,14 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - fsl,vf610-ftm-pwm
-      - fsl,imx8qm-ftm-pwm
+    oneOf:
+      - enum:
+          - fsl,vf610-ftm-pwm
+          - fsl,imx8qm-ftm-pwm
+          - nxp,s32g2-ftm-pwm
+      - items:
+          - const: nxp,s32g3-ftm-pwm
+          - const: nxp,s32g2-ftm-pwm
 
   reg:
     maxItems: 1
-- 
2.43.0

[PATCH v1 2/2] pwm: Add the S32G support in the Freescale FTM driver

[Daniel Lezcano]

From: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>

The Automotive S32G2 and S32G3 platforms include two FTM timers for
pwm. Each FTM has 6 PWM channels.

The current Freescale FTM driver supports the iMX8 and the Vybrid
Family FTM IP. The FTM IP found on the S32G platforms is almost
identical except for the number of channels and the register mapping.

These changes allow to deal with different number of channels and
support the holes found in the register memory mapping for s32gx for
suspend / resume.

Tested on a s32g274-rdb2 J5 PWM pin output with signal visualization
on oscilloscope.

Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
Co-developed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/pwm/pwm-fsl-ftm.c | 42 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
index c45a5fca4cbb..cdf2e3572c90 100644
--- a/drivers/pwm/pwm-fsl-ftm.c
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -3,6 +3,7 @@
  *  Freescale FlexTimer Module (FTM) PWM Driver
  *
  *  Copyright 2012-2013 Freescale Semiconductor, Inc.
+ *  Copyright 2020-2025 NXP
  */
 
 #include <linux/clk.h>
@@ -31,6 +32,9 @@ enum fsl_pwm_clk {
 
 struct fsl_ftm_soc {
 	bool has_enable_bits;
+	bool has_fltctrl;
+	bool has_fltpol;
+	unsigned int npwm;
 };
 
 struct fsl_pwm_periodcfg {
@@ -386,6 +390,23 @@ static bool fsl_pwm_volatile_reg(struct device *dev, unsigned int reg)
 	return false;
 }
 
+static bool fsl_pwm_is_reg(struct device *dev, unsigned int reg)
+{
+	struct pwm_chip *chip = dev_get_drvdata(dev);
+	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
+
+	if (reg >= FTM_CSC(fpc->soc->npwm) && reg < FTM_CNTIN)
+		return false;
+
+	if (reg == FTM_FLTCTRL && !fpc->soc->has_fltctrl)
+		return false;
+
+	if (reg == FTM_FLTPOL && !fpc->soc->has_fltpol)
+		return false;
+
+	return true;
+}
+
 static const struct regmap_config fsl_pwm_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
@@ -394,23 +415,26 @@ static const struct regmap_config fsl_pwm_regmap_config = {
 	.max_register = FTM_PWMLOAD,
 	.volatile_reg = fsl_pwm_volatile_reg,
 	.cache_type = REGCACHE_FLAT,
+	.writeable_reg = fsl_pwm_is_reg,
+	.readable_reg = fsl_pwm_is_reg,
 };
 
 static int fsl_pwm_probe(struct platform_device *pdev)
 {
+	const struct fsl_ftm_soc *soc = of_device_get_match_data(&pdev->dev);
 	struct pwm_chip *chip;
 	struct fsl_pwm_chip *fpc;
 	void __iomem *base;
 	int ret;
 
-	chip = devm_pwmchip_alloc(&pdev->dev, 8, sizeof(*fpc));
+	chip = devm_pwmchip_alloc(&pdev->dev, soc->npwm, sizeof(*fpc));
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
 	fpc = to_fsl_chip(chip);
 
 	mutex_init(&fpc->lock);
 
-	fpc->soc = of_device_get_match_data(&pdev->dev);
+	fpc->soc = soc;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
@@ -526,15 +550,29 @@ static const struct dev_pm_ops fsl_pwm_pm_ops = {
 
 static const struct fsl_ftm_soc vf610_ftm_pwm = {
 	.has_enable_bits = false,
+	.has_fltctrl = true,
+	.has_fltpol = true,
+	.npwm = 8,
 };
 
 static const struct fsl_ftm_soc imx8qm_ftm_pwm = {
 	.has_enable_bits = true,
+	.has_fltctrl = true,
+	.has_fltpol = true,
+	.npwm = 8,
+};
+
+static const struct fsl_ftm_soc s32g2_ftm_pwm = {
+	.has_enable_bits = true,
+	.has_fltctrl = false,
+	.has_fltpol = false,
+	.npwm = 6,
 };
 
 static const struct of_device_id fsl_pwm_dt_ids[] = {
 	{ .compatible = "fsl,vf610-ftm-pwm", .data = &vf610_ftm_pwm },
 	{ .compatible = "fsl,imx8qm-ftm-pwm", .data = &imx8qm_ftm_pwm },
+	{ .compatible = "nxp,s32g2-ftm-pwm", .data = &s32g2_ftm_pwm },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, fsl_pwm_dt_ids);
-- 
2.43.0

Re: [PATCH v1 2/2] pwm: Add the S32G support in the Freescale FTM driver

[Uwe Kleine-König]

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

Hello,

On Sun, Aug 10, 2025 at 08:52:18PM +0200, Daniel Lezcano wrote:
> From: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
> 
> The Automotive S32G2 and S32G3 platforms include two FTM timers for
> pwm. Each FTM has 6 PWM channels.
> 
> The current Freescale FTM driver supports the iMX8 and the Vybrid
> Family FTM IP. The FTM IP found on the S32G platforms is almost
> identical except for the number of channels and the register mapping.
> 
> These changes allow to deal with different number of channels and
> support the holes found in the register memory mapping for s32gx for
> suspend / resume.
> 
> Tested on a s32g274-rdb2 J5 PWM pin output with signal visualization
> on oscilloscope.
> 
> Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
> Co-developed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/pwm/pwm-fsl-ftm.c | 42 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
> index c45a5fca4cbb..cdf2e3572c90 100644
> --- a/drivers/pwm/pwm-fsl-ftm.c
> +++ b/drivers/pwm/pwm-fsl-ftm.c
> @@ -3,6 +3,7 @@
>   *  Freescale FlexTimer Module (FTM) PWM Driver
>   *
>   *  Copyright 2012-2013 Freescale Semiconductor, Inc.
> + *  Copyright 2020-2025 NXP
>   */
>  
>  #include <linux/clk.h>
> @@ -31,6 +32,9 @@ enum fsl_pwm_clk {
>  
>  struct fsl_ftm_soc {
>  	bool has_enable_bits;
> +	bool has_fltctrl;
> +	bool has_fltpol;

All variants (up to now) have .has_fltctrl == .has_fltpol. Is there a
good reason that justifies two bools for the register description?

Also I wonder about the fuss given that the two registers are not used
in the PWM driver. So this is only to prevent reading these registers
via regmap debug stuff? What happens if the memory locations are read
where the other implementations have these registers?

> +	unsigned int npwm;
>  };

Best regards
Uwe

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

Re: [PATCH v1 2/2] pwm: Add the S32G support in the Freescale FTM driver

[Daniel Lezcano]

Hi Uwe,

thanks for reviewing the changes

On 11/08/2025 07:18, Uwe Kleine-König wrote:
> Hello,
> 
> On Sun, Aug 10, 2025 at 08:52:18PM +0200, Daniel Lezcano wrote:
>> From: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
>>
>> The Automotive S32G2 and S32G3 platforms include two FTM timers for
>> pwm. Each FTM has 6 PWM channels.
>>
>> The current Freescale FTM driver supports the iMX8 and the Vybrid
>> Family FTM IP. The FTM IP found on the S32G platforms is almost
>> identical except for the number of channels and the register mapping.
>>
>> These changes allow to deal with different number of channels and
>> support the holes found in the register memory mapping for s32gx for
>> suspend / resume.
>>
>> Tested on a s32g274-rdb2 J5 PWM pin output with signal visualization
>> on oscilloscope.
>>
>> Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
>> Co-developed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>   drivers/pwm/pwm-fsl-ftm.c | 42 +++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
>> index c45a5fca4cbb..cdf2e3572c90 100644
>> --- a/drivers/pwm/pwm-fsl-ftm.c
>> +++ b/drivers/pwm/pwm-fsl-ftm.c
>> @@ -3,6 +3,7 @@
>>    *  Freescale FlexTimer Module (FTM) PWM Driver
>>    *
>>    *  Copyright 2012-2013 Freescale Semiconductor, Inc.
>> + *  Copyright 2020-2025 NXP
>>    */
>>   
>>   #include <linux/clk.h>
>> @@ -31,6 +32,9 @@ enum fsl_pwm_clk {
>>   
>>   struct fsl_ftm_soc {
>>   	bool has_enable_bits;
>> +	bool has_fltctrl;
>> +	bool has_fltpol;
> 
> All variants (up to now) have .has_fltctrl == .has_fltpol. Is there a
> good reason that justifies two bools for the register description?

Yeah, I agree it can be folded into a single has_flt_reg boolean. I can 
only guess that was done with the idea of sticking to the reference 
manual and perhaps having more variant to come with, eg.,  fltctrl=false 
and fltpol=true

Do you want me to merge these boolean ?

> Also I wonder about the fuss given that the two registers are not used
> in the PWM driver. So this is only to prevent reading these registers
> via regmap debug stuff? What happens if the memory locations are read
> where the other implementations have these registers?

The problem arises at resume time.

	/* restore all registers from cache */
         clk_prepare(fpc->ipg_clk);
	regcache_cache_only(fpc->regmap, false);
         regcache_sync(fpc->regmap);

Without skipping these registers, the kernel crashes on s32g2/3


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v1 2/2] pwm: Add the S32G support in the Freescale FTM driver

[Uwe Kleine-König]

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

Hello Daniel,

On Mon, Aug 11, 2025 at 11:44:32AM +0200, Daniel Lezcano wrote:
> On 11/08/2025 07:18, Uwe Kleine-König wrote:
> > All variants (up to now) have .has_fltctrl == .has_fltpol. Is there a
> > good reason that justifies two bools for the register description?
> 
> Yeah, I agree it can be folded into a single has_flt_reg boolean. I can only
> guess that was done with the idea of sticking to the reference manual and
> perhaps having more variant to come with, eg.,  fltctrl=false and
> fltpol=true
> 
> Do you want me to merge these boolean ?

That's the obvious thing to do if you want the new variant supported :-)

Unless you know that there is such a variant with .has_fltctrl !=
.has_fltpol to appear soon, I prefer the simplified handling with only
one bool.

> > Also I wonder about the fuss given that the two registers are not used
> > in the PWM driver. So this is only to prevent reading these registers
> > via regmap debug stuff? What happens if the memory locations are read
> > where the other implementations have these registers?
> 
> The problem arises at resume time.
> 
> 	/* restore all registers from cache */
>         clk_prepare(fpc->ipg_clk);
> 	regcache_cache_only(fpc->regmap, false);
>         regcache_sync(fpc->regmap);
> 
> Without skipping these registers, the kernel crashes on s32g2/3

That's a useful information for the commit log.

Best regards
Uwe

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

Re: [PATCH v1 2/2] pwm: Add the S32G support in the Freescale FTM driver

[Daniel Lezcano]

On 11/08/2025 23:11, Uwe Kleine-König wrote:
> Hello Daniel,
> 
> On Mon, Aug 11, 2025 at 11:44:32AM +0200, Daniel Lezcano wrote:
>> On 11/08/2025 07:18, Uwe Kleine-König wrote:
>>> All variants (up to now) have .has_fltctrl == .has_fltpol. Is there a
>>> good reason that justifies two bools for the register description?
>>
>> Yeah, I agree it can be folded into a single has_flt_reg boolean. I can only
>> guess that was done with the idea of sticking to the reference manual and
>> perhaps having more variant to come with, eg.,  fltctrl=false and
>> fltpol=true
>>
>> Do you want me to merge these boolean ?
> 
> That's the obvious thing to do if you want the new variant supported :-)
> 
> Unless you know that there is such a variant with .has_fltctrl !=
> .has_fltpol to appear soon, I prefer the simplified handling with only
> one bool.
> 
>>> Also I wonder about the fuss given that the two registers are not used
>>> in the PWM driver. So this is only to prevent reading these registers
>>> via regmap debug stuff? What happens if the memory locations are read
>>> where the other implementations have these registers?
>>
>> The problem arises at resume time.
>>
>> 	/* restore all registers from cache */
>>          clk_prepare(fpc->ipg_clk);
>> 	regcache_cache_only(fpc->regmap, false);
>>          regcache_sync(fpc->regmap);
>>
>> Without skipping these registers, the kernel crashes on s32g2/3
> 
> That's a useful information for the commit log.

Ok, thanks, I'll do the changes accordingly


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog