Skip to content

Janga: config: Adjusted tmp422 sensor position to support 2nd and main source machine #442

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

joywu-coder
Copy link
Contributor

Description

This PR is about tmp422 sensors in the Janga platform config file and sensor config file.

Motivation

  1. In the current platform config file, tmp422 can only run successfully on the main source machine. The problem was that I put tmp422 on SMB_FRU_SLOT, which can only be loaded successfully on the main source.To resolve this issue, tmp422 needs to be repositioned. In this PR, I moved the tmp422 related parts to JANGA_SLOT, so that it can run successfully on both 2nd source and main source machines.

  2. Note: This function has a limitation.The tmp422 sensors are only available in hardware from DVT2 and later. If you run the platform service and sensor service on a machine before this, an error will be reported.

Test Plan

  1. The correctness of the format has been verified on this jsonlint website.
  2. Used jq command to pretty the format.
  3. Test log as follows:

1. In main source machine platform run successfully:

image
2. In main source machine sensor_service_hw_test run successfully:

image
image
3. In 2nd source machine platform run successfully:

image
4. In 2nd source machine sensor_service_hw_test run successfully:

image
image

janga_main_source_platform_test_log_5_14.txt
janga_main_source_sensor_test_log_5_12.txt
janga_2nd_source_platform_test_log_5_12.txt
janga_2nd_source_sensor_test_log_5_12.txt

@facebook-github-bot
Copy link
Contributor

@mikechoifb has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@Scott8440
Copy link
Contributor

Thank you for the PR @joywu-coder. We are seeing merge errors on our side. Can you please rebase onto the latest main commit?

@facebook-github-bot
Copy link
Contributor

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

@joywu-coder joywu-coder force-pushed the adjust_janga_tmp422_in_sensor_config branch from a4dc071 to 0f02c51 Compare May 26, 2025 01:53
@facebook-github-bot
Copy link
Contributor

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

@joywu-coder joywu-coder reopened this May 26, 2025
@facebook-github-bot
Copy link
Contributor

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

@joywu-coder joywu-coder force-pushed the adjust_janga_tmp422_in_sensor_config branch from 3e0fa2c to da9b664 Compare June 13, 2025 02:44
@facebook-github-bot
Copy link
Contributor

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

@joywu-coder joywu-coder reopened this Jun 13, 2025
@facebook-github-bot
Copy link
Contributor

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

@joywu-coder joywu-coder reopened this Jun 13, 2025
@joywu-coder
Copy link
Contributor Author

Rebase the code and build.

@facebook-github-bot
Copy link
Contributor

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

@joywu-coder
Copy link
Contributor Author

@joywu-coder has updated the pull request. You must reimport the pull request before landing.

@mikechoifb, please reimport the pull request, thanks.

@somasun
Copy link
Contributor

somasun commented Jul 3, 2025

@joywu-coder - Regarding your comment "The problem was that I put tmp422 on SMB_FRU_SLOT, which can only be loaded successfully on the main source." Can you please explain why this is so with more details?

Copy link
Contributor

@somasun somasun left a comment

Choose a reason for hiding this comment

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

Please add more details in the PR Summary on why this sensor is being moved to a different PmUnit in the config. Is the sensor physically moved in the case of second source ?

@joywu-coder
Copy link
Contributor Author

Please add more details in the PR Summary on why this sensor is being moved to a different PmUnit in the config. Is the sensor physically moved in the case of second source ?
@somasun Thanks for your comments. Actually, the TMP422 sensor is a public device and does not have a second source. The incorrect configuration that placed the TMP422 sensor in the SMB_SLOT version caused it to fail to load. We now move the TMP422 config into the common JANGA_SLOT. @clslucas will send the email for the detailed clarification.

@clslucas
Copy link
Contributor

@somasun @mikechoifb Regarding this PR I wrote an email "[FBOSS] Minerva Janga PR#442 Confusion and TMP422 limitations" to clarify this case.
1.Maybe the PR title description confused you, the config file was updated because the TMP422 sensor is a common device, in the previous config file we assigned the tmp422 configuration to the main source through the 'pmUnitConfigs' field, but it is not a second source device, so through this PR we move the TMP422 configuration file to the generic JANGA_SLOT.
2.We know that the TMP422 device monitors the temperature of the J3 ASIC through the BJT for the OTP feature, but due to hardware limitation, the TMP422 can only work properly starting from the DVT2 stage, so the fboss platform manager will report a problem that the tmp422 cannot be found when running on previous DVT2 devices.

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.

5 participants