| Message ID | 20190206123018.24802-2-thierry.reding@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v3,1/2] r8169: Load MAC address from device tree if present | expand |
On Wed, Feb 06, 2019 at 01:30:18PM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@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 <andrew@lunn.ch> > Signed-off-by: Thierry Reding <treding@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. > > Changes in v3: > - align MAC address to u16 for is_valid_ether_addr() > > drivers/net/ethernet/realtek/r8169.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) I also forgot Andrew's Reviewed-by here, but technically the patch has changed, so I'll leave it up to him to confirm he's still okay with this version. Thierry
On Wed, Feb 06, 2019 at 01:30:18PM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@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 <andrew@lunn.ch> > Signed-off-by: Thierry Reding <treding@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. > > Changes in v3: > - align MAC address to u16 for is_valid_ether_addr() Hi Thierry The point of this patch was to try to avoid the pointer aliasing, which is what leads to the alignment requirements. But if you are forced to align it because of is_valid_ether_addr() i would just drop this patch. Aliasing is going to happen whatever. But if you want to keep it. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
From: Thierry Reding <thierry.reding@gmail.com> Date: Wed, 6 Feb 2019 13:30:18 +0100 > From: Thierry Reding <treding@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 <andrew@lunn.ch> > Signed-off-by: Thierry Reding <treding@nvidia.com> Applied.
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index 501891be7c56..1dd72137fd53 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,8 @@ 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) = {}; + /* align to u16 for is_valid_ether_addr() */ + u8 mac_addr[ETH_ALEN] __aligned(2) = {}; struct rtl8169_private *tp; struct net_device *dev; int chipset, region, i;