Re: [PATCH] zs: Move to the serial subsystem

[Russell King]

On Wed, May 30, 2007 at 01:12:24AM -0700, Andrew Morton wrote:
> > +			if (status & (Rx_SYS | Rx_BRK))
> > +				icount->brk++;
> > +			else if (status & FRM_ERR)
> > +				icount->frame++;
> > +			else if (status & PAR_ERR)
> > +				icount->parity++;
> 
> FRM_ERR and PAR_ERR are mutually exclusive, and cannot be set if either
> Rx_SYS or Rx_BRK are set?

That's actually fairly normal.  A break condition is by definition
a framing error, and possibly a parity error as well.  Also, a break
condition is not an error per-se.

Also, if you do add in the associated framing or parity errors, you're
likely to get different results from different hardware - some hardware
mask off the framing and parity errors when detecting a break condition.
Others don't.

> > +/*
> > + * Finally, routines used to initialize the serial port.
> > + */
> > +static int zs_startup(struct uart_port *uport)
> > +{
> > +	struct zs_port *zport = to_zport(uport);
> > +	struct zs_scc *scc = zport->scc;
> > +	unsigned long flags;
> > +	int ret;
> > +
> > +	if (!scc->irq_guard) {
> > +		ret = request_irq(zport->port.irq, zs_interrupt,
> > +				  IRQF_SHARED, "scc", scc);
> > +		if (ret) {
> > +			printk(KERN_ERR "zs: can't get irq %d\n",
> > +			       zport->port.irq);
> > +			return ret;
> > +		}
> > +	}
> > +	scc->irq_guard++;
> 
> The ->irq_guard handling looks a little racy?
> 
> Perhaps higher-level locks prevent this.  If so, a comment explaining this
> would be reassuring.

Does look racy if "scc" is shared between several ports.  The locking
here is only per-port, so this is racy.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/