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

[Platform] Accton as7712 32x support oom #1327

Merged
merged 10 commits into from
Feb 13, 2018
Merged

[Platform] Accton as7712 32x support oom #1327

merged 10 commits into from
Feb 13, 2018

Conversation

roylee123
Copy link
Collaborator

- What I did
Replace OOM sfp driver to optoe OOM driver.

- How I did it

  1. Add optoe driver and instantiate 32 SFP ports by it.
  2. Add SFP sideband controls to cpld driver
  3. Modify sfputil.py to read the new node paths.

- How to verify it
sensors
decode-syseeprom
psuutil status
sfputil show presence
sfputil reset

- Description for the changelog
Replace sfp driver to optoe OOM driver.

@lguohan
Copy link
Collaborator

lguohan commented Jan 19, 2018

why not use the sonic sff_8436_eeprom driver instead of optoe OOM driver? Is it because of sfp support?

@roylee123
Copy link
Collaborator Author

Yes, it supports both QSFP and SFP.
Attribute port_name also provides an identity to each individual port.

@lguohan
Copy link
Collaborator

lguohan commented Jan 26, 2018

have you tried this patch for sfp?

sonic-net/sonic-linux-kernel#20

We are moving to 4.9. We have ported sff_8436 driver to 4.9 kernel. However, this oom driver does not 4.9 kernel version. How do you plan to support 4.9 then?

@lguohan lguohan mentioned this pull request Jan 26, 2018
@roylee123
Copy link
Collaborator Author

Hi,
I've tried pathed sff_8436_eeprom.c. It can't show ddm information, the SFF-8472 part.
Even this product has only qsfp ports, but others have 10/25G sfp ones.
For kernel 4.9, my colleague will request the author of optoe driver to support as well.

@lguohan
Copy link
Collaborator

lguohan commented Jan 29, 2018

I believe you need to set sfp_compatible to 1

echo 1 > (your eeprom directory)/sfp_compatible

@roylee123
Copy link
Collaborator Author

lguohan,
The author of optoe,c, Don Bollinger, https://github.com/donboll, has responded:

  1. Why use optoe.c instead of drivers/misc/eeprom/sff_8436_eeprom.c?
    sff_8436_eeprom.c can not support pages on SFP devices. Finisar has devices that access more than 4 pages. Also sff_8436_eeprom.c copies data from the eeprom to an internal buffer, then into the user buffer. This wastes memory and is inefficient.
  2. sff_8436_eeprom.c already support Linux 4.9, will optoe.c support it too?
    Yes, optoe.c will support Linux 4.9. I have successfully built it today.

If you have more questions on it, could I involve your mail into the loop?

@donboll
Copy link

donboll commented Jan 31, 2018

lguohan,
Hi, I'm Don Bollinger the author of optoe. I am working on behalf of Finisar (major opto vendor), as the architect and implementer of the OCP Networking Project 'OOM'. The optoe driver is intended to provide better access to SFP and QSFP (and upcoming QSFP-DD/OSFP) EEPROM data. It supports all MSA standard i2c devices under the SFF-8472 and SFF-8476 standards. Optoe is being incorporated by 3 switch vendors right now (two of them are trying to submit optoe to you right now), and has already been accepted into the ONL public mainline for 6 switch platforms from two vendors (including one you are not working with right now.) Bottom line: optoe is real, solid, supported, an emerging industry standard. I and Finisar are prepared to support it and extend it as needed to accommodate future devices and evolution in the Linux base.

I have examined the version of sff_8436 in the patch you referenced above (sonic-net/sonic-linux-kernel#20). I don't think this particular patch supports SFP at all. It doesn't have the 'sfp_compat' flag that other versions of that driver have, and it doesn't have any code to access the second i2c address (A2h or 51) that holds the majority of the SFP EEPROM data. (Search for 'i2c_new_dummy' in earlier versions, note it is missing in this version.) Finally, this version still has the reference to 'struct memory_accessor' which was removed at Linux 4.6. I don't think this version will compile under Linux 4.9.

I have the same code in optoe, since optoe is based on sff_8436. I have a version that removes all the code that depends on struct memory_accessor (search for macc in optoe or in sff_8436). I think it is safe to just remove that code, but that means kernel code can no longer access the EEPROM data (user space still can). Is that OK? Specifically, does Sonic use that macc/struct memory_accessor interface to access the sff_8436 driver? If not, I can pull that code out, and submit it to Sonic. You will then have a driver that supports at least two of your switch vendors, that works for SFP and QSFP, and that works under Linux 4.9. But I need to know: Does Sonic access the optical driver through the macc interface?

@lguohan
Copy link
Collaborator

lguohan commented Feb 4, 2018

@donboll, thanks for your replying. first a few clarification.

  1. this version should support sfp_compat and it also has the i2c_new_dummy.

  2. this version does not support v4.9, but we have ported this sff_8436 to v4.9 and it can found here. We also ported to use macc interface. for v4.9, we haven't ported the sfp version to v4.9, but it should be not difficult given we have already ported the sff_8436 to 4.9.

I found the sfp_compat useful for us since in some cases we use this qsfp2sfp adapter. I believe in this case, the driver needs to dynamically detect the transceiver type which is supported in 8436 driver.

also need to check if any switch verdor use macc interface to access eeprom in the kernel or not. if there is any need,

would you be able to give an introduction for optoe driver in the coming Tuesday meeting 7-8AM?

@donboll
Copy link

donboll commented Feb 5, 2018

Yes, I can give an introduction Tuesday (Feb 5, 2018) at 7-8AM (PST). Please send meeting info, I can't find the meeting schedule, phone number, virtual meeting code, etc.

@lguohan
Copy link
Collaborator

lguohan commented Feb 5, 2018

@donboll, thanks for the confirmation. I have sent the meeting invite to you.

@lguohan
Copy link
Collaborator

lguohan commented Feb 7, 2018

@roylee123 , based on yesterday discussion, we plan to wait donboll to submit optoe patch on sonic-linux-kernel and let you adapt use the optoe driver in the sonic-linux-kernel. let me know if you have any concern on this?

@roylee123
Copy link
Collaborator Author

Good news. I'm OK with that.

@lguohan
Copy link
Collaborator

lguohan commented Feb 12, 2018

@roylee123 , optoe driver has been merged into latest sonic kernel. can you update the PR to use that?

sonic-net/sonic-linux-kernel@3525f35

@lguohan
Copy link
Collaborator

lguohan commented Feb 12, 2018

I see another commit from #1380 which also update accton driver submodule. can you coordinate?

…efile.

Signed-off-by: roy_lee <roy_lee@accton.com>
@roylee123
Copy link
Collaborator Author

I've removed related optoe files and test sfputil for

  1. sfputil show presence
  2. sfputil show eeprom
  3. sfputil reset EthernetXX

@lguohan
Copy link
Collaborator

lguohan commented Feb 13, 2018

still see as7312-54x/modules/optoe.c

@lguohan
Copy link
Collaborator

lguohan commented Feb 13, 2018

you also need to update the kernel submodule to include the optoe module.

@lguohan
Copy link
Collaborator

lguohan commented Feb 13, 2018

commit message says "Replace sfp driver to optoe OOM driver.", but I still see sfp drivers in the platform driver directories, are they still used? as7312-54x/modules/accton_as7312_54x_sfp.c

@roylee123
Copy link
Collaborator Author

No, I'll remove it on PR #1339, New model as7312 54x .

@lguohan lguohan merged commit 02e0fad into sonic-net:master Feb 13, 2018
theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this pull request Feb 5, 2022
mssonicbld added a commit that referenced this pull request Nov 28, 2023
…tically (#17314)

#### Why I did it
src/sonic-sairedis
```
* 4ee9c25 - (HEAD -> master, origin/master, origin/HEAD) Add TestSwitch missing attribute (#1327) (12 hours ago) [noaOrMlnx]
* 4cbbeed - Add SAI Notification support for host_tx_ready (#1307) (18 hours ago) [noaOrMlnx]
```
#### How I did it
#### How to verify it
#### Description for the changelog
yxieca pushed a commit that referenced this pull request Dec 13, 2023
…tically (#17454)

src/sonic-sairedis

* 9621316 - (HEAD -> 202311, origin/202311) [syncd] Remove notify pointers manual handling (#1326) (2 weeks ago) [Kamil Cudnik]
* 4ee9c25 - Add TestSwitch missing attribute (#1327) (2 weeks ago) [noaOrMlnx]
* 4cbbeed - Add SAI Notification support for host_tx_ready (#1307) (2 weeks ago) [noaOrMlnx]
* 9804bd7 - Fix compilation issue due to PORT_STATE_CHANGE_QUEUE_SIZE undefined (#1324) (3 weeks ago) [Ashish Singh]
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.

3 participants