> Subject: Re: [PATCH] bus/pci: nvme on Windows requires class id and bus > > External email: Use caution opening links or attachments > > > Hi Tal, > > Thanks for the comments. > > >> + /* Try and find PCI class ID */ > >> + for (cp = buf; !(cp[0] == 0 && cp[1] == 0); cp++) > > How about > > for (cp = buf; cp[0] || cp[1]; cp++) > That would be my preferred idiom, but the DPDK coding style (1.9.1) says 'do > not use ! for tests unless it is a boolean' (but somewhat confusingly does so > in a section on NULL pointers). I interpreted it as a general prohibition on > conditionals without an explicit operator (except for booleans). I'd love to > be > corrected here! > > >> + if (*cp == '&' && sscanf_s(cp, "&CC_%" PRIx32, > >> + &class_id) == 1) > > Could there be a case where PCI\\VEN_v(4)&DEV_d(4)&CC_c(2)s(2) exist > but PCI\\VEN_v(4)&DEV_d(4)&CC_c(2)s(2)p(2) doesn't? In that case the > parsing would be incorrect. > The MSDN documentation says that the most specific string will be returned > first, in this case &CC_c(2)s(2)p(2), so if there is a 6-digit class ID we > will > return it. That left me wondering what to do if we only encounter a 4-digit > class ID. If we ignore it, then we won't be able to match on the class ID. > We > could parse it as 4-digits and supply zero for the p(2) field, but that left > me > wondering why Windows wouldn't have already done that for us. What I see > on my own systems is that there is always a 6-digit class ID defined, even if > the pci-ids repository doesn't list any values for p(2) (e.g. > https://pci-ids.ucw.cz/read/PD/07/80 reports as &CC_078000). > > My conclusion was that was probably best just to return the first class ID > that > Windows reports even if it's only 4-digits. That way the device can still be > found and will match the CC_ info displayed in device manager.
I'd shift the class_id value to the right in case the returned value is 4 digit, just to verify the result would be "c(2)s(2) 0" and not "0 s(2)p(2)", however when testing your change I couldn't find a case where only 4 digits matched. Either than that LGTM. > > Thanks, > Nick
