[PATCH 3/4] xfree86: fbdevhw: remove superfluous pci pointer from probe function

Vignatti Tiago (Nokia-MS/Helsinki) tiago.vignatti at nokia.com
Thu Sep 9 01:01:59 PDT 2010


Mark,

On Tue, Sep 07, 2010 at 11:25:26PM +0200, ext Mark Kettenis wrote:
> > From: Tiago Vignatti <tiago.vignatti at nokia.com>
> > Date: Tue,  7 Sep 2010 15:18:48 +0300
> > 
> > The only drivers using this function are fbdev and glint. Though those driver
> > initialize with NULL argument. Other drivers do some kind of implicit probe
> > using fbdevHWInit instead.
> > 
> > API break.
> 
> Sorry, but I really fail to see the purpose of this change.

It's not used by anyone, it would open space for other major cleanup, then why
not just kill? I thought I was clear, sorry.


> It is
> true that the only open source driver that actually uses
> fbdevHWProbe() passes NULL as the first argument, but as far as I can
> tell, calling it with a non-NULL first argument is not unreasonable.
> Your diff destroys the symmetry between fbdevHWProbe() and
> fbdevHWInit().

that's a good catch. As I said before, I was targeting cleaning up only and to
be fair didn't think about the semantics about these changes - the way I
cooked was pretty much mechanical.

Going to the point now, fbdevHWProbe and fbdevHWInit lost their meaning
already in servers using libpciaccess: HWProbe is using for open fd devices
only, while HWInit open fd and does some basic probe. So meh.

 
> That said, the glint driver only calls fbdevHWProbe() inside a #ifndev
> XSERVER_LIBPCIACCESS block.  The comment says this is "for systems
> without (proper) PCI support".  So perhaps this code has outlived its
> usefulness, and fbdevHWProbe() should be removed altogether?

That might be true. Although fbdev driver stills using it when libpciaccess is
there, it's being used only for opening the fd - which seems to redundant
being done because fbdevHWInit already made it. So, if that's the case, I'm
pretty sure we can roll up another patch series after this one to remove
entirely fbdevHWProbe.

But I'd say to keep in another patch set now. What do you think?


PS: libpciaccess came for good. But the way it got incorporated was the worst
I seen.

             Tiago


More information about the xorg-devel mailing list