New NXP ENET driver #62833

decsny · 2023-09-19T21:42:56Z

ETH MCUX driver is unmaintainable and cannot support a PHY other than the one on some of the NXP EVKs. Redo, planning to deprecate old driver but keep for a release or two.

New driver selects EXPERIMENTAL until the community has had a chance to try this new driver.

This PR also makes the phy-connection-type property generic to all ethernet controllers

zephyrbot · 2023-09-19T21:43:56Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

drivers/ethernet/Kconfig.nxp_enet

drivers/ethernet/eth_nxp_enet.c

decsny · 2023-11-28T15:05:01Z

@manuargue can you check if your block is addressed

Add ENET clock value to the CCM clock decoder for both RT10XX and RT11XX CCM versions. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add a property to the ethernet controller binding indicating what type of connection the MAC has with the PHY device. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add DT Binding for Microchip KSZ8081 PHY Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add Driver for KSZ8081 Ethernet PHY. The Generic MII Driver is not sufficient to use for this PHY chip which has special vendor implemented behaviors. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add bindings for compatibles related to NXP ENET IP. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Create an include header to be used by NXP ENET drivers. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add driver for NXP ENET MDIO functionalities Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add driver for NXP ENET which is a rework of the old eth_mcux.c driver which had become unmaintainable due to fundamental problems with the lack of PHY abstraction. eth_mcux.c and the corresponding compatible nxp,kinetis-ethernet will be deprecated and this new driver will be supported instead. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

when using either old or new driver for nxp enet, enable the phy clock Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Increase the size of the system workqueue stack if using nxp ethernet driver Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add subsys value for ENET PLL / ENET Ref clk to CCM Driver Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add binding for NXP ENET PTP Clock device Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add ptp clock driver init priority kconfig Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add Driver for NXP ENET PTP Clock device Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Support PTP functionality in NXP ENET MAC driver Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

Add overlay to show how to use new experimental nxp enet driver Also make any eth_mcux related kconfigs dependent on eth_mcux in the boards' defconfigs. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

manuargue

My comments addressed. Nice job!

pdgendt · 2023-11-28T16:22:19Z

boards/arm/mimxrt1024_evk/dts/mimxrt1024_evk-enet-experimental.dtsi

+
+
+


Nit: multiple newlines

pdgendt · 2023-11-28T16:27:18Z

dts/bindings/ethernet/microchip,ksz8081.yaml

+include: ethernet-phy.yaml
+
+properties:
+  mc,reset-gpio:


No, I mean that other drivers that use reset-gpios handles, simply name them as such. Without the mc, prefix.

danieldegrasse

Couple of nits, but driver looks good to me. Verified it is working on an RT1060 EVK.

drivers/ethernet/phy/phy_microchip_ksz8081.c

danieldegrasse · 2023-11-28T18:09:04Z

include/zephyr/drivers/ethernet/eth_nxp_enet.h

