drivers: ethernet: eth_sam_gmac: Fix priority queues #22961

nandojve · 2020-02-19T22:22:44Z

The Atmel SAM SoC with ethernet port uses same GMAC driver. However, there are differences between SoC GMAC implementation. Some SoCs have priority queue and system can configure 0 up to 5, depending of SoC version. This update current GMAC driver adding missing definitions.

I already tested with SAM_V71_XULT using 6 queues [5 pri queues] with samples/net/telnet with DHCP. Ping is working OK using telnet terminal. It needs people to test SAME70[B]. I already tested SAM_V71_XULT as SAMV71A variation and seems to works well.

I added Kconfig rules to make sure user can only select proper queue size.

NOTE for SAM4E: Not yet supported.
NOTE for SAME70[B]: zephyrproject-rtos/hal_atmel#7

This work is based on slack discussion this week and the start point was #12711.

This PR supersedes #12711 (closes #12711).

drivers/ethernet/eth_sam_gmac.c

drivers/ethernet/Kconfig.sam_gmac

drivers/ethernet/eth_sam_gmac_priv.h

zephyrbot · 2020-02-21T23:35:26Z

All checks are passing now.

checkpatch (informational only, not a failure)

-:346: WARNING:LONG_LINE: line over 80 characters
#346: FILE: drivers/ethernet/eth_sam_gmac.c:873:
+	if (queue_id < GMAC_QUE_1 || queue_id > GMAC_ACTIVE_PRIORITY_QUEUE_NUM) {

-:355: WARNING:LONG_LINE: line over 80 characters
#355: FILE: drivers/ethernet/eth_sam_gmac.c:897:
+	if (queue_id < GMAC_QUE_1 || queue_id > GMAC_ACTIVE_PRIORITY_QUEUE_NUM) {

-:364: WARNING:LONG_LINE: line over 80 characters
#364: FILE: drivers/ethernet/eth_sam_gmac.c:916:
+	if (queue_id < GMAC_QUE_1 || queue_id > GMAC_ACTIVE_PRIORITY_QUEUE_NUM) {

-:373: WARNING:LONG_LINE: line over 80 characters
#373: FILE: drivers/ethernet/eth_sam_gmac.c:959:
+	if (queue_id < GMAC_QUE_1 || queue_id > GMAC_ACTIVE_PRIORITY_QUEUE_NUM) {

-:382: WARNING:LONG_LINE: line over 80 characters
#382: FILE: drivers/ethernet/eth_sam_gmac.c:1009:
+	if (queue_id < GMAC_QUE_1 || queue_id > GMAC_ACTIVE_PRIORITY_QUEUE_NUM) {

- total: 0 errors, 5 warnings, 954 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

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

nandojve · 2020-02-22T11:32:51Z

The work with queues is done! The remaining work related to DT will be addressed on #22997 and SAM4E fix and activation on #23031 just waiting this PR be merged.

stephanosio

Verified working on SAM E70 Rev. B.

mnkp

LGTM, I do have however some comments related to style.

drivers/ethernet/eth_sam_gmac.c

drivers/ethernet/eth_sam_gmac_priv.h

mnkp

Sorry for the late review. I have two minor non-blocking comments. I won't be able to re-review the code in the next few days in case there are changes.

drivers/ethernet/eth_sam_gmac.c

The first revision of the SAM E70 soc had three queues. The current revision B has six queues. If we don't initialize all queues, the DMA engine gets stuck when trying to read a descriptor from NULL. To enable the initialization of the additional queues, the correct soc has to be selected in the config options, f.ex. CONFIG_SOC_PART_NUMBER_SAME70Q21B instead of CONFIG_SOC_PART_NUMBER_SAME70Q21. Also rename GMAC_QUEUE_NO to GMAC_QUEUE_NUM as requested during review. Signed-off-by: Daniel Glöckner <dg@emlix.com>

Add missing queue entries for sam gmac. This update the queue selection to proper handle all supported SAM SoC that uses GMAC driver. Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>

The Atmel SAM SoC with ethernet port uses same GMAC driver. However, there are differences between SoC GMAC implementation. Some SoCs have priority queue and system can configure 0 up to 5, depending of SoC version. This update current GMAC driver adding missing definitions. Co-authored-by: Stephanos Ioannidis <root@stephanos.io> Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>

Move queue init block to avoid add many defines on code. Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>

Improve priority queue conditional build. Now priority queue code is enabled only if device have support to it. This enables GMAC driver for devices with only one queue for RX/TX. Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>

stephanosio · 2020-03-04T13:26:55Z

@jhedberg Could we merge this for 2.2? Without this patch, the revision B SAM7 series devices (which all newly produced SAM7s are) cannot transmit Ethernet packets.

jhedberg · 2020-03-04T15:00:22Z

@jhedberg Could we merge this for 2.2? Without this patch, the revision B SAM7 series devices (which all newly produced SAM7s are) cannot transmit Ethernet packets.

Could you explain first what's going on with the linked issues. Both of them are closed, so exactly what GitHub issue is this fixing?

stephanosio · 2020-03-04T15:12:04Z

@jhedberg Could we merge this for 2.2? Without this patch, the revision B SAM7 series devices (which all newly produced SAM7s are) cannot transmit Ethernet packets.

Could you explain first what's going on with the linked issues. Both of them are closed, so exactly what GitHub issue is this fixing?

@jhedberg Those issues should not have been closed, as the problem described there was not fixed (though both issues were more of a "question" that a proper "bug report").

This PR fixes the problem reported in #22890 and #23035.

p.s. the importance of this PR is that, for instance, if you buy a reel of ATSAME70 now, you will get the Rev. B chip and find that Ethernet is no longer working on it (without this PR, that is). Similarly, if you buy a new SAM E70 Xplained evaluation kit, the kit will be populated with the Rev. B chip and you will find that Ethernet is not working on it.

nandojve requested review from jukkar, pfalcon and tbursztyka as code owners February 19, 2020 22:22

nandojve requested review from stephanosio, galak and mnkp February 19, 2020 22:23

nandojve commented Feb 19, 2020

View reviewed changes

drivers/ethernet/eth_sam_gmac.c Outdated Show resolved Hide resolved

galak reviewed Feb 19, 2020

View reviewed changes

drivers/ethernet/Kconfig.sam_gmac Show resolved Hide resolved

pfalcon removed their request for review February 20, 2020 07:23

stephanosio reviewed Feb 20, 2020

View reviewed changes

drivers/ethernet/Kconfig.sam_gmac Outdated Show resolved Hide resolved

drivers/ethernet/Kconfig.sam_gmac Show resolved Hide resolved

drivers/ethernet/eth_sam_gmac_priv.h Outdated Show resolved Hide resolved

stephanosio mentioned this pull request Feb 21, 2020

Add GMAC device tree definition #22997

Closed

stephanosio added DNM This PR should not be merged (Do Not Merge) In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on labels Feb 21, 2020

stephanosio changed the title ~~[DNM][WIP] drivers: ethernet: eth_sam_gmac: Fix priority queues~~ drivers: ethernet: eth_sam_gmac: Fix priority queues Feb 21, 2020

stephanosio added area: Ethernet area: Networking bug The issue is a bug, or the PR is fixing a bug platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) priority: medium Medium impact/importance bug labels Feb 21, 2020

stephanosio requested review from aurel32 and pfalcon February 21, 2020 00:33

nandojve force-pushed the fix_sam_gmac_queue_size branch from c778ff6 to e3016be Compare February 21, 2020 23:25

nandojve force-pushed the fix_sam_gmac_queue_size branch from e3016be to 82a79f8 Compare February 22, 2020 00:05

nandojve mentioned this pull request Feb 22, 2020

Fix and enable sam4e ethernet port #23031

Merged

nandojve force-pushed the fix_sam_gmac_queue_size branch from 82a79f8 to 3309955 Compare February 22, 2020 01:10

stephanosio linked an issue Feb 22, 2020 that may be closed by this pull request

dhcpv4_client sample not working on sam e70 #23035

Closed

stephanosio removed the DNM This PR should not be merged (Do Not Merge) label Feb 22, 2020

stephanosio added this to the v2.2.0 milestone Feb 22, 2020

stephanosio linked an issue Feb 22, 2020 that may be closed by this pull request

IP networking does not work on ATSAME70 Rev. B #22890

Closed

stephanosio removed the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Feb 22, 2020

stephanosio approved these changes Feb 22, 2020

View reviewed changes

stephanosio mentioned this pull request Feb 24, 2020

Add GMAC device tree binding #23045

Merged

MaureenHelm approved these changes Feb 24, 2020

View reviewed changes

mnkp requested changes Feb 24, 2020

View reviewed changes

nandojve force-pushed the fix_sam_gmac_queue_size branch from 3309955 to e1ff7fb Compare February 26, 2020 03:35

nandojve closed this Feb 26, 2020

nandojve reopened this Feb 26, 2020

stephanosio requested a review from mnkp February 26, 2020 14:00

mnkp approved these changes Feb 28, 2020

View reviewed changes

drivers/ethernet/eth_sam_gmac.c Outdated Show resolved Hide resolved

drivers/ethernet/eth_sam_gmac.c Outdated Show resolved Hide resolved

dgloeck and others added 5 commits February 28, 2020 12:23

drivers: eth: Kconfig.sam_gmac: Fix queue definitions

48daf1f

Add missing queue entries for sam gmac. This update the queue selection to proper handle all supported SAM SoC that uses GMAC driver. Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>

drivers: eth: eth_sam_gmac: Move queue init block

5fa304c

Move queue init block to avoid add many defines on code. Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>

nandojve force-pushed the fix_sam_gmac_queue_size branch from e1ff7fb to 821170d Compare February 28, 2020 15:23

stephanosio requested a review from jhedberg March 4, 2020 13:23

stephanosio mentioned this pull request Mar 4, 2020

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

Closed

jhedberg merged commit f756432 into zephyrproject-rtos:master Mar 4, 2020

nandojve deleted the fix_sam_gmac_queue_size branch March 4, 2020 21:19

Julieanna97 mentioned this pull request Feb 7, 2024

ATSAME70Q21 Ethernet Issue: Can't assign IP address, subnet, etc. #68600

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: ethernet: eth_sam_gmac: Fix priority queues #22961

drivers: ethernet: eth_sam_gmac: Fix priority queues #22961

nandojve commented Feb 19, 2020 •

edited by stephanosio

Loading

zephyrbot commented Feb 21, 2020 •

edited

Loading

nandojve commented Feb 22, 2020

stephanosio left a comment

mnkp left a comment

mnkp left a comment

stephanosio commented Mar 4, 2020

jhedberg commented Mar 4, 2020

stephanosio commented Mar 4, 2020 •

edited

Loading

drivers: ethernet: eth_sam_gmac: Fix priority queues #22961

drivers: ethernet: eth_sam_gmac: Fix priority queues #22961

Conversation

nandojve commented Feb 19, 2020 • edited by stephanosio Loading

zephyrbot commented Feb 21, 2020 • edited Loading

checkpatch (informational only, not a failure)

nandojve commented Feb 22, 2020

stephanosio left a comment

Choose a reason for hiding this comment

mnkp left a comment

Choose a reason for hiding this comment

mnkp left a comment

Choose a reason for hiding this comment

stephanosio commented Mar 4, 2020

jhedberg commented Mar 4, 2020

stephanosio commented Mar 4, 2020 • edited Loading

nandojve commented Feb 19, 2020 •

edited by stephanosio

Loading

zephyrbot commented Feb 21, 2020 •

edited

Loading

stephanosio commented Mar 4, 2020 •

edited

Loading