drivers: eth: sam-e70: revision B has more queues

[dgloeck]

If we don't initialize all queues, the DMA engine gets stuck when trying to read a descriptor from NULL.

[aurel32]

@dgloeck Thanks for the patch. Looking at it quickly it seems correct. That said I also have a rev B chip and for me the driver basically works with the exception of a few bugs as you know. What are the symptoms when the DMA engine try to read from a non initialized queue?

[dgloeck]

@aurel32, both HRESP and TFC are set in GMAC_TSR and no packets are sent. The GMAC_TBQB register still points to the first descriptor. No w1 field in that queue has been modified by the DMA engine. Reception works as expected.

[pfalcon]

Please try to provide more detailed commit messages, to provide smooth intro into the topic even for folks not familiar with details of specific HW. If each of us don't do that, even an author of a patch won't be able to tell why it was done and whether it's done right enough after some time (e.g., in a year).

drivers: eth: sam-e70: revision B has more queues
If we don't initialize all queues, the DMA engine gets stuck when trying
to read a descriptor from NULL.

Can some concrete info be added, e.g. something like "Revision A has 2 queues, while B - 4. Use config var to pick up right number of queues to initialize." ?

[aurel32]

@dgloeck @mnkp Now that the Atmel SAM E70 HAL has been updated to version 2.3.98, any opinion on how we should import the revision B. I see the following options:

• Import both revisions A and B, and allow users to select the right version in Kconfig, ie adding more parts in soc/arm/atmel_sam/same70/Kconfig.soc
• Import revision B over revision A and try to detect the revision at runtime like done in the patch proposed by @dgloeck.

[dgloeck]

I'd prefer to replace the rev A headers with the rev B headers.
@mnkp, shall I make a pull request?

[mnkp]

I spent some time looking at the issue and I much prefer the first option

• Import both revisions A and B, and allow users to select the right version in Kconfig, ie adding more parts in soc/arm/atmel_sam/same70/Kconfig.soc

The second approach would make sense if we decided to drop support for A series. But I'm not convinced we should.

• Import revision B over revision A and try to detect the revision at runtime like done in the patch proposed by @dgloeck.

If we have only one set of headers, those for B series, it won't be possible any more to find out what the differences between the two versions are. We can do a runtime check in gmac driver as this PR proposes, but the next person developing another driver is going to have a rough time. Accessing a non existing register may cause difficult to debug errors ranging from a hard fault to missing functionality. It's quite frustrating when the documentation claims that flipping a given bit does something and our device doesn't seem to care. If we keep header files separate any mishaps will simply cause a compile time error.

Also I find it somehow cleaner if we stick with official part names. Someone who has a B series device will expect to choose same70q21b as a part name and not same70q21. A person may wonder if B series is supported at all.

[pfalcon]

Ping guys, any progress/resolution here? The PR needs resolving conflicts.

[dgloeck]

I'm waiting for #13155 before I rewrite this PR.

[jwu1980]

Hi, @dgloeck,

Do you still plan to merge this pull request?

Thanks.

Best,

Larry

[dgloeck]

@f7488405 Still waiting for #13155

[aurel32]

@dgloeck FYI #13155 has just been merged.

[dgloeck]

I have rebase the changes. The number of queues initialized now depends on the soc selected (CONFIG_SOC_ATMEL_SAME70_REVB). The GMAC_QUEUE_NO macro has been renamed to GMAC_QUEUE_NUM as requested. A BUILD_ASSERT_MSG line checks that GMAC_QUEUE_NUM has the correct value in case Microchip releases another soc that can use the same driver. GMAC_QUEUE_NUM must remain a number that can be evaluated by the C preprocessor, so using ARRAY_SIZE there is not possible. I have also added a few more words to the commit message.

[mnkp]

I took a liberty to mark this PR as a bugfix. I believe we should backport it to 1.14.

[stephanosio]

As reported in #22890, we need this PR to get Ethernet working on ATSAME70 Rev. B SoC. Without this PR, Ethernet frames do not get transmitted.

[stephanosio]

Is there anything preventing this PR from getting merged?

[zephyrbot]

All checks passed.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

[stephanosio]

Force pushing to rebase onto the latest master and fix checkpatch warning

[stephanosio]

Superseded by #22961.