Re: [PATCH V2 09/12] PCI: Add 'override_only' bitmap to struct pci_device_id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 8/19/2021 7:39 PM, Bjorn Helgaas wrote:
On Thu, Aug 19, 2021 at 07:16:20PM +0300, Yishai Hadas wrote:
On 8/19/2021 6:15 PM, Bjorn Helgaas wrote:
On Wed, Aug 18, 2021 at 06:16:03PM +0300, Yishai Hadas wrote:
From: Max Gurtovoy <mgurtovoy@xxxxxxxxxx>
   /**
    * struct pci_device_id - PCI device ID structure
    * @vendor:		Vendor ID to match (or PCI_ANY_ID)
@@ -34,12 +38,14 @@ typedef unsigned long kernel_ulong_t;
    *			Best practice is to use driver_data as an index
    *			into a static list of equivalent device types,
    *			instead of using it as a pointer.
+ * @override_only:	Bitmap for override_only PCI drivers.
"Match only when dev->driver_override is this driver"?
Just to be aligned here,

This field will stay __u32 and may hold at the most 1 bit value set to
represent the actual subsystem/driver.
The PCI core does not require "at most 1 bit is set."

Actually, I don't think even the file2alias code requires that.  If
you set two bits, you can generate two aliases.

This is required to later on set the correct prefix in the modules.alias
file, and you just suggested to change the comment as of above, right ?
Yes, __u32 is fine and I'm only suggesting a comment change here.

great.



As far as PCI core is concerned there's no need for this to be a
bitmap.

I think this would make more sense if split into two patches.  The
first would add override_only and change pci_match_device().  Then
there's no confusion about whether this is specific to VFIO.
Splitting may end-up the first patch with a dead-code on below, as
found_id->override_only will be always 0.

If you still believe that this is better we can do it.
I think it's fine to add the functionality in one patch and use it in
the next if it makes the commit clearer.  I wouldn't want to add
functionality that's not used at all in the series, but it's OK when
they're both posted together.

Ok. We can do the separation if all agree that the first commit is have a dead section.

Alex,

we would like to get few more reviewed-by signatures and we'll send the V3 series in a couple of days to make it to 5.15 merge window as we planned.

Are you ok with the series after we got the green light for this patch ?

do you think we need another pair of eyes to review the other patches ?

-Max.


if (found_id->override_only) {
     if (dev->driver_override)
       return found_id;
   } else
     return found_id;

The second can add PCI_ID_F_VFIO_DRIVER_OVERRIDE and make the
file2alias.c changes.  Most of the commit log applies to this part.




[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux