PWM driver turns off pin if off time is 0 in set_values

[zephyrbot]

Reported by Geoff Gustafson:

If you try to set a PWM to on or off all the time (with off = 0 or on = 0), in both cases the driver will see it as an error in __set_one_port() and silently turn off the pin entirely. For the "on = 0" case, this is great - this is exactly what you want. But for the "off = 0" case, this is exactly the opposite of what you want. Now, it seems the PWM can't be set to fully on, so the "best" solution is to change the pin to GPIO mode and set it high. But that may be impractical because it would presumably require including the GPIO module as well.

In any case, the documentation for the pwm_pin_set_values() function currently says nothing about this border case.

Another option may be that this function shouldn't just fail silently but rather report an error. But it strikes me that in the case of an accidental call to this function with "off = 0", it would be better to just leave the PWM in whatever state it was previously in.

But I think the best solutions are either:

1. Leave the PWM at its previous settings
2. Set the off time to 1 cycle and decrease on time by 1 cycle to keep the period the same

I've attached a patch with solution #1 for DW and QMSI. Not sure if the problem exists in other drivers; they are different.

(Imported from Jira ZEP-401)

[zephyrbot]

by Geoff Gustafson:

• 0001-pwm-Leave-previous-PWM-settings-when-off-time-of-zer.patch (size: 1632)

[zephyrbot]

by Kuo-Lang Tseng:

Patch for QMSI driver posted: https://gerrit.zephyrproject.org/r/#/c/2475/

[zephyrbot]

by Kuo-Lang Tseng:

Fixed in PWM QMSI shim driver.

[zephyrbot]

by Mark Linkmeyer:

Sharron LIU , this bug is in the Resolve state, but there's no label applied yet to indicate who needs to verify it. Please review is and attach appropriate label so we can get it tested and verified ASAP. Thanks!

[zephyrbot]

by ethan gao:

Verified always on and always off:
actual pulse length with always on (off=0):
LOW = 2 ms and the -reset- rest is HIGH
actual pulse length with always off(on=0):
LOW=0 and HIGH=0

[zephyrbot]

by ethan gao:

If I am not wrong, I remember that QUARK datasheet requires at least > 2 clock cycle length for both on and off value, so if we set it to 0 or 1, is it really useful ?

[zephyrbot]

by Baohong Liu:

ethan gao You are very right. But, some people in the community think setting "off" to 0 means always on.

[zephyrbot]

by Mark Linkmeyer:

Fixing incorrect priority

[zephyrbot]

Related to GH-1731