pinctrl: gecko: add low power support #83398
Conversation
zephyrbot
requested review from
asmellby,
gmarull,
jerome-pouiller and
jhedberg
RomainPelletant
force-pushed
the
pinctrl/gecko/add-low-power-support
branch
3 times, most recently
from
cd46ed5 to
ec87230
Compare
- Add low power pin configuration support in gecko Signed-off-by: Romain Pelletant <[email protected]>
- Add low power pinctrl configuration support for gecko platform Signed-off-by: Romain Pelletant <[email protected]>
RomainPelletant
force-pushed
the
pinctrl/gecko/add-low-power-support
branch
from
ec87230 to
f6543e8
Compare
| /** Input. */ | ||
| #define GECKO_LP_DISABLE 0U | ||
| /** Output. */ | ||
| #define GECKO_LP_ENABLE 1U |
There was a problem hiding this comment.
I think it does not make sense to define symbols that are finally just another name for boolean.
There was a problem hiding this comment.
you are right: it is just more "futurproof" if more bits are required for low-power feature: I let you decide.
I followed the nordic implementation (pinctrl-nrf)
There was a problem hiding this comment.
YAGNI. So, I suggest to drop them.
| GPIO_PinModeSet(pin_config.port, pin_config.pin, pin_config.mode, | ||
| pin_config.out); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
I am a bit annoyed psels is ignored if low-power-enable is set.
There was a problem hiding this comment.
I agree: if psels is not ignored, we have to define a low power pin configuration for each psels features. Maybe another way could be to call his function after psels/fun actions
There was a problem hiding this comment.
I could suggest to encapsulate GPIO_PinModeSet with all flags passing and low power flags will proceed his own pin control settings with a switch case to be psels aware. Do you prefer that way?
There was a problem hiding this comment.
I have read a bit more about GPIO_PinModeSet(). I am now a bit confused. What do you expect when low-power-enable is set?
Fixes #83397