| Message ID | 1482598381-16513-7-git-send-email-pali.rohar@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
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
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.rohar@gmail.com> 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 <pali.rohar@gmail.com> Why? What issue does this fix?
On Friday 27 January 2017 09:56:09 Kalle Valo wrote: > Pali Rohár <pali.rohar@gmail.com> 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 <pali.rohar@gmail.com> > > Why? What issue does this fix? Send permanent MAC address to wl1251 chip, same what is doing wl12xx driver.
Pali Rohár <pali.rohar@gmail.com> writes: > On Friday 27 January 2017 09:56:09 Kalle Valo wrote: >> Pali Rohár <pali.rohar@gmail.com> 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 <pali.rohar@gmail.com> >> >> 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.
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); }
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 <pali.rohar@gmail.com> --- drivers/net/wireless/ti/wl1251/main.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)