Skip to content
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

Dell:Initial commit for Z9664F platform #20768

Merged
merged 6 commits into from
Jan 30, 2025
Merged

Conversation

rkdevi27
Copy link
Contributor

@rkdevi27 rkdevi27 commented Nov 12, 2024

Why I did it

To add support for Z9664F platform

How I did it

Implemented the support for the platform Z9664F

Switch Vendor: Dell
Switch SKU: Z9664F
ASIC Vendor: Broadcom
SONiC Image: sonic-broadcom.bin

How to verify it

Verified the platform show commands and also executed the sonic-mgmt testcases.
logs.txt

Added PDDF changes as well and attaching the logs
The syncd is not up and will be raising it to broadcom for the same as it requires SAI support.
logs.zip

@rkdevi27 rkdevi27 requested a review from lguohan as a code owner November 12, 2024 11:07
Copy link

linux-foundation-easycla bot commented Nov 12, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

except ImportError as e:
raise ImportError(str(e) + "- required module not found")

FAN1_MAX_SPEED_OFFSET = 71

Choose a reason for hiding this comment

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

FAN1_MAX_SPEED_OFFSET = 71
FAN2_MAX_SPEED_OFFSET = 73
PSU_FAN_MAX_SPEED_OFFSET = 50 . Should these values be changed for this platform ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be same for the BMC platforms.

ver_info_fd.close()
updater.close()

ver_info = ver_info.get("x86_64-dellemc_z9432f_c3758-r0")

Choose a reason for hiding this comment

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

should this be 9664 or 9432 ?

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 have noticed it. Planning to fix this up in further commits. This will be 9664.

self._errno = 0

@staticmethod
def _get_command_result(cmdline):

Choose a reason for hiding this comment

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

does this require self as first argument ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not required

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jeff-yin jeff-yin requested review from jeff-yin and adyeung January 23, 2025 19:19
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adyeung
Copy link
Collaborator

adyeung commented Jan 24, 2025

@FuzailBrcm pls help review the pddf change

"num_temps":18,
"pddf_dev_types":
{
"description":"AS7726 - Below is the list of supported PDDF device types (chip names) for various components. If any component uses some other driver, we will create the client using 'echo <dev-address> <dev-type> > <path>/new_device' method",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wrong platform name here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it.

#############################################################################
# DellEMC Z9664f
#
# Platform and model specific eeprom subclass, inherits from the base class,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we have copyright headers in each of these files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not been added to any of the platform files. Hence followed same convention. Please let me know if that is required.

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@FuzailBrcm
Copy link
Contributor

@geans-pin @leeprecy
Please review this

Copy link
Contributor

Choose a reason for hiding this comment

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

Its always better to add a
platform/broadcom/sonic-platform-modules-dell/debian/platform-modules-z9664f.prerm
script. This will avoid some issues later. Also, helpful for testing platform .deb package installation & uninstall.

Its contents are primarily reverse steps of .postinst script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added platform-modules-z9664f.prerm

@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rkdevi27 rkdevi27 requested a review from jeff-yin January 28, 2025 05:09
@jeff-yin
Copy link
Collaborator

@qiluo-msft @rlhui @yxieca @prgeor -- this PR seems to have sufficient approvals. Can a repo maintainer please merge this PR? Thanks.

@qiluo-msft qiluo-msft merged commit 216a605 into sonic-net:master Jan 30, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants