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
Hauke
--
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
