<font face="Default Sans Serif,Verdana,Arial,Helvetica,sans-serif" size="2"><br><br><font color="#990099">-----Cyril Bur <<a target="_blank" href="mailto:cyrilbur@gmail.com">cyrilbur@gmail.com</a>> wrote: -----</font><br><br>>OpenBMC Patches <<a target="_blank" href="mailto:openbmc-patches@stwcx.xyz">openbmc-patches@stwcx.xyz</a>> wrote:<br>><br>>> From: "Milton D. Miller II" <<a target="_blank" href="mailto:miltonm@us.ibm.com">miltonm@us.ibm.com</a>><br>>> <br>><br>>Hi Milton,<br>><br>>Series looks nice.<br>><br>>One question about this patch, should you also have removed the call<br>>to<br>>aspeednick_write_hwaddr() from aspeednic_initialize() since the eth<br>>layer will<br>>do it for you? Not a bit deal at all, the eth layer will also detect<br>>that it's<br>>been done and just skip it?<br>><br><br>Perhaps it could, but it seems like more analysis may be required.<br><br>The generic code doesn't call write_hwaddr if the mac is invalid (a<br>broadcast address) and therefore removing the code might result<br>in different behavior.<br><br>The check for the ethernet address being set in the environment<br>seems to occur after the init function is called.  Since its only<br>two mmios with no delays I'd rather leave the code inplace at<br>least at this time.<br><br>thanks,<br>milton<br></font><BR>