On Fri, 2018-10-12 at 21:55 +0000, Adham.Abozaeid@xxxxxxxxxxxxx wrote: > > Here the driver is configuring parameters in the device by sending a > WID command for each parameters. > The val pointer points to the value of the parameter to be set, and > here all parameters being set to 0 were sharing the dummyval variable. Ok. > Looking again at this, these constant parameters can be omitted from > the driver and done on the device instead. I think it's fine, just the pointers look strange. Would probably be different once you resolve the middle layer though. > > > + if (conn_attr->bssid) > > > + memcpy(cur_byte, conn_attr->bssid, 6); > > > + cur_byte += 6; > > > > u8 bssid[ETH_ALEN]; > > > > > + if (conn_attr->bssid) > > > + memcpy(cur_byte, conn_attr->bssid, 6); > > > + cur_byte += 6; > > > > again? > > Agree. Can be changed to avoid duplication. Requires a matching change on the device. Again, like above, don't worry too much about changing the device/firmware. I was just pointing out it's duplicate, if that's the way it is then too bad, but it doesn't really matter. The more interesting thing here is to change it to use a struct and not pack it manually. johannes
