On 12/05/2009 01:55 PM, Hin-Tak Leung wrote:
> The patch looks fine - I'll have a go later. Stylistically, I don't
> think you need to copy the vendor driver though. Since you are storing
> rfkill_mask in the device structure, there is no need to have the
> variable tmp (and it is an awkful name - although, I know, it is
> temp!), because you can simply set the rfkill_mask at the beginning,
> then override it for newer models. e.g.
> I suggest you change the logic to something like this:
>
> priv->rfkill_mask = 0x02;
> if (newer product/gpio detection code) {
> priv->rfkill_mask = 0x04;
> }
>
> and while you are at it, called
> 0x02 RFKILL_MASK_FOR_MODEL_8187_89_97 and 0x04
> RFKILL_MASK_FOR_MODEL_8198 for clarity. Does it sound reasonable?
I got rid of the variable tmp; however due to the way that
eeprom_93cx6() returns its value, it is not possible to get all the
logic in one if statement. That section now looks like this:
/* Handle the differing GPIO bit in different models */
priv->rfkill_mask = RFKILL_MASK_8187_89_97;
if (product_id == 0x8197 || product_id == 0x8198) {
eeprom_93cx6_read(&eeprom, RTL8187_EEPROM_SELECT_GPIO, ®);
if (reg & 0xFF00)
priv->rfkill_mask = RFKILL_MASK_8198;
}
printk(KERN_INFO "rtl8187: rfkill mask set to %d for Product ID 0x%x",
priv->rfkill_mask, product_id);
Thanks for the suggestions.
Larry
--
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
