On Thu, Jul 19, 2012 at 11:18:23AM +0800, Chen, Chien-Chia wrote: > + if (rt2x00_rt(rt2x00dev,RT3290)){ Please add space after a comma. > + retval = rt2800_enable_wlan_rt3290(rt2x00dev); > + if (retval) > + return -EBUSY; > + } Since we enabling device here, it's probably not needed to do this on rt2800pci_probe_hw, or I'm wrong? > + if ((rt2x00_get_field32(reg, PLL_LD) == 1) && > + (rt2x00_get_field32(reg, XTAL_RDY) == 1)) > + break; Use proper coding style here (see below). Seems you did not copy this function from rt2800pci.c, but take it form other source, so it could miss some other changes as well. > + > + rt2800_register_write(rt2x00dev, 0x58, 0x018); > + udelay(REGISTER_BUSY_DELAY); > + rt2800_register_write(rt2x00dev, 0x58, 0x418); > + udelay(REGISTER_BUSY_DELAY); > + rt2800_register_write(rt2x00dev, 0x58, 0x618); We really like to know what those black magic numbers mean. > - if (rt2x00_get_field32(reg, PLL_LD) && > - rt2x00_get_field32(reg, XTAL_RDY)) > - break; Here is how above part should look like. Stanislaw -- 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