-
Notifications
You must be signed in to change notification settings - Fork 8.1k
low cost power shield solution #95056
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?
Conversation
log for mimxrt595_evk board
|
59e3ca2
to
8f0f3f4
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.
Please take a look at #94585, and only keep 1 platform per vendor.
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.
thanks @JarmouniA PR #94585 looks good, I will update according after your PR merged
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.
There is no need to wait for it to be merged, just apply the same filtering and only keep overlay/conf of 1 platform per vendor in the new sample.
|
||
config SEQUENCE_32BITS_REGISTERS | ||
bool "ADC data sequences are on 32bits" | ||
default n |
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.
default n |
samples/drivers/adc/adc_power_shield/boards/arduino_due.overlay
Outdated
Show resolved
Hide resolved
pinctrl-0 = <&adc0_default>; | ||
pinctrl-names = "default"; |
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.
move above child node
samples/drivers/adc/adc_power_shield/boards/s32z2xxdc2_s32z270_rtu0.overlay
Outdated
Show resolved
Hide resolved
c19dcd6
to
78d25a9
Compare
@bjarki-andreasen if you look at my PR, I add a 'dft' which reuse the whole pytest serial handler see
the channels_p and channel_n to the power rail "VDD_CORE" / "VDD_IO" is by your board setting definition, this could be different pre your test harness setting, and this has to be customized, here is use a yaml to control it.
so nowhat where to add, we either add shell command or use a dedicated power shield sample application, we have to parse the shell/command output to get the board settings on its probe capacities, see below.
and we need map the probe capacities to the target measure power rails as I mentioned above. My understanding is that you propose to use a series of shell command to replace the power measure sample which will be running on power shield. this will add adc shell linked with power calculations I am not sure this will accepted by ADC shell owner? As in this PR, we have an independent sample to host everything, move those to shell command needs approval, can you help to confirm on my understanding and if accepted by ADC shell group I can move the implement to shell. |
No changes to the adc shell is needed. We would just create a set of custom shell commands as part of the power shield sample. The issue is not with having a sample at all, we need that, but with how the serial communication is implemented. We can freely create sample specific shell commands, so why not use that? |
ok, I see, I will update this PR to add a sample specific shell command. but I have some concerns on the code size, I enable the stm32f030dlk as the low cost shield, which code size is very critical. Let me try and feedback. |
For this, enable SHELL_MINIMAL, the entire shell should "only" take up 6-8K ROM, if that is too much we may have to go with a custom solution |
815f2b6
to
5d04eb9
Compare
subsys/pm/Kconfig
Outdated
config ENABLE_PM_MEASURE | ||
bool "System Power Measure" | ||
depends on SYS_CLOCK_EXISTS && HAS_PM | ||
help | ||
This option enables measurement on target power state | ||
|
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 options do not belong with the subsystem, add them to the tests and/or samples where they are used
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 , I move those options to testsuite config. is it OK?
c1127b5
to
fba939c
Compare
add power measure sample for general_adc_platform power shiled, to record power/voltage/current Signed-off-by: Hake Huang <[email protected]>
1. general power shield work with the adc_power_shield sample 2. config samples to config probe 3. add readme 4. enable parse shell command outputs, see readme for details Signed-off-by: Hake Huang <[email protected]>
enable power shield testing on mimxrt595_evk ``` scripts/twister --device-testing --device-serial /dev/ttyACM0 -T samples/boards/nxp/mimxrt595_evk/system_off -X pm_probe:/dev/ttyACM1,115200 ``` an power_shield folder will be created in the build path whith measured voltage/current/power Signed-off-by: Hake Huang <[email protected]>
enable power measure on power_mgmt_soc for mimxrt1060_evk@C Signed-off-by: Hake Huang <[email protected]>
fba939c
to
e9a5f47
Compare
add power measure options to test Signed-off-by: Hake Huang <[email protected]>
demo the power measure with frdm_rw612 boards keep the system in final power state for measurement Signed-off-by: Hake Huang <[email protected]>
e9a5f47
to
df5582f
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.
I'm surprised how much python code is needed to make this work
@bjarki-andreasen, I want to make the adc_power_measure case simple, and move the complex things to python for easy change. just realize that you have approved, and sorry for re-request review. My hand acts without thinking. |
CONFIG_GEN_SW_ISR_TABLE=n is not a commonly supported option, it requires CONFIG_MULTITHREADING=n and/or rewrites of IRQ handling in drivers, and is only applicable for some archs.
No problem |
@kartben I update the sample application readme and reduce them to platforms that I tested. Please take a look. Thanks, |
@bjarki-andreasen , I see. thanks a lot for this comments, I will drop the trier with CONFIG_GEN_SW_ISR_TABLE=n. Please help to approve the PR again. Thanks a lot. |
Uh oh!
There was an error while loading. Please reload this page.