On Sun, Oct 28, 2007 at 08:49:04PM -0400, Corey Osgood wrote:
+void f71805f_pnp_set_resources(struct device *dev); +void f71805f_pnp_set_resources(struct device *dev);
Twice?
+static void pnp_enter_conf_state(struct device *dev); +static void pnp_exit_conf_state(struct device *dev);
+static void pnp_enter_conf_state(struct device *dev) +{
- outb(0x87, dev->path.u.pnp.port);
+}
+static void pnp_exit_conf_state(struct device *dev) +{
- outb(0xaa, dev->path.u.pnp.port);
+}
Are the declarations really needed when the definition is right after them?
- switch (dev->path.u.pnp.device) {
- case F71805F_SP1:
res0 = find_resource(dev, PNP_IDX_IO0);//TODO: needed? fix or remove?//init_uart8250(res0->base, &conf->sp1);break;- case F71805F_SP2:
res1 = find_resource(dev, PNP_IDX_IO0);//init_uart8250(res0->base, &conf->sp2);break;
PNP_IDX_IO0 in both cases? Plus the commented SP2 call uses res0.
+static struct device_operations ops; +static struct pnp_info pnp_dev_info[] = {
- { &ops, F71805F_SP1, PNP_IO0 | PNP_IRQ0, { 0x7f8, 0 }, },
- { &ops, F71805F_SP2, PNP_IO0 | PNP_IRQ0, { 0x7f8, 0 }, },
- /* TODO: Everything else */
+};
You could combine the declaration and definition of ops.
+static struct device_operations ops = {
- .phase2_setup_scan_bus = phase2_setup_scan_bus,
- .phase4_read_resources = pnp_read_resources,
- .phase4_set_resources = f71805f_pnp_set_resources,
- .phase4_enable_disable = f71805f_pnp_enable_resources,
- .phase5_enable_resources = f71805f_pnp_enable,
Seems these two have been mixed up.
- .phase6_init = f71805f_init,
Using ops it's of course very easy to have generic pnp functions that do what most of the chips want/need. We want that. :)
//Peter