[PATCH 0/3] ioc4: Driver rework

[PATCH 0/3] ioc4: Driver rework

[Brent Casavant]

This series of patches reworks the configuration and internal structure
of the SGI IOC4 I/O controller device drivers.  These patches are against
2.6.12-rc4, however acceptance into 2.6.13 is my actual goal.

These changes are motivated by several factors:

- The IOC4 chip PCI resources are of mixed use between functions (i.e.
  multiple functions are handled in the same address range, sometimes
  within the same register), muddling resource ownership and initialization
  issues.  Centralizing this ownership in a core driver is desirable.

- The IOC4 chip implements multiple functions (serial, IDE, others not
  yet implemented in the mainline kernel) but is not a multifunction
  PCI device.  In order to properly handle device addition and removal
  as well as module insertion and deletion, an intermediary IOC4-specific
  driver layer is needed to handle these operations cleanly.

- All IOC4 drivers are currently enabled by a single CONFIG value.  As
  not all systems need all IOC4 functions, it is desireable to enable
  these drivers independently.

- The current IOC4 core driver will trigger loading of all function-level
  drivers, as it makes direct calls to them.  This situation should be
  reversed (i.e. function-level drivers cause loading of core driver)
  in order to maintain a clear and least-surprise driver loading model.

- IOC4 hardware design necessitates some driver-level dependency on
  the PCI bus clock speed.  Current code assumes a 66MHz bus, but the
  speed should be autodetected and appropriate compensation taken.

This patch series effects the above changes by a newly and better designed
IOC4 core driver with which the function-level drivers can register and
deregister themselves upon module insertion/removal.  By tracking these
modules, device addition/removal is also handled properly.  PCI resource
management and ownership issues are centralized in this core driver, and
IOC4-wide configuration actions such as bus speed detection are also
handled in this core driver.

Brent Casavant

-- 
Brent Casavant                          If you had nothing to fear,
bcasavan@sgi.com                        how then could you be brave?
Silicon Graphics, Inc.                    -- Queen Dama, Source Wars

Re: [PATCH 0/3] ioc4: Driver rework

[Jeff Garzik]

Brent Casavant wrote:
> - The IOC4 chip implements multiple functions (serial, IDE, others not
>   yet implemented in the mainline kernel) but is not a multifunction
>   PCI device.  In order to properly handle device addition and removal
>   as well as module insertion and deletion, an intermediary IOC4-specific
>   driver layer is needed to handle these operations cleanly.

I disagree that a layer is needed.

Just write a PCI driver that does the following in probe:

	register IDE
	register serial
	...

and undoes all that in remove.

Device addition and removal work just fine with that scheme.

	Jeff

Re: [PATCH 0/3] ioc4: Driver rework

[Brent Casavant]

On Mon, 23 May 2005, Jeff Garzik wrote:

> Brent Casavant wrote:
> > - The IOC4 chip implements multiple functions (serial, IDE, others not
> >   yet implemented in the mainline kernel) but is not a multifunction
> >   PCI device.  In order to properly handle device addition and removal
> >   as well as module insertion and deletion, an intermediary IOC4-specific
> >   driver layer is needed to handle these operations cleanly.
> 
> I disagree that a layer is needed.
> 
> Just write a PCI driver that does the following in probe:
> 
> 	register IDE
> 	register serial
> 	...
> 
> and undoes all that in remove.
> 
> Device addition and removal work just fine with that scheme.

That is the structure of the current device driver, and we've found it
to be inadequate.  What the structure you mention doesn't allow is
the independent loading and unloading of modules to support the various
functions of the IOC4 chip.

On most systems the only component of IOC4 that is used is the IDE
driver.  In this case the serial devices are not needed, and there is
no use carrying around that code during runtime.  There are similar
situations for the IOC4 external interrupt capability (to be open-sourced
soon) and kbd/mouse (no plans to even write this driver, but you never
know).  This isn't a huge deal with the amount of RAM present on a typical
SGI Altix machine, however it can become an issue when we consider the
initrd images and installer kernels supplied with vendor distributions.

