On Thu, Jan 21, 2016 at 09:39:01AM +0000, David Woodhouse wrote:
> On Wed, 2016-01-20 at 11:46 -0600, Bjorn Helgaas wrote:
> >
> > I don't really want to merge things that only exist to enable
> > out-of-tree development, because (1) they're an extra maintenance
> > burden for which we get risk without benefit, and (2) we can't see
> > the out-of-tree code, so it's easy for people to make changes that
> > accidentally break that code.
> >
> > Looking at the patch again, I see that even without the export,
> > there's no current benefit,
>
> This is just a PCI quirk; I'm not sure it should be considered part of
> the driver code at all. With this patch, even without a Linux driver,
> we could correctly handle assignment to VM guests (which *might* have a
> driver), and also theoretically we should be able to handle fault
> storms and shoot the right device in the head if it was left in an odd
> state and misbehaves (not that I've hooked that up yet).
>
> So I'm not sure it makes sense to tie this patch to the existence of a
> driver.
This definitely isn't part of the driver code; I didn't mean to
suggest that. I'd like to see this as a separate patch, but as part
of a series that adds a user of the multiple-alias functionality.
All I'm saying is that as-is, this patch makes the quirks easier to
read but doesn't actually change any behavior: we set up at most one
alias, and we do it as a header quirk at enumeration-time, so there
are no new lifetime issues. Normally we merge things when they're
needed, and multiple alias support is a bit of infrastructure that
isn't used yet.
I already said I'm not rejecting the patch. Alex and I raised a few
questions. Usually that leads to a little discussion and possibly a
v2 of the patch, but so far, I haven't seen any answers.
Here are a couple more questions/concerns:
- If we export an "add" function, do we need a corresponding
"remove"? This depends on how the alias lifetimes are managed,
and I haven't seen that yet.
- Changing pci_dev_flags and struct pci_dev changes the ABI and makes
work for distros, with no current benefit.
I'm not sure why we're even having this discussion. The merge window
opened Jan 10, and IIRC, I first saw the patch Jan 11 and it first
appeared on linux-pci Jan 13, so this is just late for v4.5 to begin
with.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
