Re: [PATCH] fakephp: Allocate PCI resources before adding the device

[Trent Piepho <xyzzy@speakeasy.org>]

On Fri, 28 Nov 2008, Matthew Wilcox wrote:
> On Fri, Nov 28, 2008 at 02:11:49AM -0800, Trent Piepho wrote:
> > On Wed, 26 Nov 2008, Matthew Wilcox wrote:
> > > Because fakephp was the odd one out.  Every other hotplug driver
> > > registers a slot number and puts the address in the directory.  We're
> > > making hotplug drivers simpler (by sharing more of the logic in the
> > > core) and so fakephp had to change to match the other drivers.  I'm
> > > sorry for your inconvenience, but it's necessary.
> >
> > Seems like changing a long established interface like this should be put on
> > the feature removable schedule.
>
> sysfs is more fluid than other interfaces.  Large parts of it
> change without warning.  I suppose it could have gone through
> feature-removal-schedule, and perhaps we should try to do that in future.

Sometimes I just want to give up on Linux.  Is there a different interface
that isn't wantonly changed without warning?  No?  Do you not see the
problem this creates for developers?  Do you not care?

> > > We can discuss other ways to make your life better, but it can't be
> > > changed back to one 'slot' per PCI function.
> >
> > Why not have a slot per pci function with each hotplug driver adding
> > attributes to the slot?  The slot would be registered by the hotplug core.
>
> What we have now is the 'address' file managed by the core.  This
> doesn't include the function number.

So the race condition doesn't matter?  Alex Chiang wasn't even aware
fakephp could be used to add new devices, so this change obviously wasn't
though out very well.

> > Is there any reason the fakephp driver should not be allowed to enable and
> > disable individual functions?  There a lot of mutli-function devices that
> > allow individual functions to be enabled and disabled with registers on the
> > primary function.  For instance, it would be possible to work around a BIOS
> > that doesn't have an option to disabled a southbridge's USB controller by
> > doing so in Linux and then using fakephp to remove the device, and thus
> > free address space that other devices might need.
>
> There are lots of circumstances where enabling and disabling individual
> functions is a good idea, but I don't think that fakephp is the right
> way to do it.  We need a better interface.

So maybe this better interface should be created before breaking the
existing one?