+ * Interrupt enable: The driver's relevant interrupt was enabled in NVIC
+ */
+enum nxp_enet_callback_reason {
+	nxp_enet_module_reset,


Nit- the convention is typically to use CAPS for a enum constant in Zephyr, right?

talih0 · 2023-11-28T20:59:09Z

drivers/mdio/mdio_nxp_enet.c

+		 * ethernet driver has not initiaized, just do a busy wait
+		 */
+		k_busy_wait(USEC_PER_MSEC * config->timeout);
+		if (base->EIR && ENET_EIR_MII_MASK == ENET_EIR_MII_MASK) {


This and the same below do not seem correct. You probably meant:
(base->EIR & ENET_EIR_MII_MASK) == ENET_EIR_MII_MASK

this was fixed in #65999

talih0 · 2023-11-28T21:06:49Z

drivers/ethernet/eth_nxp_enet.c

+
+extern void nxp_enet_mdio_callback(const struct device *mdio_dev,
+				enum nxp_enet_callback_reason event);
+


I think this is already included in one of header you created.

I think this was fixed in #65998

talih0 · 2023-11-28T21:17:24Z

drivers/ethernet/eth_nxp_enet.c

+	k_mutex_lock(&data->tx_frame_buf_mutex, K_FOREVER);
+
+	/* Read network packet from upper layer into frame buffer */
+	if (net_pkt_read(pkt, data->tx_frame_buf, total_len)) {


Btw, eth_sam_gmac.c does this with zero-copy (and for receive too), perhaps the same can be done with this driver in the future.

it would be desirable, this driver is still experimental because I am planning that it will get a lot of changes such as potentially this, and I don't want it to be considered stable yet so that we can make these changes more quickly

talih0 · 2023-11-28T21:20:52Z

drivers/ethernet/eth_nxp_enet.c

+		return ret;
+	}
+
+	return ret;


nit: the if (ret) .. is unnecessary above.

my opinion on this is that it is easy to forget to add if another call is in the future added later in the code that affects ret, which can cause a bug that might not immediately appear, and the compiler will optimize this anyways, so I prefer this. I get this nit a lot

talih0 · 2023-11-28T21:48:25Z

drivers/ethernet/eth_nxp_enet.c

+	struct net_pkt *pkt;
+
+	pkt = frameinfo->context;
+	if (pkt && atomic_get(&pkt->atomic_ref) > 0) {


Should the minimum ref count be actually 2?

If the reference count == 1, then the packet will be freed by net_pkt_unref() on line 235. That means whatever we push into the queue by net_if_add_tx_timestamp() is no longer valid.

drivers/ptp_clock/Kconfig

zephyrbot added manifest manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Sep 19, 2023

This was linked to issues Sep 19, 2023

drivers: eth_mcux: manage PHY reset without interrupt GPIO #59709

Closed

Support for other PHYs than KSZ8081 missing in mcux_eth / nxp kinetis ethernet driver #49995

Closed

decsny force-pushed the feature/nxp_enet branch from 937574b to c46caf6 Compare September 26, 2023 15:16

decsny marked this pull request as ready for review September 26, 2023 15:17

decsny requested review from nashif, carlescufi, galak, MaureenHelm, mmahadevan108, dleach02, tbursztyka, jukkar and rlubos as code owners September 26, 2023 15:17

zephyrbot added area: Clocks area: Devicetree Binding PR modifies or adds a Device Tree binding area: Ethernet area: Clock Control platform: NXP NXP platform: Microchip MEC Microchip MEC Platform labels Sep 26, 2023

zephyrbot requested review from danieldegrasse, DerekSnell, EmilioCBen, manuargue, PetervdPerk-NXP and yvanderv September 26, 2023 15:18

zephyrbot assigned dleach02 Sep 26, 2023

decsny requested a review from pdgendt November 22, 2023 17:54

pdgendt reviewed Nov 23, 2023

View reviewed changes

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

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

decsny force-pushed the feature/nxp_enet branch from 6b89377 to 24bca16 Compare November 27, 2023 20:29

decsny requested a review from pdgendt November 27, 2023 20:32

decsny added 16 commits November 28, 2023 09:09

drivers: clock_control: mcux_ccm: Add ENET clock

81cb6f2

Add ENET clock value to the CCM clock decoder for both RT10XX and RT11XX CCM versions. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

dts: bindings: ethernet-controller: Add phy mode

acc85b5

Add a property to the ethernet controller binding indicating what type of connection the MAC has with the PHY device. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

dts: bindings: Add binding for KSZ8081 PHY

3669796

Add DT Binding for Microchip KSZ8081 PHY Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

drivers: ethernet: phy: Add KSZ8081 PHY Driver

62be4c7

Add Driver for KSZ8081 Ethernet PHY. The Generic MII Driver is not sufficient to use for this PHY chip which has special vendor implemented behaviors. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

dts: bindings: Add NXP ENET bindings

a5f4675

Add bindings for compatibles related to NXP ENET IP. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

include: NXP ENET driver include header

8b41427

Create an include header to be used by NXP ENET drivers. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

drivers: mdio: Add NXP ENET MDIO driver

57ab744

Add driver for NXP ENET MDIO functionalities Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

soc: rt10xx: enable phy clock with new driver

3784165

when using either old or new driver for nxp enet, enable the phy clock Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

soc: nxp: rt10xx: Increase workqueue size for enet

e13f28a

Increase the size of the system workqueue stack if using nxp ethernet driver Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

drivers: clock_control_mcux_ccm: Add ENET PLL clk

5dc6528

Add subsys value for ENET PLL / ENET Ref clk to CCM Driver Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

dts: bindings: Add NXP ENET PTP binding

6787d6b

Add binding for NXP ENET PTP Clock device Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

drivers: ptp_clock: Add init priority Kconfig

21b8c4c

Add ptp clock driver init priority kconfig Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

drivers: ptp_clock: Add NXP ENET PTP Clock Driver

7650ef4

Add Driver for NXP ENET PTP Clock device Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

drivers: eth_nxp_enet: Support PTP

35b6aed

Support PTP functionality in NXP ENET MAC driver Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

boards: mimxrt10xx_evk: Add new enet overlay

a7a1510

Add overlay to show how to use new experimental nxp enet driver Also make any eth_mcux related kconfigs dependent on eth_mcux in the boards' defconfigs. Signed-off-by: Declan Snyder <declan.snyder@nxp.com>

manuargue previously approved these changes Nov 28, 2023

View reviewed changes

decsny dismissed manuargue’s stale review via a7a1510 November 28, 2023 15:11

decsny force-pushed the feature/nxp_enet branch from 24bca16 to a7a1510 Compare November 28, 2023 15:11

decsny added this to the v3.6.0 milestone Nov 28, 2023

pdgendt approved these changes Nov 28, 2023

View reviewed changes

dleach02 approved these changes Nov 28, 2023

View reviewed changes

danieldegrasse approved these changes Nov 28, 2023

View reviewed changes

dleach02 merged commit 789addb into zephyrproject-rtos:main Nov 28, 2023
28 checks passed

talih0 reviewed Nov 28, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New NXP ENET driver #62833

New NXP ENET driver #62833

decsny commented Sep 19, 2023 •

edited

Loading

zephyrbot commented Sep 19, 2023 •

edited

Loading

decsny commented Nov 28, 2023

manuargue left a comment

pdgendt Nov 28, 2023

pdgendt Nov 28, 2023

danieldegrasse left a comment

danieldegrasse Nov 28, 2023

talih0 Nov 28, 2023

decsny Dec 6, 2023

talih0 Nov 28, 2023

decsny Dec 6, 2023

talih0 Nov 28, 2023

decsny Dec 6, 2023 •

edited

Loading

talih0 Nov 28, 2023

decsny Dec 6, 2023

talih0 Nov 28, 2023


		extern void nxp_enet_mdio_callback(const struct device *mdio_dev,
		enum nxp_enet_callback_reason event);

New NXP ENET driver #62833

New NXP ENET driver #62833

Conversation

decsny commented Sep 19, 2023 • edited Loading

zephyrbot commented Sep 19, 2023 • edited Loading

decsny commented Nov 28, 2023

manuargue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danieldegrasse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

decsny Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

decsny commented Sep 19, 2023 •

edited

Loading

zephyrbot commented Sep 19, 2023 •

edited

Loading

decsny Dec 6, 2023 •

edited

Loading