lkml.org 
[lkml]   [2006]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    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    [W:0.028 / U:92.376 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site