Hi Dmitry, On 3/21/21 11:23 PM, Dmitry Torokhov wrote:
On Tue, Mar 16, 2021 at 09:38:13PM +0100, Uwe Kleine-König wrote:max8997_haptic_enable() is the only caller of max8997_haptic_set_duty_cycle(). For the non-external case the PWM is already enabled in max8997_haptic_set_duty_cycle(), so this can be doneAre you sure about that? I think the intent was to enable it in max8997_haptic_configure(), and only after "inmotor" regulator is enabled. If the device is enabled earlier then I'd say we need to make sure we disable it until it is needed.
If you claim you understand this better, I will well believe that. I described my train of thoughts, i.e. how I understood the internal case.
Anyhow, there is little sense in separating configuration and enablement of the PWM, because the change of duty_cycle and period for a disabled PWM is expected to do nothing to the hardware's output.
So the safer approach is to do the pwm_apply_state at the place, where pwm_enable was before, but the more consistent is how I suggested in my patch. If it feels better I can do the more conservative change instead and if somebody with a deeper understanding of the driver and/or a testing possibility can be found, the internal and external cases can be unified.
Best regards Uwe
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
