-
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
LED Support for DellEMC Z9100 #2787
Conversation
platform/broadcom/sonic-platform-modules-dell/z9100/scripts/z9100_platform.sh
Outdated
Show resolved
Hide resolved
#Copy led_proc_init.soc | ||
switch_port_led | ||
if [ -e $led_proc_init ] && [ ! -e $device/$platform/led_proc_init.soc ]; then | ||
cp $led_proc_init $device/$platform |
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.
I'm not a fan of copying the file from the HwSKU directory into to the platform directory. If the LED functionality needs to differ based on SKU, then we may want to consider moving all led_proc_init.soc files into the respective HwSKU directories. For SKUs which share a common led_proc_init.soc file, we can symlink them.
I'm not sure that this should hold up this PR, but it's something I wanted to at least note here for posterity.
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.
Joe, thanks for the view. I changed copy to symlink in new commit. Right now as per the design ledinit "platform/broadcom/docker-syncd-brcm/supervisord.conf" picks led_proc_init.soc in platform specific location than HWSKU as a preference.
command=/usr/bin/bcmcmd -t 60 "rcload /usr/share/sonic/platform/led_proc_init.soc"
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, I understand that the current implementation looks for the led_proc_init.soc file in the platform directory. What I was noting for the future is that maybe we should consider moving the files into HwSKU directories and symlinking them if they share the same file.
I wasn't asking you to replace your copy operation with symlinking, but I'm OK with the change. My comment was about refactoring the mechanism completely in the future.
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.
Thanks Joe. Got it! But, when I introduce a symlink in syncd docker it points to a old hard link which is not valid in docker evnvironment.
[/usr/sonic/platfrorm] - do have only two directories hwsku and platform.
----------syncd docker-----------------------------------------------------
drwxr-xr-x 2 root root 4096 Apr 15 14:16 plugins
lrwxrwxrwx 1 root root 88 Apr 16 11:54 led_proc_init.soc -> /usr/share/sonic/device/x86_64-dell_z9100_c2538-r0/Force10-Z9100-C8D48/led_proc_init.soc
drwxr-xr-x 8 root root 4096 Apr 16 11:54 .
drwxr-xr-x 4 root root 4096 Apr 17 00:19 ..
drwxr-xr-x 2 root root 4096 Apr 17 02:11 Force10-Z9100-C32
drwxr-xr-x 2 root root 4096 Apr 17 15:18 Force10-Z9100-C8D48
root@sonic-z9100-02:/usr/share/sonic/platform#
root@sonic-z9100-02:/usr/share/sonic/platform#
----------------------------------------------------------------------------
root@sonic-z9100-02:/usr/share/sonic# ls -lar
drwxr-xr-x 8 root root 4096 Apr 16 11:54 platform
drwxr-xr-x 2 root root 4096 Apr 17 02:11 hwsku
drwxr-xr-x 81 root root 4096 Apr 17 00:19 ..
drwxr-xr-x 4 root root 4096 Apr 17 00:19 .
root@sonic-z9100-02:/usr/share/sonic#
So, copy is the right way to address this. I changed this in new commit. Additionaly added checks to remove old led files at the time of copying.
@paavaanan: Will you also need to make a similar PR on the master branch (which should also be able to be cherry-picked into the 201811 branch, or can this PR be cherry-picked cleanly into all branches? |
Joe, z9100_platform.sh do have some differences. Will raise a seperate PR to Master Branch. Better cherry-pick from that PR for 201811 branch |
Closing this PR as the 201803 branch does not officially support Z9100. Similar PR #2799 was merged into master and will be cherry-picked into the 201811 branch. |
…atically (#14752) src/sonic-utilities * ece22b7d - (HEAD -> 202205, origin/202205) Revert "[GCU] Add PFC_WD RDMA validator (#2781)" (4 minutes ago) [Ying Xie] * 7d16b184 - Remove the no use new line in show version (#2792) (21 hours ago) [xumia] * 3a880a2b - Support to display the SONiC OS Version in the command show version (#2787) (21 hours ago) [xumia] * a5199f75 - [voq][chassis][generate_dump] [BCM] Dump only the relevant BCM commands for fabric cards (#2606) (21 hours ago) [saksarav-nokia] * 2410d364 - Fixed a bug in "show vnet routes all" causing screen overrun. (#2644) (#2801) (
- What I did
Added support for Switchport LED for DellEMC Z9100 switches
- How I did it
Added led_proc_init.soc file for T0 and T1 Topologies.
- How to verify it