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

Catch up icssg driver #374

Closed
wants to merge 1 commit into from
Closed

Catch up icssg driver #374

wants to merge 1 commit into from

Conversation

AsuraZeng
Copy link
Contributor

this helps the issue #368 referred.
after several times reboots, the ethernet port can not work.

jan-kiszka
jan-kiszka reviewed Nov 11, 2022
View reviewed changes
recipes-bsp/ti-pruss-firmware/ti-pruss-firmware_08.02.00.002.bb
${@get_prueth_fw(d, 'am65x-sr2-rtu0-prueth-fw.elf', 'fb154656f7cf2dd398a35968dfeaedda1fd57e916ee82b8d6fe58ad4d018e53b')} \
${@get_prueth_fw(d, 'am65x-sr2-rtu1-prueth-fw.elf', 'fc29c8f5e34234e96dcdda3a2f8628dec61c2c150e5e2896075b13a2d859acd6')} \
${@get_prueth_fw(d, 'am65x-sr2-txpru0-prueth-fw.elf', '96e1dd06feb3f05915dc61fb1b11b27674484749f9881ef02ebb48ac07f1386e')} \
${@get_prueth_fw(d, 'am65x-sr2-txpru1-prueth-fw.elf', '389ff8bdee84d66f0b1a8e02c2b8517b0b61a263fb65c18761e40eb83e09e9d2')} \
Copy link
Collaborator

Choose a reason for hiding this comment

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

The new firmware works with the current driver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have not try to use this way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, patch ordering as shown by github always confuses me: this update comes after the driver update. I then assume you tested that the new driver already works with the older firmware.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HI jan.

Cause I have not tested new icssg whether it can work with older prufw, So I use one commit to contain new icssg driver and the prufw.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jan-kiszka pls help to review this MR. thanks a lot

jan-kiszka
jan-kiszka reviewed Nov 16, 2022
View reviewed changes
recipes-bsp/ti-pruss-firmware/ti-pruss-firmware_08.02.00.002.bb
${@get_prueth_fw(d, 'am65x-sr2-rtu0-prueth-fw.elf', 'fb154656f7cf2dd398a35968dfeaedda1fd57e916ee82b8d6fe58ad4d018e53b')} \
${@get_prueth_fw(d, 'am65x-sr2-rtu1-prueth-fw.elf', 'fc29c8f5e34234e96dcdda3a2f8628dec61c2c150e5e2896075b13a2d859acd6')} \
${@get_prueth_fw(d, 'am65x-sr2-txpru0-prueth-fw.elf', '96e1dd06feb3f05915dc61fb1b11b27674484749f9881ef02ebb48ac07f1386e')} \
${@get_prueth_fw(d, 'am65x-sr2-txpru1-prueth-fw.elf', '389ff8bdee84d66f0b1a8e02c2b8517b0b61a263fb65c18761e40eb83e09e9d2')} \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Other question: Why are we on 08.02.00.002? The firmware repo has tagged 08.05.00.004 now. What are the differences?

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://git.ti.com/cgit/processor-firmware/ti-linux-firmware/commit/?id=224f82474e4029dd2e550600ba7e59f687ff7131 on the firmware side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TI has published the SDK 08.02.00.002 officially, so I catch up on this version.
08.05.00.004 is on the git repo. this is not officially published

@jan-kiszka
Copy link
Collaborator

Already asked this in firmware context, but it equally applied to the driver changes: Why not moving to latest 08.05.00.004 release? Already the icssg-prueth driver has several changes there, some of them fixes.

@AsuraZeng
Copy link
Contributor Author

Hi jan

As described above,
TI has published the SDK 08.02.00.002 officially, so I catch up on this version.
08.05.00.004 is on the git repo. this is not officially published

@AsuraZeng AsuraZeng mentioned this pull request Dec 12, 2022
Closed
@AsuraZeng AsuraZeng force-pushed the chao/catch-up-icssg-driver branch 2 times, most recently from c14f700 to 02a9a28 Compare January 12, 2023 05:49
@AsuraZeng AsuraZeng changed the title Chao/catch up icssg driver Catch up icssg driver Jan 12, 2023
jan-kiszka
jan-kiszka reviewed Jan 13, 2023
View reviewed changes
...nel/linux/files/patches-5.10/0116-net-ethernet-ti-icssg_prueth-move-phy-init-in-.ndo_o.patch Outdated
+ phy_remove_link_mode(emac->phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+ phy_remove_link_mode(emac->phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+ phy_remove_link_mode(emac->phydev, ETHTOOL_LINK_MODE_Pause_BIT);
+ phy_remove_link_mode(emac->phydev, ETHTOOL_LINK_MODE_Asym_Pause_BIT);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This breaks half-duplex. TI bug, but we would import it.

jan-kiszka
jan-kiszka reviewed Jan 13, 2023
View reviewed changes
...nel/linux/files/patches-5.10/0116-net-ethernet-ti-icssg_prueth-move-phy-init-in-.ndo_o.patch Outdated
- if (!emac->half_duplex) {
- phy_remove_link_mode(emac->phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
- phy_remove_link_mode(emac->phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
- }
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 what we lose.

@jan-kiszka
Copy link
Collaborator

While continuing the review, my first suggestion for the kernel patch queue would to preserve our original ordering, add the new cherry-picked patches on top. Would allow some bisecting (with some extra setup effort) over the update.

Another suggestion (should be a separate commit) is to use git format-patch --no-signature from now on so that this source of change is consistently avoided. I think you tweaked your format-patch to keep the signature identical, right?

@jan-kiszka
Copy link
Collaborator

You dropped these patches from the queue:

  • recipes-kernel/linux/files/patches-5.10/0062-remoteproc-pru-Configure-firmware-based-on-client-se.patch
  • recipes-kernel/linux/files/patches-5.10/0069-remoteproc-pru-add-support-for-configuring-GPMUX-bas.patch

This causes a delta of drivers/remoteproc/pru_rproc.c even with your old BSP release baseline.

@AsuraZeng
linux-iot2050: Catchup icssg driver and prufw
050b8c2 
There is a bug, after several times reboot, the ethernet
port cat not work.

After investigation and debugging, it was found that
the problem occurred in the mac layer inside icssg.

So catchup driver from sdk 08.02.00.002 and update prufw
08.02.00.002

Signed-off-by: chao zeng <chao.zeng@siemens.com>
@AsuraZeng
Copy link
Contributor Author

While continuing the review, my first suggestion for the kernel patch queue would to preserve our original ordering, add the new cherry-picked patches on top. Would allow some bisecting (with some extra setup effort) over the update.

Another suggestion (should be a separate commit) is to use git format-patch --no-signature from now on so that this source of change is consistently avoided. I think you tweaked your format-patch to keep the signature identical, right?

I have refactored the patches based on your suggestion.

@jan-kiszka
Copy link
Collaborator

Please check out https://github.com/siemens/meta-iot2050/commits/jan/kernel-update - there was more to-do than I originally thought.

@jan-kiszka jan-kiszka mentioned this pull request Jan 18, 2023
Merged
@jan-kiszka
Copy link
Collaborator

Closing as #411 was merged.

@jan-kiszka jan-kiszka closed this Feb 21, 2023
@huaqianli huaqianli deleted the chao/catch-up-icssg-driver branch July 9, 2024 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jan-kiszka jan-kiszka jan-kiszka left review comments

@BaochengSu BaochengSu Awaiting requested review from BaochengSu

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
@AsuraZeng @jan-kiszka