weighing in

[Eric W. Biederman]

"Ronald G. Minnich" <rminnich at lanl.gov> writes:

> On 18 Feb 2003, Eric W. Biederman wrote:
> 
> > Right now I have a larger number of initialization functions being
> > called from mainboard_fixup.  Those are the real functions that need
> > to be tackled.  The superio model comes close to handling those cases
> > cleanly.  The big issue with superio code is that we use one structure
> > for all possible static values, that is silly.
> 
> There are two static structures in there, and I think I know which one you 
> mean, but want to clarify. 
> 
> This one, I assume, is OK
> 
> struct superio_control {
>   void (*pre_pci_init)(struct superio *s);
>   void (*init)(struct superio *s);
>   void (*finishup)(struct superio *s);
>   unsigned int defaultport;     /* the defaultport. Can be overridden
>                                  * by commands in config
>                                  */
>   // This is the print name for debugging
>   char *name;
> };
> 
> 
> Fairly generic and represents how to attack a superio. Universal to all 
> superios. 

Right.  The one I have for pci is a little different but roughly equivalent.
 
> This one, I think is the mistake:

I would just call it in need of a better abstraction.  Not
a really a mistake.
 
> struct superio {
>         struct superio_control *super; // the ops for the device.
>         unsigned int port; // if non-zero, overrides the default port
>         // com ports. This is not done as an array (yet).
>         // We think it's easier to set up from python if it is not an 
> array.
>         struct com_ports com1, com2, com3, com4;
>         // DMA, if it exists.
>         struct lpt_ports lpt1, lpt2;
>         /* flags for each device type. Unsigned int. */
>         // low order bit ALWAYS means enable. Next bit means to enable
>         // LPT is in transition, so we leave this here for the moment.
>         // The winbond chips really stretched the way this works.
>         // so many functions!
>         unsigned int ide, floppy, lpt;
>         unsigned int keyboard, cir, game;
>         unsigned int gpio1, gpio2, gpio3;
>         unsigned int acpi,hwmonitor;
> };
> 
> because all the superios are so different (I never realized how 
> different!) this struct is a mess and we don't want to repeat this for the 
> south and north bridge hardware. 

Correct.
 
> Make sense?

Yes.

I am thinking of something like:

struct setup_info {
        struct setup_info *next;
        int type;
};

struct irq_routing {
        struct setup_info info;
        /* Flesh this out... */
};

struct static_resources {
        struct setup_info info;
        /* Flesh this out... */
};

struct old_superio {
        struct setup_info info;
        struct com_ports com1, com2, com3, com4;
        struct lpt_ports lpt1,lpt2;
	unsigned int ide, floppy, lpt;
	unsigned int keyboard, cir, game;
	unsigned int gpio1, gpio2, gpio3;
	unsigned int acpi,hwmonitor;
        /* Don't use this for real.. */
};

The important characteristics are:

- Any developer can add new structure types and they can be
  motherboard or device specific

- There is a way for generic code to access configuration settings
  information about a device.

Eric