revyos / thead-u-boot

7 stars 16 forks source link

feat: enable load str firmware #28

Closed RevySR closed 9 months ago

nekorouter commented 9 months ago

8G memory tested, STR OK 16G memory STR failed

gilbsgilbs commented 7 months ago

Hello! Why was adec30ace4cebb0554bb246b52eebaf37c1545c4 reverted in this PR? Doesn't this regress the bug it fixed? The cache invalidation functions are wrong again as a result.

RevySR commented 7 months ago

Hello! Why was adec30a reverted in this PR? Doesn't this regress the bug it fixed? The cache invalidation functions are wrong again as a result.

  1. I reconfirmed your patch with T-Head's team, and it's causing the USB download abnormally.
  2. It will cause the new feature str(suspend to ram) to fail.

For both of these reasons, the patch is reverted.

gilbsgilbs commented 7 months ago

Thank you for the clarification @RevySR . I suspect there's a bug in the way these drivers invalidate the dcache. Reverting adec30ace4cebb0554bb246b52eebaf37c1545c4 may workaround the bugs you mentioned, but it's also causing the ethernet driver to download abnormally again (and it's only because invalidate_dcache_range performs a dcache flush when it really shouldn't).

If you or T-Head really don't want a clean fix or are afraid of side-effects, may I at least suggest to change the designware ethernet driver to clean dcache before or after copying data to DMA as a workaround? This would not be a proper fix, but it would at least make the ethernet work correctly.

RevySR commented 7 months ago

Thank you for the clarification @RevySR . I suspect there's a bug in the way these drivers invalidate the dcache. Reverting adec30a may workaround the bugs you mentioned, but it's also causing the ethernet driver to download abnormally again (and it's only because invalidate_dcache_range performs a dcache flush when it really shouldn't).

If you or T-Head really don't want a clean fix or are afraid of side-effects, may I at least suggest to change the designware ethernet driver to clean dcache before or after copying data to DMA as a workaround? This would not be a proper fix, but it would at least make the ethernet work correctly.

If you can submit a functionally correct patch I will verify it again with the T-Head team.