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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47ed1b83-9ace-475b-8279-6c7f394c35f3@linaro.org>
Date: Mon, 11 Aug 2025 11:44:32 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Uwe Kleine-König <ukleinek@...nel.org>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 Frank.Li@....com, linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Ghennadi.Procopciuc@....com, s32@....com
Subject: Re: [PATCH v1 2/2] pwm: Add the S32G support in the Freescale FTM
 driver


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@....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@....com>
>> Co-developed-by: Daniel Lezcano <daniel.lezcano@...aro.org>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.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