On Fri, 2018-10-12 at 21:55 +0000, adham.aboza...@microchip.com 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

Reply via email to