-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Using SFP refactoring framework in PDDF sfp class #10047
Conversation
@prgeor |
@@ -1109,61 +264,11 @@ def tx_disable(self, tx_disable): | |||
""" | |||
# find out a generic implementation of tx_disable for SFP, QSFP and OSFP | |||
status = False | |||
if not self.get_presence(): | |||
return tx_disable | |||
|
|||
device = 'PORT{}'.format(self.port_index) | |||
path = self.pddf_obj.get_path(device, 'xcvr_txdisable') | |||
|
|||
# TODO: put the optic based reset logic using EEPROM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why override tx_disable() ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, if CPLD setting is present for tx_disable then that would be used. Else common SfpOptoeBase implementation is used. I think this common implementation does the same eeprom register set operations which was removed from here.
return [tx1_pw, tx2_pw, tx3_pw, tx4_pw] | ||
else: | ||
return None | ||
return lpmode | ||
|
||
def get_intr_status(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be removed if NOT used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some platforms do have CPLD reg setting to read the xcvr interrupt status, hence it was added.
else: | ||
return None | ||
# Use common SfpOptoeBase implementation for get_lpmode | ||
lpmode = super().get_lpmode() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will the 'else' part be executed for QSFP-DD optics?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, given that lpmode setting cant be done via CPLD for any given platform.
status = int(output['status'].rstrip()) | ||
|
||
if status == 1: | ||
tx_fault = True | ||
else: | ||
tx_fault = False | ||
else: | ||
# Use common SfpOptoeBase implementation for get_tx_dault |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will correct
rx_los = (sffbase().test_bit(data, 1) != 0) | ||
|
||
else: | ||
if output: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why override sfp_optoe_base class's get_rx_los ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some platforms, rxlos can be set via CPLD. In such cases, PDDF JSON file would have the details of CPLD address, offset etc. If it is found, then we use PDDF parsing to write to the CPLD reg. If no such CPLD setting is present, then we use sf_optoe_base class's implementation.
@@ -749,42 +136,19 @@ def get_tx_fault(self): | |||
Note : TX fault status is lached until a call to get_tx_fault or a reset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why override get_tx_fault() ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if CPLD setting for get_tx_fault() is present for any platform, that needs to be used. Else the common get_tx_fault() implementation of SfpOptoeBase
else: | ||
# SFP doesnt support this | ||
return 0 | ||
|
||
def get_lpmode(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why override sfpopote's get_lpmode. ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if CPLD setting for lpmode is present for any platform, that needs to be used. Else the common get_lpmode implementation of SfpOptoeBase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replied to the comments
Why I did it
All the SFP platform API classes need to use SFP refactoring framework. The platforms which use PDDF, derive their SFP API class from a common pddf_sfp.py. Hence pddf_sfp.py needs to comply with SFP refactoring.
How I did it
Used sfpOptoeBase class in PddfSfp
How to verify it
Verified with the following logs on AS7326 platform.
Which release branch to backport (provide reason below if selected)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)