adin2111: OA support and fixes #68532

spectrum70 · 2024-02-05T09:44:35Z

Started to work on this driver, to optimize and add missing features.

jpmur · 2024-02-06T10:24:39Z

LGTM
Tested some net/ samples with no issues.

spectrum70 · 2024-02-08T12:09:21Z

Add other 2 small patches.

spectrum70 · 2024-02-08T12:23:48Z

Hi,

Add Open Alliance SPI support.

This is actually the most complex patch of the set, btw, i reused existing Microchip Open Alliance module for this.
Waiting for any Maintainer feedbacks, especially on the way i am zeroing gpio structure, to be able to control chip select by sw, since there may be better ways (see @@ -781,7 +916,7)

drivers/ethernet/Kconfig.adin2111

carlocaione · 2024-02-08T13:26:27Z

Waiting for any Maintainer feedbacks, especially on the way i am zeroing gpio structure, to be able to control chip select by sw, since there may be better ways (see @@ -781,7 +916,7)

@gmarull ^

drivers/ethernet/eth_adin2111.c

GeorgeCGV · 2024-02-08T17:24:07Z

drivers/ethernet/eth_adin2111.c

@@ -758,10 +891,14 @@ static int adin2111_await_device(const struct device *dev)

 	/* await reset complete (RESETC) and clear it */
 	for (count = 0U; count < ADIN2111_RESETC_AWAIT_RETRY_COUNT; ++count) {
-		ret = eth_adin2111_reg_read(dev, ADIN2111_STATUS0, &val);
+		ret = eth_adin2111_reg_read(dev, ADIN2111_PHYID, &val);


Not sure about that change. The RM mentions STATUS0 poll. Will need to test.

That also leaks PHY part into MAC (ADIN2111_PHYID, ADIN2111_PHYID_OUI).

Do you use HW reset? If no, then it could be related to clock stabilization delay (done within hw reset).

Thaks,

i tested mainly using the button reset. And i see that keeping the reset button pressed adin2111 clock stops.

Reading a read-only reg currently may not be the best, sure, but it works reliably, and allows "Generic SPI without CRC" to work.

What about if i move the check in the phy code ?

Anyway, i am asking also suggestions to the adi team right now.

@spectrum70 in cases when hw reset is not used does increasing the delay from 10ms to ADIN2111_HW_BOOT_DELAY_MS helps (line 998)?

Anyway, i am asking also suggestions to the adi team right now.

Was there any response from them?

drivers/ethernet/eth_adin2111.c

drivers/ethernet/eth_adin2111_priv.h

GeorgeCGV · 2024-02-09T11:50:29Z

@spectrum70 thank you for the addition, cleanup, and fixes. Tested legacy spi with crc, it works. I am ok with a merge as soon as the driver config is kept immutable (cs pin is set during config definition).

spectrum70 · 2024-02-10T10:29:30Z

In this PR:

remove CONFIG_ stuff for Open Alliance, used DT_PROP and COND_CODE_1
add oa and oa_protection to use/call Open Alliance features.

spectrum70 · 2024-02-12T13:50:27Z

@spectrum70 thank you for the addition, cleanup, and fixes. Tested legacy spi with crc, it works. I am ok with a merge as soon as the driver config is kept immutable (cs pin is set during config definition).

Thanks to you, a pleasure to work on this driver, nice and clean code.

drivers/ethernet/eth_adin2111.c

spectrum70 · 2024-02-12T17:41:28Z

re-pushed
changes in this version:

use DT_INST_PROP_OR and DT_INST_PROP where possible
set also spi-oa-protection enabled in DT, since adin2111 eval board is shipped OA + PROTECTION dips-witches set.
fixed macroes line-separator alignment

GeorgeCGV · 2024-02-24T19:20:20Z

stm32 re-transmit same tcp sequence packet

Does eth_adin2111_oa_data_write return any error? What is the state of TXBOE, RXBOE, and TXBUE in the STATUS0 register when it happens (maybe modification of FIFO sizes could help if the host is not fast enough)?

drivers/ethernet/eth_adin2111.c

spectrum70 · 2024-02-26T19:31:05Z

@GeorgeCGV when iperf stucks, mainly, the adin2111 receiving thread is blocked.
And loks like it is blocked at

/* await INT */ k_sem_take(&ctx->offload_sem, K_FOREVER);

In this state, packets can still be sent out (checked this by restarting zperf) but nothing is received, becouse there is no hw interrupt issued from adin2111 (checked by oscilloscope).

So strangely, only for Open Alliance, fast packet transmission by zperf for some reason locks the adin2111.
Also, if i enable CONFIG_NET_TCP_LOG_LEVEL_DBG=y, so introducing a delay between packets, iperf test completes.

[edited]
Well, looks like i miss to check for space before sending a frame in OA mode.

GeorgeCGV · 2024-03-01T20:59:52Z

Well, looks like i miss to check for space before sending a frame in OA mode.

That is great. Feel free to change the status to "Ready for review" when it works as expected.

spectrum70 · 2024-03-04T15:12:14Z

Hi,
over hard zperf/iperf testing there was a couple of issues on using oa_tc6 developed for Microchip that i could not solve without putting hands in the module, that i cannot re-test for Microchip then. So i re-implemented Open Alliance part inside the driver, featuring a single dma transfer per frame, that works much better.

oa_tc6 common code cannot be used in a profitable way, due to :

not addressing port 2
sends only chunk by chunk, in one dma transfer per chunk, while adin2111 allows a single dma transfer with all chunks.

I am attaching some zperf tests.

test-zephyr-adin2111ebz-iperf.txt

Waiting for comments.

PS: there is still a brief outstanding issue open, not related to this job, but i am investigating on it. I noticed that on incoming udp packet of size 1512, generic spi fails on "Still some length to go 2" net pkt message.

spectrum70 · 2024-03-05T10:33:12Z

Fixed the issue reported above ("Still some length to go 2" net pkt message.).
Retested all, attaching zperf test.
test-zephyr-adin2111ebz-iperf.txt

Fix IAMSK1 TX_READY_MASK bitfield position. Signed-off-by: Angelo Dureghello <[email protected]>

Enable appending of a crc32 at the end of the frame by the MAC, always. This is always needed since the driver is not adding it. This field has nothing to do with Generic SPI protocol-related 8-bit CRC, so this patch removes the CONFIG_ETH_ADIN2111_SPI_CFG0 choice related to this setting. Testing without this flag set, packets are not forwareded in the network, since the driver is not appending any crc32 header to the frame. Signed-off-by: Angelo Dureghello <[email protected]>

After some debugging related to non-working "Generic SPI without CRC" mode in eval_adin2111_ebz (CONFIG_ETH_ADIN2111_SPI_CFG0=n), noticed that even after proper STATUS0 RESETC bit detection, registers, for a certain period (some msecs) still reads as zero. This patch fixes adin2111_await_device and with it the "Generic SPI without CRC" mode. Signed-off-by: Angelo Dureghello <[email protected]>

Non functional change, removing unused variable producing compilation warning in "Generic SPI without CRC8" mode. Signed-off-by: Angelo Dureghello <[email protected]>

Add Open Alliance spi protocol support. Open Alliance is a chunk-based SPI protocol, based on sending over SPI an ethernet frame divided in smaller chunks, using a specific 32-bit header for each chunk transferred. All chunks can be sent or received by a single dma transfer. Default mode is set to Open Alliance SPI without protection, since the adin2111 dev. board comes shipped this way. Tested: - Open Alliance SPI, no protection (default board shipped) - Open Alliance SPI, protection - Generic SPI, no crc - Generic SPI, with crc8 Signed-off-by: Angelo Dureghello <[email protected]>

Fix issue related to "generic SPI" mode only, when a packet of 1512 bytes is received, net_pkt_write() fails and thrwos the error: "Still some length to go 2". This is due to net_pkt_rx_alloc_with_buffer() allocating a maximum mtu/size of 1514, and driver is not removing 4 bytes of crc32 from rx buffer, that comes to be 1516 (2 bytes over buffer limit). Fix generic SPI rx frame size removing crc32 bytes. Signed-off-by: Angelo Dureghello <[email protected]>

spectrum70 · 2024-03-06T16:34:29Z

Adding also new zperf board to board measurement, udp is at 8.5Mbit/s
test-zephyr-adin2111ebz-iperf.txt

carlocaione

not my usual cup of tea but LGTM for the general part

GeorgeCGV

Nice work, tested generic spi with enabled crc, everything works.

GeorgeCGV · 2024-09-24T08:53:10Z

drivers/ethernet/eth_adin2111.c

+		if (ctx->oa) {
+			if (status1 & ADIN2111_STATUS1_P1_RX_RDY) {
+				ret = eth_adin2111_oa_data_read(dev, 0);
+				if (ret < 0) {
+					break;
+				}
+			}
+			if (status1 & ADIN2111_STATUS1_P2_RX_RDY) {
+				ret = eth_adin2111_oa_data_read(dev, 1);
+				if (ret < 0) {
+					break;
+				}
+			}
+			goto continue_unlock;
+		}


@spectrum70 , completely missed during the review, there should be no break for the ctx->oa code; as there is no while. The break will terminate the irq offload thread. Because the thread is K_ESSENTIAL the kernel should crash.

spectrum70 requested a review from GeorgeCGV as a code owner February 5, 2024 09:44

zephyrbot added platform: ADI Analog Devices, Inc. area: Ethernet labels Feb 5, 2024

zephyrbot requested review from decsny, galak, MaureenHelm and microbuilder February 5, 2024 09:45

zephyrbot assigned MaureenHelm Feb 5, 2024

zephyrbot added the area: Devicetree Binding PR modifies or adds a Device Tree binding label Feb 7, 2024

spectrum70 force-pushed the wip/bl/adin2111-minor-fixes branch from 3106f8f to d0598b6 Compare February 8, 2024 12:08

carlocaione requested changes Feb 8, 2024

View reviewed changes

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

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

carlocaione requested a review from gmarull February 8, 2024 13:26

carlocaione changed the title ~~Wip/bl/adin2111 minor fixes~~ adin2111 minor fixes Feb 8, 2024

GeorgeCGV reviewed Feb 8, 2024

View reviewed changes

drivers/ethernet/eth_adin2111.c Show resolved Hide resolved

GeorgeCGV reviewed Feb 8, 2024

View reviewed changes

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

GeorgeCGV reviewed Feb 8, 2024

View reviewed changes

drivers/ethernet/eth_adin2111_priv.h Show resolved Hide resolved

spectrum70 force-pushed the wip/bl/adin2111-minor-fixes branch from 57d6ba1 to 2550d02 Compare February 10, 2024 10:23

spectrum70 requested review from carlocaione and GeorgeCGV February 12, 2024 13:48

carlocaione previously requested changes Feb 12, 2024

View reviewed changes

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

spectrum70 force-pushed the wip/bl/adin2111-minor-fixes branch from 2550d02 to 32c1468 Compare February 12, 2024 17:22

spectrum70 dismissed carlocaione’s stale review via 62745d6 February 24, 2024 19:06

spectrum70 force-pushed the wip/bl/adin2111-minor-fixes branch from c67a120 to 62745d6 Compare February 24, 2024 19:06

GeorgeCGV reviewed Feb 24, 2024

View reviewed changes

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

spectrum70 force-pushed the wip/bl/adin2111-minor-fixes branch 2 times, most recently from dc5cddf to c5c5aad Compare March 4, 2024 15:00

spectrum70 requested review from GeorgeCGV and carlocaione March 4, 2024 15:38

spectrum70 added 6 commits March 5, 2024 11:58

drivers: ethernet: adin2111: fix imask1 tx_ready_mask

dbbb6de

Fix IAMSK1 TX_READY_MASK bitfield position. Signed-off-by: Angelo Dureghello <[email protected]>

drivers: ethernet: adin2111: remove unused header_len variable

8ecb664

Non functional change, removing unused variable producing compilation warning in "Generic SPI without CRC8" mode. Signed-off-by: Angelo Dureghello <[email protected]>

spectrum70 force-pushed the wip/bl/adin2111-minor-fixes branch from 1f0df8a to 0e39545 Compare March 6, 2024 15:50

spectrum70 closed this Mar 6, 2024

spectrum70 reopened this Mar 6, 2024

spectrum70 marked this pull request as ready for review March 6, 2024 15:54

GeorgeCGV changed the title ~~adin2111 minor fixes~~ adin2111: OA support and fixes Mar 7, 2024

carlocaione approved these changes Mar 7, 2024

View reviewed changes

MaureenHelm approved these changes Mar 7, 2024

View reviewed changes

GeorgeCGV approved these changes Mar 8, 2024

View reviewed changes

decsny approved these changes Mar 8, 2024

View reviewed changes

carlescufi merged commit 06df4b5 into zephyrproject-rtos:main Mar 8, 2024
35 of 47 checks passed

GeorgeCGV reviewed Sep 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adin2111: OA support and fixes #68532

adin2111: OA support and fixes #68532

spectrum70 commented Feb 5, 2024

jpmur commented Feb 6, 2024 •

edited

Loading

spectrum70 commented Feb 8, 2024

spectrum70 commented Feb 8, 2024

carlocaione commented Feb 8, 2024

GeorgeCGV Feb 8, 2024 •

edited

Loading

spectrum70 Feb 9, 2024 •

edited

Loading

GeorgeCGV Feb 9, 2024 •

edited

Loading

GeorgeCGV Feb 21, 2024

GeorgeCGV commented Feb 9, 2024

spectrum70 commented Feb 10, 2024

spectrum70 commented Feb 12, 2024

spectrum70 commented Feb 12, 2024

GeorgeCGV commented Feb 24, 2024

spectrum70 commented Feb 26, 2024 •

edited

Loading

GeorgeCGV commented Mar 1, 2024

spectrum70 commented Mar 4, 2024 •

edited

Loading

spectrum70 commented Mar 5, 2024

spectrum70 commented Mar 6, 2024

carlocaione left a comment

GeorgeCGV left a comment

GeorgeCGV Sep 24, 2024

adin2111: OA support and fixes #68532

adin2111: OA support and fixes #68532

Conversation

spectrum70 commented Feb 5, 2024

jpmur commented Feb 6, 2024 • edited Loading

spectrum70 commented Feb 8, 2024

spectrum70 commented Feb 8, 2024

carlocaione commented Feb 8, 2024

GeorgeCGV Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

spectrum70 Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

GeorgeCGV Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

GeorgeCGV Feb 21, 2024

Choose a reason for hiding this comment

GeorgeCGV commented Feb 9, 2024

spectrum70 commented Feb 10, 2024

spectrum70 commented Feb 12, 2024

spectrum70 commented Feb 12, 2024

GeorgeCGV commented Feb 24, 2024

spectrum70 commented Feb 26, 2024 • edited Loading

GeorgeCGV commented Mar 1, 2024

spectrum70 commented Mar 4, 2024 • edited Loading

spectrum70 commented Mar 5, 2024

spectrum70 commented Mar 6, 2024

carlocaione left a comment

Choose a reason for hiding this comment

GeorgeCGV left a comment

Choose a reason for hiding this comment

GeorgeCGV Sep 24, 2024

Choose a reason for hiding this comment

jpmur commented Feb 6, 2024 •

edited

Loading

GeorgeCGV Feb 8, 2024 •

edited

Loading

spectrum70 Feb 9, 2024 •

edited

Loading

GeorgeCGV Feb 9, 2024 •

edited

Loading

spectrum70 commented Feb 26, 2024 •

edited

Loading

spectrum70 commented Mar 4, 2024 •

edited

Loading