lkml.org 
[lkml]   [2006]   [Jan]   [23]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
DateSun, 22 Jan 2006 22:24:25 -0800
FromAndrew Morton <>
SubjectRe: [PATCHv2] parport: add parallel port support for SGI O2
Arnaud Giersch <arnaud.giersch@free.fr> wrote:
>> From: Arnaud Giersch <arnaud.giersch@free.fr>
> 
> Add support for the built-in parallel port on SGI O2 (a.k.a. IP32).
> Define a new configuration option: PARPORT_IP32.  The module is named
> parport_ip32.
> 
> Hardware support for SPP, EPP and ECP modes along with DMA support
> when available are currently implemented.

Nice looking driver.  Big.

It does rather a lot of

	if (foo)	do_something();
whereas we prefer

	if (foo)
		do_something();
but if that was a blocker, we wouldn't have any drivers.

> +static void parport_ip32_dma_setup_context(unsigned int limit)
> +{> +	unsigned long flags;> +> +	spin_lock_irqsave(&parport_ip32_dma.lock, flags);
> +	if (parport_ip32_dma.left > 0) {
> +		volatile u64 __iomem *ctxreg = (parport_ip32_dma.ctx == 0) ?
> +			&mace->perif.ctrl.parport.context_a :
> +			&mace->perif.ctrl.parport.context_b;

Does this need to be volatile?   writeq() should do the right thing.

> +static size_t parport_ip32_epp_read(void __iomem *eppreg,
> +				    struct parport *p, void *buf,> +				    size_t len, int flags)> +{> +	struct parport_ip32_private * const priv = p->physport->private_data;
> +	size_t got;> +	parport_ip32_set_mode(p, ECR_MODE_EPP);> +	parport_ip32_data_reverse(p);> +	parport_ip32_write_control(p, DCR_nINIT);> +	if ((flags & PARPORT_EPP_FAST) && (len > 1)) {
> +		readsb(eppreg, buf, len);

readsb() is a mips thing, and doesn't seem to be documented.  What does it
do, and why does the driver use it (only) here?

> +		writesb(eppreg, buf, len);

> +static unsigned int parport_ip32_fifo_wait_break(struct parport *p,
> +						 unsigned long expire)> +{> +	cond_resched();> +	if (time_after(jiffies, expire)) {> +		printk(KERN_DEBUG PPIP32> +		       "%s: FIFO write timed out\n", p->name);
> +		return 1;> +	}> +	if (signal_pending(current)) {> +		printk(KERN_DEBUG PPIP32> +		       "%s: Signal pending\n", p->name);
> +		return 1;> +	}

This printk could be a bit noisy, if someone hoses a signal stream at a
printing program.

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

\
 
 \ /
  Last update: 2006-01-23 07:27    [from the cache]
©2003-2008