On Wed, Nov 10, 2021 at 2:28 PM Tom Rix <trix@xxxxxxxxxx> wrote: > On 11/10/21 12:24 AM, Andy Shevchenko wrote: > > On Tue, Nov 09, 2021 at 10:27:58AM -0800, Tom Rix wrote: > >> On 11/9/21 10:05 AM, Andy Shevchenko wrote: > >>> On Tue, Nov 09, 2021 at 07:55:43AM -0800, Tom Rix wrote: > >>>> On 11/9/21 7:41 AM, Andy Shevchenko wrote: > > ... > > > >>>>> + voff = pci_find_vsec_capability(dev, PCI_VENDOR_ID_INTEL, PCI_VSEC_ID_INTEL_DFLS); > >>>> This may be a weakness in the origin code, but intel isn't the exclusive > >>>> user of DFL. > >>> This does not change the original code. If you think so, this can be extended > >>> later on. > >> I would rather see this fixed now or explained why this isn't a problem. > > This is out of scope of this change in a few ways: > > - we don't do 2+ things in one patch > > - the change doesn't change behaviour > > - the change is a simple cleanup > > - another vendor may well have quite different VSEC ID for DFL > > > > If you think that it should be needed, one can come up with it later on. > > Fixing a problem is more useful than a cleanup. The fix should come first. What do you mean by that? The original code never worked with what you are suggesting. There is nothing to fix in terms of "fix". What you are proposing is a feature. And as we know the features are going into the kernel in a natural order, means fixes - priority 1, cleanups / refactoring as prerequisites to the feature enabling - priority 2, feature - priority 3, other cleanups and code improvements - priority 4. That said, the proposed change definitely falls into category 2. It makes the proposed feature to be easily realized. Also, do not forget that vendor specific stuff is _by definition_ vendor specific, and the proposed feature is doubtful until you prove there is another vendor-id pair. -- With Best Regards, Andy Shevchenko
