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.
[]
> > diff --git 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])]);
[]
> 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.
cheers, Joe
--
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
