PWM related fixes for rpi_pico board #52692
Conversation
The fractional part of the divider value is a 4 bit value. Setting it to 255 leads to an overflow in pwm_rpi_get_clkdiv(). This has resulted in a slight deviation from the reported timing, which is btw. printed in nsec. Fixes: c5cb0d1 ("samples: blinky_pwm: Fix sample for rpi_pico") Signed-off-by: Oliver Barta <[email protected]>
The default deferred logging mode is not appropriate here. The LOG_INF messages "Turned on", "Turned off", ... indicate the current state and should be printed immediately to keep log output in sync with actual LED state. Signed-off-by: Oliver Barta <[email protected]>
The fractional part of the divider value is a 4 bit value. Fixes: 91f659d ("samples: led_pwm: add overlay for rpi_pico board") Signed-off-by: Oliver Barta <[email protected]>
zephyrbot
added
area: PWM
zephyrbot
requested review from
anangl,
henrikbrixandersen,
Mani-Sadhasivam and
simonguinot
led0 and pwm_led0 are both referring to the same LED. The usage is mutualy exclusive, as only one pin muxing can be applied. Disabling child nodes is currently neither supported by gpio-leds device nor by pwm-leds device. We need to disable either of the devices completely for correct operation. Please note that even if disabling of child nodes would be supported, the devices would be empty anyway, as both have only one singe LED defined at the moment. Therefore we can completely disable one of the devices in any case. Fixes: b876ad9 ("boards: arm: rpi_pico: add pwm bindings to devictree") Signed-off-by: Oliver Barta <[email protected]>
PWM device needs to be enabled and configured on rpi_pico board. Signed-off-by: Oliver Barta <[email protected]>
pwm_set_wrap() sets the TOP value, not the number of cycles. Counter will run from 0 to TOP inclusive, generating TOP + 1 cycles. To get n cycles, we need to set TOP to (n - 1). The wrong setting made it impossible to achieve 100 % duty cycle, as there was always one extra cycle. Fixes: 7e0fff2 ("drivers: pwm: add pwm driver for rpi_pico") Signed-off-by: Oliver Barta <[email protected]>
bartao
force-pushed
the
rpi_pico_pwm_fixes
branch
from
7469dfc to
d469939
Compare
Agreed, I will revert that change. @bartao any objections? I see you reasoned that they weren't appropriate, but did you see a significant delay between the LED status changed and the printing of the message? |
@carlescufi , yes, message printing gets delayed up to 1 second in this case, which is really confusing and caused me trouble during debugging. Right after power on, the initial booting messages get printed together with "Turned on" and the LED turns on. That is fine so far. But when the LED turns off roughly one second later, the expected "Turned off" message is not printed. After another second the LED brightness starts to increase again. At this point the message "Turned off", which was expected already a second ago, and the new message "Increasing brightness gradually" are printed right after each other without any delay between them. There is a delay of one second between the two events, but no delay between the corresponding messages. When you keep it running and catch some later iteration, it can be seen that "Turned on" and "Turned off" are printed together without any delay between the messages. That is really confusing, when you know that the LED is on for a second. Thanks for review and merging. |
|
Honestly I don't remember such delays. But I am OK with this change anyway. |
Minor fixes to get PWM timing right and PWM samples working on rpi_pico board.