From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from perches-mx.perches.com ([206.117.179.246]:52605 "EHLO labridge.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S934126Ab3BTRba (ORCPT ); Wed, 20 Feb 2013 12:31:30 -0500 Message-ID: <1361381489.3065.1.camel@joe-AO722> (sfid-20130220_183135_340252_E646A02A) Subject: Re: [PATCH] ssb: fix unaligned access to mac address From: Joe Perches To: =?UTF-8?Q?Rafa=C5=82_Mi=C5=82ecki?= Cc: Hauke Mehrtens , linville@tuxdriver.com, linux-wireless@vger.kernel.org Date: Wed, 20 Feb 2013 09:31:29 -0800 In-Reply-To: References: <1361021140-19871-1-git-send-email-hauke@hauke-m.de> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Mon, 2013-02-18 at 17:09 +0100, Rafał Miłecki wrote: > 2013/2/16 Hauke Mehrtens : > > The mac address should be aligned to u16 to prevent an unaligned access > > in drivers/ssb/pci.c where it is casted to __be16. [] > > diff --git a/include/linux/ssb/ssb.h b/include/linux/ssb/ssb.h [] > > @@ -26,6 +26,7 @@ struct ssb_sprom_core_pwr_info { > > > > struct ssb_sprom { > > u8 revision; > > + u8 country_code; /* Country Code */ > > u8 il0mac[6]; /* MAC address for 802.11b/g */ > > It looks a little hacky to me too, it's easy to forget about that > requirement and break that again in the future. > > What about not casting il0mac to u16 at all? Maybe we should just fill > it as u8 (which it is)? > Perhaps this? From: Joe Perches Subject: [PATCH] ssb: pci: Standardize a function to get mac address Don't require alignment of mac addresses to u16. Signed-off-by: Joe Perches --- drivers/ssb/pci.c | 44 ++++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/drivers/ssb/pci.c b/drivers/ssb/pci.c index e9d9496..62fc9b5 100644 --- a/drivers/ssb/pci.c +++ b/drivers/ssb/pci.c @@ -231,6 +231,15 @@ static inline u8 ssb_crc8(u8 crc, u8 data) return t[crc ^ data]; } +static void sprom_get_mac(char *mac, const u16 *in) +{ + int i; + for (i = 0; i < 3; i++) { + *mac++ = in[i]; + *mac++ = in[i] >> 8; + } +} + static u8 ssb_sprom_crc(const u16 *sprom, u16 size) { int word; @@ -341,8 +350,6 @@ static s8 r123_extract_antgain(u8 sprom_revision, const u16 *in, static void sprom_extract_r123(struct ssb_sprom *out, const u16 *in) { - int i; - u16 v; u16 loc[3]; if (out->revision == 3) /* rev 3 moved MAC */ @@ -352,19 +359,10 @@ static void sprom_extract_r123(struct ssb_sprom *out, const u16 *in) loc[1] = SSB_SPROM1_ET0MAC; loc[2] = SSB_SPROM1_ET1MAC; } - for (i = 0; i < 3; i++) { - v = in[SPOFF(loc[0]) + i]; - *(((__be16 *)out->il0mac) + i) = cpu_to_be16(v); - } + sprom_get_mac(out->il0mac, &in[SPOFF(loc[0])]); if (out->revision < 3) { /* only rev 1-2 have et0, et1 */ - for (i = 0; i < 3; i++) { - v = in[SPOFF(loc[1]) + i]; - *(((__be16 *)out->et0mac) + i) = cpu_to_be16(v); - } - for (i = 0; i < 3; i++) { - v = in[SPOFF(loc[2]) + i]; - *(((__be16 *)out->et1mac) + i) = cpu_to_be16(v); - } + sprom_get_mac(out->et0mac, &in[SPOFF(loc[1])]); + sprom_get_mac(out->et1mac, &in[SPOFF(loc[2])]); } SPEX(et0phyaddr, SSB_SPROM1_ETHPHY, SSB_SPROM1_ETHPHY_ET0A, 0); SPEX(et1phyaddr, SSB_SPROM1_ETHPHY, SSB_SPROM1_ETHPHY_ET1A, @@ -454,19 +452,15 @@ static void sprom_extract_r458(struct ssb_sprom *out, const u16 *in) static void sprom_extract_r45(struct ssb_sprom *out, const u16 *in) { - int i; - u16 v; u16 il0mac_offset; if (out->revision == 4) il0mac_offset = SSB_SPROM4_IL0MAC; else il0mac_offset = SSB_SPROM5_IL0MAC; - /* extract the MAC address */ - for (i = 0; i < 3; i++) { - v = in[SPOFF(il0mac_offset) + i]; - *(((__be16 *)out->il0mac) + i) = cpu_to_be16(v); - } + + sprom_get_mac(out->il0mac, &in[SPOFF(il0mac_offset)]); + SPEX(et0phyaddr, SSB_SPROM4_ETHPHY, SSB_SPROM4_ETHPHY_ET0A, 0); SPEX(et1phyaddr, SSB_SPROM4_ETHPHY, SSB_SPROM4_ETHPHY_ET1A, SSB_SPROM4_ETHPHY_ET1A_SHIFT); @@ -530,7 +524,7 @@ static void sprom_extract_r45(struct ssb_sprom *out, const u16 *in) static void sprom_extract_r8(struct ssb_sprom *out, const u16 *in) { int i; - u16 v, o; + u16 o; u16 pwr_info_offset[] = { SSB_SROM8_PWR_INFO_CORE0, SSB_SROM8_PWR_INFO_CORE1, SSB_SROM8_PWR_INFO_CORE2, SSB_SROM8_PWR_INFO_CORE3 @@ -539,10 +533,8 @@ static void sprom_extract_r8(struct ssb_sprom *out, const u16 *in) ARRAY_SIZE(out->core_pwr_info)); /* extract the MAC address */ - for (i = 0; i < 3; i++) { - v = in[SPOFF(SSB_SPROM8_IL0MAC) + i]; - *(((__be16 *)out->il0mac) + i) = cpu_to_be16(v); - } + sprom_get_mac(out->il0mac, &in[SSB_SPROM8_IL0MAC]); + SPEX(board_rev, SSB_SPROM8_BOARDREV, 0xFFFF, 0); SPEX(alpha2[0], SSB_SPROM8_CCODE, 0xff00, 8); SPEX(alpha2[1], SSB_SPROM8_CCODE, 0x00ff, 0); -- 1.8.1.2.459.gbcd45b4.dirty