On Tue, Feb 21, 2017 at 08:54:31PM -0700, Alex Williamson wrote: [...]
> I prefer the existing code. I don't really see why you consider it a > hack. I think it's pretty elegant that we can ignore the header > through the course of iterating through the capabilities, that we drop > other masked capabilities out of the chain, and that we can so easily > and transparently insert a zero ID at the end to serve the dual purpose > of replacing the temporary ID and nullifying the list if nothing was > added. The 0xffff capability ID is a perfectly safe assumption, not > only are we ridiculously far from allocating that ID, but it's arguably > a reserved value due to its use in the root complex register block. I > also don't see any evidence that it's error prone, the entire point is > that we can arbitrarily skip capability IDs in the body of the loop and > the result is a correct, minimal capability chain. OTOH, leaving > masked capabilities in the chain with an arbitrary version number seems > messy. I see. Then please also pick this one: Tested-by: Peter Xu <pet...@redhat.com> > > The real question is why are you sneaking the virtual channel > capability into the list of masked capabilities? Thanks, Oooops. I should remove that line. It's for my testing purpose (I need to "fake" a device that with 0x100 masked to test my patch, while my SD card reader did has this VC cap at 0x100 :-). Since we now have a choice already, please just ignore that line along with the whole patch. ;) Thanks, -- peterx
