Dave Airlie <[email protected]> writes: > + else if (strcmp(udev_device_get_subsystem(udev_device), "drm")) > + goto no_probe; > + else if (strncmp(sysname, "card", 4)) > + goto no_probe;
bikeshed -- strcmp(a,b) != 0; otherwise, I'm left wondering if you
expected strcmp to return a bool.
> +
> +#if defined(XSERVER_PLATFORM_BUS)
> +extern _X_EXPORT int platformSlotClaimed;
> +#endif
> +
From my reading of the code, it seems like pciSlotClaimed,
platformSlotClaimed and sbusSlotClaimed can all be merged into a single
'we found something better than the generic driver' variable.
Looks like xf86PostProbe is also broken here -- if you don't have
LIBPCIACCESS, and try to use fbdev, it will refuse.
> +#ifdef XSERVER_PLATFORM_BUS
> + i = xf86PlatformMatchDriver(matches, nmatches);
> +#endif
Should the Platform Bus be above SBUS?
> xf86IsPrimaryPci(struct pci_device *pPci)
> {
> - return ((primaryBus.type == BUS_PCI) && (pPci == primaryBus.id.pci));
> + if (primaryBus.type == BUS_PCI)
> + return pPci == primaryBus.id.pci;
> +#ifdef XSERVER_PLATFORM_BUS
> + if (primaryBus.type == BUS_PLATFORM)
> + if (primaryBus.id.plat->pdev)
> + if (MATCH_PCI_DEVICES(primaryBus.id.plat->pdev, pPci))
> + return TRUE;
> +#endif
> + return FALSE;
> }
Ok, this seems a bit kludgy -- from what I can tell, you're essentially
pulling PCI ids out of the udev device so that you can match drivers
based on those instead of some 'other' mechanism. I don't have a better
plan though; PCI ids are the best identifiers we've ever found...
> +/*
> + * xf86IsPrimaryPlatform() -- return TRUE if primary device
> + * is PCI and bus, dev and func numbers match.
> + */
> +
> +static Bool
> +xf86IsPrimaryPlatform(struct xf86_platform_device *plat)
> +{
> + return ((primaryBus.type == BUS_PLATFORM) && (plat ==
> primaryBus.id.plat));
> +}
The comment seems to imply matching on the PCI numbers, but instead
you're matching on the platform device itself. Are those equivalent?
> +static void
> +platform_find_pci_info(int idx, char *busid)
Ick. passing around indices to global arrays. No way to actually use a
pointer to the xf86_platform_device structure here?
> + if (pci && !strncmp(busid, "pci:", 4)) {
bikeshed -- I prefer strncmp() == 0; strncmp isn't a boolean function.
--
[email protected]
pgp9AuSZgOHxy.pgp
Description: PGP signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
