Add PWM support for Renesas RA8 board #76140
Conversation
|
The following west manifest projects have been modified in this Pull Request:
Note: This message is automatically posted and updated by the Manifest GitHub Action. |
zephyrbot
added
manifest
manifest-hal_renesas
DNM
quytranpzz
force-pushed
the
support_renesas_ra8_pwm
branch
from
beb168a
to
2befa92
Compare
quytranpzz
force-pushed
the
support_renesas_ra8_pwm
branch
from
2befa92
to
d8f0a1f
Compare
quytranpzz
force-pushed
the
support_renesas_ra8_pwm
branch
from
d8f0a1f
to
1bfe133
Compare
zephyrbot
requested review from
anangl,
henrikbrixandersen,
KhiemNguyenT,
soburi and
thaoluonguw
duynguyenxa
changed the title
drivers/pwm/pwm_renesas_ra8.c
Outdated
| #if GPT_CFG_OUTPUT_SUPPORT_ENABLE | ||
|
|
||
| /* Check if custom GTIOR settings are provided. */ | ||
| if (0 == p_extend->gtior_setting.gtior) { |
There was a problem hiding this comment.
| if (0 == p_extend->gtior_setting.gtior) { | |
| if (p_extend->gtior_setting.gtior == 0) { |
I think I know what's behind the inverted condition but that's very unusual in the Zephyr code base and just makes the codde look unfamiliar.
Same for the others below.
drivers/pwm/pwm_renesas_ra8.c
Outdated
| #if GPT_PRV_EXTRA_FEATURES_ENABLED == GPT_CFG_OUTPUT_SUPPORT_ENABLE | ||
| else if (p_cfg->mode >= TIMER_MODE_TRIANGLE_WAVE_SYMMETRIC_PWM) { | ||
| gtion = GPT_PRV_GTIO_TOGGLE_COMPARE_MATCH; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
this should really be a switch (p_cfg->mode)
There was a problem hiding this comment.
Thanks for your comment but I think we have nested if else conditions here, and greater comparison as well. Could we use if else condition for more resonable?
There was a problem hiding this comment.
I see, alright it's just that it's very hard to follow these "else if" broken by the precompiler directives
There was a problem hiding this comment.
how about: could you make a separate functoin that gets p_cfg->mode as an input and returns the gtion value? Then you can have intermediate returns and you should be able to avoid the split if-else I think
There was a problem hiding this comment.
@fabiobaltieri: Sorry for the late response.
Actually we use only TIMER_MODE_PWM mode on PWM driver with our GPT hardware. For some reason, we defined this function with multiple modes, but to avoid confusion, I removed the others, only define1 value gtion for TIMER_MODE_PWM here. Thanks again for your comments.
quytranpzz
dismissed stale reviews from KhiemNguyenT, soburi, and thaoluonguw
via
384adb3
quytranpzz
force-pushed
the
support_renesas_ra8_pwm
branch
from
1bfe133
to
384adb3
Compare
Add PWM driver code support for RA8. This support is using GPT HW Signed-off-by: Duy Phuong Hoang. Nguyen <duy.nguyen.xa@renesas.com>
Add support for PWM driver on MCK-RA8T1 Signed-off-by: Quy Tran <quy.tran.pz@renesas.com>
Add support for PWM driver on EK-RA8D1 Signed-off-by: Quy Tran <quy.tran.pz@renesas.com>
quytranpzz
force-pushed
the
support_renesas_ra8_pwm
branch
from
384adb3
to
428806f
Compare
This PR add PWM driver support for Renesas RA8 board, it's using GPT hardware