Skip to content

Conversation

@TonyHan11
Copy link
Contributor

The purpose of this PR is to update sam_gmac driver from supporting only one instance to supporting multiple instances.

The following two files are updated:

  • drivers/ethernet/eth_sam_gmac.c
  • drivers/ethernet/eth_sam_gmac_priv.h

Changes include:

  • centralize the lists used for different GMAC queues
  • add instance and num_queues to configuration
  • update for supporting multi GMAC instances

Comment on lines 267 to 269
uint32_t instance:8;
uint32_t num_queues:8;
uint32_t reserved:16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why bitfields instead of one uint16_t and two uint8_t?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No special reasons for using bitfields just it's easier to keep the structure word aligned.
Will be updated with uint16_t and uint8_t, thanks.

@fabiobaltieri
Copy link
Member

Hey, this needs a rebase and force push to pick up the CI error workaround.

@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch from e667bf9 to 2f68375 Compare June 5, 2025 09:16
@TonyHan11
Copy link
Contributor Author

@maass-hamburg
Thank you for your comments, updated with the following changes:

  • remove bitfield variable instance and use dev->name for logging
  • assign .mac_addr using macro DT_INST_PROP_OR()
  • move DT_INST_FOREACH_STATUS_OKAY to the end and use it only once
  • remove init_done from the initialize procedure
  • PHY connection type: add multi instances support, remove build assert check (errors reported at runtime)
  • add multi instances support for MAC address

#define GMAC_QUEUE_NUM DT_INST_PROP(0, num_queues) in the header file not changed. Currently, we assume that the queue numbers of all instances are the same, otherwise the code and macros related to GMAC_QUEUE_NUM need to be updated. The code and macros are also be affected by some parameters come from CONFIG_. It's too complicated to put them in one pull request.

@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch from 2f68375 to a6537c6 Compare June 5, 2025 09:22
@TonyHan11
Copy link
Contributor Author

Rebased on the latest main branch to pick up the CI error workaround.

@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch 2 times, most recently from dd166f8 to 69c7a1f Compare June 9, 2025 03:02
@github-actions github-actions bot requested review from kartben and nashif June 9, 2025 03:03
@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch from 69c7a1f to 0bf5ef1 Compare June 9, 2025 03:31
@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch 2 times, most recently from d04c0e2 to 75008f4 Compare June 12, 2025 02:16
@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch from b54ec29 to 07fdcb3 Compare November 5, 2025 07:05
@TonyHan11
Copy link
Contributor Author

line 34:

#define GMAC_QUEUE_NUM DT_INST_PROP(0, num_queues)

still depends on the first instance

still relevant, now line 30

Yes, the correlation needs to be solved. GMAC_QUEUE_NUM is used as the array sizeof of queue_list in struct eth_sam_dev_data. It comes from the num_queues of the first GMAC instance in DT.

The same num_queues is also used in Kconfig.sam_gmac for some configurations (also be used for SAMX7X, SAM4E and SAME54 series MCUs). Re-organize the configurations into the DT is a bit complicated and it's too big to put in a same PR with about 10 commits.

In this PR just keep it as is and validate it with BUILD_ASSERT in the last commit. It maybe solved in an additional pull request. Thank you very much for your comment.

@TonyHan11
Copy link
Contributor Author

Updated with the following:

  • Add validation of max_frame_size for jumbo frame size at the build stage
  • Add a new commit for ref_clk_source for multi instances support

Add variable "phy_conn_type" to get and use the phy_connection_type from
DT for different GMAC instances.
Update the judgement on phy_connection_type for multi instances support.

Signed-off-by: Tony Han <[email protected]>
Add variable "ref_clk_source" to get and set the source for the GMAC
reference clock from DT for different GMAC instances.

Signed-off-by: Tony Han <[email protected]>
To allow every interface be initialized properly when there are more than
one instance, remove the static variable "init_done" which is used to make
the initialize procedure only be done once.

Signed-off-by: Tony Han <[email protected]>
@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch from 07fdcb3 to 1b93805 Compare November 6, 2025 08:23
As jumbo frame size is not supported by the networking subsystem, only
max_frame_size 1518 and 1536 can be used. The Frame size 1536 would
allow for packets with a vlan tag, so enable GMAC_NCFGR_MAXFS when
NET_VLAN is configured.

Signed-off-by: Tony Han <[email protected]>
Deprecate the 'ETH_SAM_GMAC_MAC_I2C_EEPROM' for the 'mac-eeprom' option,
Limite it to be used when there's only one activated GMAC instance.

Signed-off-by: Tony Han <[email protected]>
Add variable 'random_mac_addr' for 'zephyr,random-mac-address' from
device tree. Update generate_mac() to get random MAC address for each
GAMC interface with the 'zephyr,random-mac-address' property.

Signed-off-by: Tony Han <[email protected]>
@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch from 1b93805 to f27d14a Compare November 6, 2025 08:50
'GMAC_QUEUE_NUM' is a value of 'num-queues' for the first GMAC
instance getting from the device tree. It is used directly or
indirectly (by GMAC_PRIORITY_QUEUE_NUM) for defining and
initializing 'struct eth_sam_dev_data' with a value from Kconfig
(GMAC_ACTIVE_PRIORITY_QUEUE_NUM).

As there will be a big change for applying the corresponding
num-queues for each GMAC 'struct eth_sam_dev_data', here just
keep it as is. Adding the BUITD_ASSERT to make sure the array
queue_list[] is large enough for all GMAC instances.

Signed-off-by: Tony Han <[email protected]>
@maass-hamburg
Copy link
Member

line 34:

#define GMAC_QUEUE_NUM DT_INST_PROP(0, num_queues)

still depends on the first instance

still relevant, now line 30

Yes, the correlation needs to be solved. GMAC_QUEUE_NUM is used as the array sizeof of queue_list in struct eth_sam_dev_data. It comes from the num_queues of the first GMAC instance in DT.

The same num_queues is also used in Kconfig.sam_gmac for some configurations (also be used for SAMX7X, SAM4E and SAME54 series MCUs). Re-organize the configurations into the DT is a bit complicated and it's too big to put in a same PR with about 10 commits.

In this PR just keep it as is and validate it with BUILD_ASSERT in the last commit. It maybe solved in an additional pull request. Thank you very much for your comment.

Please open a issue for that, so we don't forget it later

@TonyHan11
Copy link
Contributor Author

Hi, @maass-hamburg ,

Please open a issue for that, so we don't forget it later

Created #99051, thanks.

@TonyHan11 TonyHan11 force-pushed the gmac_multi_instance branch 2 times, most recently from 7b894ed to ef34457 Compare November 10, 2025 09:26
The 'max-speed' property in atmel,gmac-common.yaml file is not used,
remove  it.

Signed-off-by: Tony Han <[email protected]>
@sonarqubecloud
Copy link

@nandojve
Copy link
Member

Ping @jukkar

@nashif nashif merged commit 8876d0e into zephyrproject-rtos:main Nov 14, 2025
27 checks passed
@TonyHan11 TonyHan11 deleted the gmac_multi_instance branch November 17, 2025 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants