Skip to content

[Platform] Accton add to support as9726-32d platform.#7479

Merged
jleveque merged 6 commits intosonic-net:masterfrom
ec-michael-shih:20210423_as9726_32d_add_platform
May 12, 2021
Merged

[Platform] Accton add to support as9726-32d platform.#7479
jleveque merged 6 commits intosonic-net:masterfrom
ec-michael-shih:20210423_as9726_32d_add_platform

Conversation

@ec-michael-shih
Copy link
Copy Markdown
Contributor

Why I did it

My company(Accton) have a new model names: as9726-32d.

How I did it

This pull request is based on as9716-32d, so I reference as9716-32d to create new model: as9726-32d.
This module do not need led driver to control led, FPGA can handle it.
I also implement API2.0(sonic_platform) for this model, CPLD driver, PSU driver, Fan driver to control these HW behavior.

How to verify it

=====================
QSFP and SFP can detect module present, log show as follow:

...(skip)....
Ethernet28 Not present
Ethernet29 Not present
Ethernet30 Not present
Ethernet31 Present
Ethernet32 Not present
Ethernet33 Not present
Ethernet34 Present

PSU can detect the status:

root@sonic:~# psuutil status
PSU Model Serial Voltage (V) Current (A) Power (W) Status LED


PSU-1 N/A N/A 12.08 12.81 154.25 NOT OK False
PSU-2 N/A N/A 0.00 0.00 0.00 OK False

=====================
Can get the info from eeprom:

root@sonic:~# show platform syseeprom
TlvInfo Header:
Id String: TlvInfo
Version: 1
Total Length: 175
TLV Name Code Len Value


Product Name 0x21 16 9726-32DB-O-AC-F
Part Number 0x22 13 FP5ZZ8632023A
Serial Number 0x23 15 972632DB2113016
Base MAC Address 0x24 6 90:3C:B3:F9:B0:A5
Manufacture Date 0x25 19 03/31/2021 11:02:41
Label Revision 0x27 4 R0BA
Platform Name 0x28 28 x86_64-accton_as9726_32db-r0
ONIE Version 0x29 13 2020.08.00.06
MAC Addresses 0x2A 2 256
Manufacturer 0x2B 6 ACCTON
Manufacture Country 0x2C 2 TW
Vendor Name 0x2D 8 Edgecore
Diag Version 0x2E 11 0a.0a.00.04
CRC-32 0xFE 4 0xBEA8AD7B
(checksum valid)

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

Branch of 202012 run plugins, so plugins need to merge into this branch, and master don't need (plugins).

A picture of a cute animal (not mandatory but encouraged)

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 29, 2021

This pull request introduces 28 alerts when merging 495c994 into e3b2a04 - view on LGTM.com

new alerts:

  • 9 for Unused import
  • 6 for Use of 'global' at module level
  • 5 for Unused local variable
  • 4 for Variable defined multiple times
  • 3 for Except block handles 'BaseException'
  • 1 for Unreachable code

@ec-michael-shih
Copy link
Copy Markdown
Contributor Author

I will fix the python alerts.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Apr 30, 2021

This pull request introduces 2 alerts when merging 2fce598 into 3967c28 - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'
  • 1 for Syntax error

@ec-michael-shih ec-michael-shih changed the title 20210423 as9726 32d add platform [Platform] Accton add to support as9726-32d platform. May 5, 2021
Copy link
Copy Markdown
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

Curious why you have built your own custom fan/thermal monitoring applications instead of utilizing standard SONiC applications (e.g., thermalctld)?

@ec-michael-shih
Copy link
Copy Markdown
Contributor Author

Curious why you have built your own custom fan/thermal monitoring applications instead of utilizing standard SONiC applications (e.g., thermalctld)?

Yes, I think you are right. This design I was follow the engineer who implemented before.
Therefore, I use this architecture to go on. Can I use this design?! Or suggest to modify for utilizing standard SONiC applications?!

@jleveque
Copy link
Copy Markdown
Contributor

Curious why you have built your own custom fan/thermal monitoring applications instead of utilizing standard SONiC applications (e.g., thermalctld)?

Yes, I think you are right. This design I was follow the engineer who implemented before.
Therefore, I use this architecture to go on. Can I use this design?! Or suggest to modify for utilizing standard SONiC applications?!

We would like all vendors to use the common, standard SONiC applications to allow for consistent cross-platform SONiC behavior. It is likely that the engineer before you created these solutions before the standard SONiC application(s) existed. However, I understand that refactoring your code to support the SONiC applications will take some time. How urgent is this PR?

@ec-michael-shih
Copy link
Copy Markdown
Contributor Author

Curious why you have built your own custom fan/thermal monitoring applications instead of utilizing standard SONiC applications (e.g., thermalctld)?

Yes, I think you are right. This design I was follow the engineer who implemented before.
Therefore, I use this architecture to go on. Can I use this design?! Or suggest to modify for utilizing standard SONiC applications?!

We would like all vendors to use the common, standard SONiC applications to allow for consistent cross-platform SONiC behavior. It is likely that the engineer before you created these solutions before the standard SONiC application(s) existed. However, I understand that refactoring your code to support the SONiC applications will take some time. How urgent is this PR?

Thank you for your reply.
I have a few questions about this standard SONiC application(ex: thermalctld):

  1. Can be customized for each module? Like a concept of configuration in each platform/model?

  2. Our each DUT in Accton, HW will test each DUT(model), and formulte the thermal plan,
    some is simple, and some is complex.
    In the model(as9726-32d), when temperature getting high detect from several lm75,
    there have two case: Go to mid speed, or high speed. In high speed, there have include yellow-alert, read-alert, shutdown detect
    temp from several lm75 and core-sensor. I think this is a complex rule in this model.
    As discuss with my workmate who studied this before, he cannot use stardard application to match his model.

I would like to try to use standard application to implement our thermal-plan that it can match our thermal plan.
Unn...do you have any other suggestions to help make this PR more better?! ^^

How urgent is this PR? ==> Yes, it is.

@jleveque
Copy link
Copy Markdown
Contributor

You can view the high-level design here: https://github.com/Azure/SONiC/blob/master/thermal-control-design.md

You can also examine other vendors' implementations for examples.

@ec-michael-shih
Copy link
Copy Markdown
Contributor Author

You can view the high-level design here: https://github.com/Azure/SONiC/blob/master/thermal-control-design.md

You can also examine other vendors' implementations for examples.

Thank you,I'll survey these informations.
By the way, can this PR be merged in this time?!
And when I find the way to modify by using standard api, and I will update it later?!

@jleveque
Copy link
Copy Markdown
Contributor

Thank you,I'll survey these informations.
By the way, can this PR be merged in this time?!
And when I find the way to modify by using standard api, and I will update it later?!

Sure, I will allow this to merge as-is while you research how to migrate to using our SONiC daemons.

@jleveque jleveque merged commit a070f1a into sonic-net:master May 12, 2021
@ec-michael-shih
Copy link
Copy Markdown
Contributor Author

Thank you,I'll survey these informations.
By the way, can this PR be merged in this time?!
And when I find the way to modify by using standard api, and I will update it later?!

Sure, I will allow this to merge as-is while you research how to migrate to using our SONiC daemons.

Thank you!!

qiluo-msft pushed a commit that referenced this pull request May 24, 2021
Add support for Accton as9726-32d platform

This pull request is based on as9716-32d, so I reference as9716-32d to create new model: as9726-32d.
This module do not need led driver to control led, FPGA can handle it.
I also implement API2.0(sonic_platform) for this model, CPLD driver, PSU driver, Fan driver to control these HW behavior.
@vboykox
Copy link
Copy Markdown
Member

vboykox commented May 26, 2021

@jleveque @ec-michael-shih
According to https://github.com/Azure/SONiC/wiki/Porting-Guide sonic_platform sources should be placed under platform/ directory.
Wondering if there is any reason this implementation was not placed there?

@jleveque
Copy link
Copy Markdown
Contributor

@vboykox: Good catch, I missed this. In this PR, the sonic_platform sources are under the device/ path and will thus be copied onto the device. This is not correct. They should be placed under the platform/ directory and built into a package at image build time.

@ec-michael-shih: Can you please fix this?

@vboykox
Copy link
Copy Markdown
Member

vboykox commented May 26, 2021

@jleveque
https://github.com/Azure/sonic-buildimage/tree/master/device/celestica/x86_64-cel_seastone-r0/sonic_platform
interesting that celestica also has sonic_platform under device

@jleveque
Copy link
Copy Markdown
Contributor

@jleveque
https://github.com/Azure/sonic-buildimage/tree/master/device/celestica/x86_64-cel_seastone-r0/sonic_platform
interesting that celestica also has sonic_platform under device

@mudsut4ke: Can you please explain why Celestica has placed the sonic_platform directory under the device/ directory for your platforms?

device/celestica/x86_64-cel_silverstone-r0/sonic_platform
device/celestica/x86_64-cel_seastone-r0/sonic_platform
device/celestica/x86_64-cel_e1031-r0/sonic_platform

Can you please move these directories under the platform/ directory instead? Otherwise these files are unnecessarily getting copied to the device, because the sonic-platform package is also getting installed.

@ec-michael-shih
Copy link
Copy Markdown
Contributor Author

@vboykox: Good catch, I missed this. In this PR, the sonic_platform sources are under the device/ path and will thus be copied onto the device. This is not correct. They should be placed under the platform/ directory and built into a package at image build time.

@ec-michael-shih: Can you please fix this?

No problem, I will fix it.
Thank you for your review and suggestion.

Can I ask a question about sonic_platfrom/sfp.py on 202012 branch?
The behavior about sfputil command, ex: [sfputil show presence]
I test it, and it call plugin/sfputil.py ==> so, can I say it did not support sonic_platform/sfy.py yet?!

@jleveque
Copy link
Copy Markdown
Contributor

Can I ask a question about sonic_platfrom/sfp.py on 202012 branch?
The behavior about sfputil command, ex: [sfputil show presence]
I test it, and it call plugin/sfputil.py ==> so, can I say it did not support sonic_platform/sfy.py yet?!

That's correct. The 202012 branch still has some dependencies on the old platform plugins. We are eliminating these dependencies in the master branch such that only the new sonic_platform package will be used and the old plugins will be obsolete.

carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
Add support for Accton as9726-32d platform

This pull request is based on as9716-32d, so I reference as9716-32d to create new model: as9726-32d.
This module do not need led driver to control led, FPGA can handle it.
I also implement API2.0(sonic_platform) for this model, CPLD driver, PSU driver, Fan driver to control these HW behavior.
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.

4 participants