Re: New device layout, including static and dynamic initializers

[ron minnich]

On 22 Jul 2003, ollie lho wrote:
> Why do you want to call a SINGLE function in different places with 
> different enum value ? Is there any disadvantage to make each call
> into it own function ?

OK, to elaborate.

In superio code from 1.0, we had this:
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;
};

The names pre_pci_init, init, and finishup were an implicit definition of 
those functions. They could be filled in, or NULL, as in:

struct superio_control superio_winbond_w83977ef_control = {
        pre_pci_init:   (void *) 0,
        init:           setup_devices, 
        finishup:       (void *) 0,
        defaultport:    PNPADDR,
        name:           "WinBond w83977tf"
};

These three functions roughly correspond to "passes" in hardwaremain, and 
are intended to be called as hardwaremain proceeds through platform 
configuration.

In other words, you had some degree of control of when a device's control 
functions were called. This has been very important -- some of these 
devices need code before PCI enumeration, for example.

The problem is, are the three functions defined sufficient? how many 
should there be? What if hardwaremain changes and we add new "passes", do 
we have to add some new set of functions? If we add new passes, how do we 
go about fixing all the parts that we have written code for?

Also, we wanted to generalize beyond just superios to all devices.

Finally, the PPC has very different rules than standard PCs, and we're 
trying to come up with a non-Pentium-centric way of doing things.

Now, internally, the code that walked the superio tree was ALWAYS called 
with a 'pass', as in:

void
handle_superio(int pass, struct superio *all_superio[], int nsuperio)
{
.
.
.
               // need to have both pre_pci_init and devfn defined.
                if (s->super->pre_pci_init && (pass == 0)) {
                        printk_debug("  Call pre_pci_init\n");
                        s->super->pre_pci_init(s);
                } else if (s->super->init && (pass == 1)) {
                        printk_debug("  Call init\n");
                        s->super->init(s);
                } else if (s->super->finishup && (pass == 2)) {
                        printk_debug("  Call finishup\n");
                        s->super->finishup(s);
                }

.
.
.
}

This code has been this way for two or more years.

So all we're really doing is creating ONE interface function to the chips, 
not three:

/* there is one of these for each TYPE of chip */
struct chip_control {
  void (*enable)(struct chip *, enum chip_pass);
  char *path;     /* the default path. Can be overridden
                   * by commands in config
                   */
  // This is the print name for debugging
  char *name;
};

and that code will now be called with a pass. We expect that in this 
enable function we'll see things like this:

static void enable(struct chip *, enum chip_pass) {
    struct superio *s = chip->chip_info;
    switch (chip_pass) {
    default:
    	break;
    case        DEV_PASS_PRE_CONSOLE: 
    	/* make sure serial is enabled; this chip is weird */
    	if (s->com1.enable)
    		serial_enable(&s->com1);
    	break;
        DEV_PASS_PRE_PCI: 
    	/* if the ide_enable static initializer is set, 
    	 * set the IDE controller so it is visible on PCI */
    	if (s->ide_enable)
    		ide_on();
    	break;
    }

}

In other words, most passes don't matter, but if some passes do, then the 
code can manage it. We'll thus have a standard "template" for all 
resources on the motherboard, and a standard way to enable resources 
during different passes (phases, maybe we should call it?) of 
hardwaremain().

hardwaremain changes a bit too.

For instance, this:
        post_code(0x80);
        /* displayinit MUST PRECEDE ALL PRINTK! */
        console_init();

becomes this:
    device_configure(chip_root, DEV_PASS_PRE_CONSOLE);
        post_code(0x80);
        /* displayinit MUST PRECEDE ALL PRINTK! */
        console_init();

and so on. device_configure is called at different phases in hardwaremain, 
as the platform becomes more configured. device_configure is capable for 
walking the device tree, and calling functions as needed.

That's the basic idea, anyway. It is motivated by the goal of replacing 
this type of code:

/* PCI Interface */
        .byte   0x80,   0x72    #
        .byte   0x81,   0x07    #
        .byte   0x82,   0xFF    #

with C code that people can deal with. There was a lot of confusion on how 
to enable/disable built-in-ethernet on the SiS 950, for example, and we're 
trying to reduce confusion.

Comments welcome.

ron