Messages in this thread | | | Subject | Re: [PATCH][v4] ipwireless: driver for 3G PC Card | From | Marcel Holtmann <> | Date | Fri, 01 Feb 2008 17:32:59 +0100 |
| |
Hi David,
> +irqreturn_t ipwireless_interrupt(int irq, void *dev_id, struct pt_regs *regs) > +{ > + struct ipw_hardware *hw = dev_id; > + > + if (hw->hw_version == HW_VERSION_1) > + return ipwireless_handle_v1_interrupt(irq, hw); > + else > + return ipwireless_handle_v2_v3_interrupt(irq, hw); > +}
why is this not static? I think the interrupt routine should be in the file where you actually claim the interrupt.
> +/* > + * Associate the specified network with this hardware, so it will receive events > + * from it. > + */ > +void ipwireless_associate_network(struct ipw_hardware *hw, > + struct ipw_network *network) > +{ > + hw->network = network; > +}
I think a #define would be simpler in this case.
> +module_param(tty_major, int, 0); > +module_param_named(debug, ipwireless_debug, int, 0); > +module_param_named(loopback, ipwireless_loopback, int, 0); > +module_param_named(out_queue, ipwireless_out_queue, int, 0); > +MODULE_PARM_DESC(tty_major, "ttyIPWp major number [0]");
Why is allowing to change the major still needed. I think we passed the bridge of the need for static numbers a long long long time ago.
> +static const char drv_name[] = IPWIRELESS_PCCARD_NAME;
This looks useless to. Do we need that?
Regards
Marcel
| |