Re: [PATCH v20 2/6] pwm: driver for qualcomm ipq6018 pwm block

[George Moussalem]

On 4/4/2026 11:09 PM, Uwe Kleine-König wrote:
> Hello George,
> 
> On Fri, Apr 03, 2026 at 12:40:32PM +0200, George Moussalem wrote:
>> On 4/2/2026 5:35 PM, Uwe Kleine-König wrote:
>>> diff --git a/drivers/pwm/pwm-ipq.c b/drivers/pwm/pwm-ipq.c
>>> index b944ecb456d5..4818d0170d53 100644
>>> --- a/drivers/pwm/pwm-ipq.c
>>> +++ b/drivers/pwm/pwm-ipq.c
>>> @@ -97,9 +97,10 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>>>  	if (state->polarity != PWM_POLARITY_NORMAL)
>>>  		return -EINVAL;
>>>  
>>> -	if (!ipq_chip->clk_rate)
>>> -		return -EINVAL;
>>> -
>>> +	/*
>>> +	 * XXX Why? A comment please. (Is this already covered by the checks
>>> +	 * below?)
>>> +	 */
>>
>> This check can be safely removed as it is indeed covered by the check
>> where the period_ns is limited to IPQ_PWM_MAX_PERIOD_NS which equals to
>> NSEC_PER_SEC as per macro definition above.
>>
>>>  	if (state->period < DIV64_U64_ROUND_UP(NSEC_PER_SEC,
>>>  					       ipq_chip->clk_rate))
>>>  		return -ERANGE;
>>> @@ -107,18 +108,29 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>>>  	period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
>>>  	duty_ns = min(state->duty_cycle, period_ns);
>>>  
>>> +	/*
>>> +	 * Pick the maximal value for PWM_DIV that still allows a
>>> +	 * 100% relative duty cycle. This allows a fine grained
>>> +	 * selection of duty cycles.
>>> +	 */
>>>  	pwm_div = IPQ_PWM_MAX_DIV - 1;
>>> +
>>> +	/*
>>> +	 * XXX mul_u64_u64_div_u64 returns an u64, this might overflow the
>>> +	 * unsigned int pre_div.
>>> +	 */
>>
>> Theoretically, yes, but in practice it won't due to above constraints.
>> Take the max period of 10^9 (NSEC_PER_SEC) * max clock rate of 10^9 (1
>> GHz), then the numerator becomes 10^18. Divide that by 10^9
>> (NSEC_PER_SEC) * 65,535 (IPQ_PWM_MAX_DIV) and that fits well into a
>> 32-bit integer.
> 
> OK, please put that in a comment.
> 
>> Do you want me to send a v21 or can you apply the diff in your tree with
>> above deletion and comment?
> 
> Yes, please send a v21.

Just to circle back, I've sent v21. Can you please review and get it
merged if there are no further comments?

> 
> Best regards
> Uwe

Thanks,
George