On Wednesday 04 November 2009 23:55:39 Julian Calaby wrote: > Bart, > > FWIW, this all looks good to me, except for these comments: > > 1. When you introduce struct rt2800_ops, it may telegraph your > intentions more clearly if you introduce rt2800lib.h at the same time > - this also means that we don't have (if only for a single patch) > duplicate versions of this structure and it's associated code. The current order is mostly the result of incremental steps leading to the final conclusions so indeed it can be polished a bit now. > 2. Patches #26-28 should arguably come before the conversions to use > the struct rt2800_ops methods. Done, also the patch adding rt2800lib.h has been moved in front the ones adding rt2800_ops. [ I've kept all ACKs in affected patches, I hope people are fine with it. ] > 3. I don't get the reasoning behind patch #37 (remove useless ifdefs > from rt2x00leds.h) but I'm going to assume that it's all right. struct rt2x00_led is referenced in rt2800lib.h so instead of adding more ifdefs to fix build I removed needless ones. > 4. Patch #39 should arguably come earlier in the patch set as it's a > general cleanup. This was also needed to fix build (for patch #40 IIRC). I've moved #37 and #39 near the beginning of the patch series (after "rt2x00: fix rt2x00usb_register_read() comment" patch). The rt2800 tree has been updated to reflect above changes (rt2800-v2.1 branch is now the current one), if somebody would like to see patches please ping me (I think that such minor updates don't justify spamming mailing list w/ 41 patches but that's just me). Thanks. -- Bartlomiej Zolnierkiewicz -- 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
