linux-kernel - Re: [PATCH v19 2/6] pwm: driver for qualcomm ipq6018 pwm block

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] [day] [month] [year] [list]

Message-ID:
 <BL4PR19MB8902F80FE2D7FD5BCBE340689DA2A@BL4PR19MB8902.namprd19.prod.outlook.com>
Date: Mon, 8 Dec 2025 19:40:21 +0400
From: George Moussalem <george.moussalem@...look.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
 Uwe Kleine-König <ukleinek@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Baruch Siach <baruch@...s.co.il>,
 Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, linux-pwm@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Devi Priya <quic_devipriy@...cinc.com>, Baruch Siach <baruch.siach@...lu.com>
Subject: Re: [PATCH v19 2/6] pwm: driver for qualcomm ipq6018 pwm block



On 11/28/25 15:32, Konrad Dybcio wrote:
> On 11/28/25 11:29 AM, George Moussalem via B4 Relay wrote:
>> From: Devi Priya <quic_devipriy@...cinc.com>
>>
>> Driver for the PWM block in Qualcomm IPQ6018 line of SoCs. Based on
>> driver from downstream Codeaurora kernel tree. Removed support for older
>> (V1) variants because I have no access to that hardware.
>>
>> Tested on IPQ5018 and IPQ6010 based hardware.
>>
>> Co-developed-by: Baruch Siach <baruch.siach@...lu.com>
>> Signed-off-by: Baruch Siach <baruch.siach@...lu.com>
>> Signed-off-by: Devi Priya <quic_devipriy@...cinc.com>
>> Reviewed-by: Bjorn Andersson <andersson@...nel.org>
>> Signed-off-by: George Moussalem <george.moussalem@...look.com>
>> ---
> 
> [...]
> 
>> +/* The frequency range supported is 1 Hz to clock rate */
>> +#define IPQ_PWM_MAX_PERIOD_NS	((u64)NSEC_PER_SEC)
>> +
>> +/*
>> + * The max value specified for each field is based on the number of bits
>> + * in the pwm control register for that field
>> + */
>> +#define IPQ_PWM_MAX_DIV		0xFFFF
>> +
>> +/*
>> + * Two 32-bit registers for each PWM: REG0, and REG1.
>> + * Base offset for PWM #i is at 8 * #i.
>> + */
>> +#define IPQ_PWM_REG0			0
>> +#define IPQ_PWM_REG0_PWM_DIV		GENMASK(15, 0)
>> +#define IPQ_PWM_REG0_HI_DURATION	GENMASK(31, 16)
>> +
>> +#define IPQ_PWM_REG1			4
>> +#define IPQ_PWM_REG1_PRE_DIV		GENMASK(15, 0)
> 
> Sorry for coming in so late, you may consider this as material for a
> follow-up patch (I *really* don't want to hold off your v19..)
> 
> I see that on ipq9574 the registers are named:
> 
> base = 0x1941010 = tcsr + 0xa010
> 
> 0x0	CFG0_R0
> 0x4	CFG1_R0
> 0x8	CFG0_R1
> 0xc	CFG1_R1
> 0x10	CFG0_R2
> 0x14	CFG1_R2
> 0x18	CFG0_R3
> 0x1c	CFG1_R3
> 
> CFG0 and CFG1 are what you called REG0/REG1 and Rn is confusingly the
> index of the controller/output
> 
> The other bits in CFG1 (29:16) are RESERVED so there's nothing you
> missed in there

thanks for your review and validation.

> 
>> +
>> +/*
>> + * Enable bit is set to enable output toggling in pwm device.
>> + * Update bit is set to trigger the change and is unset automatically
>> + * to reflect the changed divider and high duration values in register.
>> + */
>> +#define IPQ_PWM_REG1_UPDATE		BIT(30)
>> +#define IPQ_PWM_REG1_ENABLE		BIT(31)
>> +
>> +struct ipq_pwm_chip {
>> +	struct clk *clk;
>> +	void __iomem *mem;
>> +};
>> +
>> +static struct ipq_pwm_chip *ipq_pwm_from_chip(struct pwm_chip *chip)
>> +{
>> +	return pwmchip_get_drvdata(chip);
>> +}
>> +
>> +static unsigned int ipq_pwm_reg_read(struct pwm_device *pwm, unsigned int reg)
>> +{
>> +	struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(pwm->chip);
>> +	unsigned int off = 8 * pwm->hwpwm + reg;
> 
> This magic 8 could be #defined as IPQ6018_PWM_CONTROLLER_STRIDE or so

good suggestion, will add it should another iteration be required.

> 
> Konrad

BR,
George