-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[Micas/Platform]platform support M2-W6510 #15888
Conversation
Signed-off-by: philo <philo@micasnetworks.com>
Signed-off-by: philo <philo@micasnetworks.com>
/azp run Azure.sonic-buildimage |
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: philo <philo@micasnetworks.com>
Signed-off-by: philo <philo@micasnetworks.com>
/easycla |
@lguohan Hi guohan, help assign a code review to forward this PR, thanks! |
@prgeor Help review the code, thanks! |
Signed-off-by: philo <philo@micasnetworks.com>
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.
@philo-micas why not use existing optoe driver in sonic-linux-kernel repo?
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.
@prgeor Wb_ OPTPE. c is revised based on the kernel's OPTOE. c to fix the issue of crashes caused by switching between different OPTPE types : sonic-net/sonic-linux-kernel#317
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.
@philo-micas please raise a fix in the sonic-linux-kernel repo so that it benefits the community
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.
@prgeor OK, will raise in month, if no other review, help forward this pr, thanks!
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.
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.
@prgeor Some SSD model cannot be read according to the generic, so we have added our own implementation under the SSD specification plugin
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.
@philo-micas please raise a community fix in https://github.com/sonic-net/sonic-platform-common/blob/master/sonic_platform_base/sonic_ssd/ssd_generic.py
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.
@prgeor OK, will raise in month, if no other review, help forward this pr, thanks!
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.
@philo-micas pretty much duplicate of https://github.com/torvalds/linux/blob/master/drivers/i2c/muxes/i2c-mux-pca954x.c. So, why not use the same from sonic-linux-kernel repo?
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.
@prgeor wb_i2c_mux_pca954x.c is revised based on the i2c-mux-pca954x.c, adding 9548 reset function to improve I2C reliability
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.
@philo-micas please upstream your fix to linux kernel community.
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.
@prgeor OK, will raise in month, if no other review, help forward this pr, thanks!
@prgeor thanks for reviewing, and have replied some comments, and the reason for keeping other kernel modules show as below:
|
@lguohan for additional review |
for all duplicated drivers in the kernel, please create patch in sonic-linux-kernel, meanwhile do the upstream efforts. we can see whether some of the patch make sense or not. for example, the change to "adding AT24_FLAG_IRUGO flag for EEPROM's flag to enable none root users' read access" does not make sense to me. |
i also see binary files, we do not accept platform binary files |
Signed-off-by: philo <philo@micasnetworks.com>
Signed-off-by: philo <philo@micasnetworks.com>
Signed-off-by: philo <philo@micasnetworks.com>
Signed-off-by: philo <philo@micasnetworks.com>
@lguohan Hi Guohan, the kernel patch is only used by us for now and under Micas' platform folders. We will put forward the problems we encountered and some of our suggestions in sonic-linux-kernel community, and cooperate the community to push the solution of the problem. Also, we follow the community rules and have deleted all binary files except custom_led.bin because the solution provided by BRCM requires users to write custom_led.c and compile it to generate the custom_led.bin file, same file exists on other vendors' platforms. |
@lguohan Have done kernel 6.1 adaptation, as we have reviewed and fixed all reviewed comments month ago, (leave one semgrep error as 'defusedxml' not installed in sonic; test failed due to the test bed connect fail, compiling is fine), help push this pr forward again, thanks. |
report_6510_0110.zip |
@lguohan Hi guohan, test report has been submitted, please help to push forward this pr. Thanks. |
Signed-off-by: philo <philo@micasnetworks.com>
…c-buildimage into master-M2-W6510
@lguohan Hi Guohan, please take the time to push forward this PR, it is urgent for us, many thanks! |
Signed-off-by: philo <philo@micasnetworks.com>
Leave one semgrep error as 'defusedxml' was not installed in sonic. |
Is this issue addressed? @philo-micas |
@zhangyanzhao Yes, we have addressed and fixed it in this PR and and other PRs. |
@@ -0,0 +1,265 @@ | |||
// SPDX-License-Identifier: GPL-2.0-or-later |
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.
master image is already kernel 6.1 iirc, why still try to do linux 5.10?
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.
OK, fixed it.
AT24_CHIP_DATA(at24_data_24c01, 1024 / 8, 0); | ||
AT24_CHIP_DATA(at24_data_24cs01, 16, | ||
AT24_FLAG_SERIAL | AT24_FLAG_READONLY); | ||
AT24_CHIP_DATA(at24_data_24c02, 2048 / 8, AT24_FLAG_IRUGO); |
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.
@philo-micas can we use udev rule instead? Ref:-https://martin-jones.com/2016/08/12/making-beaglebone-black-serial-number-available-to-user-applications/
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.
DRIVER=="bone-capemgr", RUN+="/bin/chown root:i2c /sys$env{DEVPATH}/baseboard/serial-number"
SUBSYSTEM=="i2c", DEVPATH=="*0-0050", RUN+="/bin/chown root:i2c /sys$env{DEVPATH}/eeprom", RUN+="/bin/chmod 0640 /sys$env{DEVPATH}/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.
OK, done
Signed-off-by: philo <philo@micasnetworks.com>
…modules/driver/Makefile Signed-off-by: philo <philo@micasnetworks.com>
Signed-off-by: philo <philo@micasnetworks.com>
Signed-off-by: philo <philo@micasnetworks.com>
Signed-off-by: philo <philo@micasnetworks.com>
@lguohan Hi guohan, help push forward this, most comments have been addressed, and help rerun the test case for one of the failing test case, thanks. |
too many duplicated linux kernel modules in this PR. All drivers under this folder (platform/broadcom/sonic-platform-modules-micas/common/modules/linux/) are duplicated from linux kernel. This is cause lots of maintenance issues. |
Why I did it Add new platform m2-w6510(Trident 3) ASIC Vendor: Broadcom Switch ASIC: Trident 3 Port Config: 48x25G+8x100G How I did it Provide device and platform related files. How to verify it show platform fan show platform ssdhealth show platform psustatus show platform summary show platform syseeprom show platform temperature show interface status
Why I did it
Add new platform m2-w6510(Trident 3)
ASIC Vendor: Broadcom
Switch ASIC: Trident 3
Port Config: 48x25G+8x100G
How I did it
Provide device and platform related files.
How to verify it
show platform fan
show platform ssdhealth
show platform psustatus
show platform summary
show platform syseeprom
show platform temperature
show interface status
Work item tracking
How I did it
How to verify it
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)