-
Notifications
You must be signed in to change notification settings - Fork 7.9k
drivers: misc: Add Low-voltage detection (LVD) driver support for RX130 #93694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
drivers: misc: Add Low-voltage detection (LVD) driver support for RX130 #93694
Conversation
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
6018f8c
to
c11e903
Compare
c11e903
to
d012894
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Update commit ID for hal_renesas to the latest Signed-off-by: Quy Tran <[email protected]>
6a86e32
d012894
to
6a86e32
Compare
6a86e32
to
9e50a16
Compare
Add header file to support Renesas LVD APIs on Zephyr Signed-off-by: Quy Tran <[email protected]>
Introduce Low/Programmable voltage detection driver for Renesas RX which using r_lvd_rx from RDP. The target is to monitor the voltage level input to Vcc/CMPA2 pin using a program Signed-off-by: Quy Tran <[email protected]>
Add LVD node support on RX130 Signed-off-by: Quy Tran <[email protected]>
Add a simple sample for LVD driver on RSK-RX130-512kb Signed-off-by: Quy Tran <[email protected]>
9e50a16
to
3528c9e
Compare
It looks to me as if it should be using the Comparator API instead of being a misc. driver but I will let @bjarki-andreasen chime in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 until the comparator discussion happens. I will also add comments re: a few things that need fixing in the docs and sample. Thanks!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hardware maps really well to the comparator subsystem, check out https://docs.zephyrproject.org/latest/hardware/peripherals/comparator.html , and see the comments I added to this PR. The comparator subsystem comes with documentation, a shell, and test suites, so you get a lot "for free" including portability using it instead of introducing a new API :)
There is a component in tree which contains multiple comparator's as well, check out https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/comparator/renesas%2Cra-acmphs-global.yaml and https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/comparator/renesas%2Cra-acmphs.yaml Comparators don't have "channels", a comparator is "a comparator" if there are two channels, there are two comparators :)
typedef enum { | ||
/** Voltage is above threshold. */ | ||
RENESAS_LVD_POSITION_ABOVE = 0, | ||
|
||
/** Voltage is below threshold. */ | ||
RENESAS_LVD_POSITION_BELOW, | ||
} renesas_lvd_position_t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This maps well to
zephyr/include/zephyr/drivers/comparator.h
Lines 60 to 74 in 90723c8
/** | |
* @brief Get comparator's output state | |
* | |
* @param dev Comparator device | |
* | |
* @retval 1 Output state is high | |
* @retval 0 Output state is low | |
* @retval -errno code Failure | |
*/ | |
__syscall int comparator_get_output(const struct device *dev); | |
static inline int z_impl_comparator_get_output(const struct device *dev) | |
{ | |
return DEVICE_API_GET(comparator, dev)->get_output(dev); | |
} |
typedef enum { | ||
/** No threshold crossing detected */ | ||
RENESAS_LVD_CROSS_NONE = 0, | ||
|
||
/** Voltage crossed over the threshold. */ | ||
RENESAS_LVD_CROSS_OVER, | ||
} renesas_lvd_cross_t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This maps to
zephyr/include/zephyr/drivers/comparator.h
Lines 27 to 36 in 90723c8
enum comparator_trigger { | |
/** No trigger */ | |
COMPARATOR_TRIGGER_NONE = 0, | |
/** Trigger on rising edge of comparator output */ | |
COMPARATOR_TRIGGER_RISING_EDGE, | |
/** Trigger on falling edge of comparator output */ | |
COMPARATOR_TRIGGER_FALLING_EDGE, | |
/** Trigger on both edges of comparator output */ | |
COMPARATOR_TRIGGER_BOTH_EDGES | |
}; |
COMPARATOR_TRIGGER_RISING_EDGE, |
int (*get_status)(const struct device *dev, renesas_lvd_status_t *status); | ||
int (*clear_status)(const struct device *dev); | ||
int (*callback_set)(const struct device *dev, renesas_lvd_callback_t callback, | ||
void *user_data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These map to
zephyr/include/zephyr/drivers/comparator.h
Lines 51 to 56 in 90723c8
__subsystem struct comparator_driver_api { | |
comparator_api_get_output get_output; | |
comparator_api_set_trigger set_trigger; | |
comparator_api_set_trigger_callback set_trigger_callback; | |
comparator_api_trigger_is_pending trigger_is_pending; | |
}; |
get_status
-> get_output
, callback_set
-> set_trigger_callback
, clear_status
-> trigger_is_pending
. The comparator API additionally has the set_trigger
API which allows the user to enable/disable the edge detection (interrupt), and details how to implement power management as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bjarki-andreasen , Thank you. I initially considered using the comparator driver, but it doesn't fully meet the functionality. The LVD (low-voltage-detection) only monitors the VCC supply (some MCUs have an additional CMPA pin) and triggers when the voltage crosses fixed thresolds, typically generating a reset, an interrupt, or doing nothing. We also have support for a comparator on this MCU, but it uses a different hardware IP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A device does not need to meet the full functionality of a device driver API, most devices in fact don't :) Drivers return -ENOTSUP
if the application tries to make a specific device do something which it does not support, so unless the LVD has features which are not covered by the comparator API, which it seems it does not, its perfectly fit to use the comparator API to implement the features it does support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the LVD usage does not match the comparator description and may confuse users.
comparator: "Compares the voltages of two analog signals connected to its negative and positive inputs"
LVD just "compares VCC supply with voltage threshold"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the voltage threshold is the negative input, and VCC the positive input, in which case if VCC is above the threshold, the output will be 1 (high) :) Just because the inputs can't be freely selected does not make the hardware "not a comparator" does it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing my request for change as this will likely evolve to something else. Happy to re-review in the future if there are still significant documentation or sample bits in it.
Introduce low-voltage detection driver support on RX130
The voltage detection circuit (LVD) monitors the voltage level input to the VCC or external pin using a program.
Upon detecting a voltage level transition (either upward or downward) past the configured threshold, the system can generate a reset or an interrupt, enabling appropriate protective or recovery actions.