Messages in this thread | | | Date | Wed, 04 Jun 2008 02:03:06 +0200 | From | Rene Herman <> | Subject | Re: [patch 15/15] PNP: convert resource options to single linked list |
| |
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.
| |