On 04-06-08 01:03, Bjorn Helgaas wrote:
That's definitely backwards. I reversed the sizes, so we'll have 8 bits for the priority byte (including compatibility/performance/ robustness) and 16 bits for the dependent set number. Actually, I made the priority field 12 bits so we'd have space to keep PNP_RES_PRIORITY_INVALID as a truly out-of-band value.
Sounds perfect.
+ for (i = 0; i == 0 || i < dev->num_dependent_sets; i++) { + ret = pnp_assign_resources(dev, i); + if (ret == 0) return 0;Eeeew. Perhaps: i = 0; do { ret = pnp_assign_resources(dev, i); if (ret == 0) return 0; } while (++i < dev->num_dependent_sets);Heh :-) I vacillated on that one because I have a personal aversion to "do { ... } while ()", especially with a pre-increment. How would you feel about this alternative? ret = pnp_assign_resources(dev, 0); if (ret == 0) return 0; for (i = 1; i < dev->num_dependent_sets; i++) { ret = pnp_assign_resources(dev, i); if (ret == 0) return 0; }
You could fix the pre-increment by sticking a i++ inside the loop body but there's no arguing with personal aversions...
Yes, I think the latter is better. Straight-forward and clear.
Why do you do 0x800, 0x400 in that order? Shouldn't it just be 0x400, 0x800 to mimick the old order?I think they do end up in the correct order because I'm passing the same list_head to both list_add() calls, e.g., we'll have something like this: io -> ... io -> (io + 0x800) -> ... io -> (io + 0x400) -> (io + 0x800) -> ...
Yep. Just needed to see it happen once in the quirk testing I just now did.
I need to go back over all your comments and make sure I've addressed them all, then I'll post the revised patches, hopefully tomorrow. Thanks again for all your work reviewing and testing these. It's been incredibly useful.
I've been impressed by this work. This is a good redesign of PnP with a fully bisectable way to get there. And PnP was in need of some work...
Rene. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
