Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only call aspeednic_init when preparing to use network #4

Closed

Conversation

mdmillerii
Copy link

The aspeednic driver was calling init from its initialization function and
calling halt later. This left the DMA engine running and writing to
memory when Ethernet traffic arrived when control was handed off
to the operating system.

Remove this call to init and rely on the framework to call
write_hwaddr with the assigned MAC address.

In addition upstream discourages setting the random address in
drivers. Later upstream created a config for this behavior and I
back-ported that patch.

In my testing, the random mac address is about 90% 1 value after a
u-boot reset command but after a kernel reboot it does not repeat.

The code uses rand with timer as the seed source; the network stack
then uses the mac as the seed for random delays. We may prefer
to only configure u-boot setting a random mac only after a better
source of entropy, letting the kernel use its sources to set it when
we do not net-boot.

milton


v2: update README and README.ethaddr with backported CONFIG var name.

This change is Review on Reviewable

mdmillerii and others added 5 commits March 10, 2016 19:36
Tell the ethernet framework how to set the MAC address on the
aspeed nic so it can be set at the expected points in the code.

Rename set_mac_address to aspeed_write_hwaddr.  Drop the unused
argument, change the prototype to int, and return 0.

Assign the device write_hwaddr method to this new function.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
Delete the call to aspeenic_init from the aspeenic_initialize
function.  Instead rely on the framework to call the write_hwaddr
method to set the MAC address for the operating system to use.

The aspeednic driver was calling init before registering the
hardware, causing the dma engine to be left running writing
incoming ethernet packets to memory when handing control to the
operating system.

The initialize function is specifically documented to not touch
the hardware or probe in doc/README.drivers.net.

Also, the init method can take a significant amount of time
especially when NC-SI and retries are involved.  In addition
extra work is being performed to find the MAC address which the
framework will do again later.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
The device method assignments had extra spaces before the =
signs but all prior methods were 4 characters and so they
were aligned.  Since write_hwaddr is significantly longer,
the resulting spacing is not pleasing.  Since only a few other
sites had simiar whitespace replace all multi-space sequences
before an assignemnt with one space.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
Commit bef1014 upstream.

Backport changes CONFIG_NET_RANDOM_ETHADDR to CONFIG_RANDOM_MACADDR,
net_random_ethaddr to eth_random_enetaddr, and is_zero_ethaddr to
is_zero_ether_addr, and keeps existing function order.

Upstream log:

Implement the random ethaddr fallback in eth.c so it is in a common
place and not reimplemented in each board or driver that wants this
behavior.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

Signed-off-by: Milton D. Miller II <miltonm@us.ibm.com>
Do not check the u-boot environemnt or fill in a random address
in the write_hwaddr hook, instead rely on the framework to do so.

The doc/README.ethaddr specifically states that random addresses
are only to be assigned as part of a emergency such as a netboot
recovery command.

The upstream commit created a config variable to assign a
random mac when none is set leaving it zero and that has now
been backported.

Note: The hardware address is reset to 0 as part of the ethernet
reset performed at boot.  If no valid MAC address is found in
the environment the hardware will contain zeros and the operating
system will assign a valid random MAC address if u-boot is
configured not to.

The net effect is an attempt to use the network will result in
the ethernet address not set warning being printed if the ethaddr
variable is not set, and a warning iwth the random mac address
if the config is set.  If a valid ethernet address is set
in the environment it will be programmed in the hardware and
used by the operating system.

Signed-off-by: Milton Miller <miltonm@us.ibm.com>
@shenki
Copy link
Member

shenki commented Mar 29, 2016

Merged to v2013.07-aspeed-openbmc branch

@shenki shenki closed this Mar 29, 2016
eddiejames pushed a commit to eddiejames/u-boot that referenced this pull request Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants