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

[device] Adding platform support for Accton as5912-54x #16297

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fischerdouglas
Copy link

Why I did it

Add device and platform files for Accton as5912-54 - AGR100.

Work item tracking

No tracking yet.

How I did it

Add as SONiC Porting Guide.

How to verify it

I sincerely don't know!
Still a work in progress.

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

Until now I'm guessing no backport will be needed.

Tested branch (Please provide the tested image version)

No tests yet.

Description for the changelog

Add device and platform files for Accton as5912-54 - AGR100.

Link to config_db schema for YANG module changes

No module changes were made.

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 27, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: fischerdouglas / name: Douglas Fischer (bc111b7)

@fischerdouglas
Copy link
Author

I'm not a programmer, and I don't really know what I'm doing.

But I used your PR for AS7315 (which is also Qumran), and decided to give it a try.

I have no idea how lanes are defined for interfaces in port_config.ini. I basically copied it from another model and it seemed to make sense.
I also added a 6th fan in sensors.conf, but I don't know if that makes sense.

@roylee123 could you take a look and see if what's been done here is a far cry from what really needs to be done?

@akenliu
Copy link

akenliu commented Aug 28, 2023

AS7315 and AS5912 are two different devices, the hardware layout are also difference.
You should not copy the files from AS7315 and rename all as7315 to as5912, it is wrong.
I think you also did not test your codes on real AS5912.
If you buy the device, AS5912-54x, from Edgecore, I think you can contact with Edgecore support website.

@fischerdouglas
Copy link
Author

fischerdouglas commented Aug 28, 2023

Hello @akenliu

it is wrong.

I Actually know that it is "wrong"... But I needed to start somewhere.
Sorry if I exceeded the limit requesting PR.

I think you also did not test your codes on real AS5912.

Yep, I did not test it on any real device. Exactly because I knew that something would be wrong.
But we have 18 of those and we want to migrate to SONiC.

I need to reinforce the "sorry" part.
I was just following the docs on Porting-Guide.

I think you can contact with Edgecore support website.

Well... I made some contacts in the past.
And at that moment they basically limited themselves to answering that such equipment is not supported in SONiC.
Maybe now the conversation with them can be more productive.

P.S.:
I felt that the orientations on Porting-Guide could be improved.
Maybe some fluorograms and more detailed instructions on the modules part.
Fans, BCM, and especially the Lanes mapping to the interfaces.
I'm just starting... But I will make an effort on that.

@akenliu
Copy link

akenliu commented Aug 29, 2023

I Actually know that it is "wrong"... But I needed to start somewhere. Sorry if I exceeded the limit requesting PR.

I think you also did not test your codes on real AS5912.

Yep, I did not test it on any real device. Exactly because I knew that something would be wrong. But we have 18 of those and we want to migrate to SONiC.

I need to reinforce the "sorry" part. I was just following the docs on Porting-Guide.

Porting-Guide is a general guideline, it would not describe how to control the hardware components in the switch device.
I think you understand the lane mapping should be modified, so you ask the questions.
However, the other hardware components are also all different, such as FAN, LED, PSU, thermal.
Therefore, the next step is that you need to understand the hardware layout for each component.
I mean we does not just modify the name/string from AS7315 to AS5912, we need to modify driver codes for real hardware design regarding the FAN, PSU, LED, thermal sensor, transceiver, EERPOM....

@yxieca yxieca requested a review from prgeor September 5, 2023 01:55
@lguohan lguohan added the device label Sep 23, 2023
@Lewis-Kang
Copy link

@fischerdouglas Could you please confirm if you will be verifying your PR modifications for as5912-54x?

@fischerdouglas
Copy link
Author

Hello @Lewis-Kang!

I will not have the opportunity to deal with that in the next few months.
You are probably asking that to remove the tests, right?
Sorry for that!

What are the instructions to remove that from the pipeline tests?

@Lewis-Kang
Copy link

Hi @fischerdouglas
I think it's improper to submit a PR without verifying the code's functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants