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
--
linuxbios mailing list
linuxbios@linuxbios.org
http://www.linuxbios.org/mailman/listinfo/linuxbios
