Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data

2017-01-27 Thread Pali Rohár
On Friday 27 January 2017 09:56:09 Kalle Valo wrote:
> Pali Rohár  writes:
> 
> > In case there is no valid MAC address kernel generates random one. This
> > patch propagate this generated MAC address back to NVS data which will be
> > uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
> > uses.
> >
> > Signed-off-by: Pali Rohár 
> 
> Why? What issue does this fix?

Send permanent MAC address to wl1251 chip, same what is doing wl12xx
driver.

-- 
Pali Rohár
pali.ro...@gmail.com


Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data

2017-01-27 Thread Kalle Valo
Pali Rohár  writes:

> On Friday 27 January 2017 09:56:09 Kalle Valo wrote:
>> Pali Rohár  writes:
>> 
>> > In case there is no valid MAC address kernel generates random one. This
>> > patch propagate this generated MAC address back to NVS data which will be
>> > uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
>> > uses.
>> >
>> > Signed-off-by: Pali Rohár 
>> 
>> Why? What issue does this fix?
>
> Send permanent MAC address to wl1251 chip, same what is doing wl12xx
> driver.

Ok, so this doesn't change functionality in any way and you are adding
it only because wl12xx does the same? You should document that in the
the commit log.

If there's no change I don't really see the point of this. But if
there's harm, hopefully, I guess it's ok.

-- 
Kalle Valo


Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data

2017-01-26 Thread Kalle Valo
Pali Rohár  writes:

> In case there is no valid MAC address kernel generates random one. This
> patch propagate this generated MAC address back to NVS data which will be
> uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
> uses.
>
> Signed-off-by: Pali Rohár 

Why? What issue does this fix?

-- 
Kalle Valo


Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data

2016-12-24 Thread Pali Rohár
On Saturday 24 December 2016 19:17:30 Pavel Machek wrote:
> Hi!
> 
> > In case there is no valid MAC address kernel generates random one.
> > This patch propagate this generated MAC address back to NVS data
> > which will be uploaded to wl1251 chip. So HW would have same MAC
> > address as linux kernel uses.
> > 
> > return 0;
> >  
> >  }
> > 
> > +static int wl1251_write_nvs_mac(struct wl1251 *wl)
> > +{
> 
> The name is quite confusing, this sounds like writing into
> non-volatile storage.
> 
> > +   int i;
> > +
> > +   if (wl->nvs_len < 0x24)
> > +   return -ENODATA;
> > +
> > +   /* length is 2 and data address is 0x546c (mask is 0xfffe) */
> 
> You don't actually check for the mask.

It is quite complicated. { 0x6d, 0x54 } (= 0x546d) in data represent 
address 0x546c and content are data. You need to apply mask 0xfffe for 
0x546d and you get address where data will be written (so 0x546c).

> > +   if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b]
> > != 0x54) +  return -EINVAL;
> 
> You have two copies of these. Does it make sense to move it to helper
> function?

I'm thinking if checks is really needed. But probably moving it to 
separate function is good idea.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.


Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data

2016-12-24 Thread Pavel Machek
Hi!

> In case there is no valid MAC address kernel generates random one. This
> patch propagate this generated MAC address back to NVS data which will be
> uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
> uses.

>   return 0;
>  }
>  
> +static int wl1251_write_nvs_mac(struct wl1251 *wl)
> +{

The name is quite confusing, this sounds like writing into
non-volatile storage.

> + int i;
> +
> + if (wl->nvs_len < 0x24)
> + return -ENODATA;
> +
> + /* length is 2 and data address is 0x546c (mask is 0xfffe) */

You don't actually check for the mask.

> + if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b] != 
> 0x54)
> + return -EINVAL;

You have two copies of these. Does it make sense to move it to helper
function?

Thanks,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


[PATCH 6/6] wl1251: Set generated MAC address back to NVS data

2016-12-24 Thread Pali Rohár
In case there is no valid MAC address kernel generates random one. This
patch propagate this generated MAC address back to NVS data which will be
uploaded to wl1251 chip. So HW would have same MAC address as linux kernel
uses.

Signed-off-by: Pali Rohár 
---
 drivers/net/wireless/ti/wl1251/main.c |   20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/net/wireless/ti/wl1251/main.c 
b/drivers/net/wireless/ti/wl1251/main.c
index 1454ba2..895ae05 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1555,6 +1555,24 @@ static int wl1251_read_nvs_mac(struct wl1251 *wl)
return 0;
 }
 
+static int wl1251_write_nvs_mac(struct wl1251 *wl)
+{
+   int i;
+
+   if (wl->nvs_len < 0x24)
+   return -ENODATA;
+
+   /* length is 2 and data address is 0x546c (mask is 0xfffe) */
+   if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b] != 
0x54)
+   return -EINVAL;
+
+   /* MAC is stored in reverse order */
+   for (i = 0; i < ETH_ALEN; i++)
+   wl->nvs[0x1c + i] = wl->mac_addr[ETH_ALEN - i - 1];
+
+   return 0;
+}
+
 static int wl1251_register_hw(struct wl1251 *wl)
 {
int ret;
@@ -1620,6 +1638,8 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
memcpy(wl->mac_addr, nokia_oui, 3);
get_random_bytes(wl->mac_addr + 3, 3);
+   if (!wl->use_eeprom)
+   wl1251_write_nvs_mac(wl);
wl1251_warning("MAC address in eeprom or nvs data is not 
valid");
wl1251_warning("Setting random MAC address: %pM", wl->mac_addr);
}
-- 
1.7.9.5