[PATCH] pwm: samsung: Implement .apply() callback

[Krzysztof Kozlowski]

On 25/04/2022 19:16, Uwe Kleine-König wrote:

>>> +	/*
>>> +	 * We currently avoid using 64bit arithmetic by using the
>>> +	 * fact that anything faster than 1Hz is easily representable
>>> +	 * by 32bits.
>>> +	 */
>>> +	if (state->period > NSEC_PER_SEC)
>>
>> Isn't this changing a bit logic in case of setting wrong period and
>> inverted signal?
>>
>> Before, the config would return early and do nothing. Now, you disable
>> the PWM, check the condition here and exit with PWM disabled. I think
>> this should reverse the tasks done, or the check should be done early.
> 
> The intension here is to just push the legacy implementation of .apply()
> (i.e.  pwm_apply_legacy()) into the driver, to be able to get rid of the
> legacy handing in the core. I think the problem you point out is real,
> but it is not introduced by my change which doesn't change behaviour.
> 
> The problem is just that it's not possible to "see" that period is
> invalid at the time the polarity is changed and so it exists since at
> least 5ec803edcb703fe379836f13560b79dfac79b01d, my patch just made it
> more obvious.
> 
> So yes, there is potential to further improve the driver, but that's out
> of scope for this 1:1 conversion patch.

Sounds reasonable, thanks for explanation. Everything else was looking
good, so:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org>




Best regards,
Krzysztof