Hi Stanislaw, 2012/7/19 Stanislaw Gruszka <sgruszka@xxxxxxxxxx>: > 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. will fix it. >> + >> + 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. I have no idea, Ralink gave it to me for the fixing. >> - if (rt2x00_get_field32(reg, PLL_LD) && >> - rt2x00_get_field32(reg, XTAL_RDY)) >> - break; > Here is how above part should look like. Sounds good to me. :) > Stanislaw > -- Thank you. -- 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