I've update the patch with changes from the comments below.

ftp://oss.sgi.com/projects/sn2/sn2-update/033-ioc4-support

As usual forgot this:

Signed-off-by: Patrick Gefre <[EMAIL PROTECTED]>




Patrick Gefre wrote:
I've update the patch with changes from the comments below.

ftp://oss.sgi.com/projects/sn2/sn2-update/033-ioc4-support




Christoph Hellwig wrote:

On Mon, Feb 07, 2005 at 09:58:33AM -0600, Patrick Gefre wrote:

Latest version with review mods:
ftp://oss.sgi.com/projects/sn2/sn2-update/033-ioc4-support




- still not __iomem annotations.


I *think* I have this right ??



 - still a ->remove method

more comments (mostly nipicks I missed last time, nothing too exciting):


+#define DEVICE_NAME_DYNAMIC "ttyIOC0" /* need full name for misc_register */


this one is completely unused.

+#define PENDING(_p)    readl(&(_p)->ip_mem->sio_ir) & _p->ip_ienb

probably wants some braces around the macro body

+static struct ioc4_port *get_ioc4_port(struct uart_port *the_port)
+{
+    struct ioc4_control *control = dev_get_drvdata(the_port->dev);
+    int ii;
+
+    if (control) {
+        for ( ii = 0; ii < IOC4_NUM_SERIAL_PORTS; ii++ ) {
+            if (!control->ic_port[ii].icp_port)
+                continue;
+            if (the_port == control->ic_port[ii].icp_port->ip_port)
+                return control->ic_port[ii].icp_port;
+        }
+    }
+    return (struct ioc4_port *)0;

just return NULL here.

+static irqreturn_t ioc4_intr(int irq, void *arg, struct pt_regs *regs)
+{
+    struct ioc4_soft *soft;
+    uint32_t this_ir, this_mir;
+    int xx, num_intrs = 0;
+    int intr_type;
+    int handled = 0;
+    struct ioc4_intr_info *ii;
+
+    soft = (struct ioc4_soft *)arg;
+    if (!soft)
+        return IRQ_NONE;    /* Polled but no ioc4 registered */

no need to cast.  and it can't be NULL either.

+    spin_lock_irqsave(&port->ip_lock, port_flags);
+    wake_up_interruptible(&info->delta_msr_wait);
+    spin_unlock_irqrestore(&port->ip_lock, port_flags);

no need to lock around a wake_up()

+    /* Start up the serial port */
+    spin_lock_irqsave(&port->ip_lock, port_flags);
+    retval = ic4_startup_local(the_port);
+    if (retval) {
+        spin_unlock_irqrestore(&port->ip_lock, port_flags);
+        return retval;
+    }
+    spin_unlock_irqrestore(&port->ip_lock, port_flags);
+    return 0;

what about just

    spin_lock_irqsave(&port->ip_lock, port_flags);
    retval = ic4_startup_local(the_port);
    spin_unlock_irqrestore(&port->ip_lock, port_flags);
    return reval;

?
+ struct ioc4_port *port = get_ioc4_port(the_port);
+ unsigned long port_flags;
+
+ spin_lock_irqsave(&port->ip_lock, port_flags);
+ ioc4_change_speed(the_port, termios, old_termios);
+ spin_unlock_irqrestore(&port->ip_lock, port_flags);
+ return;


no need for empty returns at the end of void functions

+static struct uart_driver ioc4_uart = {
+    .owner        = THIS_MODULE,
+    .driver_name    = "ioc4_serial",
+    .dev_name    = DEVICE_NAME,
+    .major        = DEVICE_MAJOR,
+    .minor        = DEVICE_MINOR,
+    .nr        = IOC4_NUM_CARDS * IOC4_NUM_SERIAL_PORTS,
+    .cons        = NULL,
+};

no need to initialize .cons to zero, the compiler does that for you.

+ if ( !request_region(tmp_addr, sizeof(struct ioc4_mem), "sioc4_mem")) {

superflous space before the !

+    if (!request_irq(pdev->irq, ioc4_intr, SA_SHIRQ,
+                "sgi-ioc4serial", (void *)soft)) {
+        control->ic_irq = pdev->irq;
+    } else {
+        printk(KERN_WARNING
+            "%s : request_irq fails for IRQ 0x%x\n ",
+            __FUNCTION__, pdev->irq);
+    }

Can the driver work without an irq?


Not in its current state.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

Reply via email to