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.

Sign up for GitHub

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

Jump to bottom

[Inventec][D6356] Kernel upgrade and few updates #5145

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

[Inventec][D6356] Kernel upgrade and few updates #5145

wants to merge 1 commit into from

Conversation

CynthiaINV
Copy link
Contributor

- Why I did it

  • Support Inventec D6356 with kernel 4.19
  • fix xcvrd can't run issue after updated on master branch (cause by platform API)
  • add auto find gpio base in swps driver to detect base changes when kernel changed

- How I did it

  • Upgrade kernel to 4.19
  • Update SFP/QSFP platform API base on latest xcvrd
  • Add auto find gpio base in swps driver

- How to verify it

  • Basic traffic tests
  • Run "show interface status" "show interface transceiver presence" to check transceiver/port status
  • Run "show platform fan", "show temperature", "show environment" ...etc to check device status
    Please refer to the test report after these fixes for details.
    D6356_master.pdf

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

  • 201811
  • 201911
  • 202006

- Description for the changelog

  • Upgrade kernel to 4.19
  • Update SFP/QSFP platform API base on latest xcvrd
  • Add auto find gpio base in swps driver

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

@ghost
Copy link

ghost commented Aug 11, 2020 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

lguohan
lguohan reviewed Aug 17, 2020
View reviewed changes
platform/broadcom/sonic-platform-modules-inventec/d6356/modules/i2c-mux-pca9541.c
pca9641_release_bus(client);
i2c_unlock_adapter(adap);
i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER);
Copy link
Collaborator

@lguohan lguohan Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i do not know how this driver got into sonic repo, we should use standard linux driver for 9541.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is modified after previous feedback, please help to review, thank you!

lguohan
lguohan reviewed Aug 17, 2020
View reviewed changes
platform/broadcom/sonic-platform-modules-inventec/d6356/utils/inventec_d6356_util.py Outdated
@@ -141,7 +141,7 @@ def system_install():
status, output = exec_cmd("rmmod lpc_ich ", 1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use blacklist module as the standard way to block loading certain drivers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is modified after previous feedback, please help to review, thank you!

lguohan
lguohan reviewed Aug 17, 2020
View reviewed changes
platform/broadcom/sonic-platform-modules-inventec/d6356/utils/inventec_d6356_util.py Outdated
@@ -141,7 +141,7 @@ def system_install():
status, output = exec_cmd("rmmod lpc_ich ", 1)

#insert extra module
status, output = exec_cmd("insmod /lib/modules/4.9.0-11-2-amd64/kernel/drivers/gpio/gpio-ich.ko gpiobase=0",1)
status, output = exec_cmd("insmod /usr/lib/modules/4.19.0-9-2-amd64/kernel/drivers/gpio/gpio-ich.ko gpiobase=0",1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is easily broken. we should not hardcode the driver path into the installer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is modified after previous feedback, please help to review, thank you!

@CynthiaINV
[Inventec][D6356] Kernel upgrade and few updates
2a84aa6 
- Upgrade kernel to 4.19
- Update SFP/QSFP platform API base on latest xcvrd
- Add auto find gpio base in swps driver

Signed-off-by: cynthia <wu.cynthia@inventec.com>
@lgtm-com
Copy link

lgtm-com bot commented Aug 25, 2020

This pull request fixes 8 alerts when merging 2a84aa6 into f3feb56 - view on LGTM.com

fixed alerts:

  • 8 for Variable defined multiple times

@CynthiaINV
Copy link
Contributor Author

hi @lguohan ,
I'm sorry for bothering.
I've updated the code after previous review, and it seems already passed the autotests. Would you please help to take a look at this pull request?
Thank you very much for your help!

@CynthiaINV CynthiaINV requested a review from lguohan October 7, 2020 03:47
@CynthiaINV
Copy link
Contributor Author

hi @lguohan ,
I'm sorry for bothering again.
I've modified the code after previous review, and the changes seems already passed the tests. Would you please help to review this pull request again?
If there is something I missed to do, please let me know! Thank you very much for your help!

@CynthiaINV
Copy link
Contributor Author

hi @lguohan ,
I'm sorry for keep flooding with messages, but this pull request hasn't been reviewed for a while after last update, and I has became a bit urgent for our team these days.
Would you please help to take a look this pull request? If there's something I need to do, please let me know, I'll try to fix the issues as soon as possible.
Thank you very much for your help!

hi @jleveque ,
I'm sorry for bothering.
Because this pull request hasn't been reviewed after a while and it has became a bit urgent recently, I'm asked to tag you here by my supervisor. Would you mind to give us some advices to make the merge procedure go smoothly? I'm thinking maybe I missed something I should do for the review.
Thank you very much for your assistance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CynthiaINV @lguohan