On 04.02.2019 17:42, Thierry Reding wrote: > From: Thierry Reding <tred...@nvidia.com> > > Read MAC address 32-bit at a time and manually extract the individual > bytes. This avoids pointer aliasing and gives the compiler a better > chance of optimizing the operation. > > Suggested-by: Andrew Lunn <and...@lunn.ch> > Signed-off-by: Thierry Reding <tred...@nvidia.com> > --- > Applies to net-next. > > I tested this on a Jetson TX2 with an add-in Realtek ethernet card that > has a properly programmed OTP to verify that I got the endianess right. > Seems like everything works and the device behaves the same with or > without this patch. > > drivers/net/ethernet/realtek/r8169.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/realtek/r8169.c > b/drivers/net/ethernet/realtek/r8169.c > index 501891be7c56..192fbb36bc9f 100644 > --- a/drivers/net/ethernet/realtek/r8169.c > +++ b/drivers/net/ethernet/realtek/r8169.c > @@ -7113,12 +7113,21 @@ static int rtl_alloc_irq(struct rtl8169_private *tp) > static void rtl_read_mac_address(struct rtl8169_private *tp, > u8 mac_addr[ETH_ALEN]) > { > + u32 value; > + > /* Get MAC address */ > switch (tp->mac_version) { > case RTL_GIGA_MAC_VER_35 ... RTL_GIGA_MAC_VER_38: > case RTL_GIGA_MAC_VER_40 ... RTL_GIGA_MAC_VER_51: > - *(u32 *)&mac_addr[0] = rtl_eri_read(tp, 0xe0, ERIAR_EXGMAC); > - *(u16 *)&mac_addr[4] = rtl_eri_read(tp, 0xe4, ERIAR_EXGMAC); > + value = rtl_eri_read(tp, 0xe0, ERIAR_EXGMAC); > + mac_addr[0] = (value >> 0) & 0xff; > + mac_addr[1] = (value >> 8) & 0xff; > + mac_addr[2] = (value >> 16) & 0xff; > + mac_addr[3] = (value >> 24) & 0xff; > + > + value = rtl_eri_read(tp, 0xe4, ERIAR_EXGMAC); > + mac_addr[4] = (value >> 0) & 0xff; > + mac_addr[5] = (value >> 8) & 0xff; > break; > default: > break; > @@ -7316,7 +7325,7 @@ static int rtl_get_ether_clk(struct rtl8169_private *tp) > static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id > *ent) > { > const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data; > - u8 mac_addr[ETH_ALEN] __aligned(4) = {}; > + u8 mac_addr[ETH_ALEN] = {}; > struct rtl8169_private *tp; > struct net_device *dev; > int chipset, region, i; > I just have one concern / question:
After this there's a call to is_valid_ether_addr(mac_addr) and kernel-doc of is_valid_ether_addr() states that argument must be u16-aligned. AFAIK that's not guaranteed for a byte array. Heiner