2013/2/21 Larry Finger <Larry.Finger@xxxxxxxxxxxx>: > On 02/20/2013 02:11 PM, Joe Perches wrote: >> >> On Wed, 2013-02-20 at 20:17 +0100, Hauke Mehrtens wrote: >>> >>> On 02/20/2013 07:29 PM, Joe Perches wrote: >>>> >>>> On Wed, 2013-02-20 at 11:56 -0600, Larry Finger wrote: >>>>> >>>>> On 02/20/2013 11:31 AM, Joe Perches wrote: >>>>>> >>>>>> On Mon, 2013-02-18 at 17:09 +0100, Rafał Miłecki wrote: >>>>>>> >>>>>>> 2013/2/16 Hauke Mehrtens <hauke@xxxxxxxxxx>: >>>>>>>> >>>>>>>> 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 <joe@xxxxxxxxxxx> >>>>>> Subject: [PATCH] ssb: pci: Standardize a function to get mac address >>>>>> >>>>>> Don't require alignment of mac addresses to u16. >>> >>> >>> ... >>> >>>>> I like the looks of sprom_get_mac() over that ugly *(((__be16 >>>>> *)out->il0mac) >>>>> construct, but this patch breaks ssb. The resulting MAC address is all >>>>> ones. I >>>>> have not yet figured out the problem. >>>> >>>> >>>> Dunno, I must have done something stupid. >>>> >>>> I don't have one of these but the transform >>>> looked correct when I did it. >>>> >>>> I'm not sure it's the best solution anyway >>>> because some of the other ether address >>>> functions like compare_ether_addr also >>>> require 2 byte alignment and cast to u16. >>> >>> >>> Your patch looks good (haven't tested it), but as you said we should >>> still align the mac addresses to u16 so functions like >>> compare_ether_addr work with them. >>> >>> There is also a v2 of my patch: >>> http://www.spinics.net/lists/linux-wireless/msg103628.html >> >> >> Seems OK to me, though I suggest using ETH_ALEN >> instead of 6. >> >> Maybe using both patches would be better still. >> >> And as long as I'm futzing with ssb, there's another >> logging cleanup to go with it. >> >> Here's a V2 with the missing SPOFF fixed. >> >> Subject: [PATCH 1/2] ssb: pci: Standardize a function to get mac address >> From: Joe Perches <joe@xxxxxxxxxxx> >> >> Don't require alignment of mac addresses to u16. >> >> Signed-off-by: Joe Perches <joe@xxxxxxxxxxx> >> --- >> >> V2: Add missing SPOFF > > > Tested-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx> I didn't confirm my issue to be related to this patch, but with recent kernels my MAC has changed from 00:11:22:33:44:55 to 11:00:33:22:55:44 Larry: did you verify MAC didn't change for you? -- Rafał -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html