Skip to content

Conversation

ZhaoxiangJin
Copy link
Contributor

@ZhaoxiangJin ZhaoxiangJin commented Sep 8, 2025

  1. Enable nxp pmc temperature sensor (pmc-tmpsns) driver.
  2. enable pmc-tmpsns on RT700
  3. tested with samples/sensor/die_temp_polling
    test result:
*** Booting Zephyr OS build v4.2.0-3691-gd3f239dcd8d6 ***
CPU Die temperature[temp0]: 25.8 °C
CPU Die temperature[temp0]: 25.6 °C
CPU Die temperature[temp0]: 24.9 °C
CPU Die temperature[temp0]: 25.2 °C
CPU Die temperature[temp0]: 25.3 °C
CPU Die temperature[temp0]: 25.6 °C
CPU Die temperature[temp0]: 25.1 °C
CPU Die temperature[temp0]: 25.1 °C
CPU Die temperature[temp0]: 25.1 °C
CPU Die temperature[temp0]: 25.1 °C

@ZhaoxiangJin ZhaoxiangJin force-pushed the enable-temperature-sensor-on-rt700 branch from aa65fbd to aa11bf2 Compare September 8, 2025 03:32
Copy link

github-actions bot commented Sep 8, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@github-actions github-actions bot added manifest manifest-hal_nxp DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Sep 8, 2025
@ZhaoxiangJin ZhaoxiangJin force-pushed the enable-temperature-sensor-on-rt700 branch 3 times, most recently from 604ed81 to cb2e633 Compare September 8, 2025 04:40
@ZhaoxiangJin ZhaoxiangJin marked this pull request as ready for review September 8, 2025 05:32
@zephyrbot zephyrbot added area: Samples Samples platform: NXP NXP platform: NXP Drivers NXP Semiconductors, drivers area: Sensors Sensors labels Sep 8, 2025
@ZhaoxiangJin ZhaoxiangJin force-pushed the enable-temperature-sensor-on-rt700 branch from cb2e633 to 65b5238 Compare September 14, 2025 14:44
@ZhaoxiangJin
Copy link
Contributor Author

Hello @MaureenHelm, could you please revisit this pr? I think all your comments have been fixed. Thanks!

Comment on lines 1113 to 1136
mbox4: mailbox@189000 {
#mbox-cells = <1>;

compatible = "nxp,mbox-imx-mu";
reg = <0x189000 0x1000>;

interrupts = <31 0>;
rx-channels = <4>;

status = "disabled";
};

pmc_tmpsns: pmc-tmpsns {
compatible = "nxp,pmc-tmpsns";
status = "disabled";
};
};

&systick {
/*
* RT700 cm33_cpu0 relies by default on the OS Timer for system
* clock implementation, so the SysTick node is not to be enabled.
*/
status = "disabled";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some extra stuff is being added that looks unrelated to the sensor. Rebasing mistake?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for that, it would be a accident, i will update shortly

@ZhaoxiangJin ZhaoxiangJin force-pushed the enable-temperature-sensor-on-rt700 branch from 645d098 to a1a07f9 Compare September 29, 2025 22:17
@ZhaoxiangJin
Copy link
Contributor Author

This PR will get twister failures as know issue #96718

@ZhaoxiangJin ZhaoxiangJin force-pushed the enable-temperature-sensor-on-rt700 branch 2 times, most recently from 830dbdd to 1fa40eb Compare October 3, 2025 12:04
@ZhaoxiangJin
Copy link
Contributor Author

@jeppenodgaard, please review

Copy link

sonarqubecloud bot commented Oct 3, 2025

@jeppenodgaard
Copy link

@jeppenodgaard, please review

LGTM. Other requests needs to be addressed before I can approve.

Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok, but need to resolve the twister failures

@ZhaoxiangJin
Copy link
Contributor Author

Looks ok, but need to resolve the twister failures

I can reproduce this issue on zephyr latest main branch, hello @hakehuang, did you catch this issue? Thanks.

@hakehuang
Copy link
Contributor

I can reproduce this issue on zephyr latest main branch, hello @hakehuang, did you catch this issue? Thanks.

Thanks #97020 @ZhaoxiangJin

Pull in romapi for RT7xx

Signed-off-by: Zhaoxiang Jin <[email protected]>
This commit introduced NXP pmc temperature sensor
(pmc-tmpsns) driver.

Signed-off-by: Zhaoxiang Jin <[email protected]>
1. add pmc-tmpsns to rt7xx dts
2. add pmc-tmpsns to rt700 evk

Signed-off-by: Zhaoxiang Jin <[email protected]>
enable rt700 die_temp_polling sample

Signed-off-by: Zhaoxiang Jin <[email protected]>
@ZhaoxiangJin ZhaoxiangJin force-pushed the enable-temperature-sensor-on-rt700 branch from 1fa40eb to e57e5fd Compare October 8, 2025 04:23
@zephyrbot zephyrbot requested a review from MaureenHelm October 8, 2025 04:24
@ZhaoxiangJin
Copy link
Contributor Author

rebased to main, ci issue should be fixed in main.

@ZhaoxiangJin
Copy link
Contributor Author

ZhaoxiangJin commented Oct 8, 2025

Hello @hakehuang, I see #97020 is closed, I tried to rebase to the latest main, but the CI show new failures, could you please help check again? Thanks!

@hakehuang
Copy link
Contributor

hakehuang commented Oct 9, 2025

@ZhaoxiangJin all RT700 testing is blocked by #96718, I would suggest to block all RT700 PRs until this gets resolved.
also, CI issue is same root cause as:
#97243


cm_vref = cm_ctat + (953.36f + calibration) * cm_temp / 2048;

data->pmc_tmpsns_value = 370.98f * (cm_temp / cm_vref) - 273.15f;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to remove the float calculation above and change to int32 instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to know your starting point of the suggestion, actually this driver will only be used by RT500, RT600, and RT700, which all have FPU, so I think it is necessary to maintain accuracy as much as possible during the calculation process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants