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

Update the Wistron platform support in master branch #12110

Merged
merged 10 commits into from
Feb 23, 2023

Conversation

RogerX87
Copy link
Contributor

Signed-off-by: RogerX87 RogerX87@gmail.com

Why I did it

Update the latest code for better platform support for Wistron platform

How I did it

Modified platform related code in 202111 branch and make it works

How to verify it

Run the codes in Wistron platform

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205

Description for the changelog

Ensure to add label/tag for the feature raised. example - PR#2174 where, Generic Config and Update feature has been labelled as GCU.

Link to config_db schema for YANG module changes

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

dog

Signed-off-by: RogerX87 <RogerX87@gmail.com>
@lgtm-com
Copy link

lgtm-com bot commented Sep 19, 2022

This pull request introduces 15 alerts when merging 9d13a21 into de68f10 - view on LGTM.com

new alerts:

  • 9 for Unused local variable
  • 4 for Except block handles 'BaseException'
  • 1 for Unused import
  • 1 for Unreachable code

@lgtm-com
Copy link

lgtm-com bot commented Sep 21, 2022

This pull request introduces 1 alert when merging 7410e246ba8b3d51f91017e109e66fc5fbfa5832 into 63c14d2 - view on LGTM.com

new alerts:

  • 1 for Except block handles 'BaseException'

Signed-off-by: RogerX87 <RogerX87@gmail.com>
Signed-off-by: RogerX87 <RogerX87@gmail.com>
@RogerX87
Copy link
Contributor Author

@yxieca @sujinmkang . please help to review, thanks.

Signed-off-by: RogerX87 <RogerX87@gmail.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 13, 2022

This pull request introduces 11 alerts and fixes 2 when merging 80edd9f into 7087763 - view on LGTM.com

new alerts:

  • 8 for Unused local variable
  • 2 for Except block handles 'BaseException'
  • 1 for Unused import

fixed alerts:

  • 2 for Except block handles 'BaseException'

Signed-off-by: RogerX87 <RogerX87@gmail.com>
@lgtm-com
Copy link

lgtm-com bot commented Oct 13, 2022

This pull request fixes 2 alerts when merging 0949ebc into 7087763 - view on LGTM.com

fixed alerts:

  • 2 for Except block handles 'BaseException'

@RogerX87
Copy link
Contributor Author

@yxieca @sujinmkang . please help to review, thanks.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@lgtm-com
Copy link

lgtm-com bot commented Oct 27, 2022

This pull request fixes 2 alerts when merging 9db9193 into 558c904 - view on LGTM.com

fixed alerts:

  • 2 for Except block handles 'BaseException'

@lgtm-com
Copy link

lgtm-com bot commented Oct 27, 2022

This pull request fixes 2 alerts when merging fbd64db into 558c904 - view on LGTM.com

fixed alerts:

  • 2 for Except block handles 'BaseException'

@WistronNetwork
Copy link
Contributor

@yxieca @sujinmkang it's been hold for a long time, since our customer ask for this, would you please help review and merge it? thanks in advance.

Comment on lines +13 to +15
from sonic_platform_base.sonic_sfp.sffbase import sffbase
from sonic_platform_base.sonic_sfp.sff8024 import type_abbrv_name
from sonic_platform_base.sonic_sfp.sff8024 import type_of_media_interface
Copy link
Contributor

Choose a reason for hiding this comment

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

@RogerX87 please use sonic_xcvr instead of sonic_sfp. sonic_sfp is deprecated.
SFP-refactor HLD- #9142
Sample PR to migrate to SFP-refactor:- #9142

@maipbui
Copy link
Contributor

maipbui commented Nov 30, 2022

@RogerX87 please help add relevant code in this PR #12102

@@ -141,15 +151,28 @@ def get_reboot_cause(self):
return (reboot_cause, description)

def _get_sku_name(self):
p = subprocess.Popen(GET_HWSKU_CMD, shell=True, stdout=subprocess.PIPE)
p = subprocess.Popen(GET_HWSKU_CMD, stdout=subprocess.PIPE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maipbui , the code related with PR#12102 at here.

there are many modifications like this, I only pointed one here.

Copy link
Contributor

Choose a reason for hiding this comment

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

what about code under device/wistron directory? The changed file in PR12102 is device/wistron/x86_64-wistron_6512_32r-r0/sonic_platform/chassis.py but I don’t see it 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.

we moved the code from device folder to platform folder.

@WistronNetwork
Copy link
Contributor

@yxieca please help merge this, thanks in advance.

Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@RogerX87 please use sonic_xcvr instead of sonic_sfp. sonic_sfp is deprecated.
SFP-refactor HLD- #9142
Sample PR to migrate to SFP-refactor:- #9142

@WistronNetwork
Copy link
Contributor

WistronNetwork commented Jan 16, 2023

@prgeor Hi Prince, we had checked the HLD. But our HW design for both 6512_32r and sw_to3200k is to access transceiver eeprom through BMC, not from the i2c interface of CPU, it's quite difficult for us to suit new xcvr design. And the code already had full tested via testbed by customer request, we don't have time to re-factor it since tight schedule. Shall we have this merged first and then get back to open another PR for sfp re-factoring? thanks in advance.

@prgeor
Copy link
Contributor

prgeor commented Jan 16, 2023

@prgeor Hi Prince, we had checked the HLD. But our HW design for both 6512_32r and sw_to3200k is to access transceiver eeprom through BMC, not from the i2c interface of CPU, it's quite difficult for us to suit new xcvr design. And the code already had full tested via testbed by customer request, we don't have time to re-factor it since tight schedule. Shall we have this merged first and then get back to open another PR for sfp re-factoring? thanks in advance.

@RogerX87 https://github.com/sonic-net/sonic-platform-common/tree/master/sonic_platform_base/sonic_sfp is going to be purged from the repo by June 2023. Will your platform adapt by then?

@prgeor
Copy link
Contributor

prgeor commented Jan 16, 2023

@RogerX87 I don't understand why you think sonic_xcvr can support only I2C interface. This is incorrect. Platform can override read and write APIs. For example, This barefoot platform does RPC call to read the eeprom (it also caches the data for performance) https://github.com/sonic-net/sonic-buildimage/blob/master/platform/barefoot/sonic-platform-modules-bfn-montara/sonic_platform/sfp.py#L105

@WistronNetwork
Copy link
Contributor

@prgeor Hi Prince, we had checked the HLD. But our HW design for both 6512_32r and sw_to3200k is to access transceiver eeprom through BMC, not from the i2c interface of CPU, it's quite difficult for us to suit new xcvr design. And the code already had full tested via testbed by customer request, we don't have time to re-factor it since tight schedule. Shall we have this merged first and then get back to open another PR for sfp re-factoring? thanks in advance.

@RogerX87 https://github.com/sonic-net/sonic-platform-common/tree/master/sonic_platform_base/sonic_sfp is going to be purged from the repo by June 2023. Will your platform adapt by then?

Sure, we will raise another PR for sfp adaption before June 2023. In the mean time, shall we get this PR merged first?

@prgeor
Copy link
Contributor

prgeor commented Feb 3, 2023

@lguohan can you merge?

@lguohan lguohan merged commit 33db298 into sonic-net:master Feb 23, 2023
StormLiangMS pushed a commit to StormLiangMS/sonic-buildimage that referenced this pull request Mar 28, 2023
Related work items: sonic-net#276, sonic-net#305, sonic-net#332, sonic-net#338, sonic-net#339, sonic-net#1188, sonic-net#1192, sonic-net#1197, sonic-net#1206, sonic-net#1685, sonic-net#1690, sonic-net#1696, sonic-net#1699, sonic-net#1709, sonic-net#1727, sonic-net#1737, sonic-net#1741, sonic-net#1742, sonic-net#2511, sonic-net#2512, sonic-net#2532, sonic-net#2559, sonic-net#2626, sonic-net#2638, sonic-net#2645, sonic-net#2649, sonic-net#2660, sonic-net#2669, sonic-net#2670, sonic-net#2678, sonic-net#10084, sonic-net#11442, sonic-net#11873, sonic-net#12047, sonic-net#12110, sonic-net#12207, sonic-net#12529, sonic-net#12678, sonic-net#13235, sonic-net#13287, sonic-net#13372, sonic-net#13395, sonic-net#13456, sonic-net#13497, sonic-net#13522, sonic-net#13545, sonic-net#13547, sonic-net#13552, sonic-net#13569, sonic-net#13572, sonic-net#13578, sonic-net#13591, sonic-net#13611, sonic-net#13647, sonic-net#13649, sonic-net#13660, sonic-net#13710, sonic-net#13716, sonic-net#13724, sonic-net#13726, sonic-net#13732, sonic-net#13735, sonic-net#13739, sonic-net#13757, sonic-net#13786, sonic-net#13792, sonic-net#13800, sonic-net#13801, sonic-net#13802, sonic-net#13805, sonic-net#13806, sonic-net#13812, sonic-net#13814, sonic-net#13822, sonic-net#13831, sonic-net#13834, sonic-net#13847, sonic-net#13870, sonic-net#13882, sonic-net#13884, sonic-net#13885, sonic-net#13894, sonic-net#13895, sonic-net#13926, sonic-net#13932, sonic-net#13935, sonic-net#13942, sonic-net#13951, sonic-net#13953, sonic-net#13964
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.

None yet

6 participants