A quarter meg here, a quarter meg there, and before you know it, you're
talking about real disk space. :)

Another minor point, the current driver code inverts the mental model
that makes sense to most people.  Having the core driver cause a load
of the function-level drivers (via modprobe) seems counterintuitive.
The new method seems to make sense to most everyone I've bounced it
off of (granted, internal to SGI).  Very much related to this point,
the new code is just far far cleaner, code-wise and mentally.

On a more personal level, having a structure which allows loading and
unloading of the drivers for the individual functions of the IOC4
greatly eases the debugging cycle.  While in theory my code should be
perfect...

Brent

-- 
Brent Casavant                          If you had nothing to fear,
bcasavan@sgi.com                        how then could you be brave?
Silicon Graphics, Inc.                    -- Queen Dama, Source Wars

Re: [PATCH 0/3] ioc4: Driver rework

[Brent Casavant]

On Mon, 23 May 2005, Jeff Garzik wrote:

> Brent Casavant wrote:
> > - The IOC4 chip implements multiple functions (serial, IDE, others not
> >   yet implemented in the mainline kernel) but is not a multifunction
> >   PCI device.  In order to properly handle device addition and removal
> >   as well as module insertion and deletion, an intermediary IOC4-specific
> >   driver layer is needed to handle these operations cleanly.
> 
> I disagree that a layer is needed.
> 
> Just write a PCI driver that does the following in probe:
> 
> 	register IDE
> 	register serial
> 	...
> 
> and undoes all that in remove.

Robin Holt just stopped by my office and clued me in to something I
may not have made clear, and which may be why you disagree with me.

The IOC4 hardware is very strange PCI-wise.  While it implements
what are a disparate set of features (IDE, serial ports, kbd/mouse,
etc), in PCI-speak IOC4 is not a multifunction device. That is, in
all ways it appears to the PCI infrastructure as a single device
(e.g.  a single shared interrupt for all these features, no subdevice
IDs for each independent feature, etc).

This complicates the driver model fairly significantly.  The PCI
infrastructure cannot natively deal with this situation because
multiple kernel modules cannot register themselves for the same
PCI subdevice because pci_device_probe_dynamic() will find only
the first match.  Consequently we cannot write free-standing drivers
for the seperate IOC4 features; actions that would normally be
invoked by a probe function must be handled by some sort of IOC4
core code which knows about the various IOC4 features, and how to
probe these features whenever the core code has its probe function
invoked.

(Yes, I suppose I could alter the PCI code to deal with this situation,
but considering the complications that may cause for less-weird devices,
it hardly seems like a good path to pursue.)

The main thrust of my code changes is to allow these logically
independent features of the IOC4 to be implemented as independent
(from eachother, not the core driver) kernel modules.  Rather than
the IOC4 core module having hard-coded knowledge of each feature of
the chip, it instead relies on the per-feature kernel modules to
register their comings and goings.

This provides a very useful level of flexibility, requiring neither
RAM nor disk space (which can be quite contended for install media
or initrd images) for unused features.  It certainly speeds up
driver debugging iterations.  And (personal opinion here), the
resulting code structure is cleaner than the existing code.

In some ways, you could conceptualize the IOC4 chip as a bus.  In
fact, I once toyed with the idea of implementing it as a new bus
type.  If you view it in those terms, the IOC4 core driver is
essentially a "PCI to IOC4 bridge" driver, and the IOC4 feature
drivers would be independent IOC4 devices.  However, IOC4 isn't
a bridge, and implementing it as a bus would be overkill. Thus
a simple core driver that allows feature registration and
deregistration seems a fitting solution.

Thoughts?

Thanks,
Brent

-- 
Brent Casavant                          If you had nothing to fear,
bcasavan@sgi.com                        how then could you be brave?
Silicon Graphics, Inc.                    -- Queen Dama, Source Wars